| Message ID | 20240307160823.3800932-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: pxa2xx: Clean up linux/spi/pxa2xx_spi.h | expand |
On Thu, Mar 7, 2024, at 17:07, Andy Shevchenko wrote: > There is the only one user of the pxa2xx_set_spi_info(). Unexport it > and inline to the actual user. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> I have no idea why you care about this, but it's a nice cleanup, so I'm happy to see this get merged through the spi tree if that helps. Let me know if I should take it through the soc tree instead. > -/* pxa2xx-spi platform-device ID equals respective SSP platform-device ID + 1 */ This comment might still be useful. > @@ -592,7 +595,15 @@ static void __init spitz_spi_init(void) > > gpiod_add_lookup_table(&spitz_ads7846_gpio_table); > gpiod_add_lookup_table(&spitz_spi_gpio_table); > - pxa2xx_set_spi_info(2, &spitz_spi_info); > + > + pd = platform_device_alloc("pxa2xx-spi", id); > + if (pd == NULL) { > + pr_err("pxa2xx-spi: failed to allocate device id %d\n", id); > + } else { > + pd->dev.platform_data = info; > + platform_device_add(pd); > + } > + > spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices)); I think the normal interface these days would be platform_device_register_data(), which does it all in one step. Arnd
On Thu, Mar 07, 2024 at 05:30:10PM +0100, Arnd Bergmann wrote: > On Thu, Mar 7, 2024, at 17:07, Andy Shevchenko wrote: > > There is the only one user of the pxa2xx_set_spi_info(). Unexport it > > and inline to the actual user. > > > I have no idea why you care about this, PXA2xx headers contain a lot of legacy information for pre-OF era. Besides slowing down the builds, this also pollutes an exported namespace on many platforms where it's even not being used! But this is just an ad-hoc clean up, as the main target to me was CS clean up (as per next patch). > but it's a nice cleanup, > so I'm happy to see this get merged through the spi tree if > that helps. Let me know if I should take it through the soc > tree instead. Thank you for the review! ... > > -/* pxa2xx-spi platform-device ID equals respective SSP platform-device ID + 1 */ > > This comment might still be useful. Okay. ... > > spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices)); > > I think the normal interface these days would be > platform_device_register_data(), which does it all in one step. I'm not sure how is this related to the SPI board info registration.
On Thu, Mar 7, 2024, at 17:42, Andy Shevchenko wrote: > On Thu, Mar 07, 2024 at 05:30:10PM +0100, Arnd Bergmann wrote: >> On Thu, Mar 7, 2024, at 17:07, Andy Shevchenko wrote: >> > spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices)); >> >> I think the normal interface these days would be >> platform_device_register_data(), which does it all in one step. > > I'm not sure how is this related to the SPI board info registration. It's not. What I meant is that you could use platform_device_register_data() instead of the "platform_device_alloc(); platform_data = info; platform_device_add();" sequence. It should be a safe conversion, but it's also fine to stay with the existing version if you are worried about regressions. Arnd
On Thu, Mar 07, 2024 at 05:47:56PM +0100, Arnd Bergmann wrote: > On Thu, Mar 7, 2024, at 17:42, Andy Shevchenko wrote: > > On Thu, Mar 07, 2024 at 05:30:10PM +0100, Arnd Bergmann wrote: > >> On Thu, Mar 7, 2024, at 17:07, Andy Shevchenko wrote: > >> > spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices)); > >> > >> I think the normal interface these days would be > >> platform_device_register_data(), which does it all in one step. > > > > I'm not sure how is this related to the SPI board info registration. > > It's not. What I meant is that you could use > platform_device_register_data() instead of the > "platform_device_alloc(); platform_data = info; platform_device_add();" > sequence. Ah, thank you for elaboration. > It should be a safe conversion, but it's also fine to stay > with the existing version if you are worried about regressions. Yeah, I prefer not to change it here (as it out of scope of my little cleanup series).
diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c index 661b3fc43275..1e4cd502340e 100644 --- a/arch/arm/mach-pxa/devices.c +++ b/arch/arm/mach-pxa/devices.c @@ -7,7 +7,6 @@ #include <linux/clk-provider.h> #include <linux/dma-mapping.h> #include <linux/dmaengine.h> -#include <linux/spi/pxa2xx_spi.h> #include <linux/platform_data/i2c-pxa.h> #include <linux/soc/pxa/cpu.h> @@ -665,23 +664,6 @@ struct platform_device pxa27x_device_gpio = { .resource = pxa_resource_gpio, }; -/* pxa2xx-spi platform-device ID equals respective SSP platform-device ID + 1. - * See comment in arch/arm/mach-pxa/ssp.c::ssp_probe() */ -void __init pxa2xx_set_spi_info(unsigned id, struct pxa2xx_spi_controller *info) -{ - struct platform_device *pd; - - pd = platform_device_alloc("pxa2xx-spi", id); - if (pd == NULL) { - printk(KERN_ERR "pxa2xx-spi: failed to allocate device id %d\n", - id); - return; - } - - pd->dev.platform_data = info; - platform_device_add(pd); -} - static struct resource pxa_dma_resource[] = { [0] = { .start = 0x40000000, diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c index cc691b199429..70e4623907fa 100644 --- a/arch/arm/mach-pxa/spitz.c +++ b/arch/arm/mach-pxa/spitz.c @@ -585,6 +585,9 @@ static struct gpiod_lookup_table spitz_spi_gpio_table = { static void __init spitz_spi_init(void) { + struct platform_device *pd; + int id = 2; + if (machine_is_akita()) gpiod_add_lookup_table(&akita_lcdcon_gpio_table); else @@ -592,7 +595,15 @@ static void __init spitz_spi_init(void) gpiod_add_lookup_table(&spitz_ads7846_gpio_table); gpiod_add_lookup_table(&spitz_spi_gpio_table); - pxa2xx_set_spi_info(2, &spitz_spi_info); + + pd = platform_device_alloc("pxa2xx-spi", id); + if (pd == NULL) { + pr_err("pxa2xx-spi: failed to allocate device id %d\n", id); + } else { + pd->dev.platform_data = info; + platform_device_add(pd); + } + spi_register_board_info(ARRAY_AND_SIZE(spitz_spi_devices)); } #else diff --git a/include/linux/spi/pxa2xx_spi.h b/include/linux/spi/pxa2xx_spi.h index ca2cd4e30ead..56aba2f737b1 100644 --- a/include/linux/spi/pxa2xx_spi.h +++ b/include/linux/spi/pxa2xx_spi.h @@ -45,12 +45,4 @@ struct pxa2xx_spi_chip { u32 timeout; }; -#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP) - -#include <linux/clk.h> - -extern void pxa2xx_set_spi_info(unsigned id, struct pxa2xx_spi_controller *info); - -#endif - #endif /* __LINUX_SPI_PXA2XX_SPI_H */
There is the only one user of the pxa2xx_set_spi_info(). Unexport it and inline to the actual user. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- arch/arm/mach-pxa/devices.c | 18 ------------------ arch/arm/mach-pxa/spitz.c | 13 ++++++++++++- include/linux/spi/pxa2xx_spi.h | 8 -------- 3 files changed, 12 insertions(+), 27 deletions(-)