| Message ID | 1460316600-15978-2-git-send-email-robert.jarzmik@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sunday 10 April 2016 21:29:59 Robert Jarzmik wrote: > + > +DT_MACHINE_START(PXA_DT, "Marvell PXA3xx (Device Tree Support)") > + .map_io = pxa3xx_map_io, > + .init_irq = pxa3xx_dt_init_irq, > + .handle_irq = pxa3xx_handle_irq, > + .restart = pxa_restart, > + .dt_compat = pxa3xx_dt_board_compat, > +MACHINE_END > Nothing wrong with your series, it's a straightforward continuation of what you have for the other platforms, but I have a few comments on the method overall, and it might be good if you could work on improving those next, basically eliminating most of the machine descriptor contents in the long run: - It would be nice not to call map_io() at all and instead ensure that all drivers that have DT bindings use ioremap. The main reason for this is that relying on the hardwired mapping makes it easy to get things wrong in the bindings, by leaving out required memory ranges. - The init_irq()/handle_irq() callbacks can probably be replaced with a IRQCHIP_DECLARE() statement per irqchip variant, which then goes on to initialize the controller and set the handler. - The restart method is the least important here, but I guess we can convert that into a driver, or use an existing one from DT, like drivers/power/reset/gpio-restart.c Arnd
Arnd Bergmann <arnd@arndb.de> writes: > On Sunday 10 April 2016 21:29:59 Robert Jarzmik wrote: >> + >> +DT_MACHINE_START(PXA_DT, "Marvell PXA3xx (Device Tree Support)") >> + .map_io = pxa3xx_map_io, >> + .init_irq = pxa3xx_dt_init_irq, >> + .handle_irq = pxa3xx_handle_irq, >> + .restart = pxa_restart, >> + .dt_compat = pxa3xx_dt_board_compat, >> +MACHINE_END >> > > Nothing wrong with your series, it's a straightforward continuation of what > you have for the other platforms, but I have a few comments on the method > overall, and it might be good if you could work on improving those next, > basically eliminating most of the machine descriptor contents in the long > run: > > - It would be nice not to call map_io() at all and instead ensure that all > drivers that have DT bindings use ioremap. The main reason for this is > that relying on the hardwired mapping makes it easy to get things wrong > in the bindings, by leaving out required memory ranges. Okay, that sounds good, I'll add it to my todo list. I seem to remember some legacy driver relying on one of these mapping (cpufreq for pxa relying on memory controller maybe ...), but that's a fix easy enough to queue. Let's hope I'm not bitten by something else. > - The init_irq()/handle_irq() callbacks can probably be replaced with > a IRQCHIP_DECLARE() statement per irqchip variant, which then goes > on to initialize the controller and set the handler. Okay, I'll verify that, especially that the ordering is ensured, ie. that interrupts are available at the same time that when it was the machine code calling the irq init, and that's also going to my todo list. > > - The restart method is the least important here, but I guess we can > convert that into a driver, or use an existing one from DT, like > drivers/power/reset/gpio-restart.c I think most of the required stuff is already done. The only remaining part is the reset status clearing specific to pxa, which as you said would ask for a very tiny driver. As soon as I'm done with the ac97 rework and if no v4l2 pressure is applied to pxa, I'll work on this and the MMP to pinctrl conversion. Thanks for the comments, and cheers. -- Robert
diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig index cd894d69e766..76fbc115ec33 100644 --- a/arch/arm/mach-pxa/Kconfig +++ b/arch/arm/mach-pxa/Kconfig @@ -4,6 +4,17 @@ menu "Intel PXA2xx/PXA3xx Implementations" comment "Intel/Marvell Dev Platforms (sorted by hardware release time)" +config MACH_PXA25X_DT + bool "Support PXA25x platforms from device tree" + select PINCTRL + select POWER_SUPPLY + select PXA25x + select USE_OF + help + Include support for Marvell PXA25x based platforms using + the device tree. Needn't select any other machine while + MACH_PXA25x_DT is enabled. + config MACH_PXA27X_DT bool "Support PXA27x platforms from device tree" select PINCTRL diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile index 2ceed407eda9..ef25dc597f30 100644 --- a/arch/arm/mach-pxa/Makefile +++ b/arch/arm/mach-pxa/Makefile @@ -19,8 +19,9 @@ obj-$(CONFIG_CPU_PXA930) += pxa930.o # NOTE: keep the order of boards in accordance to their order in Kconfig # Device Tree support -obj-$(CONFIG_MACH_PXA3XX_DT) += pxa-dt.o +obj-$(CONFIG_MACH_PXA25X_DT) += pxa-dt.o obj-$(CONFIG_MACH_PXA27X_DT) += pxa-dt.o +obj-$(CONFIG_MACH_PXA3XX_DT) += pxa-dt.o # Intel/Marvell Dev Platforms obj-$(CONFIG_ARCH_LUBBOCK) += lubbock.o diff --git a/arch/arm/mach-pxa/pxa-dt.c b/arch/arm/mach-pxa/pxa-dt.c index f128133a8f30..3e331e61f995 100644 --- a/arch/arm/mach-pxa/pxa-dt.c +++ b/arch/arm/mach-pxa/pxa-dt.c @@ -18,20 +18,18 @@ #include "generic.h" -#ifdef CONFIG_PXA3xx -static const char *const pxa3xx_dt_board_compat[] __initconst = { - "marvell,pxa300", - "marvell,pxa310", - "marvell,pxa320", +#ifdef CONFIG_PXA25x +static const char * const pxa25x_dt_board_compat[] __initconst = { + "marvell,pxa250", NULL, }; -DT_MACHINE_START(PXA_DT, "Marvell PXA3xx (Device Tree Support)") - .map_io = pxa3xx_map_io, - .init_irq = pxa3xx_dt_init_irq, - .handle_irq = pxa3xx_handle_irq, +DT_MACHINE_START(PXA25X_DT, "Marvell PXA25x (Device Tree Support)") + .map_io = pxa25x_map_io, + .init_irq = pxa25x_dt_init_irq, + .handle_irq = pxa25x_handle_irq, .restart = pxa_restart, - .dt_compat = pxa3xx_dt_board_compat, + .dt_compat = pxa25x_dt_board_compat, MACHINE_END #endif @@ -41,7 +39,7 @@ static const char * const pxa27x_dt_board_compat[] __initconst = { NULL, }; -DT_MACHINE_START(PXA27X_DT, "Marvell PXA2xx (Device Tree Support)") +DT_MACHINE_START(PXA27X_DT, "Marvell PXA27x (Device Tree Support)") .map_io = pxa27x_map_io, .init_irq = pxa27x_dt_init_irq, .handle_irq = pxa27x_handle_irq, @@ -49,3 +47,20 @@ DT_MACHINE_START(PXA27X_DT, "Marvell PXA2xx (Device Tree Support)") .dt_compat = pxa27x_dt_board_compat, MACHINE_END #endif + +#ifdef CONFIG_PXA3xx +static const char *const pxa3xx_dt_board_compat[] __initconst = { + "marvell,pxa300", + "marvell,pxa310", + "marvell,pxa320", + NULL, +}; + +DT_MACHINE_START(PXA_DT, "Marvell PXA3xx (Device Tree Support)") + .map_io = pxa3xx_map_io, + .init_irq = pxa3xx_dt_init_irq, + .handle_irq = pxa3xx_handle_irq, + .restart = pxa_restart, + .dt_compat = pxa3xx_dt_board_compat, +MACHINE_END +#endif diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c index a0de9a9ae64e..6110fa91be5e 100644 --- a/arch/arm/mach-pxa/pxa25x.c +++ b/arch/arm/mach-pxa/pxa25x.c @@ -212,12 +212,12 @@ static int __init pxa25x_init(void) register_syscore_ops(&pxa_irq_syscore_ops); register_syscore_ops(&pxa2xx_mfp_syscore_ops); - pxa2xx_set_dmac_info(16, 40); - pxa_register_device(&pxa25x_device_gpio, &pxa25x_gpio_info); - ret = platform_add_devices(pxa25x_devices, - ARRAY_SIZE(pxa25x_devices)); - if (ret) - return ret; + if (!of_have_populated_dt()) { + pxa2xx_set_dmac_info(16, 40); + pxa_register_device(&pxa25x_device_gpio, &pxa25x_gpio_info); + ret = platform_add_devices(pxa25x_devices, + ARRAY_SIZE(pxa25x_devices)); + } } return ret;
Add a device-tree machine entry (DT_MACHINE_START) for pxa25x based platforms. Take the opportunity to sort the file machine descriptions by alphabetical order. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- arch/arm/mach-pxa/Kconfig | 11 +++++++++++ arch/arm/mach-pxa/Makefile | 3 ++- arch/arm/mach-pxa/pxa-dt.c | 37 ++++++++++++++++++++++++++----------- arch/arm/mach-pxa/pxa25x.c | 12 ++++++------ 4 files changed, 45 insertions(+), 18 deletions(-)