| Message ID | 20240414154957.127113-3-krzk@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] eeprom: at25: drop unneeded MODULE_ALIAS | expand |
On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: > The ID table already has respective entry and MODULE_DEVICE_TABLE and > creates proper alias for SPI driver. Having another MODULE_ALIAS causes > the alias to be duplicated. ... > MODULE_ALIAS("spi:93xx46"); I was stumbled over this (leftover?). Commit message doesn't elaborate this bit. Any comments? > -MODULE_ALIAS("spi:eeprom-93xx46"); > -MODULE_ALIAS("spi:93lc46b");
On 08/05/2024 19:53, Andy Shevchenko wrote: > On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: >> The ID table already has respective entry and MODULE_DEVICE_TABLE and >> creates proper alias for SPI driver. Having another MODULE_ALIAS causes >> the alias to be duplicated. > > ... > >> MODULE_ALIAS("spi:93xx46"); > > I was stumbled over this (leftover?). > Commit message doesn't elaborate this bit. > Any comments? It is not present in ID table and commit msg removes only duplicated aliases. That alias has meaning - someone might be actually relying on it. Best regards, Krzysztof
On Wed, May 08, 2024 at 08:15:00PM +0200, Krzysztof Kozlowski wrote: > On 08/05/2024 19:53, Andy Shevchenko wrote: > > On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: > >> The ID table already has respective entry and MODULE_DEVICE_TABLE and > >> creates proper alias for SPI driver. Having another MODULE_ALIAS causes > >> the alias to be duplicated. ... > >> MODULE_ALIAS("spi:93xx46"); > > > > I was stumbled over this (leftover?). > > Commit message doesn't elaborate this bit. > > Any comments? > > It is not present in ID table and commit msg removes only duplicated > aliases. That alias has meaning - someone might be actually relying on it. It seems no users for it. The only user of platform data of this EEPROM uses board files which AFAIU bypasses modalias matching.
On 08/05/2024 20:28, Andy Shevchenko wrote: > On Wed, May 08, 2024 at 08:15:00PM +0200, Krzysztof Kozlowski wrote: >> On 08/05/2024 19:53, Andy Shevchenko wrote: >>> On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: >>>> The ID table already has respective entry and MODULE_DEVICE_TABLE and >>>> creates proper alias for SPI driver. Having another MODULE_ALIAS causes >>>> the alias to be duplicated. > > ... > >>>> MODULE_ALIAS("spi:93xx46"); >>> >>> I was stumbled over this (leftover?). >>> Commit message doesn't elaborate this bit. >>> Any comments? >> >> It is not present in ID table and commit msg removes only duplicated >> aliases. That alias has meaning - someone might be actually relying on it. > > It seems no users for it. The only user of platform data of this EEPROM uses > board files which AFAIU bypasses modalias matching. I don't think that's correct. The modalias of SPI board is there on purpose. Why do you think it is not used? IOW, what changed in kernel since the issue was encountered and fixed by adding that alias (see explanation and users in kernel of modalias from that commit)? Best regards, Krzysztof
On Wed, May 8, 2024, at 20:44, Krzysztof Kozlowski wrote: > On 08/05/2024 20:28, Andy Shevchenko wrote: >> On Wed, May 08, 2024 at 08:15:00PM +0200, Krzysztof Kozlowski wrote: >>> On 08/05/2024 19:53, Andy Shevchenko wrote: >>>> On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: >>>>> The ID table already has respective entry and MODULE_DEVICE_TABLE and >>>>> creates proper alias for SPI driver. Having another MODULE_ALIAS causes >>>>> the alias to be duplicated. >> >> ... >> >>>>> MODULE_ALIAS("spi:93xx46"); >>>> >>>> I was stumbled over this (leftover?). >>>> Commit message doesn't elaborate this bit. >>>> Any comments? >>> >>> It is not present in ID table and commit msg removes only duplicated >>> aliases. That alias has meaning - someone might be actually relying on it. >> >> It seems no users for it. The only user of platform data of this EEPROM uses >> board files which AFAIU bypasses modalias matching. > > I don't think that's correct. The modalias of SPI board is there on > purpose. Right, but I think a better workaround would have been to change the board_info to pick a modalias that is part of the ID table: --- a/drivers/misc/eeprom/digsy_mtc_eeprom.c +++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c @@ -76,7 +76,7 @@ static struct gpiod_lookup_table eeprom_spi_gpiod_table = { static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = { { - .modalias = "93xx46", + .modalias = "eeprom-93xx46", .max_speed_hz = 1000000, .bus_num = EE_SPI_BUS_NUM, .chip_select = 0, Arnd
On 08/05/2024 21:11, Arnd Bergmann wrote: > On Wed, May 8, 2024, at 20:44, Krzysztof Kozlowski wrote: >> On 08/05/2024 20:28, Andy Shevchenko wrote: >>> On Wed, May 08, 2024 at 08:15:00PM +0200, Krzysztof Kozlowski wrote: >>>> On 08/05/2024 19:53, Andy Shevchenko wrote: >>>>> On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: >>>>>> The ID table already has respective entry and MODULE_DEVICE_TABLE and >>>>>> creates proper alias for SPI driver. Having another MODULE_ALIAS causes >>>>>> the alias to be duplicated. >>> >>> ... >>> >>>>>> MODULE_ALIAS("spi:93xx46"); >>>>> >>>>> I was stumbled over this (leftover?). >>>>> Commit message doesn't elaborate this bit. >>>>> Any comments? >>>> >>>> It is not present in ID table and commit msg removes only duplicated >>>> aliases. That alias has meaning - someone might be actually relying on it. >>> >>> It seems no users for it. The only user of platform data of this EEPROM uses >>> board files which AFAIU bypasses modalias matching. >> >> I don't think that's correct. The modalias of SPI board is there on >> purpose. > > Right, but I think a better workaround would have been to change > the board_info to pick a modalias that is part of the ID table: > > --- a/drivers/misc/eeprom/digsy_mtc_eeprom.c > +++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c > @@ -76,7 +76,7 @@ static struct gpiod_lookup_table eeprom_spi_gpiod_table = { > > static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = { > { > - .modalias = "93xx46", > + .modalias = "eeprom-93xx46", > .max_speed_hz = 1000000, > .bus_num = EE_SPI_BUS_NUM, > .chip_select = 0, > That's kind of independent change. This commit *only* removed duplicated aliases, thus should have not functional impact except dropping redundant code. What you propose is to change spi board matching, which is reasonable, just a different thing. Best regards, Krzysztof
On Wed, May 08, 2024 at 09:17:59PM +0200, Krzysztof Kozlowski wrote: > On 08/05/2024 21:11, Arnd Bergmann wrote: > > On Wed, May 8, 2024, at 20:44, Krzysztof Kozlowski wrote: > >> On 08/05/2024 20:28, Andy Shevchenko wrote: > >>> On Wed, May 08, 2024 at 08:15:00PM +0200, Krzysztof Kozlowski wrote: > >>>> On 08/05/2024 19:53, Andy Shevchenko wrote: > >>>>> On Sun, Apr 14, 2024 at 05:49:57PM +0200, Krzysztof Kozlowski wrote: > >>>>>> The ID table already has respective entry and MODULE_DEVICE_TABLE and > >>>>>> creates proper alias for SPI driver. Having another MODULE_ALIAS causes > >>>>>> the alias to be duplicated. > >>> > >>> ... > >>> > >>>>>> MODULE_ALIAS("spi:93xx46"); > >>>>> > >>>>> I was stumbled over this (leftover?). > >>>>> Commit message doesn't elaborate this bit. > >>>>> Any comments? > >>>> > >>>> It is not present in ID table and commit msg removes only duplicated > >>>> aliases. That alias has meaning - someone might be actually relying on it. > >>> > >>> It seems no users for it. The only user of platform data of this EEPROM uses > >>> board files which AFAIU bypasses modalias matching. > >> > >> I don't think that's correct. The modalias of SPI board is there on > >> purpose. > > > > Right, but I think a better workaround would have been to change > > the board_info to pick a modalias that is part of the ID table: > > > > --- a/drivers/misc/eeprom/digsy_mtc_eeprom.c > > +++ b/drivers/misc/eeprom/digsy_mtc_eeprom.c > > @@ -76,7 +76,7 @@ static struct gpiod_lookup_table eeprom_spi_gpiod_table = { > > > > static struct spi_board_info digsy_mtc_eeprom_info[] __initdata = { > > { > > - .modalias = "93xx46", > > + .modalias = "eeprom-93xx46", > > .max_speed_hz = 1000000, > > .bus_num = EE_SPI_BUS_NUM, > > .chip_select = 0, > > > > That's kind of independent change. I have done similar change in my patch series, after which I am 100% sure the modalias can be removed. Can you look at that? https://lore.kernel.org/r/20240508184905.2102633-1-andriy.shevchenko@linux.intel.com > This commit *only* removed duplicated > aliases, thus should have not functional impact except dropping > redundant code. What you propose is to change spi board matching, which > is reasonable, just a different thing.
diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c index e78a76d74ff4..45c8ae0db8f9 100644 --- a/drivers/misc/eeprom/eeprom_93xx46.c +++ b/drivers/misc/eeprom/eeprom_93xx46.c @@ -578,5 +578,3 @@ MODULE_LICENSE("GPL"); MODULE_DESCRIPTION("Driver for 93xx46 EEPROMs"); MODULE_AUTHOR("Anatolij Gustschin <agust@denx.de>"); MODULE_ALIAS("spi:93xx46"); -MODULE_ALIAS("spi:eeprom-93xx46"); -MODULE_ALIAS("spi:93lc46b");
The ID table already has respective entry and MODULE_DEVICE_TABLE and creates proper alias for SPI driver. Having another MODULE_ALIAS causes the alias to be duplicated. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- drivers/misc/eeprom/eeprom_93xx46.c | 2 -- 1 file changed, 2 deletions(-)