Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Thursday 05 July 2012, Qiao Zhou wrote:
+
+static const struct i2c_device_id pm80x_id_table[] = {
+ {88PM800, CHIP_PM800},
+ {88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
The point of moving the table to the individual drivers was that the
right one can get loaded automatically. This requires of course that
you put only one in there. It really needs to be
static const struct i2c_device_id pm800_id_table[] = {
{88PM800, CHIP_PM800},
};
MODULE_DEVICE_TABLE(i2c, pm800_id_table);
and
static const struct i2c_device_id pm805_id_table[] = {
{88PM805, CHIP_PM805},
};
MODULE_DEVICE_TABLE(i2c, pm805_id_table);
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 75f6ed6..22dba98 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -3,7 +3,12 @@
#
88pm860x-objs:= 88pm860x-core.o 88pm860x-i2c.o
+88pm80x-objs := 88pm80x-i2c.o
+88pm800-objs := 88pm800-core.o
+88pm805-objs := 88pm805-core.o
obj-$(CONFIG_MFD_88PM860X) += 88pm860x.o
+obj-$(CONFIG_MFD_88PM800)+= 88pm800.o 88pm80x.o
+obj-$(CONFIG_MFD_88PM805)+= 88pm805.o 88pm80x.o
This can be written much shorter if you leave out the 88pm80?-objs:= lines
and just build each module from one file as in
obj-$(CONFIG_MFD_88PM800) += 88pm800-core.o 88pm80x-i2c.o
It might make sense to drop the core and i2c postfixes on the names,
your choice.
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
new file mode 100644
index 000..687f296
--- /dev/null
+++ b/include/linux/mfd/88pm80x.h
I don't really like repeating myself, but this still contains a lot
of crap that needs to be moved out of this file, into the places
where it's used:
+enum {
+ /* Procida */
+ PM800_CHIP_A0 = 0x60,
+ PM800_CHIP_A1 = 0x61,
+ PM800_CHIP_B0 = 0x62,
+ PM800_CHIP_C0 = 0x63,
+ PM800_CHIP_END = PM800_CHIP_C0,
+
+ /* Make sure to update this to the last stepping */
+ PM8XXX_CHIP_END = PM800_CHIP_END
+};
88pm800-core.c
+enum {
+ PM800_ID_INVALID,
+ PM800_ID_VIBRATOR,
+ PM800_ID_SOUND,
+ PM800_ID_MAX,
+};
unused?
+enum {
+ PM800_ID_BUCK1 = 0,
+ PM800_ID_BUCK2,
+ PM800_ID_BUCK3,
+ PM800_ID_BUCK4,
+ PM800_ID_BUCK5,
+
+ PM800_ID_LDO1,
+ PM800_ID_LDO2,
+ PM800_ID_LDO3,
+ PM800_ID_LDO4,
+ PM800_ID_LDO5,
+ PM800_ID_LDO6,
+ PM800_ID_LDO7,
+ PM800_ID_LDO8,
+ PM800_ID_LDO9,
+ PM800_ID_LDO10,
+ PM800_ID_LDO11,
+ PM800_ID_LDO12,
+ PM800_ID_LDO13,
+ PM800_ID_LDO14,
+ PM800_ID_LDO15,
+ PM800_ID_LDO16,
+ PM800_ID_LDO17,
+ PM800_ID_LDO18,
+ PM800_ID_LDO19,
+
+ PM800_ID_RG_MAX,
+};
+#define PM800_MAX_REGULATOR PM800_ID_RG_MAX /* 5 Bucks, 19 LDOs */
+#define PM800_NUM_BUCK (5) /*5 Bucks */
+#define PM800_NUM_LDO (19) /*19 Bucks */
unused? should probably go into the regulator driver
+/* 88PM805 Registers */
+#define PM805_CHIP_ID(0x00)
88pm805-core.c
+/* Audio */
+
+/* 88PM800 registers */
+enum {
+ PM80X_INVALID_PAGE = 0,
+ PM80X_BASE_PAGE,
+ PM80X_POWER_PAGE,
+ PM80X_GPADC_PAGE,
+ PM80X_TEST_PAGE,
+};
unused?
+/* page 0 basic: slave adder 0x60 */
+
+/* Interrupt Registers */
+#define PM800_CHIP_ID(0x00)
+
+#define PM800_STATUS_1 (0x01)
+#define PM800_ONKEY_STS1 (1 0)
+#define PM800_EXTON_STS1 (1 1)
+#define PM800_CHG_STS1 (1 2)
+#define PM800_BAT_STS1 (1 3)
+#define PM800_VBUS_STS1 (1 4)
+#define PM800_LDO_PGOOD_STS1 (1 5)
+#define PM800_BUCK_PGOOD_STS1(1 6)
+
+#define PM800_STATUS_2 (0x02)
+#define PM800_RTC_ALARM_STS2 (1 0)
These can probably stay here.
+#define PM800_INT_STATUS1(0x05)
+#define PM800_ONKEY_INT_STS1 (1 0)
+#define PM800_EXTON_INT_STS1 (1 1)
+#define PM800_CHG_INT_STS1 (1 2)
+#define PM800_BAT_INT_STS1 (1 3)
+#define PM800_RTC_INT_STS1 (1 4)
+#define PM800_CLASSD_OC_INT_STS1 (1 5)
...
+/* number of INT_ENA INT_STATUS regs */
+#define PM800_INT_REG_NUM(4)
all the interrupt handling can go into 88pm800-core.c
+/* RTC Registers */
+#define PM800_RTC_CONTROL(0xD0)
+#define PM800_RTC_COUNTER1 (0xD1)
+#define PM800_RTC_COUNTER2 (0xD2)
+#define PM800_RTC_COUNTER3 (0xD3)
+#define PM800_RTC_COUNTER4 (0xD4)
+#define PM800_RTC_EXPIRE1_1 (0xD5)
+#define PM800_RTC_EXPIRE1_2 (0xD6)
+#define PM800_RTC_EXPIRE1_3 (0xD7)
+#define PM800_RTC_EXPIRE1_4 (0xD8)
+#define PM800_RTC_TRIM1 (0xD9)
+#define PM800_RTC_TRIM2
Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Wednesday 04 July 2012, Qiao Zhou wrote:
88PM800 and 88PM805 are two discrete chips used for power management.
Hardware designer can use them together or only one of them according
to requirement.
88pm80x_i2c.c provides common i2c driver handling for both 800 and
805, such as i2c_driver init, regmap init, read/write api etc.
88pm800_core.c handles specifically for 800, such as chip init, irq
init/handle, mfd device register, including rtc, onkey, regulator(
to be add later) etc. besides that, 800 has three i2c device, one
regular i2c client, two other i2c dummy for gpadc and power purpose.
88pm805_core.c handles specifically for 805, such as chip init, irq
init/handle, mfd device register, including codec, headset/mic detect
etc.
the i2c operation of both 800 and 805 are via regmap.
Signed-off-by: Qiao Zhou zhouq...@marvell.com
The split between the two files looks very good now, I think this way
it makes much more sense for the reader.
+ ret = mfd_add_devices(chip-dev, 0, onkey_devs[0],
+ ARRAY_SIZE(onkey_devs), onkey_resources[0],
+ chip-irq_base);
According to what I discussed with Mark in the previous version, I think you
need to pass 0 instead of chip-irq_base here, and transform the interrupt
numbers using the domain in the client drivers.
+
+const struct i2c_device_id pm80x_id_table[] = {
+ {88PM800, CHIP_PM800},
+ {88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
Since these are separate modules now, you have to move the device table
into the split files as well.
+
+/**
+ * pm80x_reg_read: Read a single 88PM80x register.
+ *
+ * @map: regmap to read from.
+ * @reg: Register to read.
+ */
+int pm80x_reg_read(struct regmap *map, u32 reg)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(map, reg, val);
+
+ if (ret 0)
+ return ret;
+ else
+ return val;
+}
+EXPORT_SYMBOL_GPL(pm80x_reg_read);
In your introductory email you write
Exported r/w API which requires regmap handle. as currently the pm800
chip has 3 i2c device, only passing a pm80x_chip info can't ensure r/w the
register in correct i2c device.
Your first driver version had this, then you removed the functions
after I asked you to, and now they are back, so I assume there is something
I don't see yet. It looks like the function is just an unnecessary wrapper
that is better open-coded in the caller. Can you explain again what the
difference is?
+/**
+ * pm80x_bulk_read: Read multiple 88PM80x registers
+ *
+ * @map: regmap to read from
+ * @reg: First register
+ * @buf: Buffer to fill. The data will be returned big endian.
+ * @count: Number of registers
+ */
+int pm80x_bulk_read(struct regmap *map, u32 reg, u8 *buf, int count)
+{
+ return regmap_raw_read(map, reg, buf, count);
+}
Unused function? Either export this if you want to provide it as
the general API, or drop the function.
+int __devinit pm80x_device_init(struct pm80x_chip *chip,
+ struct pm80x_platform_data *pdata)
+void __devexit pm80x_device_exit(struct pm80x_chip *chip)
+SIMPLE_DEV_PM_OPS(pm80x_pm_ops, pm80x_suspend, pm80x_resume);
I would think that these need to be exported as well, at least if
you want the driver to be modular.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e129c82..96dd2d7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -20,6 +20,18 @@ config MFD_88PM860X
select individual components like voltage regulators, RTC and
battery-charger under the corresponding menus.
+config MFD_88PM80X
+ bool Support Marvell 88PM800/88PM805
+ depends on I2C=y GENERIC_HARDIRQS
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ select MFD_CORE
+ help
+ This supports for Marvell 88PM800/88PM805 Power Management IC.
+ This includes the I2C driver and the core APIs _only_, you have to
+ select individual components like voltage regulators, RTC and
+ battery-charger under the corresponding menus.
+
config MFD_SM501
tristate Support for Silicon Motion SM501
---help---
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 75f6ed6..dc2584e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -3,7 +3,9 @@
#
88pm860x-objs:= 88pm860x-core.o 88pm860x-i2c.o
+88pm80x-objs := 88pm800-core.o 88pm805-core.o 88pm80x-i2c.o
obj-$(CONFIG_MFD_88PM860X) += 88pm860x.o
+obj-$(CONFIG_MFD_88PM80X)+= 88pm80x.o
obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
I just noticed that it can currently only be builtin. Is there a strict
requirement for that? If not, better make it a tristate option.
If you do that, aside from adding the exports mentioned above, you have
to use three separate modules,
RE: [PATCH 1/4] mfd: support 88pm80x in 80x driver
+ret = mfd_add_devices(chip-dev, 0, onkey_devs[0],
+ ARRAY_SIZE(onkey_devs), onkey_resources[0],
+ chip-irq_base);
According to what I discussed with Mark in the previous version, I think you
need to pass 0 instead of chip-irq_base here, and transform the interrupt
numbers using the domain in the client drivers.
+
+const struct i2c_device_id pm80x_id_table[] = {
+{88PM800, CHIP_PM800},
+{88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
Since these are separate modules now, you have to move the device table
into the split files as well.
Is it ok to export it in 88pm80x.h?
+
+/**
+ * pm80x_reg_read: Read a single 88PM80x register.
+ *
+ * @map: regmap to read from.
+ * @reg: Register to read.
+ */
+int pm80x_reg_read(struct regmap *map, u32 reg)
+{
+unsigned int val;
+int ret;
+
+ret = regmap_read(map, reg, val);
+
+if (ret 0)
+return ret;
+else
+return val;
+}
+EXPORT_SYMBOL_GPL(pm80x_reg_read);
In your introductory email you write
Exported r/w API which requires regmap handle. as currently the pm800
chip has 3 i2c device, only passing a pm80x_chip info can't ensure r/w the
register in correct i2c device.
Your first driver version had this, then you removed the functions
after I asked you to, and now they are back, so I assume there is something
I don't see yet. It looks like the function is just an unnecessary wrapper
that is better open-coded in the caller. Can you explain again what the
difference is?
After you suggest to change the r/w API so that caller doesn't care about it's
via i2c or spi, it makes sense. However due to pm800 has 3 i2c devices, and
it's hard to export such interface for pm800. Currently to add such interface
via regmap handle, caller still doesn't care about the actual hw implement,
also it's clear that all pm80x sub-driver or plat call the unified r/w API.
+/**
+ * pm80x_bulk_read: Read multiple 88PM80x registers
+ *
+ * @map: regmap to read from
+ * @reg: First register
+ * @buf: Buffer to fill. The data will be returned big endian.
+ * @count: Number of registers
+ */
+int pm80x_bulk_read(struct regmap *map, u32 reg, u8 *buf, int count)
+{
+return regmap_raw_read(map, reg, buf, count);
+}
Unused function? Either export this if you want to provide it as
the general API, or drop the function.
It's used by rtc driver.
+int __devinit pm80x_device_init(struct pm80x_chip *chip,
+struct pm80x_platform_data *pdata)
+void __devexit pm80x_device_exit(struct pm80x_chip *chip)
+SIMPLE_DEV_PM_OPS(pm80x_pm_ops, pm80x_suspend, pm80x_resume);
I would think that these need to be exported as well, at least if
you want the driver to be modular.
Would update. Thanks.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e129c82..96dd2d7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -20,6 +20,18 @@ config MFD_88PM860X
select individual components like voltage regulators, RTC and
battery-charger under the corresponding menus.
+config MFD_88PM80X
+bool Support Marvell 88PM800/88PM805
+depends on I2C=y GENERIC_HARDIRQS
+select REGMAP_I2C
+select REGMAP_IRQ
+select MFD_CORE
+help
+ This supports for Marvell 88PM800/88PM805 Power Management IC.
+ This includes the I2C driver and the core APIs _only_, you have to
+ select individual components like voltage regulators, RTC and
+ battery-charger under the corresponding menus.
+
config MFD_SM501
tristate Support for Silicon Motion SM501
---help---
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 75f6ed6..dc2584e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -3,7 +3,9 @@
#
88pm860x-objs := 88pm860x-core.o 88pm860x-i2c.o
+88pm80x-objs:= 88pm800-core.o 88pm805-core.o
88pm80x-i2c.o
obj-$(CONFIG_MFD_88PM860X) += 88pm860x.o
+obj-$(CONFIG_MFD_88PM80X) += 88pm80x.o
obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
I just noticed that it can currently only be builtin. Is there a strict
requirement for that? If not, better make it a tristate option.
If you do that, aside from adding the exports mentioned above, you have
to use three separate modules, as in
obj-$(CONFIG_MFD_88PM80X) += 88pm800-core.o 88pm805-core.o 88pm80x-i2c.o
or even
obj-$(CONFIG_MFD_88PM800) += 88pm800-core.o 88pm800-core.o
obj-$(CONFIG_MFD_88PM805) += 88pm800-core.o 88pm805-core.o
with the respective Kconfig change.
Would update. Thanks.
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
new file mode 100644
index 000..115348a
--- /dev/null
+++ b/include/linux/mfd/88pm80x.h
@@ -0,0 +1,521 @@
+#ifndef __LINUX_MFD_88PM80X_H
+#define __LINUX_MFD_88PM80X_H
+
Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Wednesday 04 July 2012, Qiao Zhou wrote:
+ ret = mfd_add_devices(chip-dev, 0, onkey_devs[0],
+ARRAY_SIZE(onkey_devs), onkey_resources[0],
+chip-irq_base);
According to what I discussed with Mark in the previous version, I think you
need to pass 0 instead of chip-irq_base here, and transform the interrupt
numbers using the domain in the client drivers.
+
+const struct i2c_device_id pm80x_id_table[] = {
+ {88PM800, CHIP_PM800},
+ {88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
Since these are separate modules now, you have to move the device table
into the split files as well.
Is it ok to export it in 88pm80x.h?
You mean putting the MODULE_DEVICE_TABLE() into 88pm80x.h? No, that would
not work.
The correct way is to have two MODULE_DEVICE_TABLE statements, one per file.
Actually the table only makes sense for loadable modules, so if you decide
to keep the driver only built-in, it's best to just drop this table.
+
+/**
+ * pm80x_reg_read: Read a single 88PM80x register.
+ *
+ * @map: regmap to read from.
+ * @reg: Register to read.
+ */
+int pm80x_reg_read(struct regmap *map, u32 reg)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(map, reg, val);
+
+ if (ret 0)
+ return ret;
+ else
+ return val;
+}
+EXPORT_SYMBOL_GPL(pm80x_reg_read);
In your introductory email you write
Exported r/w API which requires regmap handle. as currently the pm800
chip has 3 i2c device, only passing a pm80x_chip info can't ensure r/w the
register in correct i2c device.
Your first driver version had this, then you removed the functions
after I asked you to, and now they are back, so I assume there is something
I don't see yet. It looks like the function is just an unnecessary wrapper
that is better open-coded in the caller. Can you explain again what the
difference is?
After you suggest to change the r/w API so that caller doesn't care about it's
via i2c or spi, it makes sense. However due to pm800 has 3 i2c devices, and
it's hard to export such interface for pm800. Currently to add such interface
via regmap handle, caller still doesn't care about the actual hw implement,
also it's clear that all pm80x sub-driver or plat call the unified r/w API.
But there is nothing unified about the function above, it just calls
regmap_read(), so the caller already has access to the regmap pointer.
+/**
+ * pm80x_bulk_read: Read multiple 88PM80x registers
+ *
+ * @map: regmap to read from
+ * @reg: First register
+ * @buf: Buffer to fill. The data will be returned big endian.
+ * @count: Number of registers
+ */
+int pm80x_bulk_read(struct regmap *map, u32 reg, u8 *buf, int count)
+{
+ return regmap_raw_read(map, reg, buf, count);
+}
Unused function? Either export this if you want to provide it as
the general API, or drop the function.
It's used by rtc driver.
Then it needs to be exported, so the rtc driver can be a module.
I would still argue that the majority of the constants in this file
should get moved into the driver .c file that uses them. Putting them
into the header is better done only for interfaces between the
driver parts, and for constants that are used by multiple drivers.
These registers still in this header file are either used by mfd core
driver, regulator/rtc/onkey/codec, or used in platform initial setting.
Exactly. All the values that are only used by the core driver should be
part of the core driver (or a local header in the mfd directory if they
need to be shared between multiple files). The platform code should not
really touch the registers, but only things like the platform_data
structure, which indeed belongs into the global header.
+struct pm80x_subchip {
+ struct i2c_client *power_page; /* chip client for power page */
+ struct i2c_client *gpadc_page; /* chip client for gpadc page */
+ struct regmap *regmap_power;
+ struct regmap *regmap_gpadc;
+ unsigned short power_page_addr; /* power page I2C address */
+ unsigned short gpadc_page_addr; /* gpadc page I2C address */
+};
+
+struct pm80x_chip {
+ struct pm80x_subchip *subchip;
+ struct device *dev;
+ struct i2c_client *client;
+ struct regmap *regmap;
+ struct regmap_irq_chip *regmap_irq_chip;
+ struct regmap_irq_chip_data *irq_data;
+ unsigned char version;
+ int id;
+ int irq;
+ int irq_mode;
+ int irq_base;
+ unsigned int wu_flag;
+};
One thing I forgot to ask in the previous review although I had already
noticed it then: What is the separation of pm80x_chip and pm80x_subchip
used for?
Pm80x_chip is common for both pm800 and pm805, while pm80x_subchip
handles the other i2c devices as discussed before, only that these two
i2c devices are not as much as pm800 and pm805 i2c device, and they are
only for gpadc power settings purpose. The subchip can be
Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Wed, Jul 04, 2012 at 03:27:13PM +, Arnd Bergmann wrote: On Wednesday 04 July 2012, Qiao Zhou wrote: On the other hand, I think it probably makes sense to drop the irq_base member in this struct and rely on irq domains to allocate them dynamically as mentioned before. Do you mean that both regmap_add_irq_chip and mfd_add_devices api pass -1 as the irq_base, so that system can dynamically allocate the irq_base for it? regmap_add_irq_chip should pass -1, mfd_add_devices should pass 0. Mark can probably correct me if that's wrong. That's right. I do need to grovel through the irqdomain code and try to figure out if the stuff added recently for MFDs to pass an irqdomain about would also support doing the same mapping. Unfortunately all the irqdomain code I found that I didn't write was rather tied to DT which makes things more obscure, it's not clear what's for irqdomain and what's for DT. signature.asc Description: Digital signature
RE: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Wednesday 04 July 2012, Qiao Zhou wrote:
+ret = mfd_add_devices(chip-dev, 0, onkey_devs[0],
+ ARRAY_SIZE(onkey_devs),
onkey_resources[0],
+ chip-irq_base);
According to what I discussed with Mark in the previous version, I think
you
need to pass 0 instead of chip-irq_base here, and transform the interrupt
numbers using the domain in the client drivers.
+
+const struct i2c_device_id pm80x_id_table[] = {
+{88PM800, CHIP_PM800},
+{88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
Since these are separate modules now, you have to move the device table
into the split files as well.
Is it ok to export it in 88pm80x.h?
You mean putting the MODULE_DEVICE_TABLE() into 88pm80x.h? No, that would
not work.
The correct way is to have two MODULE_DEVICE_TABLE statements, one per file.
Actually the table only makes sense for loadable modules, so if you decide
to keep the driver only built-in, it's best to just drop this table.
Understood and would update.
+
+/**
+ * pm80x_reg_read: Read a single 88PM80x register.
+ *
+ * @map: regmap to read from.
+ * @reg: Register to read.
+ */
+int pm80x_reg_read(struct regmap *map, u32 reg)
+{
+unsigned int val;
+int ret;
+
+ret = regmap_read(map, reg, val);
+
+if (ret 0)
+return ret;
+else
+return val;
+}
+EXPORT_SYMBOL_GPL(pm80x_reg_read);
In your introductory email you write
Exported r/w API which requires regmap handle. as currently the pm800
chip has 3 i2c device, only passing a pm80x_chip info can't ensure r/w the
register in correct i2c device.
Your first driver version had this, then you removed the functions
after I asked you to, and now they are back, so I assume there is something
I don't see yet. It looks like the function is just an unnecessary wrapper
that is better open-coded in the caller. Can you explain again what the
difference is?
After you suggest to change the r/w API so that caller doesn't care about
it's
via i2c or spi, it makes sense. However due to pm800 has 3 i2c devices, and
it's hard to export such interface for pm800. Currently to add such
interface
via regmap handle, caller still doesn't care about the actual hw implement,
also it's clear that all pm80x sub-driver or plat call the unified r/w API.
But there is nothing unified about the function above, it just calls
regmap_read(), so the caller already has access to the regmap pointer.
would just use open-coded regmap api.
+/**
+ * pm80x_bulk_read: Read multiple 88PM80x registers
+ *
+ * @map: regmap to read from
+ * @reg: First register
+ * @buf: Buffer to fill. The data will be returned big endian.
+ * @count: Number of registers
+ */
+int pm80x_bulk_read(struct regmap *map, u32 reg, u8 *buf, int count)
+{
+return regmap_raw_read(map, reg, buf, count);
+}
Unused function? Either export this if you want to provide it as
the general API, or drop the function.
It's used by rtc driver.
Then it needs to be exported, so the rtc driver can be a module.
Would update.
On the other hand, I think it probably makes sense to drop the irq_base
member in this struct and rely on irq domains to allocate them dynamically
as mentioned before.
Do you mean that both regmap_add_irq_chip and mfd_add_devices api pass -1 as
the irq_base, so that system can dynamically allocate the irq_base for it?
regmap_add_irq_chip should pass -1, mfd_add_devices should pass 0.
Mark can probably correct me if that's wrong.
Given the proper regmap_irq_chip device resource, is there anything else
needed? As I don't want to miss the exact meaning of transform the
interrupt numbers using the domain in the client drivers you mentioned
above.
The drivers using the numbers need to call regmap_irq_get_virq() to get the
real interrupt number. See include/linux/mfd/wm8994/core.h for an example.
OK, understood. Will check the reference and update accordingly.
Arnd
Arnd, thanks for the review.
Best Regards
Qiao
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] mfd: support 88pm80x in 80x driver
On Wed, Jul 04, 2012 at 03:27:13PM +, Arnd Bergmann wrote: On Wednesday 04 July 2012, Qiao Zhou wrote: On the other hand, I think it probably makes sense to drop the irq_base member in this struct and rely on irq domains to allocate them dynamically as mentioned before. Do you mean that both regmap_add_irq_chip and mfd_add_devices api pass -1 as the irq_base, so that system can dynamically allocate the irq_base for it? regmap_add_irq_chip should pass -1, mfd_add_devices should pass 0. Mark can probably correct me if that's wrong. That's right. Mark, thanks for the confirmation. Best Regards Qiao -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
