2021-07-21 18:21:23

by Andreas Schwab

[permalink] [raw]
Subject: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

This allows the driver to be auto loaded.

Signed-off-by: Andreas Schwab <[email protected]>
---
drivers/mmc/host/mmc_spi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 65c65bb5737f..103c450773df 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1542,3 +1542,4 @@ MODULE_AUTHOR("Mike Lavender, David Brownell, Hans-Peter Nilsson, Jan Nikitenko"
MODULE_DESCRIPTION("SPI SD/MMC host driver");
MODULE_LICENSE("GPL");
MODULE_ALIAS("spi:mmc_spi");
+MODULE_ALIAS("spi:mmc-spi-slot");
--
2.32.0


--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


2021-07-21 19:27:07

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Wed, Jul 21, 2021 at 03:26:49PM +0200, Andreas Schwab wrote:
> This allows the driver to be auto loaded.

Can you elaborate a bit?

The driver has OF compatible strings and should be loaded automatically.

--
With Best Regards,
Andy Shevchenko


2021-07-21 19:27:54

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 21 2021, Andy Shevchenko wrote:

> The driver has OF compatible strings and should be loaded automatically.

They are never being used.

# udevadm info /sys/devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
P: /devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
L: 0
E: DEVPATH=/devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
E: DRIVER=mmc_spi
E: OF_NAME=mmc
E: OF_FULLNAME=/soc/spi@10050000/mmc@0
E: OF_COMPATIBLE_0=mmc-spi-slot
E: OF_COMPATIBLE_N=1
E: MODALIAS=spi:mmc-spi-slot
E: SUBSYSTEM=spi

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-21 19:28:46

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Wed, Jul 21, 2021 at 04:37:51PM +0200, Andreas Schwab wrote:
> On Jul 21 2021, Andy Shevchenko wrote:
>
> > The driver has OF compatible strings and should be loaded automatically.
>
> They are never being used.

What do you mean by that?

Can you explain what is the _practical_ issue here? What is your HW setup and
what you are trying to do?

> # udevadm info /sys/devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
> P: /devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
> L: 0
> E: DEVPATH=/devices/platform/soc/10050000.spi/spi_master/spi1/spi1.0
> E: DRIVER=mmc_spi
> E: OF_NAME=mmc
> E: OF_FULLNAME=/soc/spi@10050000/mmc@0
> E: OF_COMPATIBLE_0=mmc-spi-slot
> E: OF_COMPATIBLE_N=1
> E: MODALIAS=spi:mmc-spi-slot
> E: SUBSYSTEM=spi

--
With Best Regards,
Andy Shevchenko


2021-07-21 21:01:24

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 21 2021, Andy Shevchenko wrote:

> Can you explain what is the _practical_ issue here?

The module is not loaded by udev.

> What is your HW setup and what you are trying to do?

Booting the HiFive board without having to force loading the mmc_spi
module.

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-21 21:03:26

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Wed, Jul 21, 2021 at 06:05:48PM +0200, Andreas Schwab wrote:
> On Jul 21 2021, Andy Shevchenko wrote:
>
> > What is your DT excerpt for it?
>
> arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts:
>
> &qspi2 {
> status = "okay";
> mmc@0 {
> compatible = "mmc-spi-slot";
> reg = <0>;
> spi-max-frequency = <20000000>;
> voltage-ranges = <3300 3300>;
> disable-wp;
> };
> };

Hmm...

I have counted 89 device drivers in the kernel that have OF ID table without
SPI ID table. I'm wondering if all of them need to be fixed.

Or problem is somewhere else?


--
With Best Regards,
Andy Shevchenko


2021-07-21 21:04:40

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 21 2021, Andy Shevchenko wrote:

> What is your DT excerpt for it?

arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts:

&qspi2 {
status = "okay";
mmc@0 {
compatible = "mmc-spi-slot";
reg = <0>;
spi-max-frequency = <20000000>;
voltage-ranges = <3300 3300>;
disable-wp;
};
};

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-21 21:06:21

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 21 2021, Andy Shevchenko wrote:

> I have counted 89 device drivers in the kernel that have OF ID table without
> SPI ID table. I'm wondering if all of them need to be fixed.

How does a SPI ID table look like?

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-21 21:08:12

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Wed, Jul 21, 2021 at 6:16 PM Andreas Schwab <[email protected]> wrote:
> On Jul 21 2021, Andy Shevchenko wrote:
> > Can you explain what is the _practical_ issue here?
> The module is not loaded by udev.
>
> > What is your HW setup and what you are trying to do?
>
> Booting the HiFive board without having to force loading the mmc_spi
> module.

What is your DT excerpt for it?

--
With Best Regards,
Andy Shevchenko

2021-07-21 21:08:13

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Wed, Jul 21, 2021 at 10:53 PM Andreas Schwab <[email protected]> wrote:
>
> On Jul 21 2021, Andy Shevchenko wrote:
>
> > I have counted 89 device drivers in the kernel that have OF ID table without
> > SPI ID table. I'm wondering if all of them need to be fixed.
>
> How does a SPI ID table look like?

Plenty examples [1], like [2].

[1]: https://elixir.bootlin.com/linux/latest/A/ident/spi_device_id
[2]: https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/tiny/mi0283qt.c#L174

--
With Best Regards,
Andy Shevchenko

2021-07-22 10:12:01

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 21 2021, Andy Shevchenko wrote:

> Or problem is somewhere else?

I don't know. Why does the spi subsystem put "spi:mmc-spi-slot" into
the modalias file, instead of "of:N(null)T(null)Cmmc-spi-slot" or
similar? The same problem exists with the other spi port on the board,
which has a jedec,spi-nor instance attached, also not auto loading.

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-22 10:31:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Thu, Jul 22, 2021 at 1:03 PM Andreas Schwab <[email protected]> wrote:
> On Jul 21 2021, Andy Shevchenko wrote:
>
> > Or problem is somewhere else?
>
> I don't know. Why does the spi subsystem put "spi:mmc-spi-slot" into
> the modalias file, instead of "of:N(null)T(null)Cmmc-spi-slot" or
> similar? The same problem exists with the other spi port on the board,
> which has a jedec,spi-nor instance attached, also not auto loading.

You see, there are two unrelated drivers that share the same issue
(the common denominator is that they are SPI devices). I believe the
issue is somewhere in the SPI core rather than here.

Compare the code of
https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L649
vs.
https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L56

and

https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L139
vs.
https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L361

The culprit is this one:
https://lore.kernel.org/lkml/[email protected]/

and in my humble opinion must be reverted.

--
With Best Regards,
Andy Shevchenko

2021-07-22 10:39:52

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Thu, Jul 22, 2021 at 1:28 PM Andy Shevchenko
<[email protected]> wrote:
> On Thu, Jul 22, 2021 at 1:03 PM Andreas Schwab <[email protected]> wrote:
> > On Jul 21 2021, Andy Shevchenko wrote:
> >
> > > Or problem is somewhere else?
> >
> > I don't know. Why does the spi subsystem put "spi:mmc-spi-slot" into
> > the modalias file, instead of "of:N(null)T(null)Cmmc-spi-slot" or
> > similar? The same problem exists with the other spi port on the board,
> > which has a jedec,spi-nor instance attached, also not auto loading.
>
> You see, there are two unrelated drivers that share the same issue
> (the common denominator is that they are SPI devices). I believe the
> issue is somewhere in the SPI core rather than here.
>
> Compare the code of
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L649
> vs.
> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L56
>
> and
>
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L139
> vs.
> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L361


> The culprit is this one:
> https://lore.kernel.org/lkml/[email protected]/
>
> and in my humble opinion must be reverted.

Oops, I have been too fast. It's not related to SPI in general, sorry
guys. But the idea you have got from above, I believe.



--
With Best Regards,
Andy Shevchenko

2021-07-22 12:06:06

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 22 2021, Andy Shevchenko wrote:

> Compare the code of
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L649
> vs.
> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L56
>
> and
>
> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L139
> vs.
> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L361

Thanks, I think I now know how to fix it correctly.

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2021-07-22 12:38:56

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] mmc: mmc_spi: add spi:mmc-spi-slot alias

On Jul 22 2021, Andreas Schwab wrote:

> On Jul 22 2021, Andy Shevchenko wrote:
>
>> Compare the code of
>> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L649
>> vs.
>> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L56
>>
>> and
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L139
>> vs.
>> https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi.c#L361
>
> Thanks, I think I now know how to fix it correctly.

In fact, the correct fix has already been installed in commit
3ce6c9e2617e ("spi: add of_device_uevent_modalias support"), which is
part of -rc1, just incomplete, which explains the inconsistency above.

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."