| Message ID | 201207051357.07195.arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 05, 2012 at 01:57:06PM +0000, Arnd Bergmann wrote: > The non-DT path for this is a huge mess, I'd rather focus on making > it obsolete than trying to fix it. Other than that, I agree that > we should be registering the ab8500 from the prcmu from both the > DT and the non-DT case. If that's the decision then we should probably have a comment in the code saying that this is what's going on to warn off anyone who's inclined to cut'n'paste.
On 05/07/12 14:57, Arnd Bergmann wrote: > On Thursday 05 July 2012, Mark Brown wrote: >> On Thu, Jul 05, 2012 at 01:41:12PM +0100, Lee Jones wrote: >>> On 05/07/12 13:29, Mark Brown wrote: >> >>> If DT is not enabled, we do: >> >>> From platform code: >>> - Register the DB8500-PRCMU >>> - Register the AB8500 >> >>> So you see the registration is separate. >> >> Right, so what I'm saying is that what I'd expect unless there's >> something unusual going on is that you wouldn't be doing the separate >> registration of the AB8500 here and would instead be passing the >> platform data for the AB8500 through in the same way you pass the DT >> data through. >> >> DT and non-DT do have a very similar model for this stuff. > > The non-DT path for this is a huge mess, I'd rather focus on making > it obsolete than trying to fix it. Other than that, I agree that > we should be registering the ab8500 from the prcmu from both the > DT and the non-DT case. Ah, is that what you were saying Mark? If so, I apologise. I thought you meant register both from platform code. I'm happy to register the AB8500 from the DB8500 for _both_ DT and !DT. > Right now, for non-DT, we register ab8500 as a platform device > with board specific platform_data from arch/arm/mach-ux500/board-mop500.c > and the device just accesses the prcmu driver through its exported > functions. > > Making it registered through the prcmu sounds like the right thing > to do, but it requires funneling the board specific ab8500 platform > data through to the prcmu device registration, something like the > patch below, which is not really making things nicer overall. The patch doesn't look awful. There are more '-' than '+', and it's only a temporary solution, as the plan is to go solely DT once it's been proven viable and ST-Ericsson's delta has been DT:ed and upstreamed anyway. > diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c > index 1509a3c..f8fae8c 100644 > --- a/arch/arm/mach-ux500/board-mop500.c > +++ b/arch/arm/mach-ux500/board-mop500.c > @@ -197,24 +197,6 @@ static struct ab8500_platform_data ab8500_platdata = { > .gpio = &ab8500_gpio_pdata, > }; > > -static struct resource ab8500_resources[] = { > - [0] = { > - .start = IRQ_DB8500_AB8500, > - .end = IRQ_DB8500_AB8500, > - .flags = IORESOURCE_IRQ > - } > -}; > - > -struct platform_device ab8500_device = { > - .name = "ab8500-core", > - .id = 0, > - .dev = { > - .platform_data = &ab8500_platdata, > - }, > - .num_resources = 1, > - .resource = ab8500_resources, > -}; > - > /* > * TPS61052 > */ > @@ -460,7 +442,6 @@ static struct hash_platform_data u8500_hash1_platform_data = { > /* add any platform devices here - TODO */ > static struct platform_device *mop500_platform_devs[] __initdata = { > &mop500_gpio_keys_device, > - &ab8500_device, > }; > > #ifdef CONFIG_STE_DMA40 > @@ -622,7 +603,6 @@ static struct platform_device *snowball_platform_devs[] __initdata = { > &snowball_led_dev, > &snowball_key_dev, > &snowball_sbnet_dev, > - &ab8500_device, > }; > > static struct platform_device *snowball_of_platform_devs[] __initdata = { > @@ -639,9 +619,8 @@ static void __init mop500_init_machine(void) > mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR; > > mop500_pinmaps_init(); > - parent = u8500_init_devices(); > + parent = u8500_init_devices(&ab8500_platform_data); > > - /* FIXME: parent of ab8500 should be prcmu */ > for (i = 0; i < ARRAY_SIZE(mop500_platform_devs); i++) > mop500_platform_devs[i]->dev.parent = parent; > > @@ -674,7 +653,8 @@ static void __init snowball_init_machine(void) > int i; > > snowball_pinmaps_init(); > - parent = u8500_init_devices(); > + > + parent = u8500_init_devices(&ab8500_platform_data); > > for (i = 0; i < ARRAY_SIZE(snowball_platform_devs); i++) > snowball_platform_devs[i]->dev.parent = parent; > @@ -706,7 +686,7 @@ static void __init hrefv60_init_machine(void) > mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO; > > hrefv60_pinmaps_init(); > - parent = u8500_init_devices(); > + parent = u8500_init_devices(&ab8500_platform_data); > > for (i = 0; i < ARRAY_SIZE(mop500_platform_devs); i++) > mop500_platform_devs[i]->dev.parent = parent; > diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c > index 33275eb..6cc247c 100644 > --- a/arch/arm/mach-ux500/cpu-db8500.c > +++ b/arch/arm/mach-ux500/cpu-db8500.c > @@ -207,7 +207,7 @@ static struct device * __init db8500_soc_device_init(void) > /* > * This function is called from the board init > */ > -struct device * __init u8500_init_devices(void) > +struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500) > { > struct device *parent; > int i; > @@ -224,6 +224,8 @@ struct device * __init u8500_init_devices(void) > for (i = 0; i < ARRAY_SIZE(platform_devs); i++) > platform_devs[i]->dev.parent = parent; > > + db8500_prcmu_device.dev.platform_data = ab8500; > + > platform_add_devices(platform_devs, ARRAY_SIZE(platform_devs)); > > return parent; > diff --git a/arch/arm/mach-ux500/include/mach/setup.h b/arch/arm/mach-ux500/include/mach/setup.h > index 8b7ed82..7940615 100644 > --- a/arch/arm/mach-ux500/include/mach/setup.h > +++ b/arch/arm/mach-ux500/include/mach/setup.h > @@ -17,7 +17,7 @@ > void __init ux500_map_io(void); > extern void __init u8500_map_io(void); > > -extern struct device * __init u8500_init_devices(void); > +extern struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500); > > extern void __init ux500_init_irq(void); > extern void __init ux500_init_late(void); > diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c > index 50e83dc5..fc0bd4e 100644 > --- a/drivers/mfd/db8500-prcmu.c > +++ b/drivers/mfd/db8500-prcmu.c > @@ -2987,6 +2987,8 @@ static int __devinit db8500_prcmu_probe(struct platform_device *pdev) > goto no_irq_return; > } > > + db8500_prcmu_devs[AB8500].platform_data = pdev->dev.platform_data; > + > if (cpu_is_u8500v20_or_later()) > prcmu_config_esram0_deep_sleep(ESRAM0_DEEP_SLEEP_STATE_RET); > >
On Thu, Jul 05, 2012 at 03:06:20PM +0100, Lee Jones wrote: > On 05/07/12 14:57, Arnd Bergmann wrote: > >The non-DT path for this is a huge mess, I'd rather focus on making > >it obsolete than trying to fix it. Other than that, I agree that > >we should be registering the ab8500 from the prcmu from both the > >DT and the non-DT case. > Ah, is that what you were saying Mark? > If so, I apologise. I thought you meant register both from platform > code. I'm happy to register the AB8500 from the DB8500 for _both_ DT > and !DT. Yes, that's what I was trying to say - sorry that I wasn't clear!
On 05/07/12 15:13, Mark Brown wrote: > On Thu, Jul 05, 2012 at 03:06:20PM +0100, Lee Jones wrote: >> On 05/07/12 14:57, Arnd Bergmann wrote: > >>> The non-DT path for this is a huge mess, I'd rather focus on making >>> it obsolete than trying to fix it. Other than that, I agree that >>> we should be registering the ab8500 from the prcmu from both the >>> DT and the non-DT case. > >> Ah, is that what you were saying Mark? > >> If so, I apologise. I thought you meant register both from platform >> code. I'm happy to register the AB8500 from the DB8500 for _both_ DT >> and !DT. > > Yes, that's what I was trying to say - sorry that I wasn't clear! Well it seems silly of me just to copy Arnd's work. Do you want to author and submit the patch to do this Arnd?
On Thursday 05 July 2012, Lee Jones wrote: > > On 05/07/12 15:13, Mark Brown wrote: > > On Thu, Jul 05, 2012 at 03:06:20PM +0100, Lee Jones wrote: > >> On 05/07/12 14:57, Arnd Bergmann wrote: > > > >>> The non-DT path for this is a huge mess, I'd rather focus on making > >>> it obsolete than trying to fix it. Other than that, I agree that > >>> we should be registering the ab8500 from the prcmu from both the > >>> DT and the non-DT case. > > > >> Ah, is that what you were saying Mark? > > > >> If so, I apologise. I thought you meant register both from platform > >> code. I'm happy to register the AB8500 from the DB8500 for both DT > >> and !DT. > > > > Yes, that's what I was trying to say - sorry that I wasn't clear! > > Well it seems silly of me just to copy Arnd's work. Do you want to > author and submit the patch to do this Arnd? No, just assume ownership of it. I haven't tried building the patch, so I assume you will have to change it some more. Just list me as Suggested-by:. Arnd
On 05/07/12 16:41, Arnd Bergmann wrote: > On Thursday 05 July 2012, Lee Jones wrote: >> >> On 05/07/12 15:13, Mark Brown wrote: >>> On Thu, Jul 05, 2012 at 03:06:20PM +0100, Lee Jones wrote: >>>> On 05/07/12 14:57, Arnd Bergmann wrote: >>> >>>>> The non-DT path for this is a huge mess, I'd rather focus on making >>>>> it obsolete than trying to fix it. Other than that, I agree that >>>>> we should be registering the ab8500 from the prcmu from both the >>>>> DT and the non-DT case. >>> >>>> Ah, is that what you were saying Mark? >>> >>>> If so, I apologise. I thought you meant register both from platform >>>> code. I'm happy to register the AB8500 from the DB8500 for both DT >>>> and !DT. >>> >>> Yes, that's what I was trying to say - sorry that I wasn't clear! >> >> Well it seems silly of me just to copy Arnd's work. Do you want to >> author and submit the patch to do this Arnd? > > No, just assume ownership of it. I haven't tried building the > patch, so I assume you will have to change it some more. Just > list me as Suggested-by:. It'll have to wait until tomorrow now, but okay, I'll take it on.
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c index 1509a3c..f8fae8c 100644 --- a/arch/arm/mach-ux500/board-mop500.c +++ b/arch/arm/mach-ux500/board-mop500.c @@ -197,24 +197,6 @@ static struct ab8500_platform_data ab8500_platdata = { .gpio = &ab8500_gpio_pdata, }; -static struct resource ab8500_resources[] = { - [0] = { - .start = IRQ_DB8500_AB8500, - .end = IRQ_DB8500_AB8500, - .flags = IORESOURCE_IRQ - } -}; - -struct platform_device ab8500_device = { - .name = "ab8500-core", - .id = 0, - .dev = { - .platform_data = &ab8500_platdata, - }, - .num_resources = 1, - .resource = ab8500_resources, -}; - /* * TPS61052 */ @@ -460,7 +442,6 @@ static struct hash_platform_data u8500_hash1_platform_data = { /* add any platform devices here - TODO */ static struct platform_device *mop500_platform_devs[] __initdata = { &mop500_gpio_keys_device, - &ab8500_device, }; #ifdef CONFIG_STE_DMA40 @@ -622,7 +603,6 @@ static struct platform_device *snowball_platform_devs[] __initdata = { &snowball_led_dev, &snowball_key_dev, &snowball_sbnet_dev, - &ab8500_device, }; static struct platform_device *snowball_of_platform_devs[] __initdata = { @@ -639,9 +619,8 @@ static void __init mop500_init_machine(void) mop500_gpio_keys[0].gpio = GPIO_PROX_SENSOR; mop500_pinmaps_init(); - parent = u8500_init_devices(); + parent = u8500_init_devices(&ab8500_platform_data); - /* FIXME: parent of ab8500 should be prcmu */ for (i = 0; i < ARRAY_SIZE(mop500_platform_devs); i++) mop500_platform_devs[i]->dev.parent = parent; @@ -674,7 +653,8 @@ static void __init snowball_init_machine(void) int i; snowball_pinmaps_init(); - parent = u8500_init_devices(); + + parent = u8500_init_devices(&ab8500_platform_data); for (i = 0; i < ARRAY_SIZE(snowball_platform_devs); i++) snowball_platform_devs[i]->dev.parent = parent; @@ -706,7 +686,7 @@ static void __init hrefv60_init_machine(void) mop500_gpio_keys[0].gpio = HREFV60_PROX_SENSE_GPIO; hrefv60_pinmaps_init(); - parent = u8500_init_devices(); + parent = u8500_init_devices(&ab8500_platform_data); for (i = 0; i < ARRAY_SIZE(mop500_platform_devs); i++) mop500_platform_devs[i]->dev.parent = parent; diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c index 33275eb..6cc247c 100644 --- a/arch/arm/mach-ux500/cpu-db8500.c +++ b/arch/arm/mach-ux500/cpu-db8500.c @@ -207,7 +207,7 @@ static struct device * __init db8500_soc_device_init(void) /* * This function is called from the board init */ -struct device * __init u8500_init_devices(void) +struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500) { struct device *parent; int i; @@ -224,6 +224,8 @@ struct device * __init u8500_init_devices(void) for (i = 0; i < ARRAY_SIZE(platform_devs); i++) platform_devs[i]->dev.parent = parent; + db8500_prcmu_device.dev.platform_data = ab8500; + platform_add_devices(platform_devs, ARRAY_SIZE(platform_devs)); return parent; diff --git a/arch/arm/mach-ux500/include/mach/setup.h b/arch/arm/mach-ux500/include/mach/setup.h index 8b7ed82..7940615 100644 --- a/arch/arm/mach-ux500/include/mach/setup.h +++ b/arch/arm/mach-ux500/include/mach/setup.h @@ -17,7 +17,7 @@ void __init ux500_map_io(void); extern void __init u8500_map_io(void); -extern struct device * __init u8500_init_devices(void); +extern struct device * __init u8500_init_devices(struct ab8500_platform_data *ab8500); extern void __init ux500_init_irq(void); extern void __init ux500_init_late(void); diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c index 50e83dc5..fc0bd4e 100644 --- a/drivers/mfd/db8500-prcmu.c +++ b/drivers/mfd/db8500-prcmu.c @@ -2987,6 +2987,8 @@ static int __devinit db8500_prcmu_probe(struct platform_device *pdev) goto no_irq_return; } + db8500_prcmu_devs[AB8500].platform_data = pdev->dev.platform_data; + if (cpu_is_u8500v20_or_later()) prcmu_config_esram0_deep_sleep(ESRAM0_DEEP_SLEEP_STATE_RET);