Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/05/2012 08:06 PM, Arnd Bergmann wrote:
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);
would update. thanks!

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.
would update. thanks!

diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
new file mode 100644
index 0000000..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
would move to 88pm800.c

+enum {
+	PM800_ID_INVALID,
+	PM800_ID_VIBRATOR,
+	PM800_ID_SOUND,
+	PM800_ID_MAX,
+};

unused?
unused. would remove.

+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
it's regulator resource and be used in 88pm800_core.c & regulator.c. keep it here currently.

+/* 88PM805 Registers */
+#define PM805_CHIP_ID			(0x00)

88pm805-core.c
would move it to 805-core.c

+/* Audio */
+
+/* 88PM800 registers */
+enum {
+	PM80X_INVALID_PAGE = 0,
+	PM80X_BASE_PAGE,
+	PM80X_POWER_PAGE,
+	PM80X_GPADC_PAGE,
+	PM80X_TEST_PAGE,
+};

unused?
unused and would remove

+/* 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
would move to pm800-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			(0xDA)
+#define PM800_RTC_TRIM3			(0xDB)
+#define PM800_RTC_TRIM4			(0xDC)
+#define PM800_RTC_EXPIRE2_1		(0xDD)
+#define PM800_RTC_EXPIRE2_2		(0xDE)
+#define PM800_RTC_EXPIRE2_3		(0xDF)
+#define PM800_RTC_EXPIRE2_4		(0xE0)
+#define PM800_RTC_MISC1			(0xE1)
+#define PM800_RTC_MISC2			(0xE2)
+#define PM800_RTC_MISC3			(0xE3)
+#define PM800_RTC_MISC4			(0xE4)
+
+/* bit definitions of RTC Register 1 (0xD0) */
+#define PM800_ALARM1_EN			(1 << 0)
+#define PM800_ALARM_WAKEUP		(1 << 4)
+#define PM800_ALARM			(1 << 5)
+#define PM800_RTC1_USE_XO		(1 << 7)
+
+#define PM800_POWER_DOWN_LOG1	(0xE5)
+#define PM800_POWER_DOWN_LOG2	(0xE6)
+
+#define PM800_RTC_MISC5			(0xE7)

rtc-88pm80x.c
RTC_CONTROL & MISC reg still be needed by platform, so keep it here. move others to rtc-88pm80x.c

+/* Regulator Control Registers: BUCK1,BUCK5,LDO1 have DVC */
+
+/* LDO1 with DVC[0..3] */
+#define PM800_LDO1_VOUT		(0x08) /* VOUT1 */
+#define PM800_LDO1_VOUT_2	(0x09)
+#define PM800_LDO1_VOUT_3	(0x0A)
+#define PM800_LDO2_VOUT		(0x0B)
+#define PM800_LDO3_VOUT		(0x0C)
+#define PM800_LDO4_VOUT		(0x0D)
+#define PM800_LDO5_VOUT		(0x0E)
+#define PM800_LDO6_VOUT		(0x0F)
+#define PM800_LDO7_VOUT		(0x10)
+#define PM800_LDO8_VOUT		(0x11)

unused
it's used in regulator, and would move to regulator.c.

+#define get_pmic_version(chip) (*(unsigned char *) chip)

unused
would remove.

+/* Interrupt Number in 88PM800 */
+enum {
+	PM800_IRQ_ONKEY,	/*EN1b0 *//*0 */
+	PM800_IRQ_EXTON,	/*EN1b1 */
+	PM800_IRQ_CHG,		/*EN1b2 */
+	PM800_IRQ_BAT,		/*EN1b3 */
+	PM800_IRQ_RTC,		/*EN1b4 */
+	PM800_IRQ_CLASSD,	/*EN1b5 *//*5 */
+	PM800_IRQ_VBAT,		/*EN2b0 */
+	PM800_IRQ_VSYS,		/*EN2b1 */
+	PM800_IRQ_VCHG,		/*EN2b2 */
+	PM800_IRQ_TINT,		/*EN2b3 */
+	PM800_IRQ_GPADC0,	/*EN3b0 *//*10 */
+	PM800_IRQ_GPADC1,	/*EN3b1 */
+	PM800_IRQ_GPADC2,	/*EN3b2 */
+	PM800_IRQ_GPADC3,	/*EN3b3 */
+	PM800_IRQ_GPADC4,	/*EN3b4 */
+	PM800_IRQ_GPIO0,	/*EN4b0 *//*15 */
+	PM800_IRQ_GPIO1,	/*EN4b1 */
+	PM800_IRQ_GPIO2,	/*EN4b2 */
+	PM800_IRQ_GPIO3,	/*EN4b3 */
+	PM800_IRQ_GPIO4,	/*EN4b4 *//*19 */
+	PM800_MAX_IRQ,
+};

88pm800-core.c
would move it to 800-core.c

+/* Interrupt Number in 88PM805 */
+enum {
+	PM805_IRQ_LDO_OFF,	/*0 */
+	PM805_IRQ_SRC_DPLL_LOCK,	/*1 */
+	PM805_IRQ_CLIP_FAULT,
+	PM805_IRQ_MIC_CONFLICT,
+	PM805_IRQ_HP2_SHRT,
+	PM805_IRQ_HP1_SHRT,	/*5 */
+	PM805_IRQ_FINE_PLL_FAULT,
+	PM805_IRQ_RAW_PLL_FAULT,
+	PM805_IRQ_VOLP_BTN_DET,
+	PM805_IRQ_VOLM_BTN_DET,
+	PM805_IRQ_SHRT_BTN_DET,	/*10 */
+	PM805_IRQ_MIC_DET,	/*11 */
+
+	PM805_MAX_IRQ,
+};

88pm805-core.c
would move it to 805-core.c

	Arnd

Arnd, thanks for review and sorry for the inconvenience.

--

Best Regards
Qiao


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux