* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-04-14 20:53 ` Thomas Petazzoni
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-04-14 20:53 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Wolfram Sang
Cc: linux-i2c, linux-arm-kernel, 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.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/i2c/busses/i2c-designware-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 7a3faa5..7f9da6e 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -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 */
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-04-14 20:53 ` Thomas Petazzoni
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-04-14 20:53 UTC (permalink / raw)
To: linux-arm-kernel
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.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/i2c/busses/i2c-designware-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 7a3faa5..7f9da6e 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -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 */
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-04-14 20:53 ` Thomas Petazzoni
@ 2017-04-18 7:54 ` Andy Shevchenko
-1 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2017-04-18 7:54 UTC (permalink / raw)
To: Thomas Petazzoni, Jarkko Nikula, Mika Westerberg, Wolfram Sang
Cc: linux-i2c, linux-arm-kernel
On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
> 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?
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> drivers/i2c/busses/i2c-designware-core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c
> b/drivers/i2c/busses/i2c-designware-core.c
> index 7a3faa5..7f9da6e 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -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 */
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-04-18 7:54 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2017-04-18 7:54 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
> 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?
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> ?drivers/i2c/busses/i2c-designware-core.c | 5 +++--
> ?1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c
> b/drivers/i2c/busses/i2c-designware-core.c
> index 7a3faa5..7f9da6e 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -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 */
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-04-18 7:54 ` Andy Shevchenko
@ 2017-04-18 7:59 ` Thomas Petazzoni
-1 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-04-18 7:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mika Westerberg, Jarkko Nikula, linux-i2c, linux-arm-kernel,
Wolfram Sang
Hello,
On Tue, 18 Apr 2017 10:54:47 +0300, Andy Shevchenko wrote:
> On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
> > 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?
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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-04-18 7:59 ` Thomas Petazzoni
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-04-18 7:59 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Tue, 18 Apr 2017 10:54:47 +0300, Andy Shevchenko wrote:
> On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
> > 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?
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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-04-18 7:59 ` Thomas Petazzoni
@ 2017-04-18 10:23 ` Jarkko Nikula
-1 siblings, 0 replies; 14+ messages in thread
From: Jarkko Nikula @ 2017-04-18 10:23 UTC (permalink / raw)
To: Thomas Petazzoni, Andy Shevchenko
Cc: Mika Westerberg, Wolfram Sang, linux-i2c, linux-arm-kernel
On 04/18/2017 10:59 AM, Thomas Petazzoni wrote:
> Hello,
>
> On Tue, 18 Apr 2017 10:54:47 +0300, Andy Shevchenko wrote:
>> On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
>>> 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?
>
> 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 ()).
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-04-18 10:23 ` Jarkko Nikula
0 siblings, 0 replies; 14+ messages in thread
From: Jarkko Nikula @ 2017-04-18 10:23 UTC (permalink / raw)
To: linux-arm-kernel
On 04/18/2017 10:59 AM, Thomas Petazzoni wrote:
> Hello,
>
> On Tue, 18 Apr 2017 10:54:47 +0300, Andy Shevchenko wrote:
>> On Fri, 2017-04-14 at 22:53 +0200, Thomas Petazzoni wrote:
>>> 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?
>
> 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 ()).
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-04-18 10:23 ` Jarkko Nikula
@ 2017-06-02 20:15 ` Wolfram Sang
-1 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2017-06-02 20:15 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Thomas Petazzoni, Andy Shevchenko, Mika Westerberg, linux-i2c,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 306 bytes --]
> 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 <jarkko.nikula@linux.intel.com>
I agree with Jarkko on everything, including the 'else if' fix.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-06-02 20:15 ` Wolfram Sang
0 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2017-06-02 20:15 UTC (permalink / raw)
To: linux-arm-kernel
> 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 <jarkko.nikula@linux.intel.com>
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-06-02 20:15 ` Wolfram Sang
@ 2017-06-03 13:18 ` Thomas Petazzoni
-1 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-06-03 13:18 UTC (permalink / raw)
To: Wolfram Sang
Cc: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, linux-i2c,
linux-arm-kernel
Hello,
On Fri, 2 Jun 2017 22:15:02 +0200, Wolfram Sang wrote:
> > 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 <jarkko.nikula@linux.intel.com>
>
> 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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-06-03 13:18 ` Thomas Petazzoni
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2017-06-03 13:18 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Fri, 2 Jun 2017 22:15:02 +0200, Wolfram Sang wrote:
> > 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 <jarkko.nikula@linux.intel.com>
>
> 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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: designware: do not show SDA hold time warning when not needed
2017-06-03 13:18 ` Thomas Petazzoni
@ 2017-06-03 21:25 ` Wolfram Sang
-1 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2017-06-03 21:25 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, linux-i2c,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
Hi Thomas,
On Sat, Jun 03, 2017 at 03:18:21PM +0200, Thomas Petazzoni wrote:
> Hello,
>
> On Fri, 2 Jun 2017 22:15:02 +0200, Wolfram Sang wrote:
> > > 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 <jarkko.nikula@linux.intel.com>
> >
> > 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] i2c: designware: do not show SDA hold time warning when not needed
@ 2017-06-03 21:25 ` Wolfram Sang
0 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2017-06-03 21:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
On Sat, Jun 03, 2017 at 03:18:21PM +0200, Thomas Petazzoni wrote:
> Hello,
>
> On Fri, 2 Jun 2017 22:15:02 +0200, Wolfram Sang wrote:
> > > 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 <jarkko.nikula@linux.intel.com>
> >
> > 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>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-06-03 21:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-14 20:53 [PATCH] i2c: designware: do not show SDA hold time warning when not needed Thomas Petazzoni
2017-04-14 20:53 ` Thomas Petazzoni
2017-04-18 7:54 ` Andy Shevchenko
2017-04-18 7:54 ` Andy Shevchenko
2017-04-18 7:59 ` Thomas Petazzoni
2017-04-18 7:59 ` Thomas Petazzoni
2017-04-18 10:23 ` Jarkko Nikula
2017-04-18 10:23 ` Jarkko Nikula
2017-06-02 20:15 ` Wolfram Sang
2017-06-02 20:15 ` Wolfram Sang
2017-06-03 13:18 ` Thomas Petazzoni
2017-06-03 13:18 ` Thomas Petazzoni
2017-06-03 21:25 ` Wolfram Sang
2017-06-03 21:25 ` Wolfram Sang
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.