* [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model @ 2008-11-19 16:46 Mark Brown 2008-11-19 16:47 ` [PATCH 2/2] mfd: Refactor WM8350 chip identification Mark Brown 2008-11-24 10:37 ` [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Samuel Ortiz 0 siblings, 2 replies; 6+ messages in thread From: Mark Brown @ 2008-11-19 16:46 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel, Mark Brown Rather than check for chip revisions in the WM8350 drivers have the core code set flags for relevant differences. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/mfd/wm8350-core.c | 14 ++++++-------- drivers/power/wm8350_power.c | 2 +- include/linux/mfd/wm8350/core.h | 8 -------- include/linux/mfd/wm8350/supply.h | 2 ++ 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c index 3f2a68d..84fc90e 100644 --- a/drivers/mfd/wm8350-core.c +++ b/drivers/mfd/wm8350-core.c @@ -1238,21 +1238,19 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq, if (id1 == 0x6143) { switch ((id2 & WM8350_CHIP_REV_MASK) >> 12) { - case WM8350_REV_E: + case 0x4: dev_info(wm8350->dev, "Found Rev E device\n"); - wm8350->rev = WM8350_REV_E; break; - case WM8350_REV_F: + case 0x5: dev_info(wm8350->dev, "Found Rev F device\n"); - wm8350->rev = WM8350_REV_F; break; - case WM8350_REV_G: + case 0x6: dev_info(wm8350->dev, "Found Rev G device\n"); - wm8350->rev = WM8350_REV_G; + wm8350->power.rev_g_coeff = 1; break; - case WM8350_REV_H: + case 0x7: dev_info(wm8350->dev, "Found Rev H device\n"); - wm8350->rev = WM8350_REV_H; + wm8350->power.rev_g_coeff = 1; break; default: /* For safety we refuse to run on unknown hardware */ diff --git a/drivers/power/wm8350_power.c b/drivers/power/wm8350_power.c index 9c0a847..74e7593 100644 --- a/drivers/power/wm8350_power.c +++ b/drivers/power/wm8350_power.c @@ -44,7 +44,7 @@ static int wm8350_read_usb_uvolts(struct wm8350 *wm8350) static inline int wm8350_charge_time_min(struct wm8350 *wm8350, int min) { - if (wm8350->rev < WM8350_REV_G) + if (!wm8350->power.rev_g_coeff) return (((min - 30) / 15) & 0xf) << 8; else return (((min - 30) / 30) & 0xf) << 8; diff --git a/include/linux/mfd/wm8350/core.h b/include/linux/mfd/wm8350/core.h index d2614df..8563408 100644 --- a/include/linux/mfd/wm8350/core.h +++ b/include/linux/mfd/wm8350/core.h @@ -558,12 +558,6 @@ #define WM8350_IRQ_WKUP_ONKEY 48 #define WM8350_IRQ_WKUP_GP_WAKEUP 49 -/* wm8350 chip revisions */ -#define WM8350_REV_E 0x4 -#define WM8350_REV_F 0x5 -#define WM8350_REV_G 0x6 -#define WM8350_REV_H 0x7 - #define WM8350_NUM_IRQ 63 struct wm8350_reg_access { @@ -585,8 +579,6 @@ struct wm8350_irq { }; struct wm8350 { - int rev; /* chip revision */ - struct device *dev; /* device IO */ diff --git a/include/linux/mfd/wm8350/supply.h b/include/linux/mfd/wm8350/supply.h index 7972151..2b94793 100644 --- a/include/linux/mfd/wm8350/supply.h +++ b/include/linux/mfd/wm8350/supply.h @@ -127,6 +127,8 @@ struct wm8350_power { struct power_supply usb; struct power_supply ac; struct wm8350_charger_policy *policy; + + int rev_g_coeff; }; #endif -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] mfd: Refactor WM8350 chip identification 2008-11-19 16:46 [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Mark Brown @ 2008-11-19 16:47 ` Mark Brown 2008-11-24 10:37 ` [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Samuel Ortiz 1 sibling, 0 replies; 6+ messages in thread From: Mark Brown @ 2008-11-19 16:47 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel, Mark Brown Since the WM8350 driver was originally written the semantics for the identification registers of the chip have been clarified, allowing us to do an exact match on all the fields. This avoids mistakenly running on unsupported hardware. Also change to using the datasheet names more consistently for legibility and fix a printk() that should be dev_err(). Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- drivers/mfd/wm8350-core.c | 54 ++++++++++++++++++++++++++------------ include/linux/mfd/wm8350/core.h | 6 ++++ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c index 84fc90e..d7836ad 100644 --- a/drivers/mfd/wm8350-core.c +++ b/drivers/mfd/wm8350-core.c @@ -1227,52 +1227,72 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq, struct wm8350_platform_data *pdata) { int ret = -EINVAL; - u16 id1, id2, mask, mode; + u16 id1, id2, mask_rev; + u16 cust_id, mode, chip_rev; /* get WM8350 revision and config mode */ wm8350->read_dev(wm8350, WM8350_RESET_ID, sizeof(id1), &id1); wm8350->read_dev(wm8350, WM8350_ID, sizeof(id2), &id2); + wm8350->read_dev(wm8350, WM8350_REVISION, sizeof(mask_rev), &mask_rev); id1 = be16_to_cpu(id1); id2 = be16_to_cpu(id2); + mask_rev = be16_to_cpu(mask_rev); - if (id1 == 0x6143) { - switch ((id2 & WM8350_CHIP_REV_MASK) >> 12) { + if (id1 != 0x6143) { + dev_err(wm8350->dev, + "Device with ID %x is not a WM8350\n", id1); + ret = -ENODEV; + goto err; + } + + mode = id2 & WM8350_CONF_STS_MASK >> 10; + cust_id = id2 & WM8350_CUST_ID_MASK; + chip_rev = (id2 & WM8350_CHIP_REV_MASK) >> 12; + dev_info(wm8350->dev, + "CONF_STS %d, CUST_ID %d, MASK_REV %d, CHIP_REV %d\n", + mode, cust_id, mask_rev, chip_rev); + + if (cust_id != 0) { + dev_err(wm8350->dev, "Unsupported CUST_ID\n"); + ret = -ENODEV; + goto err; + } + + switch (mask_rev) { + case 0: + switch (chip_rev) { case 0x4: - dev_info(wm8350->dev, "Found Rev E device\n"); + dev_info(wm8350->dev, "WM8350 Rev E\n"); break; case 0x5: - dev_info(wm8350->dev, "Found Rev F device\n"); + dev_info(wm8350->dev, "WM8350 Rev F\n"); break; case 0x6: - dev_info(wm8350->dev, "Found Rev G device\n"); + dev_info(wm8350->dev, "WM8350 Rev G\n"); wm8350->power.rev_g_coeff = 1; break; case 0x7: - dev_info(wm8350->dev, "Found Rev H device\n"); + dev_info(wm8350->dev, "WM8350 Rev H\n"); wm8350->power.rev_g_coeff = 1; break; default: /* For safety we refuse to run on unknown hardware */ - dev_info(wm8350->dev, "Found unknown rev %x\n", - (id2 & WM8350_CHIP_REV_MASK) >> 12); + dev_err(wm8350->dev, "Unknown WM8350 CHIP_REV\n"); ret = -ENODEV; goto err; } - } else { - dev_info(wm8350->dev, "Device with ID %x is not a WM8350\n", - id1); + break; + + default: + dev_err(wm8350->dev, "Unknown MASK_REV\n"); ret = -ENODEV; goto err; } - mode = id2 & WM8350_CONF_STS_MASK >> 10; - mask = id2 & WM8350_CUST_ID_MASK; - dev_info(wm8350->dev, "Config mode %d, ROM mask %d\n", mode, mask); - ret = wm8350_create_cache(wm8350, mode); if (ret < 0) { - printk(KERN_ERR "wm8350: failed to create register cache\n"); + dev_err(wm8350->dev, "Failed to create register cache\n"); return ret; } diff --git a/include/linux/mfd/wm8350/core.h b/include/linux/mfd/wm8350/core.h index 8563408..0d37572 100644 --- a/include/linux/mfd/wm8350/core.h +++ b/include/linux/mfd/wm8350/core.h @@ -29,6 +29,7 @@ */ #define WM8350_RESET_ID 0x00 #define WM8350_ID 0x01 +#define WM8350_REVISION 0x02 #define WM8350_SYSTEM_CONTROL_1 0x03 #define WM8350_SYSTEM_CONTROL_2 0x04 #define WM8350_SYSTEM_HIBERNATE 0x05 @@ -80,6 +81,11 @@ #define WM8350_CUST_ID_MASK 0x00FF /* + * R2 (0x02) - Revision + */ +#define WM8350_MASK_REV_MASK 0x00FF + +/* * R3 (0x03) - System Control 1 */ #define WM8350_CHIP_ON 0x8000 -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model 2008-11-19 16:46 [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Mark Brown 2008-11-19 16:47 ` [PATCH 2/2] mfd: Refactor WM8350 chip identification Mark Brown @ 2008-11-24 10:37 ` Samuel Ortiz 2008-11-24 13:31 ` Mark Brown 1 sibling, 1 reply; 6+ messages in thread From: Samuel Ortiz @ 2008-11-24 10:37 UTC (permalink / raw) To: Mark Brown; +Cc: linux-kernel Hi Mark, On Wed, Nov 19, 2008 at 04:46:59PM +0000, Mark Brown wrote: > Rather than check for chip revisions in the WM8350 drivers have the core > code set flags for relevant differences. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > --- > drivers/mfd/wm8350-core.c | 14 ++++++-------- > drivers/power/wm8350_power.c | 2 +- > include/linux/mfd/wm8350/core.h | 8 -------- > include/linux/mfd/wm8350/supply.h | 2 ++ > 4 files changed, 9 insertions(+), 17 deletions(-) > > diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c > index 3f2a68d..84fc90e 100644 > --- a/drivers/mfd/wm8350-core.c > +++ b/drivers/mfd/wm8350-core.c > @@ -1238,21 +1238,19 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq, > > if (id1 == 0x6143) { > switch ((id2 & WM8350_CHIP_REV_MASK) >> 12) { > - case WM8350_REV_E: > + case 0x4: Dont you prefer to use the WM8350_REV_* values in your code rather than some undocumented constants like that ? I understand that reading the code below will make those constants obvious, but still... Cheers, Samuel. > dev_info(wm8350->dev, "Found Rev E device\n"); > - wm8350->rev = WM8350_REV_E; > break; > - case WM8350_REV_F: > + case 0x5: > dev_info(wm8350->dev, "Found Rev F device\n"); > - wm8350->rev = WM8350_REV_F; > break; > - case WM8350_REV_G: > + case 0x6: > dev_info(wm8350->dev, "Found Rev G device\n"); > - wm8350->rev = WM8350_REV_G; > + wm8350->power.rev_g_coeff = 1; > break; > - case WM8350_REV_H: > + case 0x7: > dev_info(wm8350->dev, "Found Rev H device\n"); > - wm8350->rev = WM8350_REV_H; > + wm8350->power.rev_g_coeff = 1; > break; > default: > /* For safety we refuse to run on unknown hardware */ > diff --git a/drivers/power/wm8350_power.c b/drivers/power/wm8350_power.c > index 9c0a847..74e7593 100644 > --- a/drivers/power/wm8350_power.c > +++ b/drivers/power/wm8350_power.c > @@ -44,7 +44,7 @@ static int wm8350_read_usb_uvolts(struct wm8350 *wm8350) > > static inline int wm8350_charge_time_min(struct wm8350 *wm8350, int min) > { > - if (wm8350->rev < WM8350_REV_G) > + if (!wm8350->power.rev_g_coeff) > return (((min - 30) / 15) & 0xf) << 8; > else > return (((min - 30) / 30) & 0xf) << 8; > diff --git a/include/linux/mfd/wm8350/core.h b/include/linux/mfd/wm8350/core.h > index d2614df..8563408 100644 > --- a/include/linux/mfd/wm8350/core.h > +++ b/include/linux/mfd/wm8350/core.h > @@ -558,12 +558,6 @@ > #define WM8350_IRQ_WKUP_ONKEY 48 > #define WM8350_IRQ_WKUP_GP_WAKEUP 49 > > -/* wm8350 chip revisions */ > -#define WM8350_REV_E 0x4 > -#define WM8350_REV_F 0x5 > -#define WM8350_REV_G 0x6 > -#define WM8350_REV_H 0x7 > - > #define WM8350_NUM_IRQ 63 > > struct wm8350_reg_access { > @@ -585,8 +579,6 @@ struct wm8350_irq { > }; > > struct wm8350 { > - int rev; /* chip revision */ > - > struct device *dev; > > /* device IO */ > diff --git a/include/linux/mfd/wm8350/supply.h b/include/linux/mfd/wm8350/supply.h > index 7972151..2b94793 100644 > --- a/include/linux/mfd/wm8350/supply.h > +++ b/include/linux/mfd/wm8350/supply.h > @@ -127,6 +127,8 @@ struct wm8350_power { > struct power_supply usb; > struct power_supply ac; > struct wm8350_charger_policy *policy; > + > + int rev_g_coeff; > }; > > #endif > -- > 1.5.6.5 > -- Intel Open Source Technology Centre http://oss.intel.com/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model 2008-11-24 10:37 ` [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Samuel Ortiz @ 2008-11-24 13:31 ` Mark Brown 2008-11-24 19:25 ` Samuel Ortiz 0 siblings, 1 reply; 6+ messages in thread From: Mark Brown @ 2008-11-24 13:31 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel On Mon, Nov 24, 2008 at 11:37:55AM +0100, Samuel Ortiz wrote: > Dont you prefer to use the WM8350_REV_* values in your code rather than some > undocumented constants like that ? > I understand that reading the code below will make those constants obvious, > but still... I don't find them enormously helpful, TBH - the constant gets used in exactly one place and there's been a bit of skew between what some of the ID registers are called and thier actual meaning which doesn't help. I'll repost with the #defines for the revisons added back, though. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model 2008-11-24 13:31 ` Mark Brown @ 2008-11-24 19:25 ` Samuel Ortiz 0 siblings, 0 replies; 6+ messages in thread From: Samuel Ortiz @ 2008-11-24 19:25 UTC (permalink / raw) To: Mark Brown; +Cc: linux-kernel On Mon, Nov 24, 2008 at 01:31:16PM +0000, Mark Brown wrote: > On Mon, Nov 24, 2008 at 11:37:55AM +0100, Samuel Ortiz wrote: > > > Dont you prefer to use the WM8350_REV_* values in your code rather than some > > undocumented constants like that ? > > I understand that reading the code below will make those constants obvious, > > but still... > > I don't find them enormously helpful, TBH - the constant gets used in > exactly one place and there's been a bit of skew between what some of > the ID registers are called and thier actual meaning which doesn't help. > > I'll repost with the #defines for the revisons added back, though. Thanks for the patches. Both of them are now sitting in my for-next branch. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model @ 2008-11-24 15:53 Mark Brown 0 siblings, 0 replies; 6+ messages in thread From: Mark Brown @ 2008-11-24 15:53 UTC (permalink / raw) To: Samuel Ortiz; +Cc: linux-kernel, Mark Brown Rather than check for chip revisions in the WM8350 drivers have the core code set flags for relevant differences. Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com> --- Updated to keep the constants for the revisions. drivers/mfd/wm8350-core.c | 6 ++---- drivers/power/wm8350_power.c | 2 +- include/linux/mfd/wm8350/core.h | 2 -- include/linux/mfd/wm8350/supply.h | 2 ++ 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c index 60439bd..764bf15 100644 --- a/drivers/mfd/wm8350-core.c +++ b/drivers/mfd/wm8350-core.c @@ -1240,19 +1240,17 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq, switch ((id2 & WM8350_CHIP_REV_MASK) >> 12) { case WM8350_REV_E: dev_info(wm8350->dev, "Found Rev E device\n"); - wm8350->rev = WM8350_REV_E; break; case WM8350_REV_F: dev_info(wm8350->dev, "Found Rev F device\n"); - wm8350->rev = WM8350_REV_F; break; case WM8350_REV_G: dev_info(wm8350->dev, "Found Rev G device\n"); - wm8350->rev = WM8350_REV_G; + wm8350->power.rev_g_coeff = 1; break; case WM8350_REV_H: dev_info(wm8350->dev, "Found Rev H device\n"); - wm8350->rev = WM8350_REV_H; + wm8350->power.rev_g_coeff = 1; break; default: /* For safety we refuse to run on unknown hardware */ diff --git a/drivers/power/wm8350_power.c b/drivers/power/wm8350_power.c index 9c0a847..74e7593 100644 --- a/drivers/power/wm8350_power.c +++ b/drivers/power/wm8350_power.c @@ -44,7 +44,7 @@ static int wm8350_read_usb_uvolts(struct wm8350 *wm8350) static inline int wm8350_charge_time_min(struct wm8350 *wm8350, int min) { - if (wm8350->rev < WM8350_REV_G) + if (!wm8350->power.rev_g_coeff) return (((min - 30) / 15) & 0xf) << 8; else return (((min - 30) / 30) & 0xf) << 8; diff --git a/include/linux/mfd/wm8350/core.h b/include/linux/mfd/wm8350/core.h index d2614df..3c97356 100644 --- a/include/linux/mfd/wm8350/core.h +++ b/include/linux/mfd/wm8350/core.h @@ -585,8 +585,6 @@ struct wm8350_irq { }; struct wm8350 { - int rev; /* chip revision */ - struct device *dev; /* device IO */ diff --git a/include/linux/mfd/wm8350/supply.h b/include/linux/mfd/wm8350/supply.h index 7972151..2b94793 100644 --- a/include/linux/mfd/wm8350/supply.h +++ b/include/linux/mfd/wm8350/supply.h @@ -127,6 +127,8 @@ struct wm8350_power { struct power_supply usb; struct power_supply ac; struct wm8350_charger_policy *policy; + + int rev_g_coeff; }; #endif -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-11-24 19:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-19 16:46 [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Mark Brown 2008-11-19 16:47 ` [PATCH 2/2] mfd: Refactor WM8350 chip identification Mark Brown 2008-11-24 10:37 ` [PATCH 1/2] mfd: Switch WM8350 revision detection to a feature based model Samuel Ortiz 2008-11-24 13:31 ` Mark Brown 2008-11-24 19:25 ` Samuel Ortiz -- strict thread matches above, loose matches on Subject: below -- 2008-11-24 15:53 Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox