Messages in this thread | | | Date | Fri, 23 May 2014 16:29:33 +0530 | From | Keerthy <> | Subject | Re: [PATCH v3 3/4] mfd: tps65917: Add driver for the TPS65917 PMIC |
| |
On Friday 23 May 2014 03:36 PM, Lee Jones wrote: >>>>>> The TPS65917 chip is a power management IC for Portable Navigation Systems >>>>>> and Tablet Computing devices. It contains the following components: >>>>>> >>>>>> - Regulators. >>>>>> - Over Temperature warning and Shut down. >>>>>> >>>>>> This patch adds support for tps65917 mfd device. At this time only >>>>>> the regulator functionality is made available. >>>>>> >>>>>> Signed-off-by: Keerthy <j-keerthy@ti.com> >>>>>> --- >>>>>> v3 Changes: >>>>>> >>>>>> * Header file formating >>>>>> * Changed the cache_type to REGCACHE_RBTREE >>>>>> * removed unnecessary code >>>>>> * Corrected documentation style >>>>>> * Added pm_power_off function >>>>>> >>>>>> v2 Changes: >>>>>> >>>>>> * Added volatile register check as some of the registers >>>>>> in the set are volatile. >>>>>> drivers/mfd/Kconfig | 12 + >>>>>> drivers/mfd/Makefile | 1 + >>>>>> drivers/mfd/tps65917.c | 594 +++++++++++++++++ >>>>>> include/linux/mfd/tps65917.h | 1485 ++++++++++++++++++++++++++++++++++++++++++ >>>>>> 4 files changed, 2092 insertions(+) >>>>>> create mode 100644 drivers/mfd/tps65917.c >>>>>> create mode 100644 include/linux/mfd/tps65917.h > [...] > >>>>>> + ret = regmap_read(tps65917->regmap[slave], addr, ®); >>>>>> + if (ret) >>>>>> + goto err_irq; >>>>>> + }i >>>>> What does the read do? You're not doing anything with the value. >>>> This pad1 and pad2 stuff is not needed. I will remove this. >>> Then why is it in here? >>> >>> Did you copy this code from somewhere, if so, where? >>> >>> Okay, I just answered my own question. There is so much common code >>> in between this and palmas, there is no way I'm going to accept this >>> driver. Please merge it in with the palmas driver! >>> >> The chip is more like a subset of palmas with lot of register offset changes >> and register bit field changes. Adding this would make it clumsy. >> There could >> be lot of checks. That is why i chose to write a new driver. >> >> Palmas driver already supports palmas variants and tps659038. Merging >> this would mean more and more checks :-/. > Then find an elegant way of representing the variants. I'm not > prepared to carry that much duplicated code in MFD. It's already > overladened and in need of an overhaul. This will exacerbate the > matter. >
Okay. I am working on that.
Regards, Keerthy
| |