| Message ID | 20180926215842.23125-2-jae.hyun.yoo@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | i2c: aspeed: Add bus idle waiting logic for multi-master use cases | expand |
On Wed, Sep 26, 2018 at 02:58:40PM -0700, Jae Hyun Yoo wrote: > This commit adds 'aspeed,timeout' property as an optional property > which can be used for setting 'timeout' value of > 'struct i2c_adapter'. With this patch, the timeout value can be > set through an I2C_TIMEOUT ioctl on cdev, or through this optional > DT property. Isn't controlling this from userspace or relying on a default sufficient? I can't see this needing to be highly tuned for each platform. However, if we do have a property, it should be common. > > 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..d6965b360fbc 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. > > +- aspeed,timeout : I2C bus timeout in microseconds defaults to 5 seconds when > + not specified. > + > Example: > > i2c { > -- > 2.19.0 >
Hi Rob, On 9/27/2018 1:56 PM, Rob Herring wrote: > On Wed, Sep 26, 2018 at 02:58:40PM -0700, Jae Hyun Yoo wrote: >> This commit adds 'aspeed,timeout' property as an optional property >> which can be used for setting 'timeout' value of >> 'struct i2c_adapter'. With this patch, the timeout value can be >> set through an I2C_TIMEOUT ioctl on cdev, or through this optional >> DT property. > > Isn't controlling this from userspace or relying on a default > sufficient? I can't see this needing to be highly tuned for each > platform. > It can be controlled using an ioctl command on an I2C cdev from userspace if CONFIG_I2C_CHARDEV is enabled. A couple of I2C drivers use their own specific default value for it but in general the common default value (1 second) which is set by i2c core is sufficient. But it still needs to be tuned for specific cases based on attached devices' characteristic, on packet length, on bus speed and on etc. Specifically in Aspeed I2C driver for BMC, it should be tuned to support multi-master use cases properly, and it needs a device tree property to apply this timeout value from the probing time of the module. > However, if we do have a property, it should be common. > Okay, I'll change it to 'timeout'. Thanks a lot, Jae
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt index 8fbd8633a387..d6965b360fbc 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. +- aspeed,timeout : I2C bus timeout in microseconds defaults to 5 seconds when + not specified. + Example: i2c {
This commit adds 'aspeed,timeout' property as an optional property which can be used for setting 'timeout' value of 'struct i2c_adapter'. With this patch, the timeout value can be set through an I2C_TIMEOUT ioctl on cdev, or through this optional DT property. 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(+)