| Message ID | 20250415075230.16235-1-johan+linaro@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: omap: fix deprecated of_property_read_bool() use | expand |
On 4/15/2025 1:22 PM, Johan Hovold wrote: > Using of_property_read_bool() for non-boolean properties is deprecated > and results in a warning during runtime since commit c141ecc3cecd ("of: > Warn when of_property_read_bool() is used on non-boolean properties"). > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > Cc: Jayesh Choudhary <j-choudhary@ti.com> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/i2c/busses/i2c-omap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 16afb9ca19bb..876791d20ed5 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -1454,7 +1454,7 @@ omap_i2c_probe(struct platform_device *pdev) > (1000 * omap->speed / 8); > } > > - if (of_property_read_bool(node, "mux-states")) { > + if (of_property_present(node, "mux-states")) { > struct mux_state *mux_state; > > mux_state = devm_mux_state_get(&pdev->dev, NULL); Acked-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
Hi Johan, On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > Using of_property_read_bool() for non-boolean properties is deprecated > and results in a warning during runtime since commit c141ecc3cecd ("of: > Warn when of_property_read_bool() is used on non-boolean properties"). > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > Cc: Jayesh Choudhary <j-choudhary@ti.com> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Thanks for your patch! I'm going to drop the Fixes tag, as this isn't really a bug fix but rather a warning suppression during boot time. Thanks, Andi
On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > Using of_property_read_bool() for non-boolean properties is deprecated > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > Thanks for your patch! I'm going to drop the Fixes tag, as this > isn't really a bug fix but rather a warning suppression during > boot time. forgot to say that I merged the patch in i2c/i2c-host. Thanks, Andi
On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > Using of_property_read_bool() for non-boolean properties is deprecated > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > Thanks for your patch! I'm going to drop the Fixes tag, as this > isn't really a bug fix but rather a warning suppression during > boot time. Thanks, but I think you should have kept the Fixes tag and merged this for 6.15 (i2c-host-fixes) since this is a new warning in 6.15-rc1 (and that does warrant a Fixes tag). Perhaps I should have highlighted that better. If the offending patch had been posted or merged before such uses started generating warnings in 6.14-rc1 then that would have been a different matter. Johan
Hi Johan, On Fri, Apr 18, 2025 at 11:57:57AM +0200, Johan Hovold wrote: > On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > > Using of_property_read_bool() for non-boolean properties is deprecated > > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > > Thanks for your patch! I'm going to drop the Fixes tag, as this > > isn't really a bug fix but rather a warning suppression during > > boot time. > > Thanks, but I think you should have kept the Fixes tag and merged this > for 6.15 (i2c-host-fixes) since this is a new warning in 6.15-rc1 (and > that does warrant a Fixes tag). Perhaps I should have highlighted that > better. > > If the offending patch had been posted or merged before such uses > started generating warnings in 6.14-rc1 then that would have been a > different matter. I'm sorry, but as I understand it, the Fixes tag should be used only when an actual bug is being fixed. I've seen stable maintainers getting annoyed when it's used for non-bug issues. The system works perfectly fine even with the warning printed. It might confuse CI systems, but that shouldn't really be our concern. In any case, I see your point and I'm open to hearing a third opinion. Thanks, Andi
Am Tue, 29 Apr 2025 15:10:13 +0200 schrieb Andi Shyti <andi.shyti@kernel.org>: > Hi Johan, > > On Fri, Apr 18, 2025 at 11:57:57AM +0200, Johan Hovold wrote: > > On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > > > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > > > Using of_property_read_bool() for non-boolean properties is deprecated > > > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > > > > Thanks for your patch! I'm going to drop the Fixes tag, as this > > > isn't really a bug fix but rather a warning suppression during > > > boot time. > > > > Thanks, but I think you should have kept the Fixes tag and merged this > > for 6.15 (i2c-host-fixes) since this is a new warning in 6.15-rc1 (and > > that does warrant a Fixes tag). Perhaps I should have highlighted that > > better. > > > > If the offending patch had been posted or merged before such uses > > started generating warnings in 6.14-rc1 then that would have been a > > different matter. > > I'm sorry, but as I understand it, the Fixes tag should be used > only when an actual bug is being fixed. I've seen stable > maintainers getting annoyed when it's used for non-bug issues. > hmm, some issue new in -rc1 could be fixed in a later -rcX. I have seen a lot of typos and other minor stuff getting fixed that way. So it does not need to be backported to any stable/longterm tree at all. Are the rules for that really that tough as for stable trees? I really doubt. Regards, Andreas
On Tue, Apr 29, 2025 at 03:10:13PM +0200, Andi Shyti wrote: > On Fri, Apr 18, 2025 at 11:57:57AM +0200, Johan Hovold wrote: > > On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > > > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > > > Using of_property_read_bool() for non-boolean properties is deprecated > > > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > > > > Thanks for your patch! I'm going to drop the Fixes tag, as this > > > isn't really a bug fix but rather a warning suppression during > > > boot time. > > > > Thanks, but I think you should have kept the Fixes tag and merged this > > for 6.15 (i2c-host-fixes) since this is a new warning in 6.15-rc1 (and > > that does warrant a Fixes tag). Perhaps I should have highlighted that > > better. > > > > If the offending patch had been posted or merged before such uses > > started generating warnings in 6.14-rc1 then that would have been a > > different matter. > > I'm sorry, but as I understand it, the Fixes tag should be used > only when an actual bug is being fixed. I've seen stable > maintainers getting annoyed when it's used for non-bug issues. You seem to confuse the Fixes tag with a CC stable tag. A Fixes tag is used to indicate which commit introduced an issue, while the CC stable tag is used to flag a commit for backporting (and the fact that autosel tends to pick up patches with just a Fixes doesn't change this). It's perfectly fine to fix an issue and use a Fixes tag when doing so even if the fix itself does not qualify for backporting (for whatever reason). > The system works perfectly fine even with the warning printed. > It might confuse CI systems, but that shouldn't really be our > concern. You should not knowingly be introducing new warnings. The Fixes tag I added showed that this was an issue introduced in 6.15-rc1, and, unless discovered really late in the cycle, it should be fixed before 6.15 is out. Johan
Hi Johan, On Mon, May 05, 2025 at 12:00:07PM +0200, Johan Hovold wrote: > On Tue, Apr 29, 2025 at 03:10:13PM +0200, Andi Shyti wrote: > > On Fri, Apr 18, 2025 at 11:57:57AM +0200, Johan Hovold wrote: > > > On Thu, Apr 17, 2025 at 11:41:51PM +0200, Andi Shyti wrote: > > > > On Tue, Apr 15, 2025 at 09:52:30AM +0200, Johan Hovold wrote: > > > > > Using of_property_read_bool() for non-boolean properties is deprecated > > > > > and results in a warning during runtime since commit c141ecc3cecd ("of: > > > > > Warn when of_property_read_bool() is used on non-boolean properties"). > > > > > > > > > > Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") > > > > > Cc: Jayesh Choudhary <j-choudhary@ti.com> > > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > > > > > > > Thanks for your patch! I'm going to drop the Fixes tag, as this > > > > isn't really a bug fix but rather a warning suppression during > > > > boot time. > > > > > > Thanks, but I think you should have kept the Fixes tag and merged this > > > for 6.15 (i2c-host-fixes) since this is a new warning in 6.15-rc1 (and > > > that does warrant a Fixes tag). Perhaps I should have highlighted that > > > better. > > > > > > If the offending patch had been posted or merged before such uses > > > started generating warnings in 6.14-rc1 then that would have been a > > > different matter. > > > > I'm sorry, but as I understand it, the Fixes tag should be used > > only when an actual bug is being fixed. I've seen stable > > maintainers getting annoyed when it's used for non-bug issues. > > You seem to confuse the Fixes tag with a CC stable tag. A Fixes tag is > used to indicate which commit introduced an issue, while the CC stable > tag is used to flag a commit for backporting (and the fact that autosel > tends to pick up patches with just a Fixes doesn't change this). (the Cc tag for fixes is not mandatory, it's more a courtesy) > It's perfectly fine to fix an issue and use a Fixes tag when doing so > even if the fix itself does not qualify for backporting (for whatever > reason). Oh yes, I forgot that patch was part of the 6.15 merge window. I will then move it to the -fixes and send it for this week's merge request. Thanks and sorry for the confusion, Andi > > The system works perfectly fine even with the warning printed. > > It might confuse CI systems, but that shouldn't really be our > > concern. > > You should not knowingly be introducing new warnings. The Fixes tag I > added showed that this was an issue introduced in 6.15-rc1, and, unless > discovered really late in the cycle, it should be fixed before 6.15 is > out. > > Johan
On Tue, May 06, 2025 at 12:13:00AM +0200, Andi Shyti wrote: > On Mon, May 05, 2025 at 12:00:07PM +0200, Johan Hovold wrote: > > On Tue, Apr 29, 2025 at 03:10:13PM +0200, Andi Shyti wrote: > > > I'm sorry, but as I understand it, the Fixes tag should be used > > > only when an actual bug is being fixed. I've seen stable > > > maintainers getting annoyed when it's used for non-bug issues. > > > > You seem to confuse the Fixes tag with a CC stable tag. A Fixes tag is > > used to indicate which commit introduced an issue, while the CC stable > > tag is used to flag a commit for backporting (and the fact that autosel > > tends to pick up patches with just a Fixes doesn't change this). > > (the Cc tag for fixes is not mandatory, it's more a courtesy) You should still add it as described by the stable tree docs: To have a patch you submit for mainline inclusion later automatically picked up for stable trees, add this tag in the sign-off area:: Cc: stable@vger.kernel.org The stable team also scans through patches with just a Fixes tag because some people forget to add the tag, don't know that they should, or don't care about stable, but you should not rely on that (as I alluded to above). > > It's perfectly fine to fix an issue and use a Fixes tag when doing so > > even if the fix itself does not qualify for backporting (for whatever > > reason). > > Oh yes, I forgot that patch was part of the 6.15 merge window. I > will then move it to the -fixes and send it for this week's merge > request. Perfect, thanks. Johan
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 16afb9ca19bb..876791d20ed5 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1454,7 +1454,7 @@ omap_i2c_probe(struct platform_device *pdev) (1000 * omap->speed / 8); } - if (of_property_read_bool(node, "mux-states")) { + if (of_property_present(node, "mux-states")) { struct mux_state *mux_state; mux_state = devm_mux_state_get(&pdev->dev, NULL);
Using of_property_read_bool() for non-boolean properties is deprecated and results in a warning during runtime since commit c141ecc3cecd ("of: Warn when of_property_read_bool() is used on non-boolean properties"). Fixes: b6ef830c60b6 ("i2c: omap: Add support for setting mux") Cc: Jayesh Choudhary <j-choudhary@ti.com> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/i2c/busses/i2c-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)