i2c: designware: do not show SDA hold time warning when not needed
Subsystems (3)
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
When the I2C controller IP block has a revision too old to be able to configure the SDA hold time, the driver currently displays a warning. However, it does so unconditionally, even if no SDA hold time has been configured through the Device Tree. This causes useless warnings when running the system, so only show the warning if a SDA hold time was specified.
--- drivers/i2c/busses/i2c-designware-core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/i2c/busses/i2c-designware-core.c +3 -2
@@ -436,8 +436,9 @@ int i2c_dw_init(struct dw_i2c_dev *dev) dev->sda_hold_time |=1 <<DW_IC_SDA_HOLD_RX_SHIFT ;dw_writel (dev, dev->sda_hold_time,DW_IC_SDA_HOLD ); }else { -dev_warn (dev->dev, -"Hardware too old to adjust SDA hold time.\n" ); +if (dev->sda_hold_time) +dev_warn (dev->dev, +"Hardware too old to adjust SDA hold time.\n" ); }/* Configure Tx/Rx FIFO threshold levels */
Signature · 1 line
2.7.4
Andy Shevchenko
reviewerPATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Quoted 6 lines · Thomas Petazzoni
When the I2C controller IP block has a revision too old to be able to configure the SDA hold time, the driver currently displays a warning. However, it does so unconditionally, even if no SDA hold time has been configured through the Device Tree. This causes useless warnings when running the system, so only show the warning if a SDA hold time was specified.
As far as I understand the warning it would be better to keep it in either way, though you may shift it to debug level.
Wolfram, Jarkko, thoughts?
Quoted 26 lines
Signed-off-by Thomas Petazzoni <[email protected]>---
?drivers/i2c/busses/i2c-designware-core.c | 5 +++-- ?1 file changed, 3 insertions(+), 2 deletions(-)drivers/i2c/busses/i2c-designware-core.c +0
-0@@ -436,8 +436,9 @@ int i2c_dw_init(struct dw_i2c_dev *dev)?
dev->sda_hold_time|= 1 <<DW_IC_SDA_HOLD_RX_SHIFT;
?dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
? } else {drivers/i2c/busses/i2c-designware-core.c +0
-2- dev_warn(dev->dev, - "Hardware too old to adjust SDA holdtime.\n");
drivers/i2c/busses/i2c-designware-core.c +3
-0+if (dev->sda_hold_time) + dev_warn(dev->dev, + ?"Hardware too old to adjust SDA holdtime.\n");
? }
?
? /* Configure Tx/Rx FIFO threshold levels */
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Hello,
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Thomas Petazzoni
When the I2C controller IP block has a revision too old to be able to configure the SDA hold time, the driver currently displays a warning. However, it does so unconditionally, even if no SDA hold time has been configured through the Device Tree. This causes useless warnings when running the system, so only show the warning if a SDA hold time was specified.
Quoted 4 lines
As far as I understand the warning it would be better to keep it in either way, though you may shift it to debug level.
Wolfram, Jarkko, thoughts?
Why show a message when the user has not requested a custom SDA hold time? Getting a warning about something you haven't requested seems really odd.
I think it makes a lot more sense to keep it at the warning level (because it's important to get this message if you configure a custom SDA hold time), but only show it when appropriate.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Quoted 16 lines · Thomas Petazzoni
Hello,
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Thomas Petazzoni
When the I2C controller IP block has a revision too old to be able to configure the SDA hold time, the driver currently displays a warning. However, it does so unconditionally, even if no SDA hold time has been configured through the Device Tree. This causes useless warnings when running the system, so only show the warning if a SDA hold time was specified.
Quoted 4 lines
As far as I understand the warning it would be better to keep it in either way, though you may shift it to debug level.
Wolfram, Jarkko, thoughts?
Quoted 8 lines
Why show a message when the user has not requested a custom SDA hold time? Getting a warning about something you haven't requested seems really odd.
I think it makes a lot more sense to keep it at the warning level (because it's important to get this message if you configure a custom SDA hold time), but only show it when appropriate.
I guess warning over debug level could have slightly better chance to prevent someone not adding needless "i2c-sda-hold-time-ns" property in a hardware that doesn't support SDA hold time. But needless spamming have negative value so this is worth to fix. (I would do this as a single liner by else if ()).
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Quoted 6 lines
hardware that doesn't support SDA hold time. But needless spamming have negative value so this is worth to fix. (I would do this as a single liner by else if ()).
Acked-by Jarkko Nikula <[email protected]>
I agree with Jarkko on everything, including the 'else if' fix.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170602/93213df3/attachment.sig>
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Hello,
Quoted 7 lines · Wolfram Sang
Quoted 5 lines
hardware that doesn't support SDA hold time. But needless spamming have negative value so this is worth to fix. (I would do this as a single liner by else if ()).
Acked-by Jarkko Nikula <[email protected]>I agree with Jarkko on everything, including the 'else if' fix.
I am not sure to understand what Jarkko suggested to fix, besides using "else if" in one line. Could you be more specific about the changes you expect me to do?
Andy suggested to turn the warning into a debug message, to which I objected, and my understanding was that Jarkko agreed with me that a warning message was better, and only made the suggestion of the "else if".
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 i2c: designware: do not show SDA hold time warning when not needed
Hi Thomas,
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 4 lines · Wolfram Sang
Quoted 4 lines
hardware that doesn't support SDA hold time. But needless spamming have negative value so this is worth to fix. (I would do this as a single liner by else if ()).
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Acked-by Jarkko Nikula <[email protected]>I agree with Jarkko on everything, including the 'else if' fix.
I am not sure to understand what Jarkko suggested to fix, besides using "else if" in one line. Could you be more specific about the changes you expect me to do?
I agree with Jarkko that needless spamming is worth fixing. Yet, it should be done as a single line using "else if". So, basically the paragraph I quoted from him. More clear now?
Regards,
Wolfram
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170603/96a6744b/attachment.sig>