MFD: DA9052 MFD core module v8
Subsystems (2)
PATCH 1/11 MFD: DA9052 MFD core module v8
The DA9052/53 is a highly integrated PMIC subsystem with supply domain flexibility to support wide range of high performance application.
It provides voltage regulators, GPIO controller, Touch Screen, RTC, Battery control and other functionality.
---
Changes since v8:
- Add read/write mask flag for SPI
Changes since v7:
- Replace ADC polling with interrupt mechanism
- Move macros related to battery module IRQs to header file Changes since v6:
- Move inlined device I/O functions to header file
Changes since v5:
- Inline device I/O
- Unhandle IRQ return IRQ_NONE
- Remove register address conversion
Changes since v4:
- Modify KConfig
- Use device registration for detemining chip id
- Remove preprocessor directive with runtime bustype variable
- Replace module_init() for spi with subsys_initcall()
- Remove platform string from MODULE_ALIAS of spi module. Changes since v3:
- Code refactored to use REGMAP API.
- Add Battery resources.
- da9052_set_bits() and da9052_clear_bits() replaced by da9052_reg_update().
- Add support for DA9053 PMIC
Changes since v2:
- Drop da9052_irqs[] table.
- Move struct da9052_subdev_info[].
- Remove initialization of static member.
- Care for NULL pdata init().
- Check removal of subdevices on errors.
- Remove open source spi code.
- Remove '_spi' from the driver name.
- Move tbat_lookup table from header file.
- Remove irq.h
- Remove num_gpio variable from pdata
--- drivers/mfd/Kconfig | 26 ++ drivers/mfd/Makefile | 6 + drivers/mfd/da9052-core.c | 371 ++++++++++++++++++ drivers/mfd/da9052-i2c.c | 144 +++++++ drivers/mfd/da9052-irq.c | 175 +++++++++ drivers/mfd/da9052-spi.c | 169 ++++++++ include/linux/mfd/da9052/da9052.h | 154 ++++++++ include/linux/mfd/da9052/pdata.h | 42 ++ include/linux/mfd/da9052/reg.h | 777 +++++++++++++++++++++++++++++++++++++ 9 files changed, 1864 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/da9052-core.c create mode 100644 drivers/mfd/da9052-i2c.c create mode 100644 drivers/mfd/da9052-irq.c create mode 100644 drivers/mfd/da9052-spi.c create mode 100644 include/linux/mfd/da9052/da9052.h create mode 100644 include/linux/mfd/da9052/pdata.h create mode 100644 include/linux/mfd/da9052/reg.h
drivers/mfd/Kconfig +26 -0
@@ -328,6 +328,32 @@ config PMIC_DA903X individual components like LCD backlight, voltage regulators, LEDs and battery-charger under the corresponding menus. +config PMIC_DA9052 + bool + select MFD_CORE + +config MFD_DA9052_SPI + bool "Support Dialog Semiconductor DA9052/53 PMIC variants with SPI" + select REGMAP_SPI + select PMIC_DA9052 + depends on SPI_MASTER=y + help + Support for the Dialog Semiconductor DA9052 PMIC + when controlled using SPI. This driver provides common support + for accessing the device, additional drivers must be enabled in + order to use the functionality of the device. + +config MFD_DA9052_I2C + bool "Support Dialog Semiconductor DA9052/53 PMIC variants with I2C" + select REGMAP_I2C + select PMIC_DA9052 + depends on I2C=y + help + Support for the Dialog Semiconductor DA9052 PMIC + when controlled using I2C. This driver provides common support + for accessing the device, additional drivers must be enabled in + order to use the functionality of the device. + config PMIC_ADP5520 bool "Analog Devices ADP5520/01 MFD PMIC Core Support" depends on I2C=y
drivers/mfd/Makefile +6 -0
@@ -67,6 +67,12 @@ endif obj-$(CONFIG_UCB1400_CORE) += ucb1400_core.o obj-$(CONFIG_PMIC_DA903X) += da903x.o + +da9052-objs := da9052-core.o da9052-irq.o +obj-$(CONFIG_PMIC_DA9052) += da9052.o +obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o +obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o + max8925-objs := max8925-core.o max8925-i2c.o obj-$(CONFIG_MFD_MAX8925) += max8925.o obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o
drivers/mfd/da9052-core.c new file +371 -0
371 lines not on this page · raw message
drivers/mfd/da9052-i2c.c new file +144 -0
144 lines not on this page · raw message
drivers/mfd/da9052-irq.c new file +175 -0
175 lines not on this page · raw message
drivers/mfd/da9052-spi.c new file +169 -0
169 lines not on this page · raw message
include/linux/mfd/da9052/da9052.h new file +154 -0
154 lines not on this page · raw message
include/linux/mfd/da9052/pdata.h new file +42 -0
42 lines not on this page · raw message
include/linux/mfd/da9052/reg.h new file +777 -0
777 lines not on this page · raw message
Signature · 1 line
1.7.1
On Fri, Nov 18, 2011 at 02:49:54PM +0530, Ashish Jangam wrote:
There's still a few issues in here. It'd be very much easier to review this stuff if you split the patch down into smaller patches, for example having the ADC, SPI and I2C as separate patches. The bigger each individual e-mail is the more work it is to review.
Please also try to fix whatever problems you're having posting patch serieses - this claims to be patch 1/11 but it looked like only patches numbered 1, 4 and 5 got posted and none of those were threaded together.
drivers/mfd/da9052-core.c line 117
Quoted 11 lines
drivers/mfd/da9052-core.c new file +11
-0+int da9052_adc_manual_read (struct da9052 *da9052,unsigned char channel) +{ +struct da9052_auxadc_req *req; +int ret; +unsigned short calc_data; +unsigned short data; +unsigned char mux_sel; + + req =kzalloc (sizeof (*req),GFP_KERNEL ); +if (!req) +return -ENOMEM ;
This function sometimes returns errnos but at other times...
drivers/mfd/da9052-core.c line 153
Quoted 3 lines
drivers/mfd/da9052-core.c new file +3
-0+ ret =da9052_reg_read (da9052,DA9052_ADC_RES_H_REG ); +if (ret <0 ) +return IRQ_NONE ;
...it returns irqreturn_t values. In the above case I'd *really* expect to see the error from the return passed back to the caller. It also looks like this error case returns with the auxadc_lock held which is going to deadlock the next time someone tries to use the ADC - the same problem exists for some other code.
Quoted 7 lines
drivers/mfd/da9052-core.c new file +7
-0+int da9052_add_regulator_devices (struct da9052 *da9052, +struct da9052_pdata *pdata) +{ +struct platform_device *pdev; +int i, ret; + +for (i =0 ; i < pdata->num_regulators; i++) {
drivers/mfd/da9052-core.c line 275
Quoted 10 lines
drivers/mfd/da9052-core.c new file +10
-0+ pdev =platform_device_alloc ("da9052-regulator" , i); +if (!pdev) +return -ENOMEM ; + + pdev->dev.parent = da9052->dev; + ret =platform_device_add (pdev); +if (ret) { +platform_device_put (pdev); +return ret; + }
As I think has been previously said it's better style to unconditionally register all the regulators the device has. The core should cope fine with missing platform data and this gives readback support for the regulators that aren't software configured yet.
drivers/mfd/da9052-core.c line 283
Quoted 3 lines
drivers/mfd/da9052-core.c new file +3
-0+static struct mfd_cell __initdata da9052_subdev_info[] = { + {"da9052-onkey" , .resources = &da9052_onkey_resource, + .num_resources =1 },
Spaces around the { }.
drivers/mfd/da9052-core.c line 343
Quoted 2 lines
drivers/mfd/da9052-core.c new file +2
-0+ ret = request_threaded_irq(pdata->irq_base + 13, + NULL, da9052_auxadc_irq,
Should replace the magic number with a define.
drivers/mfd/da9052-i2c.c line 66
Quoted 1 line
drivers/mfd/da9052-i2c.c new file +1
-0+ da9052_i2c->bustype =BUS_I2C ;
bustype should be redundant now, it certianly doesn't seem to be referred to in this patch.
Quoted 2 lines
+/*
+ * Interrupt controller support for Dilaog DA9052 PMICs.
This looks very much like it could be replaced with regmap-irq. The code would be slightly less efficient due to the support for sparse interrupt registers but it'd be less code.
drivers/mfd/da9052-spi.c line 119
Quoted 11 lines
drivers/mfd/da9052-spi.c new file +11
-0+static struct spi_driver da9053_ba_spi_driver = { + .probe = da9052_spi_probe, + .remove =__devexit_p (da9052_spi_remove), + .driver = { + .name ="da9053-ba" , + .bus = &spi_bus_type, + .owner =THIS_MODULE , + }, +}; + +static struct spi_driver da9053_bb_spi_driver = {
Use spi_device_id rather than registering multiple driver instances.
Quoted 8 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Thursday, November 24, 2011 5:36 PM
To: Ashish Jangam
Cc: [email protected]; [email protected]; [email protected]; Dajun; [email protected]
Subject: Re: [PATCH 01/11] MFD: DA9052 MFD core module v8
Quoted 6 lines
drivers/mfd/da9052-i2c.c line 66
Quoted 1 line · Ashish Jangam
drivers/mfd/da9052-i2c.c new file +1
-0+ da9052_i2c->bustype =BUS_I2C ;bustype should be redundant now, it certianly doesn't seem to be referred to in this patch.
This is required in the TSI driver.
Quoted 7 lines
Quoted 3 lines
+/*
+ * Interrupt controller support for Dilaog DA9052 PMICs.This looks very much like it could be replaced with regmap-irq. The code would be slightly less efficient due to the support for sparse interrupt registers but it'd be less code.
regmap-irq has a opaque struct regmap_irq_chip_data which has a member irq_base and this is required for non-primary irqs registration and also the clean-up function regmap_del_irq_chip() requires it. So as of now I will keep the current irq implementation as it is.
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
Quoted 8 lines · Ashish Jangam
Quoted 6 lines · Ashish Jangam
drivers/mfd/da9052-i2c.c line 66
Quoted 2 lines
drivers/mfd/da9052-i2c.c new file +1
-0+ da9052_i2c->bustype =BUS_I2C ;bustype should be redundant now, it certianly doesn't seem to be referred to in this patch.
This is required in the TSI driver.
You can just query the bus_type of the struct device if you really need this.
Quoted 10 lines
Quoted 5 lines
Quoted 1 line
+ * Interrupt controller support for Dilaog DA9052 PMICs.
This looks very much like it could be replaced with regmap-irq. The code would be slightly less efficient due to the support for sparse interrupt registers but it'd be less code.
regmap-irq has a opaque struct
regmap_irq_chip_datawhich has a memberirq_baseand this is required for non-primary irqs registration and also the clean-up functionregmap_del_irq_chip()requires it. So as of now I will keep the current irq implementation as it is.
I'm not sure how this is relevant to my above comment? The struct is of course opaque since it is only used by the implementation.
Quoted 18 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Friday, December 02, 2011 7:33 PM
To: Ashish Jangam
Cc: [email protected]; [email protected]; [email protected]; Dajun; [email protected]
Subject: Re: [PATCH 01/11] MFD: DA9052 MFD core module v8Quoted 7 lines · Ashish Jangam
Quoted 7 lines · Ashish Jangam
Quoted 2 lines
+ * Interrupt controller support for Dilaog DA9052 PMICs.
This looks very much like it could be replaced with regmap-irq. The code would be slightly less efficient due to the support for sparse interrupt registers but it'd be less code.
Quoted 7 lines
Quoted 4 lines
regmap-irq has a opaque struct
regmap_irq_chip_datawhich has a memberirq_baseand this is required for non-primary irqs registration and also the clean-up functionregmap_del_irq_chip()requires it. So as of now I will keep the current irq implementation as it is.I'm not sure how this is relevant to my above comment? The struct is of course opaque since it is only used by the implementation.
No issues on that but it has got irq_base as its member which gets initialize in the function regmap_add_irq_chip() and since this member value is not available to the user how can the mfd dependent modules like touch, battery etc can register for interrupt because during irq registration we add irq_base to the irq nmber for e.g. irq_base + battery_irq
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
On Sun, Dec 04, 2011 at 11:50:02AM +0000, Ashish Jangam wrote:
You should fix your mailer to word wrap at less than 80 columns, I've reflowed your text for legibility.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
regmap-irq has a opaque struct
regmap_irq_chip_datawhich has a memberirq_baseand this is required for non-primary irqs registration and also the clean-up functionregmap_del_irq_chip()requires it. So as of now I will keep the current irq implementation as it is.I'm not sure how this is relevant to my above comment? The struct is of course opaque since it is only used by the implementation.
Quoted 6 lines
No issues on that but it has got
irq_baseas its member which gets initialize in the functionregmap_add_irq_chip()and since this member value is not available to the user how can the mfd dependent modules like touch, battery etc can register for interrupt because during irq registration we addirq_baseto the irq nmber for e.g.irq_base+battery_irq
There's several straightforward solutions to that - for example, we can add an accessor to retrieve the information, we can use resources to pass the interrupt ranges to the MFD children or we can have the core remember the interrupt base it's passing on to the children.
Whichever one we choose we can certainly solve the problem without replicating the entire interrupt controller implementation.