mfd: twl4030: simplified child creation code
Subsystems (2)
PATCH 1/3 mfd: twl4030: simplified child creation code
From: David Brownell <[email protected]>
Minor cleanup to twl4030-core: define a helper function to populate a single child node, and use it to replace six inconsistent versions of the same logic. Both object and source code shrink.
As part of this, some devices now have more IRQ resources: battery charger, keypad, ADC, and USB transceiver. That helps to remove some irq #defines that block the children's drivers code from compiling on non-OMAP platforms.
Signed-off-by David Brownell <[email protected]>
Signed-off-by Tony Lindgren <[email protected]>
---
These three patches are not essential for 2.6.28-final, although having the first two merge would be appreciated by ASoc folk so they can test-build the twl4030 codec support on non-OMAP hw.
The diff on this first one is particularly deceptive; ugh.
drivers/mfd/twl4030-core.c | 298 +++++++++++-------------------------------- 1 file changed, 83 insertions(+), 215 deletions(-)
drivers/mfd/twl4030-core.c +83 -215
@@ -352,258 +352,126 @@ EXPORT_SYMBOL(twl4030_i2c_read_u8);/*----------------------------------------------------------------------*/ -/* -* NOTE: We know the first 8 IRQs after pdata->base_irq are -* for the PIH, and the next are for the PWR_INT SIH, since -* that's how twl_init_irq() sets things up. -*/ - -static int add_children (struct twl4030_platform_data *pdata) +static struct device *add_child (unsigned chip,const char *name, +void *pdata,unsigned pdata_len, +bool can_wakeup,int irq0,int irq1) { -struct platform_device *pdev =NULL ; -struct twl4030_client *twl =NULL ; -int status =0 ; - -if (twl_has_bci () && pdata->bci) { - twl = &twl4030_modules[3 ]; - - pdev =platform_device_alloc ("twl4030_bci" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc bci dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } - -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; - status =platform_device_add_data (pdev, pdata->bci, -sizeof (*pdata->bci)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add bci data, %d\n" , - status); - goto err; - } - } - -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +1 , - .flags =IORESOURCE_IRQ , - }; +struct platform_device *pdev; +struct twl4030_client *twl = &twl4030_modules[chip]; +int status; - status =platform_device_add_resources (pdev, &r,1 ); - } + pdev =platform_device_alloc (name,-1 ); +if (!pdev) { +dev_dbg (&twl->client->dev,"can't alloc dev\n" ); + status = -ENOMEM ; + goto err; + } -if (status ==0 ) - status =platform_device_add (pdev); +device_init_wakeup (&pdev->dev, can_wakeup); + pdev->dev.parent = &twl->client->dev; +if (pdata) { + status =platform_device_add_data (pdev, pdata, pdata_len);if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create bci dev, %d\n" , - status); +dev_dbg (&pdev->dev,"can't add platform_data\n" ); goto err; } } -if (twl_has_gpio () && pdata->gpio) { - twl = &twl4030_modules[1 ]; +if (irq0) { +struct resource r[2 ] = { + { .start = irq0, .flags =IORESOURCE_IRQ , }, + { .start = irq1, .flags =IORESOURCE_IRQ , }, + }; - pdev =platform_device_alloc ("twl4030_gpio" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc gpio dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; + status =platform_device_add_resources (pdev, r, irq1 ?2 :1 ); +if (status <0 ) { +dev_dbg (&pdev->dev,"can't add irqs\n" ); goto err; } + } -/* more driver model init */ -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; -/* device_init_wakeup(&pdev->dev, 1); */ + status =platform_device_add (pdev); - status =platform_device_add_data (pdev, pdata->gpio, -sizeof (*pdata->gpio)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add gpio data, %d\n" , - status); - goto err; - } - } +err: +if (status <0 ) { +platform_device_put (pdev); +dev_err (&twl->client->dev,"can't add %s dev\n" , name); +return ERR_PTR (status); + } +return &pdev->dev; +} -/* GPIO module IRQ */ -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +0 , - .flags =IORESOURCE_IRQ , - }; +/* +* NOTE: We know the first 8 IRQs after pdata->base_irq are +* for the PIH, and the next are for the PWR_INT SIH, since +* that's how twl_init_irq() sets things up. +*/ - status =platform_device_add_resources (pdev, &r,1 ); - } +static int add_children (struct twl4030_platform_data *pdata) +{ +struct device *child; -if (status ==0 ) - status =platform_device_add (pdev); +if (twl_has_bci () && pdata->bci) { + child =add_child (3 ,"twl4030_bci" , + pdata->bci,sizeof (*pdata->bci), + false, +/* irq0 = CHG_PRES, irq1 = BCI */ + pdata->irq_base +8 +1 , pdata->irq_base +2 ); +if (IS_ERR (child)) +return PTR_ERR (child); + } -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create gpio dev, %d\n" , - status); - goto err; - } +if (twl_has_gpio () && pdata->gpio) { + child =add_child (1 ,"twl4030_gpio" , + pdata->gpio,sizeof (*pdata->gpio), + false, pdata->irq_base +0 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_keypad () && pdata->keypad) { - pdev =platform_device_alloc ("twl4030_keypad" ,-1 ); -if (pdev) { - twl = &twl4030_modules[2 ]; - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->keypad, -sizeof (*pdata->keypad)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add keypad data, %d\n" , - status); -platform_device_put (pdev); - goto err; - } - status =platform_device_add (pdev); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create keypad dev, %d\n" , - status); - goto err; - } - }else { -pr_debug ("%s: can't alloc keypad dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } + child =add_child (2 ,"twl4030_keypad" , + pdata->keypad,sizeof (*pdata->keypad), + true, pdata->irq_base +1 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_madc () && pdata->madc) { - pdev =platform_device_alloc ("twl4030_madc" ,-1 ); -if (pdev) { - twl = &twl4030_modules[2 ]; - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->madc, -sizeof (*pdata->madc)); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't add madc data, %d\n" , - status); - goto err; - } - status =platform_device_add (pdev); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create madc dev, %d\n" , - status); - goto err; - } - }else { -pr_debug ("%s: can't alloc madc dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } + child =add_child (2 ,"twl4030_madc" , + pdata->madc,sizeof (*pdata->madc), + true, pdata->irq_base +3 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_rtc ()) { - twl = &twl4030_modules[3 ]; - - pdev =platform_device_alloc ("twl4030_rtc" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc rtc dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - }else { - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - } -/* -* REVISIT platform_data here currently might use of +* REVISIT platform_data here currently might expose the * "msecure" line ... but for now we just expect board -* setup to tell the chip "we are secure" at all times. +* setup to tell the chip "it's always ok to SET_TIME". * Eventually, Linux might become more aware of such * HW security concerns, and "least privilege". */ - -/* RTC module IRQ */ -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +3 , - .flags =IORESOURCE_IRQ , - }; - - status =platform_device_add_resources (pdev, &r,1 ); - } - -if (status ==0 ) - status =platform_device_add (pdev); - -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create rtc dev, %d\n" , - status); - goto err; - } + child =add_child (3 ,"twl4030_rtc" , +NULL ,0 , + true, pdata->irq_base +8 +3 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_usb () && pdata->usb) { - twl = &twl4030_modules[0 ]; - - pdev =platform_device_alloc ("twl4030_usb" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc usb dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } - -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->usb, -sizeof (*pdata->usb)); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't add usb data, %d\n" , - status); - goto err; - } - } - -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +2 , - .flags =IORESOURCE_IRQ , - }; - - status =platform_device_add_resources (pdev, &r,1 ); - } - -if (status ==0 ) - status =platform_device_add (pdev); - -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create usb dev, %d\n" , - status); - } + child =add_child (0 ,"twl4030_usb" , + pdata->usb,sizeof (*pdata->usb), + true, +/* irq0 = USB_PRES, irq1 = USB */ + pdata->irq_base +8 +2 , pdata->irq_base +4 ); +if (IS_ERR (child)) +return PTR_ERR (child); } -err: -if (status) -pr_err ("failed to add twl4030's children (status %d)\n" , status); -return status; +return 0 ; }/*----------------------------------------------------------------------*/
Hi David,
Quoted 11 lines · David Brownell
From: David Brownell <[email protected]>
Minor cleanup to twl4030-core: define a helper function to populate a single child node, and use it to replace six inconsistent versions of the same logic. Both object and source code shrink.
As part of this, some devices now have more IRQ resources: battery charger, keypad, ADC, and USB transceiver. That helps to remove some irq #defines that block the children's drivers code from compiling on non-OMAP platforms.
Quoted 6 lines
Signed-off-by David Brownell <[email protected]>Signed-off-by Tony Lindgren <[email protected]>---
These three patches are not essential for 2.6.28-final, although having the first two merge would be appreciated by ASoc folk so they can test-build the twl4030 codec support on non-OMAP hw.
All 3 patches pushed to my for-next branch, thanks.
Cheers,
Samuel.
Quoted 349 lines
The diff on this first one is particularly deceptive; ugh.
drivers/mfd/twl4030-core.c | 298 +++++++++++-------------------------------- 1 file changed, 83 insertions(+), 215 deletions(-)drivers/mfd/twl4030-core.c +83
-215@@ -352,258 +352,126 @@ EXPORT_SYMBOL(twl4030_i2c_read_u8);/*----------------------------------------------------------------------*/ -/* -* NOTE: We know the first 8 IRQs after pdata->base_irq are -* for the PIH, and the next are for the PWR_INT SIH, since -* that's how twl_init_irq() sets things up. -*/ - -static int add_children (struct twl4030_platform_data *pdata) +static struct device *add_child (unsigned chip,const char *name, +void *pdata,unsigned pdata_len, +bool can_wakeup,int irq0,int irq1) { -struct platform_device *pdev =NULL ; -struct twl4030_client *twl =NULL ; -int status =0 ; - -if (twl_has_bci () && pdata->bci) { - twl = &twl4030_modules[3 ]; - - pdev =platform_device_alloc ("twl4030_bci" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc bci dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } - -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; - status =platform_device_add_data (pdev, pdata->bci, -sizeof (*pdata->bci)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add bci data, %d\n" , - status); - goto err; - } - } - -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +1 , - .flags =IORESOURCE_IRQ , - }; +struct platform_device *pdev; +struct twl4030_client *twl = &twl4030_modules[chip]; +int status; - status =platform_device_add_resources (pdev, &r,1 ); - } + pdev =platform_device_alloc (name,-1 ); +if (!pdev) { +dev_dbg (&twl->client->dev,"can't alloc dev\n" ); + status = -ENOMEM ; + goto err; + } -if (status ==0 ) - status =platform_device_add (pdev); +device_init_wakeup (&pdev->dev, can_wakeup); + pdev->dev.parent = &twl->client->dev; +if (pdata) { + status =platform_device_add_data (pdev, pdata, pdata_len);if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create bci dev, %d\n" , - status); +dev_dbg (&pdev->dev,"can't add platform_data\n" ); goto err; } } -if (twl_has_gpio () && pdata->gpio) { - twl = &twl4030_modules[1 ]; +if (irq0) { +struct resource r[2 ] = { + { .start = irq0, .flags =IORESOURCE_IRQ , }, + { .start = irq1, .flags =IORESOURCE_IRQ , }, + }; - pdev =platform_device_alloc ("twl4030_gpio" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc gpio dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; + status =platform_device_add_resources (pdev, r, irq1 ?2 :1 ); +if (status <0 ) { +dev_dbg (&pdev->dev,"can't add irqs\n" ); goto err; } + } -/* more driver model init */ -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; -/* device_init_wakeup(&pdev->dev, 1); */ + status =platform_device_add (pdev); - status =platform_device_add_data (pdev, pdata->gpio, -sizeof (*pdata->gpio)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add gpio data, %d\n" , - status); - goto err; - } - } +err: +if (status <0 ) { +platform_device_put (pdev); +dev_err (&twl->client->dev,"can't add %s dev\n" , name); +return ERR_PTR (status); + } +return &pdev->dev; +} -/* GPIO module IRQ */ -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +0 , - .flags =IORESOURCE_IRQ , - }; +/* +* NOTE: We know the first 8 IRQs after pdata->base_irq are +* for the PIH, and the next are for the PWR_INT SIH, since +* that's how twl_init_irq() sets things up. +*/ - status =platform_device_add_resources (pdev, &r,1 ); - } +static int add_children (struct twl4030_platform_data *pdata) +{ +struct device *child; -if (status ==0 ) - status =platform_device_add (pdev); +if (twl_has_bci () && pdata->bci) { + child =add_child (3 ,"twl4030_bci" , + pdata->bci,sizeof (*pdata->bci), + false, +/* irq0 = CHG_PRES, irq1 = BCI */ + pdata->irq_base +8 +1 , pdata->irq_base +2 ); +if (IS_ERR (child)) +return PTR_ERR (child); + } -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create gpio dev, %d\n" , - status); - goto err; - } +if (twl_has_gpio () && pdata->gpio) { + child =add_child (1 ,"twl4030_gpio" , + pdata->gpio,sizeof (*pdata->gpio), + false, pdata->irq_base +0 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_keypad () && pdata->keypad) { - pdev =platform_device_alloc ("twl4030_keypad" ,-1 ); -if (pdev) { - twl = &twl4030_modules[2 ]; - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->keypad, -sizeof (*pdata->keypad)); -if (status <0 ) { -dev_dbg (&twl->client->dev, -"can't add keypad data, %d\n" , - status); -platform_device_put (pdev); - goto err; - } - status =platform_device_add (pdev); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create keypad dev, %d\n" , - status); - goto err; - } - }else { -pr_debug ("%s: can't alloc keypad dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } + child =add_child (2 ,"twl4030_keypad" , + pdata->keypad,sizeof (*pdata->keypad), + true, pdata->irq_base +1 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_madc () && pdata->madc) { - pdev =platform_device_alloc ("twl4030_madc" ,-1 ); -if (pdev) { - twl = &twl4030_modules[2 ]; - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->madc, -sizeof (*pdata->madc)); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't add madc data, %d\n" , - status); - goto err; - } - status =platform_device_add (pdev); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create madc dev, %d\n" , - status); - goto err; - } - }else { -pr_debug ("%s: can't alloc madc dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } + child =add_child (2 ,"twl4030_madc" , + pdata->madc,sizeof (*pdata->madc), + true, pdata->irq_base +3 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_rtc ()) { - twl = &twl4030_modules[3 ]; - - pdev =platform_device_alloc ("twl4030_rtc" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc rtc dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - }else { - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - } -/* -* REVISIT platform_data here currently might use of +* REVISIT platform_data here currently might expose the * "msecure" line ... but for now we just expect board -* setup to tell the chip "we are secure" at all times. +* setup to tell the chip "it's always ok to SET_TIME". * Eventually, Linux might become more aware of such * HW security concerns, and "least privilege". */ - -/* RTC module IRQ */ -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +3 , - .flags =IORESOURCE_IRQ , - }; - - status =platform_device_add_resources (pdev, &r,1 ); - } - -if (status ==0 ) - status =platform_device_add (pdev); - -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create rtc dev, %d\n" , - status); - goto err; - } + child =add_child (3 ,"twl4030_rtc" , +NULL ,0 , + true, pdata->irq_base +8 +3 ,0 ); +if (IS_ERR (child)) +return PTR_ERR (child); }if (twl_has_usb () && pdata->usb) { - twl = &twl4030_modules[0 ]; - - pdev =platform_device_alloc ("twl4030_usb" ,-1 ); -if (!pdev) { -pr_debug ("%s: can't alloc usb dev\n" ,DRIVER_NAME ); - status = -ENOMEM ; - goto err; - } - -if (status ==0 ) { - pdev->dev.parent = &twl->client->dev; -device_init_wakeup (&pdev->dev,1 ); - status =platform_device_add_data (pdev, pdata->usb, -sizeof (*pdata->usb)); -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't add usb data, %d\n" , - status); - goto err; - } - } - -if (status ==0 ) { -struct resource r = { - .start = pdata->irq_base +8 +2 , - .flags =IORESOURCE_IRQ , - }; - - status =platform_device_add_resources (pdev, &r,1 ); - } - -if (status ==0 ) - status =platform_device_add (pdev); - -if (status <0 ) { -platform_device_put (pdev); -dev_dbg (&twl->client->dev, -"can't create usb dev, %d\n" , - status); - } + child =add_child (0 ,"twl4030_usb" , + pdata->usb,sizeof (*pdata->usb), + true, +/* irq0 = USB_PRES, irq1 = USB */ + pdata->irq_base +8 +2 , pdata->irq_base +4 ); +if (IS_ERR (child)) +return PTR_ERR (child); } -err: -if (status) -pr_err ("failed to add twl4030's children (status %d)\n" , status); -return status; +return 0 ; }/*----------------------------------------------------------------------*/
Signature · 2 lines
Intel Open Source Technology Centre http://oss.intel.com/
Quoted 1 line · Samuel Ortiz
All 3 patches pushed to my for-next branch, thanks.
Great ... here's a small tweak, which you may want to just merge with the last one.
- Dave
========== CUT HERE
From: David Brownell <[email protected]>
This contains two bugfixes to the initial twl4030 regulator support patch related to USB:
(a) always overwrite the old list of consumers ... else
the regulator handles all use the same "usb1v5" name;
(b) don't set up the "usbcp" regulator, which turns out to be managed through separate controls, usually ULPI directly from the OTG controller.
Signed-off-by David Brownell <[email protected]>
--- drivers/mfd/twl4030-core.c | 11 +---------- include/linux/i2c/twl4030.h | 1 - 2 files changed, 1 insertion(+), 11 deletions(-)
drivers/mfd/twl4030-core.c +1 -10
@@ -433,7 +433,7 @@ add_regulator_linked(int num, struct regif (!pdata)return NULL ; -if (consumers && !pdata->consumer_supplies) { +if (consumers) { pdata->consumer_supplies = consumers; pdata->num_consumer_supplies = num_consumers; }
@@ -556,9 +556,6 @@ add_children(struct twl4030_platform_datstatic struct regulator_consumer_supply usb3v1 = { .supply ="usb3v1" , }; -static struct regulator_consumer_supply usbcp = { - .supply ="usbcp" , - };/* this is a template that gets copied */ struct regulator_init_data usb_fixed = {
@@ -573,7 +570,6 @@ add_children(struct twl4030_platform_dat usb1v5.dev = usb_transceiver; usb1v8.dev = usb_transceiver; usb3v1.dev = usb_transceiver; - usbcp.dev = usb_transceiver; child =add_regulator_linked (TWL4030_REG_VUSB1V5 , &usb_fixed, &usb1v5,1 );
@@ -589,11 +585,6 @@ add_children(struct twl4030_platform_dat &usb3v1, 1);if (IS_ERR (child))return PTR_ERR (child); - - child =add_regulator_linked (TWL4030_REG_VUSBCP , &usb_fixed, - &usbcp,1 ); -if (IS_ERR (child)) -return PTR_ERR (child); }/* maybe add LDOs that are omitted on cost-reduced parts */
include/linux/i2c/twl4030.h +0 -1
@@ -351,6 +351,5 @@ int twl4030_sih_setup(int module);#define TWL4030_REG_VUSB1V5 17#define TWL4030_REG_VUSB1V8 18#define TWL4030_REG_VUSB3V1 19 -#define TWL4030_REG_VUSBCP 20#endif /* End of __TWL4030_H */