All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i2c-next v5 0/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
@ 2018-10-03 22:31 ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

In multi-master environment, this driver's master cannot know
exactly when peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver. To prevent state corruption in the
case, this patch adds checking code if any slave operation is
ongoing and it waits up to the bus timeout duration before starting
a master_xfer operation.

To support this change, it introduces changes on i2c-core-base to
make that able to read the bus timeout and master transfer retries
count values from device tree properties.

Please review this patch set.

Thanks,

-Jae

Changes since v3:
- Changed the property name to 'timeout' and made it use the
  default setting in i2c-core when not specified.

Changes since v2:
- Changed the property name to 'aspeed,timeout' and made it to
  update the adapter's timeout configuration.

Changes since v1:
- Changed define names of timeout related.

Jae Hyun Yoo (5):
  dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common
    optional
  i2c: Add support for 'timeout-ms' and '#retries' OF properties
  dt-bindings: i2c: aspeed: Add 'timeout-ms' property as an optional
    property
  i2c: aspeed: Remove hard-coded bus timeout value setting
  i2c: aspeed: Add bus idle waiting logic for multi-master use cases

 .../devicetree/bindings/i2c/i2c-aspeed.txt    |  3 ++
 Documentation/devicetree/bindings/i2c/i2c.txt |  6 +++
 drivers/i2c/busses/i2c-aspeed.c               | 54 +++++++++++++------
 drivers/i2c/i2c-core-base.c                   | 18 ++++++-
 4 files changed, 63 insertions(+), 18 deletions(-)

-- 
2.19.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 0/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
@ 2018-10-03 22:31 ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

In multi-master environment, this driver's master cannot know
exactly when peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver. To prevent state corruption in the
case, this patch adds checking code if any slave operation is
ongoing and it waits up to the bus timeout duration before starting
a master_xfer operation.

To support this change, it introduces changes on i2c-core-base to
make that able to read the bus timeout and master transfer retries
count values from device tree properties.

Please review this patch set.

Thanks,

-Jae

Changes since v3:
- Changed the property name to 'timeout' and made it use the
  default setting in i2c-core when not specified.

Changes since v2:
- Changed the property name to 'aspeed,timeout' and made it to
  update the adapter's timeout configuration.

Changes since v1:
- Changed define names of timeout related.

Jae Hyun Yoo (5):
  dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common
    optional
  i2c: Add support for 'timeout-ms' and '#retries' OF properties
  dt-bindings: i2c: aspeed: Add 'timeout-ms' property as an optional
    property
  i2c: aspeed: Remove hard-coded bus timeout value setting
  i2c: aspeed: Add bus idle waiting logic for multi-master use cases

 .../devicetree/bindings/i2c/i2c-aspeed.txt    |  3 ++
 Documentation/devicetree/bindings/i2c/i2c.txt |  6 +++
 drivers/i2c/busses/i2c-aspeed.c               | 54 +++++++++++++------
 drivers/i2c/i2c-core-base.c                   | 18 ++++++-
 4 files changed, 63 insertions(+), 18 deletions(-)

