[PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Daniel Palmer]

When building kernels without ACPI support the table is declared
but is not used because ACPI_PTR() turns it into a NULL.

Add the __maybe_unused attribute to stop the compiler whining.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index a3a0154da567..599d2bfff045 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -704,7 +704,7 @@ static const struct of_device_id dw8250_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, dw8250_of_match);
 
-static const struct acpi_device_id dw8250_acpi_match[] = {
+static const struct acpi_device_id __maybe_unused dw8250_acpi_match[] = {
 	{ "INT33C4", 0 },
 	{ "INT33C5", 0 },
 	{ "INT3434", 0 },
-- 
2.33.0

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Andy Shevchenko]

On Thu, Sep 30, 2021 at 09:49:50PM +0900, Daniel Palmer wrote:
> When building kernels without ACPI support the table is declared
> but is not used because ACPI_PTR() turns it into a NULL.

Okay.

> Add the __maybe_unused attribute to stop the compiler whining.

And incorrect fix. See my patches regarding to the topic
(`git log --grep ACPI_PTR`) and do accordingly, i.e. drop
ACPI_PTR() for good.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Daniel Palmer]

Hi Andy,

On Fri, 1 Oct 2021 at 00:24, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> And incorrect fix. See my patches regarding to the topic
> (`git log --grep ACPI_PTR`) and do accordingly, i.e. drop
> ACPI_PTR() for good.

Something like 349bff48ae0f5f8aa2075d0bdc2091a30bd634f6?

Doesn't this mean the ACPI table ends up in kernels that will never use ACPI?

Cheers,

Daniel

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Andy Shevchenko]

On Fri, Oct 01, 2021 at 12:31:34AM +0900, Daniel Palmer wrote:
> On Fri, 1 Oct 2021 at 00:24, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > And incorrect fix. See my patches regarding to the topic
> > (`git log --grep ACPI_PTR`) and do accordingly, i.e. drop
> > ACPI_PTR() for good.
> 
> Something like 349bff48ae0f5f8aa2075d0bdc2091a30bd634f6?
> 
> Doesn't this mean the ACPI table ends up in kernels that will never use ACPI?

Yes. Is it a problem (*)? If so, you need to use ifdeffery, since __maybe_unused is
not for the ID tables.

*) while justifying this you also need to show why it's a problem specific
to the ACPI IDs and not a problem for OF ones, which we have tons of in the
Linux kernel without any guards (ifdeffery).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Daniel Palmer]

Hi Andy,

On Fri, 1 Oct 2021 at 06:04, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> > Doesn't this mean the ACPI table ends up in kernels that will never use ACPI?
>
> Yes. Is it a problem (*)? If so, you need to use ifdeffery, since __maybe_unused is
> not for the ID tables.

Ok, is there a reason it's not for the ID tables? Does it break something?

> *) while justifying this you also need to show why it's a problem specific
> to the ACPI IDs and not a problem for OF ones, which we have tons of in the
> Linux kernel without any guards (ifdeffery).

To be honest I don't care about this too much. I just wanted to cut
down some of the noise when I build my patch backlog so that warnings
in the stuff I'm trying to mainline are more visible.

For what it's worth I think the OF ids are a bit wasteful. For some
drivers where there are tons of broken variations they add a few K of
unneeded data. But since everyone now has gigabytes of memory I doubt
they care...
I'm working with 64MB. :)

Cheers,

Daniel

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Andy Shevchenko]

On Fri, Oct 01, 2021 at 09:16:24AM +0900, Daniel Palmer wrote:
> On Fri, 1 Oct 2021 at 06:04, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > > Doesn't this mean the ACPI table ends up in kernels that will never use ACPI?
> >
> > Yes. Is it a problem (*)? If so, you need to use ifdeffery, since __maybe_unused is
> > not for the ID tables.
> 
> Ok, is there a reason it's not for the ID tables? Does it break something?

It will look ugly. Why we define a table that may or may not be used?
Sounds fishy.

On top of that why you should tell linker to waste resources on something
that you may well know beforehand will be thrown away?

> > *) while justifying this you also need to show why it's a problem specific
> > to the ACPI IDs and not a problem for OF ones, which we have tons of in the
> > Linux kernel without any guards (ifdeffery).
> 
> To be honest I don't care about this too much. I just wanted to cut
> down some of the noise when I build my patch backlog so that warnings
> in the stuff I'm trying to mainline are more visible.

