| Message ID | 20250225100838.362125-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] at24: Drop of_match_ptr() and ACPI_PTR() protections | expand |
On Tue, Feb 25, 2025, at 11:08, Andy Shevchenko wrote: > These result in a very small reduction in driver size, but at the cost > of more complex build and slightly harder to read code. In the case of > of_match_ptr() it also prevents use of PRP0001 ACPI based identification. > In this particular case we have a valid ACPI/PNP ID that should be used > in preference to PRP0001 but doesn't mean we should prevent that route. > > With this done, drop unneeded of*.h inclusions and __maybe_unused markers. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Arnd Bergmann <arnd@arndb.de> For reference, see below for a couple of patches in this area that I have sent in the past. Ideally I think we should try to fix these all up and enable -Wunused-const-variable, which is useful in its own right. Your patch does not address a warning, but it's still a step in that direction. Arnd
On Tue, Feb 25, 2025, at 11:37, Andy Shevchenko wrote: > On Tue, Feb 25, 2025 at 11:29:05AM +0100, Arnd Bergmann wrote: >> On Tue, Feb 25, 2025, at 11:08, Andy Shevchenko wrote: > >> Subject: [PATCH] [SUBMITTED 20240403] spi: remove incorrect of_match_ptr >> annotations > > Was it applied (and the rest you provided here)? It was part of a longer series. Some were applied, but the ones I provided here are those that for some reason did not make it. They should apply cleanly to today's linux-next. Arnd
On 25/02/2025 11:29, Arnd Bergmann wrote: > On Tue, Feb 25, 2025, at 11:08, Andy Shevchenko wrote: >> These result in a very small reduction in driver size, but at the cost >> of more complex build and slightly harder to read code. In the case of >> of_match_ptr() it also prevents use of PRP0001 ACPI based identification. >> In this particular case we have a valid ACPI/PNP ID that should be used >> in preference to PRP0001 but doesn't mean we should prevent that route. >> >> With this done, drop unneeded of*.h inclusions and __maybe_unused markers. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > For reference, see below for a couple of patches in this area that > I have sent in the past. Ideally I think we should try to fix these > all up and enable -Wunused-const-variable, which is useful in its > own right. > I tried to fix this in SPI, regulator and ASoC 2 years ago and Mark rejected such approach of dropping ACPI/of_match_ptr. AFAIU, Mark wants this to be fixed in more generic way, on the OF and ACPI common code, not per driver. SPI: https://lore.kernel.org/all/7a65d775-cf07-4393-8b10-2cef4d5266ab@sirena.org.uk/ regulator: https://lore.kernel.org/all/20230310214553.275450-1-krzysztof.kozlowski@linaro.org/ ASoC: https://lore.kernel.org/all/20230310214333.274903-1-krzysztof.kozlowski@linaro.org/ Best regards, Krzysztof
On Tue, Feb 25, 2025, at 11:48, Andy Shevchenko wrote: > On Tue, Feb 25, 2025 at 11:42:48AM +0100, Arnd Bergmann wrote: >> On Tue, Feb 25, 2025, at 11:37, Andy Shevchenko wrote: >> > On Tue, Feb 25, 2025 at 11:29:05AM +0100, Arnd Bergmann wrote: >> >> On Tue, Feb 25, 2025, at 11:08, Andy Shevchenko wrote: >> > >> >> Subject: [PATCH] [SUBMITTED 20240403] spi: remove incorrect of_match_ptr >> >> annotations >> > >> > Was it applied (and the rest you provided here)? >> >> It was part of a longer series. Some were applied, but the ones >> I provided here are those that for some reason did not make it. >> They should apply cleanly to today's linux-next. > > I can review them and give a tag if you issue a new version. I've sent a bunch of those to maintainers directly now but in the end forgot to Cc you. Let's see what gets accepted, and then I'll send the rest as a series fro the endgame. Arnd
On Tue, Feb 25, 2025 at 06:21:29PM +0100, Krzysztof Kozlowski wrote: > On 25/02/2025 11:29, Arnd Bergmann wrote: > > On Tue, Feb 25, 2025, at 11:08, Andy Shevchenko wrote: > >> These result in a very small reduction in driver size, but at the cost > >> of more complex build and slightly harder to read code. In the case of > >> of_match_ptr() it also prevents use of PRP0001 ACPI based identification. > >> In this particular case we have a valid ACPI/PNP ID that should be used > >> in preference to PRP0001 but doesn't mean we should prevent that route. > >> > >> With this done, drop unneeded of*.h inclusions and __maybe_unused markers. > >> > >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > > > For reference, see below for a couple of patches in this area that > > I have sent in the past. Ideally I think we should try to fix these > > all up and enable -Wunused-const-variable, which is useful in its > > own right. > > I tried to fix this in SPI, regulator and ASoC 2 years ago and Mark > rejected such approach of dropping ACPI/of_match_ptr. AFAIU, Mark wants > this to be fixed in more generic way, on the OF and ACPI common code, > not per driver. > > SPI: > https://lore.kernel.org/all/7a65d775-cf07-4393-8b10-2cef4d5266ab@sirena.org.uk/ > > regulator: > https://lore.kernel.org/all/20230310214553.275450-1-krzysztof.kozlowski@linaro.org/ > > ASoC: > https://lore.kernel.org/all/20230310214333.274903-1-krzysztof.kozlowski@linaro.org/ It was almost two years ago. Things may be changed :-) At least I have no impediments so far with converting drivers I'm supporting in the SPI. For ASoC there might be a new attempt by Cezary Rojewski in the future (he does some cleanups in that area, and we discussed cleaning up ACPI_PTR() at minimum). Also note $ git grep -lw ACPI_PTR | wc -l 238 $ git grep -lw of_match_ptr | wc -l 841 So, at least dropping ACPI_PTR() seems on the track to getting rid of. And I checked, they are spread all over the kernel with top subsystems as ... 10 drivers/hwtracing/coresight 11 drivers/input/touchscreen 15 drivers/media/i2c 53 sound/soc/codecs
On Wed, Feb 26, 2025, at 15:18, Andy Shevchenko wrote: > On Tue, Feb 25, 2025 at 06:21:29PM +0100, Krzysztof Kozlowski wrote: >> On 25/02/2025 11:29, Arnd Bergmann wrote: >> >> I tried to fix this in SPI, regulator and ASoC 2 years ago and Mark >> rejected such approach of dropping ACPI/of_match_ptr. AFAIU, Mark wants >> this to be fixed in more generic way, on the OF and ACPI common code, >> not per driver. >> >> SPI: >> https://lore.kernel.org/all/7a65d775-cf07-4393-8b10-2cef4d5266ab@sirena.org.uk/ >> >> regulator: >> https://lore.kernel.org/all/20230310214553.275450-1-krzysztof.kozlowski@linaro.org/ >> >> ASoC: >> https://lore.kernel.org/all/20230310214333.274903-1-krzysztof.kozlowski@linaro.org/ > > It was almost two years ago. Things may be changed :-) > At least I have no impediments so far with converting drivers I'm supporting in > the SPI. For ASoC there might be a new attempt by Cezary Rojewski in the future > (he does some cleanups in that area, and we discussed cleaning up ACPI_PTR() at > minimum). I skipped those three subsystems when I sent my backlog. Comparing what I have left with the version from the patches above I see that about 40% of the warnings in all three are already addressed in the meantime, leaving just drivers/regulator/pbias-regulator.c | 2 +- drivers/regulator/twl-regulator.c | 2 +- drivers/regulator/twl6030-regulator.c | 2 +- drivers/spi/spi-armada-3700.c | 2 +- drivers/spi/spi-img-spfi.c | 2 +- drivers/spi/spi-meson-spicc.c | 2 +- drivers/spi/spi-meson-spifc.c | 2 +- drivers/spi/spi-orion.c | 2 +- drivers/spi/spi-pic32-sqi.c | 2 +- drivers/spi/spi-pic32.c | 2 +- drivers/spi/spi-rockchip.c | 2 +- drivers/spi/spi-s3c64xx.c | 2 +- drivers/spi/spi-st-ssc4.c | 2 +- sound/soc/amd/acp3x-rt5682-max9836.c | 2 +- sound/soc/atmel/sam9x5_wm8731.c | 2 +- sound/soc/codecs/rt1318.c | 6 ++---- sound/soc/codecs/rt5514-spi.c | 2 +- sound/soc/qcom/lpass-sc7280.c | 2 +- sound/soc/samsung/aries_wm8994.c | 2 +- I send everything else that I have to address the warnings, and they are slowly making their way into the tree, as of today the remaining ones are drivers/char/apm-emulation.c | 5 ++--- drivers/char/tpm/tpm_ftpm_tee.c | 2 +- drivers/comedi/drivers/ni_atmio.c | 2 +- drivers/dma/img-mdc-dma.c | 2 +- drivers/fpga/versal-fpga.c | 2 +- drivers/input/touchscreen/stmpe-ts.c | 2 +- drivers/mux/adg792a.c | 2 +- drivers/net/ethernet/apm/xgene-v2/main.c | 4 +--- drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +- drivers/rtc/rtc-fsl-ftm-alarm.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +- drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +- drivers/tty/serial/amba-pl011.c | 6 +++--- drivers/tty/serial/ma35d1_serial.c | 2 +- and I'm optimistic about most of them making it into the next merge window. Once few enough are left, I can send it again as a series that turns on the warning by default and hopefully by that time we can wear down Mark enough that he takes the patches even if he still disagrees ;-) Arnd
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Tue, 25 Feb 2025 12:08:38 +0200, Andy Shevchenko wrote: > These result in a very small reduction in driver size, but at the cost > of more complex build and slightly harder to read code. In the case of > of_match_ptr() it also prevents use of PRP0001 ACPI based identification. > In this particular case we have a valid ACPI/PNP ID that should be used > in preference to PRP0001 but doesn't mean we should prevent that route. > > With this done, drop unneeded of*.h inclusions and __maybe_unused markers. > > [...] Applied, thanks! [1/1] at24: Drop of_match_ptr() and ACPI_PTR() protections commit: 51e36ca2251c5a47d8f7069d60fd07153cff3f36 Best regards,
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 0a7c7f29406c..f721825199ce 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -18,8 +18,6 @@ #include <linux/module.h> #include <linux/mutex.h> #include <linux/nvmem-provider.h> -#include <linux/of.h> -#include <linux/of_device.h> #include <linux/pm_runtime.h> #include <linux/property.h> #include <linux/regmap.h> @@ -252,7 +250,7 @@ static const struct i2c_device_id at24_ids[] = { }; MODULE_DEVICE_TABLE(i2c, at24_ids); -static const struct of_device_id __maybe_unused at24_of_match[] = { +static const struct of_device_id at24_of_match[] = { { .compatible = "atmel,24c00", .data = &at24_data_24c00 }, { .compatible = "atmel,24c01", .data = &at24_data_24c01 }, { .compatible = "atmel,24cs01", .data = &at24_data_24cs01 }, @@ -286,7 +284,7 @@ static const struct of_device_id __maybe_unused at24_of_match[] = { }; MODULE_DEVICE_TABLE(of, at24_of_match); -static const struct acpi_device_id __maybe_unused at24_acpi_ids[] = { +static const struct acpi_device_id at24_acpi_ids[] = { { "INT3499", (kernel_ulong_t)&at24_data_INT3499 }, { "TPF0001", (kernel_ulong_t)&at24_data_24c1024 }, { /* END OF LIST */ } @@ -848,8 +846,8 @@ static struct i2c_driver at24_driver = { .driver = { .name = "at24", .pm = &at24_pm_ops, - .of_match_table = of_match_ptr(at24_of_match), - .acpi_match_table = ACPI_PTR(at24_acpi_ids), + .of_match_table = at24_of_match, + .acpi_match_table = at24_acpi_ids, }, .probe = at24_probe, .remove = at24_remove,
These result in a very small reduction in driver size, but at the cost of more complex build and slightly harder to read code. In the case of of_match_ptr() it also prevents use of PRP0001 ACPI based identification. In this particular case we have a valid ACPI/PNP ID that should be used in preference to PRP0001 but doesn't mean we should prevent that route. With this done, drop unneeded of*.h inclusions and __maybe_unused markers. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/misc/eeprom/at24.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)