-- 
2.19.0

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 1/5] dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common optional
  2018-10-03 22:31 ` Jae Hyun Yoo
  (?)
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

This commit adds 'timeout-ms' and '#retries' properties as common
optional properties that can be used for setting 'timeout' and
'retries' values of 'struct i2c_adapter'. With this patch, the bus
timeout value and the master transfer retries count can be set
through these DT properties at the registration time of an adapter.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.

These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in this document.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 11263982470e..6d7a6e919cf8 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -80,6 +80,12 @@ wants to support one of the below features, it should adapt the bindings below.
 	Names of map programmable addresses.
 	It can contain any map needing another address than default one.
 
+- timeout-ms
+	Bus timeout in milliseconds.
+
+- #retries
+	Number of retries for master transfer.
+
 Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 1/5] dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common optional
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Vernon Mauery, Jae Hyun Yoo, Jarkko Nikula, James Feist

This commit adds 'timeout-ms' and '#retries' properties as common
optional properties that can be used for setting 'timeout' and
'retries' values of 'struct i2c_adapter'. With this patch, the bus
timeout value and the master transfer retries count can be set
through these DT properties at the registration time of an adapter.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.

These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in this document.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 11263982470e..6d7a6e919cf8 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -80,6 +80,12 @@ wants to support one of the below features, it should adapt the bindings below.
 	Names of map programmable addresses.
 	It can contain any map needing another address than default one.
 
+- timeout-ms
+	Bus timeout in milliseconds.
+
+- #retries
+	Number of retries for master transfer.
+
 Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 1/5] dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common optional
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds 'timeout-ms' and '#retries' properties as common
optional properties that can be used for setting 'timeout' and
'retries' values of 'struct i2c_adapter'. With this patch, the bus
timeout value and the master transfer retries count can be set
through these DT properties at the registration time of an adapter.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.

These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in this document.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index 11263982470e..6d7a6e919cf8 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -80,6 +80,12 @@ wants to support one of the below features, it should adapt the bindings below.
 	Names of map programmable addresses.
 	It can contain any map needing another address than default one.
 
+- timeout-ms
+	Bus timeout in milliseconds.
+
+- #retries
+	Number of retries for master transfer.
+
 Binding may contain optional "interrupts" property, describing interrupts
 used by the device. I2C core will assign "irq" interrupt (or the very first
 interrupt if not using interrupt names) as primary interrupt for the slave.
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 2/5] i2c: Add support for 'timeout-ms' and '#retries' OF properties
  2018-10-03 22:31 ` Jae Hyun Yoo
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

This commit adds support for 'timeout-ms' and '#retries' OF
properties to set 'timeout' and 'retries' values in
'struct i2c_adapter' in case an adapter node has the properties.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.