Which is good intention and thanks for doing this!

> For what it's worth I think the OF ids are a bit wasteful.

Exactly my point, but fixing one driver of zillions does not solve the issue
in general.

> For some
> drivers where there are tons of broken variations they add a few K of
> unneeded data. But since everyone now has gigabytes of memory I doubt
> they care...

Some actually cares.

> I'm working with 64MB. :)

Then I would imagine that you already using as less kernel configuration as
possible and have as many modules as you want for the hardware that might
appear to be connected to that board, right? Then again one driver with 100+
bytes doesn't affect really your case. Disabling, for example PRINTK, will
win much more for you.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Daniel Palmer]

Hi Andy,

On Tue, 5 Oct 2021 at 21:14, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> > Ok, is there a reason it's not for the ID tables? Does it break something?
>
> It will look ugly. Why we define a table that may or may not be used?
> Sounds fishy.

I guess it's a toss up between is the attribute more ugly than #ifdefs
and is the ugliness of either worth it..
Not going to say I have an answer here. :)

> On top of that why you should tell linker to waste resources on something
> that you may well know beforehand will be thrown away?

That's true but the linker on my machine with 64GB of RAM compiling
for a single core machine with 64MB of RAM doesn't mind too much.

> > For what it's worth I think the OF ids are a bit wasteful.
>
> Exactly my point, but fixing one driver of zillions does not solve the issue
> in general.

I looked into making OF ids smaller globally. There seems to be 64
bytes wasted from the start for the name and type fields as nothing
uses them as far as I can tell.
Then you have the array for the compatible string which is currently
128 bytes but the longest compatible string in the kernel is less than
64 from what I can tell.
I understand that it's for future proofing etc. Adding a few hacks to
my kernel to remove the unused fields and reduce the size of the
compatible string saved a few tens of K.
Which isn't a lot but might be the difference between the kernel
fitting in a tiny SPI NOR partition or not.

> > For some
> > drivers where there are tons of broken variations they add a few K of
> > unneeded data. But since everyone now has gigabytes of memory I doubt
> > they care...
> Some actually cares.
>

Ok.. I might consider pushing my changes to remove unused ids all over
the place then.
drivers/cpufreq/cpufreq-dt-platdev.c is a really good example of
adding ~10K to kernels for no reason.

> > I'm working with 64MB. :)
>
> Then I would imagine that you already using as less kernel configuration as
> possible and have as many modules as you want for the hardware that might
> appear to be connected to that board, right?

I have a minimal config but compiling in macb for the ethernet
compiles in code and ids for stuff like zynq that I could do without.

>Then again one driver with 100+
> bytes doesn't affect really your case. Disabling, for example PRINTK, will
> win much more for you.

It's not *that* bad just yet. :)

Anyhow, thankyou for the interesting discussion. I'll just leave this
in my tree for now so I don't have to see the warning.

Cheers,

Daniel

Re: [PATCH] serial: 8250_dw: Mark acpi match table as maybe unused

[Andy Shevchenko]

On Tue, Oct 05, 2021 at 09:41:33PM +0900, Daniel Palmer wrote:
> On Tue, 5 Oct 2021 at 21:14, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> > > For what it's worth I think the OF ids are a bit wasteful.
> >
> > Exactly my point, but fixing one driver of zillions does not solve the issue
> > in general.
> 
> I looked into making OF ids smaller globally. There seems to be 64
> bytes wasted from the start for the name and type fields as nothing
> uses them as far as I can tell.
> Then you have the array for the compatible string which is currently
> 128 bytes but the longest compatible string in the kernel is less than
> 64 from what I can tell.
> I understand that it's for future proofing etc. Adding a few hacks to
> my kernel to remove the unused fields and reduce the size of the
> compatible string saved a few tens of K.
> Which isn't a lot but might be the difference between the kernel
> fitting in a tiny SPI NOR partition or not.

What can you try is to somehow cut the sections during linking phase
if you know, that OF=n, or ACPI=n. Not sure how it would work, because
basically we will need to replace them with stubs to prevent drivers
from crashing. Nevertheless, One terminator entry is much better than
full bloated ID table(s)!

...

> Anyhow, thankyou for the interesting discussion. I'll just leave this
> in my tree for now so I don't have to see the warning.

You're welcome!

FWIW, I have sent my version of the patch, you are in Cc list there,
can you test and confirm that warning is gone for you?

-- 
With Best Regards,
Andy Shevchenko