2022-06-30 07:15:38

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

Add spi_device_id entries to silent following warnings:
SPI driver sja1105 has no spi_device_id for nxp,sja1105e
SPI driver sja1105 has no spi_device_id for nxp,sja1105t
SPI driver sja1105 has no spi_device_id for nxp,sja1105p
SPI driver sja1105 has no spi_device_id for nxp,sja1105q
SPI driver sja1105 has no spi_device_id for nxp,sja1105r
SPI driver sja1105 has no spi_device_id for nxp,sja1105s
SPI driver sja1105 has no spi_device_id for nxp,sja1110a
SPI driver sja1105 has no spi_device_id for nxp,sja1110b
SPI driver sja1105 has no spi_device_id for nxp,sja1110c
SPI driver sja1105 has no spi_device_id for nxp,sja1110d

Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index b253e27bcfb4..b03d0d0c3dbf 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3382,12 +3382,28 @@ static const struct of_device_id sja1105_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, sja1105_dt_ids);

+static const struct spi_device_id sja1105_spi_ids[] = {
+ { "sja1105e" },
+ { "sja1105t" },
+ { "sja1105p" },
+ { "sja1105q" },
+ { "sja1105r" },
+ { "sja1105s" },
+ { "sja1110a" },
+ { "sja1110b" },
+ { "sja1110c" },
+ { "sja1110d" },
+ { },
+};
+MODULE_DEVICE_TABLE(spi, sja1105_spi_ids);
+
static struct spi_driver sja1105_driver = {
.driver = {
.name = "sja1105",
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(sja1105_dt_ids),
},
+ .id_table = sja1105_spi_ids,
.probe = sja1105_probe,
.remove = sja1105_remove,
.shutdown = sja1105_shutdown,
--
2.30.2


2022-06-30 16:37:56

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On Thu, Jun 30, 2022 at 09:10:13AM +0200, Oleksij Rempel wrote:
> Add spi_device_id entries to silent following warnings:
> SPI driver sja1105 has no spi_device_id for nxp,sja1105e
> SPI driver sja1105 has no spi_device_id for nxp,sja1105t
> SPI driver sja1105 has no spi_device_id for nxp,sja1105p
> SPI driver sja1105 has no spi_device_id for nxp,sja1105q
> SPI driver sja1105 has no spi_device_id for nxp,sja1105r
> SPI driver sja1105 has no spi_device_id for nxp,sja1105s
> SPI driver sja1105 has no spi_device_id for nxp,sja1110a
> SPI driver sja1105 has no spi_device_id for nxp,sja1110b
> SPI driver sja1105 has no spi_device_id for nxp,sja1110c
> SPI driver sja1105 has no spi_device_id for nxp,sja1110d
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index b253e27bcfb4..b03d0d0c3dbf 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -3382,12 +3382,28 @@ static const struct of_device_id sja1105_dt_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, sja1105_dt_ids);
>
> +static const struct spi_device_id sja1105_spi_ids[] = {
> + { "sja1105e" },
> + { "sja1105t" },
> + { "sja1105p" },
> + { "sja1105q" },
> + { "sja1105r" },
> + { "sja1105s" },
> + { "sja1110a" },
> + { "sja1110b" },
> + { "sja1110c" },
> + { "sja1110d" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(spi, sja1105_spi_ids);
> +
> static struct spi_driver sja1105_driver = {
> .driver = {
> .name = "sja1105",
> .owner = THIS_MODULE,
> .of_match_table = of_match_ptr(sja1105_dt_ids),
> },
> + .id_table = sja1105_spi_ids,
> .probe = sja1105_probe,
> .remove = sja1105_remove,
> .shutdown = sja1105_shutdown,
> --
> 2.30.2
>

Do we also need these?

MODULE_ALIAS("spi:sja1105e");
MODULE_ALIAS("spi:sja1105t");
MODULE_ALIAS("spi:sja1105p");
MODULE_ALIAS("spi:sja1105q");
MODULE_ALIAS("spi:sja1105r");
MODULE_ALIAS("spi:sja1105s");
MODULE_ALIAS("spi:sja1110a");
MODULE_ALIAS("spi:sja1110b");
MODULE_ALIAS("spi:sja1110c");
MODULE_ALIAS("spi:sja1110d");

To be honest I don't do much testing with modules at all, so I'm not
sure if udev-based module loading is broken or not. I remember becoming
vaguely curious after commit 5fa6863ba692 ("spi: Check we have a
spi_device_id for each DT compatible"), and I did some basic testing
without the spi_device_id table and MODULE_ALIASes, and it appeared that
udev could still autoload the sja1105 kernel module just fine.
So I'm not really sure what's broken.

2022-07-01 07:33:00

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On Thu, Jun 30, 2022 at 07:10:59PM +0300, Vladimir Oltean wrote:
> On Thu, Jun 30, 2022 at 09:10:13AM +0200, Oleksij Rempel wrote:
> > Add spi_device_id entries to silent following warnings:
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105e
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105t
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105p
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105q
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105r
> > SPI driver sja1105 has no spi_device_id for nxp,sja1105s
> > SPI driver sja1105 has no spi_device_id for nxp,sja1110a
> > SPI driver sja1105 has no spi_device_id for nxp,sja1110b
> > SPI driver sja1105 has no spi_device_id for nxp,sja1110c
> > SPI driver sja1105 has no spi_device_id for nxp,sja1110d
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/dsa/sja1105/sja1105_main.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> > index b253e27bcfb4..b03d0d0c3dbf 100644
> > --- a/drivers/net/dsa/sja1105/sja1105_main.c
> > +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> > @@ -3382,12 +3382,28 @@ static const struct of_device_id sja1105_dt_ids[] = {
> > };
> > MODULE_DEVICE_TABLE(of, sja1105_dt_ids);
> >
> > +static const struct spi_device_id sja1105_spi_ids[] = {
> > + { "sja1105e" },
> > + { "sja1105t" },
> > + { "sja1105p" },
> > + { "sja1105q" },
> > + { "sja1105r" },
> > + { "sja1105s" },
> > + { "sja1110a" },
> > + { "sja1110b" },
> > + { "sja1110c" },
> > + { "sja1110d" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(spi, sja1105_spi_ids);
> > +
> > static struct spi_driver sja1105_driver = {
> > .driver = {
> > .name = "sja1105",
> > .owner = THIS_MODULE,
> > .of_match_table = of_match_ptr(sja1105_dt_ids),
> > },
> > + .id_table = sja1105_spi_ids,
> > .probe = sja1105_probe,
> > .remove = sja1105_remove,
> > .shutdown = sja1105_shutdown,
> > --
> > 2.30.2
> >
>
> Do we also need these?
>
> MODULE_ALIAS("spi:sja1105e");
> MODULE_ALIAS("spi:sja1105t");
> MODULE_ALIAS("spi:sja1105p");
> MODULE_ALIAS("spi:sja1105q");
> MODULE_ALIAS("spi:sja1105r");
> MODULE_ALIAS("spi:sja1105s");
> MODULE_ALIAS("spi:sja1110a");
> MODULE_ALIAS("spi:sja1110b");
> MODULE_ALIAS("spi:sja1110c");
> MODULE_ALIAS("spi:sja1110d");

No, it is not needed. With this patch modinfo will show this additional
aliases:
alias: spi:sja1110d
alias: spi:sja1110c
alias: spi:sja1110b
alias: spi:sja1110a
alias: spi:sja1105s
alias: spi:sja1105r
alias: spi:sja1105q
alias: spi:sja1105p
alias: spi:sja1105t
alias: spi:sja1105e

This seems to be enough for properly working module auto loading.

> To be honest I don't do much testing with modules at all, so I'm not
> sure if udev-based module loading is broken or not. I remember becoming
> vaguely curious after commit 5fa6863ba692 ("spi: Check we have a
> spi_device_id for each DT compatible"), and I did some basic testing
> without the spi_device_id table and MODULE_ALIASes, and it appeared that
> udev could still autoload the sja1105 kernel module just fine.
> So I'm not really sure what's broken.

Without this patch, module is not automatically loaded on my testing
system.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-07-01 10:32:37

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On Fri, Jul 01, 2022 at 09:18:35AM +0200, Oleksij Rempel wrote:
> Without this patch, module is not automatically loaded on my testing
> system.

Ok, in that case do we need to target 'net' and split the patch into 2,
one with Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port L2 switch")
and one with Fixes: 3e77e59bf8cf ("net: dsa: sja1105: add support for the SJA1110 switch family")?

2022-07-01 11:26:24

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On Fri, Jul 01, 2022 at 01:20:24PM +0300, Vladimir Oltean wrote:
> On Fri, Jul 01, 2022 at 09:18:35AM +0200, Oleksij Rempel wrote:
> > Without this patch, module is not automatically loaded on my testing
> > system.
>
> Ok, in that case do we need to target 'net' and split the patch into 2,
> one with Fixes: 8aa9ebccae87 ("net: dsa: Introduce driver for NXP SJA1105 5-port L2 switch")
> and one with Fixes: 3e77e59bf8cf ("net: dsa: sja1105: add support for the SJA1110 switch family")?

Hard to say, it looks like no one reported module autoload issue till
kernel started warn about missing spi_device_id.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-07-01 17:45:48

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On Thu, Jun 30, 2022 at 09:10:13AM +0200, Oleksij Rempel wrote:
> Add spi_device_id entries to silent following warnings:
> SPI driver sja1105 has no spi_device_id for nxp,sja1105e
> SPI driver sja1105 has no spi_device_id for nxp,sja1105t
> SPI driver sja1105 has no spi_device_id for nxp,sja1105p
> SPI driver sja1105 has no spi_device_id for nxp,sja1105q
> SPI driver sja1105 has no spi_device_id for nxp,sja1105r
> SPI driver sja1105 has no spi_device_id for nxp,sja1105s
> SPI driver sja1105 has no spi_device_id for nxp,sja1110a
> SPI driver sja1105 has no spi_device_id for nxp,sja1110b
> SPI driver sja1105 has no spi_device_id for nxp,sja1110c
> SPI driver sja1105 has no spi_device_id for nxp,sja1110d
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

Reviewed-by: Vladimir Oltean <[email protected]>

2022-07-01 17:49:07

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

On 6/30/22 00:10, Oleksij Rempel wrote:
> Add spi_device_id entries to silent following warnings:
> SPI driver sja1105 has no spi_device_id for nxp,sja1105e
> SPI driver sja1105 has no spi_device_id for nxp,sja1105t
> SPI driver sja1105 has no spi_device_id for nxp,sja1105p
> SPI driver sja1105 has no spi_device_id for nxp,sja1105q
> SPI driver sja1105 has no spi_device_id for nxp,sja1105r
> SPI driver sja1105 has no spi_device_id for nxp,sja1105s
> SPI driver sja1105 has no spi_device_id for nxp,sja1110a
> SPI driver sja1105 has no spi_device_id for nxp,sja1110b
> SPI driver sja1105 has no spi_device_id for nxp,sja1110c
> SPI driver sja1105 has no spi_device_id for nxp,sja1110d
>
> Signed-off-by: Oleksij Rempel <[email protected]>

Reviewed-by: Florian Fainelli <[email protected]>
--
Florian

2022-07-06 15:05:12

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v1 1/1] net: dsa: sja1105: silent spi_device_id warnings

Hi Oleksij,

On Thu, 30 Jun 2022 at 10:10, Oleksij Rempel <[email protected]> wrote:
> Add spi_device_id entries to silent following warnings:
> SPI driver sja1105 has no spi_device_id for nxp,sja1105e
> SPI driver sja1105 has no spi_device_id for nxp,sja1105t
> SPI driver sja1105 has no spi_device_id for nxp,sja1105p
> SPI driver sja1105 has no spi_device_id for nxp,sja1105q
> SPI driver sja1105 has no spi_device_id for nxp,sja1105r
> SPI driver sja1105 has no spi_device_id for nxp,sja1105s
> SPI driver sja1105 has no spi_device_id for nxp,sja1110a
> SPI driver sja1105 has no spi_device_id for nxp,sja1110b
> SPI driver sja1105 has no spi_device_id for nxp,sja1110c
> SPI driver sja1105 has no spi_device_id for nxp,sja1110d
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---

This was marked as "Changes Requested" in patchwork. Could you please
slap a Fixes: tag onto it and resend to net?
Also, would you mind applying the same treatment to
vitesse-vsc73xx-spi.c so that we get rid of the warnings for all DSA
drivers?
You can blame the spi.c commit that introduced the warning.

Thanks!