These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in the dt-bindings document.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/i2c-core-base.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9ee9a15e7134..b58aa17b287b 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1214,6 +1214,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
 static int i2c_register_adapter(struct i2c_adapter *adap)
 {
 	int res = -EINVAL;
+	u32 timeout_ms;
 
 	/* Can't register until after driver model init */
 	if (WARN_ON(!is_registered)) {
@@ -1239,8 +1240,21 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	INIT_LIST_HEAD(&adap->userspace_clients);
 
 	/* Set default timeout to 1 second if not already set */
-	if (adap->timeout == 0)
-		adap->timeout = HZ;
+	if (adap->timeout == 0) {
+		if (adap->dev.of_node)
+			res = of_property_read_u32(adap->dev.of_node,
+						   "timeout-ms",
+						   &timeout_ms);
+		if (res)
+			timeout_ms = 0;
+
+		adap->timeout = timeout_ms ? msecs_to_jiffies(timeout_ms) : HZ;
+	}
+
+	/* Set retries value if it has the OF property */
+	if (adap->dev.of_node)
+		of_property_read_u32(adap->dev.of_node, "#retries",
+				     &adap->retries);
 
 	/* register soft irqs for Host Notify */
 	res = i2c_setup_host_notify_irq_domain(adap);
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 2/5] i2c: Add support for 'timeout-ms' and '#retries' OF properties
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds support for 'timeout-ms' and '#retries' OF
properties to set 'timeout' and 'retries' values in
'struct i2c_adapter' in case an adapter node has the properties.
Still the values can be set by I2C_TIMEOUT and I2C_RETRIES ioctls
on cdev at runtime too.

These properties may not be supported by all drivers. However, if
a driver wants to support one of them, it should adapt the
bindings in the dt-bindings document.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/i2c-core-base.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9ee9a15e7134..b58aa17b287b 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1214,6 +1214,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
 static int i2c_register_adapter(struct i2c_adapter *adap)
 {
 	int res = -EINVAL;
+	u32 timeout_ms;
 
 	/* Can't register until after driver model init */
 	if (WARN_ON(!is_registered)) {
@@ -1239,8 +1240,21 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	INIT_LIST_HEAD(&adap->userspace_clients);
 
 	/* Set default timeout to 1 second if not already set */
-	if (adap->timeout == 0)
-		adap->timeout = HZ;
+	if (adap->timeout == 0) {
+		if (adap->dev.of_node)
+			res = of_property_read_u32(adap->dev.of_node,
+						   "timeout-ms",
+						   &timeout_ms);
+		if (res)
+			timeout_ms = 0;
+
+		adap->timeout = timeout_ms ? msecs_to_jiffies(timeout_ms) : HZ;
+	}
+
+	/* Set retries value if it has the OF property */
+	if (adap->dev.of_node)
+		of_property_read_u32(adap->dev.of_node, "#retries",
+				     &adap->retries);
 
 	/* register soft irqs for Host Notify */
 	res = i2c_setup_host_notify_irq_domain(adap);
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 3/5] dt-bindings: i2c: aspeed: Add 'timeout-ms' property as an optional property
  2018-10-03 22:31 ` Jae Hyun Yoo
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

This commit adds 'timeout-ms' property as an optional property
which can be used for setting the bus timeout value of an adapter.
With this patch, the bus timeout value can be set through this
DT property at the probing time of this module. Still the bus
timeout value can be set by an I2C_TIMEOUT ioctl on cdev at
runtime too.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
index 8fbd8633a387..af9be200383c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -17,6 +17,9 @@ Optional Properties:
 		  specified
 - multi-master	: states that there is another master active on this bus.
 
+- timeout-ms	: bus timeout in milliseconds defaults to 1 second when not
+		  specified.
+
 Example:
 
 i2c {
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 3/5] dt-bindings: i2c: aspeed: Add 'timeout-ms' property as an optional property
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds 'timeout-ms' property as an optional property
which can be used for setting the bus timeout value of an adapter.
With this patch, the bus timeout value can be set through this
DT property at the probing time of this module. Still the bus
timeout value can be set by an I2C_TIMEOUT ioctl on cdev at
runtime too.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
index 8fbd8633a387..af9be200383c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -17,6 +17,9 @@ Optional Properties:
 		  specified
 - multi-master	: states that there is another master active on this bus.
 
+- timeout-ms	: bus timeout in milliseconds defaults to 1 second when not
+		  specified.
+
 Example:
 
 i2c {
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
  2018-10-03 22:31 ` Jae Hyun Yoo
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

This commit removes hard-coded bus timeout value setting so that
it can be set by i2c-core-base.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..833b6b6a4c7e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -930,7 +930,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	init_completion(&bus->cmd_complete);
 	bus->adap.owner = THIS_MODULE;
 	bus->adap.retries = 0;
-	bus->adap.timeout = 5 * HZ;
 	bus->adap.algo = &aspeed_i2c_algo;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = pdev->dev.of_node;
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

This commit removes hard-coded bus timeout value setting so that
it can be set by i2c-core-base.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 8dc9161ced38..833b6b6a4c7e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -930,7 +930,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	init_completion(&bus->cmd_complete);
 	bus->adap.owner = THIS_MODULE;
 	bus->adap.retries = 0;
-	bus->adap.timeout = 5 * HZ;
 	bus->adap.algo = &aspeed_i2c_algo;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = pdev->dev.of_node;
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
  2018-10-03 22:31 ` Jae Hyun Yoo
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

In multi-master environment, this driver's master cannot know
exactly when peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver.

To prevent any state corruption in the case, this patch adds
checking code if any slave operation is ongoing and it waits up to
the bus timeout duration before starting a master_xfer operation.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 53 +++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 833b6b6a4c7e..8d60d7e5b323 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -12,6 +12,7 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
@@ -115,6 +116,9 @@
 /* 0x18 : I2CD Slave Device Address Register   */
 #define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
 
+/* Busy checking */
+#define ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US		(10 * 1000)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 	ASPEED_I2C_MASTER_START,
@@ -156,6 +160,8 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+	/* Multi-master */
+	bool				multi_master;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -596,27 +602,42 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 	return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
 }
 
+static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
+{
+	ktime_t timeout;
+
+	if (bus->multi_master) {
+		might_sleep();
+		timeout = ktime_add_ms(ktime_get(),
+				       jiffies_to_msecs(bus->adap.timeout));
+	}
+
+	for (;;) {
+		if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+		      ASPEED_I2CD_BUS_BUSY_STS) &&
+		    bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+			return 0;
+		if (!bus->multi_master)
+			break;
+		if (ktime_compare(ktime_get(), timeout) > 0)
+			break;
+		usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
+			     ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
+	}
+
+	return aspeed_i2c_recover_bus(bus);
+}
+
 static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
 				  struct i2c_msg *msgs, int num)
 {
 	struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
 	unsigned long time_left, flags;
-	int ret = 0;
 
-	spin_lock_irqsave(&bus->lock, flags);
-	bus->cmd_err = 0;
-
-	/* If bus is busy, attempt recovery. We assume a single master
-	 * environment.
-	 */
-	if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
-		spin_unlock_irqrestore(&bus->lock, flags);
-		ret = aspeed_i2c_recover_bus(bus);
-		if (ret)
-			return ret;
-		spin_lock_irqsave(&bus->lock, flags);
-	}
+	if (aspeed_i2c_check_bus_busy(bus))
+		return -EAGAIN;
 
+	spin_lock_irqsave(&bus->lock, flags);
 	bus->cmd_err = 0;
 	bus->msgs = msgs;
 	bus->msgs_index = 0;
@@ -827,7 +848,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	if (ret < 0)
 		return ret;
 
-	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		bus->multi_master = true;
+	else
 		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
 
 	/* Enable Master Mode */
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases
@ 2018-10-03 22:31   ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-03 22:31 UTC (permalink / raw)
  To: linux-arm-kernel

In multi-master environment, this driver's master cannot know
exactly when peer master sends data to this driver's slave so a
case can be happened that this master tries to send data through
the master_xfer function but slave data from peer master is still
being processed by this driver.

To prevent any state corruption in the case, this patch adds
checking code if any slave operation is ongoing and it waits up to
the bus timeout duration before starting a master_xfer operation.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 53 +++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 833b6b6a4c7e..8d60d7e5b323 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -12,6 +12,7 @@
 
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
@@ -115,6 +116,9 @@
 /* 0x18 : I2CD Slave Device Address Register   */
 #define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
 
+/* Busy checking */
+#define ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US		(10 * 1000)
+
 enum aspeed_i2c_master_state {
 	ASPEED_I2C_MASTER_INACTIVE,
 	ASPEED_I2C_MASTER_START,
@@ -156,6 +160,8 @@ struct aspeed_i2c_bus {
 	int				cmd_err;
 	/* Protected only by i2c_lock_bus */
 	int				master_xfer_result;
+	/* Multi-master */
+	bool				multi_master;
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	struct i2c_client		*slave;
 	enum aspeed_i2c_slave_state	slave_state;
@@ -596,27 +602,42 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 	return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
 }
 
+static int aspeed_i2c_check_bus_busy(struct aspeed_i2c_bus *bus)
+{
+	ktime_t timeout;
+
+	if (bus->multi_master) {
+		might_sleep();
+		timeout = ktime_add_ms(ktime_get(),
+				       jiffies_to_msecs(bus->adap.timeout));
+	}
+
+	for (;;) {
+		if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+		      ASPEED_I2CD_BUS_BUSY_STS) &&
+		    bus->slave_state == ASPEED_I2C_SLAVE_STOP)
+			return 0;
+		if (!bus->multi_master)
+			break;
+		if (ktime_compare(ktime_get(), timeout) > 0)
+			break;
+		usleep_range((ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US >> 2) + 1,
+			     ASPEED_I2C_BUS_BUSY_CHECK_INTERVAL_US);
+	}
+
+	return aspeed_i2c_recover_bus(bus);
+}
+
 static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
 				  struct i2c_msg *msgs, int num)
 {
 	struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
 	unsigned long time_left, flags;
-	int ret = 0;
 
-	spin_lock_irqsave(&bus->lock, flags);
-	bus->cmd_err = 0;
-
-	/* If bus is busy, attempt recovery. We assume a single master
-	 * environment.
-	 */
-	if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
-		spin_unlock_irqrestore(&bus->lock, flags);
-		ret = aspeed_i2c_recover_bus(bus);
-		if (ret)
-			return ret;
-		spin_lock_irqsave(&bus->lock, flags);
-	}
+	if (aspeed_i2c_check_bus_busy(bus))
+		return -EAGAIN;
 
+	spin_lock_irqsave(&bus->lock, flags);
 	bus->cmd_err = 0;
 	bus->msgs = msgs;
 	bus->msgs_index = 0;
@@ -827,7 +848,9 @@ static int aspeed_i2c_init(struct aspeed_i2c_bus *bus,
 	if (ret < 0)
 		return ret;
 
-	if (!of_property_read_bool(pdev->dev.of_node, "multi-master"))
+	if (of_property_read_bool(pdev->dev.of_node, "multi-master"))
+		bus->multi_master = true;
+	else
 		fun_ctrl_reg |= ASPEED_I2CD_MULTI_MASTER_DIS;
 
 	/* Enable Master Mode */
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
  2018-10-03 22:31   ` Jae Hyun Yoo
  (?)
@ 2018-10-04  8:32     ` Joel Stanley
  -1 siblings, 0 replies; 20+ messages in thread
From: Joel Stanley @ 2018-10-04  8:32 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Rob Herring, Mark Rutland, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, devicetree, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, James Feist,
	Vernon Mauery

On Thu, 4 Oct 2018 at 00:31, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit removes hard-coded bus timeout value setting so that
> it can be set by i2c-core-base.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
@ 2018-10-04  8:32     ` Joel Stanley
  0 siblings, 0 replies; 20+ messages in thread
From: Joel Stanley @ 2018-10-04  8:32 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Rob Herring, Mark Rutland, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, devicetree, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, James Feist,
	Vernon Mauery

On Thu, 4 Oct 2018 at 00:31, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit removes hard-coded bus timeout value setting so that
> it can be set by i2c-core-base.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
@ 2018-10-04  8:32     ` Joel Stanley
  0 siblings, 0 replies; 20+ messages in thread
From: Joel Stanley @ 2018-10-04  8:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 4 Oct 2018 at 00:31, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>
> This commit removes hard-coded bus timeout value setting so that
> it can be set by i2c-core-base.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Reviewed-by: Joel Stanley <joel@jms.id.au>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
  2018-10-04  8:32     ` Joel Stanley
@ 2018-10-04 16:11       ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-04 16:11 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Rob Herring, Mark Rutland, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, devicetree, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, James Feist,
	Vernon Mauery

On 10/4/2018 1:32 AM, Joel Stanley wrote:
> On Thu, 4 Oct 2018 at 00:31, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> This commit removes hard-coded bus timeout value setting so that
>> it can be set by i2c-core-base.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 

Thanks for the review, Joel!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting
@ 2018-10-04 16:11       ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-04 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 10/4/2018 1:32 AM, Joel Stanley wrote:
> On Thu, 4 Oct 2018 at 00:31, Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> This commit removes hard-coded bus timeout value setting so that
>> it can be set by i2c-core-base.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 

Thanks for the review, Joel!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH i2c-next v5 2/5] i2c: Add support for 'timeout-ms' and '#retries' OF properties
  2018-10-03 22:31   ` Jae Hyun Yoo
@ 2018-10-04 16:26     ` Jae Hyun Yoo
  -1 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-04 16:26 UTC (permalink / raw)
  To: Brendan Higgins, Wolfram Sang, Benjamin Herrenschmidt,
	Joel Stanley, Rob Herring, Mark Rutland, Andrew Jeffery,
	linux-i2c, openbmc, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery

> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1214,6 +1214,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
>   static int i2c_register_adapter(struct i2c_adapter *adap)
>   {
>   	int res = -EINVAL;
> +	u32 timeout_ms;

Will change it to:

u32 timeout_ms = 0;

>   
>   	/* Can't register until after driver model init */
>   	if (WARN_ON(!is_registered)) {
> @@ -1239,8 +1240,21 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>   	INIT_LIST_HEAD(&adap->userspace_clients);
>   
>   	/* Set default timeout to 1 second if not already set */
> -	if (adap->timeout == 0)
> -		adap->timeout = HZ;
> +	if (adap->timeout == 0) {
> +		if (adap->dev.of_node)
> +			res = of_property_read_u32(adap->dev.of_node,
> +						   "timeout-ms",
> +						   &timeout_ms);
> +		if (res)
> +			timeout_ms = 0;
> +

Also, it should be:

if (adap->dev.of_node)
	of_property_read_u32(adap->dev.of_node, "timeout-ms",
			     &timeout_ms);

Will submit a new patch set.

Thanks,
Jae

> +		adap->timeout = timeout_ms ? msecs_to_jiffies(timeout_ms) : HZ;
> +	}
> +
> +	/* Set retries value if it has the OF property */
> +	if (adap->dev.of_node)
> +		of_property_read_u32(adap->dev.of_node, "#retries",
> +				     &adap->retries);
>   
>   	/* register soft irqs for Host Notify */
>   	res = i2c_setup_host_notify_irq_domain(adap);
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH i2c-next v5 2/5] i2c: Add support for 'timeout-ms' and '#retries' OF properties
@ 2018-10-04 16:26     ` Jae Hyun Yoo
  0 siblings, 0 replies; 20+ messages in thread
From: Jae Hyun Yoo @ 2018-10-04 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1214,6 +1214,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
>   static int i2c_register_adapter(struct i2c_adapter *adap)
>   {
>   	int res = -EINVAL;
> +	u32 timeout_ms;

Will change it to:

u32 timeout_ms = 0;

>   
>   	/* Can't register until after driver model init */
>   	if (WARN_ON(!is_registered)) {
> @@ -1239,8 +1240,21 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>   	INIT_LIST_HEAD(&adap->userspace_clients);
>   
>   	/* Set default timeout to 1 second if not already set */
> -	if (adap->timeout == 0)
> -		adap->timeout = HZ;
> +	if (adap->timeout == 0) {
> +		if (adap->dev.of_node)
> +			res = of_property_read_u32(adap->dev.of_node,
> +						   "timeout-ms",
> +						   &timeout_ms);
> +		if (res)
> +			timeout_ms = 0;
> +

Also, it should be:

if (adap->dev.of_node)
	of_property_read_u32(adap->dev.of_node, "timeout-ms",
			     &timeout_ms);

Will submit a new patch set.

Thanks,
Jae

> +		adap->timeout = timeout_ms ? msecs_to_jiffies(timeout_ms) : HZ;
> +	}
> +
> +	/* Set retries value if it has the OF property */
> +	if (adap->dev.of_node)
> +		of_property_read_u32(adap->dev.of_node, "#retries",
> +				     &adap->retries);
>   
>   	/* register soft irqs for Host Notify */
>   	res = i2c_setup_host_notify_irq_domain(adap);
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-10-04 16:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 22:31 [PATCH i2c-next v5 0/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases Jae Hyun Yoo
2018-10-03 22:31 ` Jae Hyun Yoo
2018-10-03 22:31 ` [PATCH i2c-next v5 1/5] dt-bindings: i2c: Add 'timeout-ms' and '#retries' properties as common optional Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo
2018-10-03 22:31 ` [PATCH i2c-next v5 2/5] i2c: Add support for 'timeout-ms' and '#retries' OF properties Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo
2018-10-04 16:26   ` Jae Hyun Yoo
2018-10-04 16:26     ` Jae Hyun Yoo
2018-10-03 22:31 ` [PATCH i2c-next v5 3/5] dt-bindings: i2c: aspeed: Add 'timeout-ms' property as an optional property Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo
2018-10-03 22:31 ` [PATCH i2c-next v5 4/5] i2c: aspeed: Remove hard-coded bus timeout value setting Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo
2018-10-04  8:32   ` Joel Stanley
2018-10-04  8:32     ` Joel Stanley
2018-10-04  8:32     ` Joel Stanley
2018-10-04 16:11     ` Jae Hyun Yoo
2018-10-04 16:11       ` Jae Hyun Yoo
2018-10-03 22:31 ` [PATCH i2c-next v5 5/5] i2c: aspeed: Add bus idle waiting logic for multi-master use cases Jae Hyun Yoo
2018-10-03 22:31   ` Jae Hyun Yoo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.