[PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Jean Delvare]

When CONFIG_OF isn't set, we can optimize out dmard06_of_match as it
will never be used.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/accel/dmard06.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-5.19.orig/drivers/iio/accel/dmard06.c	2022-08-25 14:19:11.742351430 +0200
+++ linux-5.19/drivers/iio/accel/dmard06.c	2022-08-25 14:20:13.505276596 +0200
@@ -209,7 +209,7 @@ static const struct i2c_device_id dmard0
 };
 MODULE_DEVICE_TABLE(i2c, dmard06_id);
 
-static const struct of_device_id dmard06_of_match[] = {
+static const struct of_device_id __maybe_unused dmard06_of_match[] = {
 	{ .compatible = "domintech,dmard05" },
 	{ .compatible = "domintech,dmard06" },
 	{ .compatible = "domintech,dmard07" },
@@ -222,7 +222,7 @@ static struct i2c_driver dmard06_driver
 	.id_table = dmard06_id,
 	.driver = {
 		.name = DMARD06_DRV_NAME,
-		.of_match_table = dmard06_of_match,
+		.of_match_table = of_match_ptr(dmard06_of_match),
 		.pm = pm_sleep_ptr(&dmard06_pm_ops),
 	},
 };


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Andy Shevchenko]

On Thu, Aug 25, 2022 at 3:54 PM Jean Delvare <jdelvare@suse.de> wrote:
>
> When CONFIG_OF isn't set, we can optimize out dmard06_of_match as it
> will never be used.

NACK. There is a magic PRP0001 for ACPI case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Jean Delvare]

Hi Andy,

On Thu, 25 Aug 2022 23:12:43 +0300, Andy Shevchenko wrote:
> On Thu, Aug 25, 2022 at 3:54 PM Jean Delvare <jdelvare@suse.de> wrote:
> >
> > When CONFIG_OF isn't set, we can optimize out dmard06_of_match as it
> > will never be used.  
> 
> NACK. There is a magic PRP0001 for ACPI case.

OK, I wasn't aware of this. As of_match_device() is a stub when
CONFIG_OF isn't set, I thought the table could never be used. But now I
see that the acpi subsystem accesses the table directly, so you are
correct, the optimization I suggested is bogus.

Now I'm curious, is there a well-defined subset of device names that
can be found in an ACPI table? If not, does that mean that any driver
with an OF entry could match, therefore of_match_ptr() should be
removed from the kernel entirely?

Another possibility would be to stub out of_match_ptr() only if neither
CONFIG_OF nor CONFIG_ACPI is set. But I'm not sure that would be worth,
as I expected either to be set on almost all kernels in practice
(except on s390x, but you wouldn't build any of these drivers there
anyway).

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Andy Shevchenko]

On Fri, Aug 26, 2022 at 1:46 PM Jean Delvare <jdelvare@suse.de> wrote:
> On Thu, 25 Aug 2022 23:12:43 +0300, Andy Shevchenko wrote:
> > On Thu, Aug 25, 2022 at 3:54 PM Jean Delvare <jdelvare@suse.de> wrote:

...

> Now I'm curious, is there a well-defined subset of device names that
> can be found in an ACPI table? If not, does that mean that any driver
> with an OF entry could match,

Yes, anything can be matched by ACPI with any of the compatible strings.

> therefore of_match_ptr() should be
> removed from the kernel entirely?

In most cases yes, like for discrete components that can be connected
to any SoC on ACPI/DT/whatever platform. But for some cases it still
makes sense: platform is known to never be non-OF, component is known
to be used only on such platforms, etc.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Jean Delvare]

On Fri, 26 Aug 2022 18:18:20 +0300, Andy Shevchenko wrote:
> On Fri, Aug 26, 2022 at 1:46 PM Jean Delvare <jdelvare@suse.de> wrote:
> > therefore of_match_ptr() should be
> > removed from the kernel entirely?  
> 
> (...) But for some cases it still
> makes sense: platform is known to never be non-OF, component is known
> to be used only on such platforms, etc.

Well, I can't see the value of of_match_ptr() in such case either. In
fact I've submitted a couple patches to remove such occurrences lately:

https://patchwork.kernel.org/project/linux-mediatek/patch/20220730144833.0a0d9825@endymion.delvare/
https://patchwork.kernel.org/project/linux-pm/patch/20220804135938.7f69f5d9@endymion.delvare/

-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Andy Shevchenko]

On Fri, Aug 26, 2022 at 7:06 PM Jean Delvare <jdelvare@suse.de> wrote:
> On Fri, 26 Aug 2022 18:18:20 +0300, Andy Shevchenko wrote:
> > On Fri, Aug 26, 2022 at 1:46 PM Jean Delvare <jdelvare@suse.de> wrote:
> > > therefore of_match_ptr() should be
> > > removed from the kernel entirely?
> >
> > (...) But for some cases it still
> > makes sense: platform is known to never be non-OF, component is known
> > to be used only on such platforms, etc.
>
> Well, I can't see the value of of_match_ptr() in such case either. In
> fact I've submitted a couple patches to remove such occurrences lately:
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20220730144833.0a0d9825@endymion.delvare/
> https://patchwork.kernel.org/project/linux-pm/patch/20220804135938.7f69f5d9@endymion.delvare/

They are different to what we are discussing here, but yes, in common
denominator the of_match_ptr() is useless in almost 100% cases.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio:accel:dmard06: Optimize when CONFIG_OF isn't set

[Jonathan Cameron]

On Fri, 26 Aug 2022 19:10:33 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Aug 26, 2022 at 7:06 PM Jean Delvare <jdelvare@suse.de> wrote:
> > On Fri, 26 Aug 2022 18:18:20 +0300, Andy Shevchenko wrote:  
> > > On Fri, Aug 26, 2022 at 1:46 PM Jean Delvare <jdelvare@suse.de> wrote:  
> > > > therefore of_match_ptr() should be
> > > > removed from the kernel entirely?  
> > >
> > > (...) But for some cases it still
> > > makes sense: platform is known to never be non-OF, component is known
> > > to be used only on such platforms, etc.  
> >
> > Well, I can't see the value of of_match_ptr() in such case either. In
> > fact I've submitted a couple patches to remove such occurrences lately:
> >
> > https://patchwork.kernel.org/project/linux-mediatek/patch/20220730144833.0a0d9825@endymion.delvare/
> > https://patchwork.kernel.org/project/linux-pm/patch/20220804135938.7f69f5d9@endymion.delvare/  
> 
> They are different to what we are discussing here, but yes, in common
> denominator the of_match_ptr() is useless in almost 100% cases.
> 

Agreed. Ever since PRP0001 came in, it's made little to no sense to have
an of_match_ptr() but it's perhaps also not worth the noise of generally
removing 1588 cases of it!

As a side note, of_match_ptr() did make sense, then moving to trickery along the lines
of what was recently done for pm_ptr() to get rid of the need for __maybe_unused
would be good.

#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))

I 'think' the same trick could be used to make the use of the array visible
to the compiler but still allow it to remove the array. Not actually tried
it though...

Jonathan