| Message ID | 20200415162030.16414-3-sravanhome@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add battery charger driver support for MP2629 | expand |
On Wed, 15 Apr 2020, Saravanan Sekar wrote: > mp2629 is a highly-integrated switching-mode battery charge management > device for single-cell Li-ion or Li-polymer battery. > > Add MFD core enables chip access for ADC driver for battery readings, > and a power supply battery-charger driver > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> > --- > drivers/mfd/Kconfig | 9 ++++ > drivers/mfd/Makefile | 2 + > drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/mp2629.h | 19 +++++++++ > 4 files changed, 116 insertions(+) > create mode 100644 drivers/mfd/mp2629.c > create mode 100644 include/linux/mfd/mp2629.h How is this driver registered? Looks like it has device tree support. Is there another way? > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 3c547ed575e6..85be799795aa 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C > help > Select this if your MC13xxx is connected via an I2C bus. > > +config MFD_MP2629 > + tristate "Monolithic power system MP2629 ADC and Battery charger" > + depends on I2C > + select REGMAP_I2C > + help > + Select this option to enable support for monolithic power system > + battery charger. This provides ADC, thermal, battery charger power > + management functions on the systems.
Hi Lee, On 24/04/20 9:18 am, Lee Jones wrote: > On Wed, 15 Apr 2020, Saravanan Sekar wrote: > >> mp2629 is a highly-integrated switching-mode battery charge management >> device for single-cell Li-ion or Li-polymer battery. >> >> Add MFD core enables chip access for ADC driver for battery readings, >> and a power supply battery-charger driver >> >> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> >> --- >> drivers/mfd/Kconfig | 9 ++++ >> drivers/mfd/Makefile | 2 + >> drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/mp2629.h | 19 +++++++++ >> 4 files changed, 116 insertions(+) >> create mode 100644 drivers/mfd/mp2629.c >> create mode 100644 include/linux/mfd/mp2629.h > How is this driver registered? > > Looks like it has device tree support. Is there another way? Yes, only using device tree > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 3c547ed575e6..85be799795aa 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C >> help >> Select this if your MC13xxx is connected via an I2C bus. >> >> +config MFD_MP2629 >> + tristate "Monolithic power system MP2629 ADC and Battery charger" >> + depends on I2C >> + select REGMAP_I2C >> + help >> + Select this option to enable support for monolithic power system >> + battery charger. This provides ADC, thermal, battery charger power >> + management functions on the systems.
On Fri, 24 Apr 2020, saravanan sekar wrote: > Hi Lee, > > On 24/04/20 9:18 am, Lee Jones wrote: > > On Wed, 15 Apr 2020, Saravanan Sekar wrote: > > > > > mp2629 is a highly-integrated switching-mode battery charge management > > > device for single-cell Li-ion or Li-polymer battery. > > > > > > Add MFD core enables chip access for ADC driver for battery readings, > > > and a power supply battery-charger driver > > > > > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> > > > --- > > > drivers/mfd/Kconfig | 9 ++++ > > > drivers/mfd/Makefile | 2 + > > > drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ > > > include/linux/mfd/mp2629.h | 19 +++++++++ > > > 4 files changed, 116 insertions(+) > > > create mode 100644 drivers/mfd/mp2629.c > > > create mode 100644 include/linux/mfd/mp2629.h > > How is this driver registered? > > > > Looks like it has device tree support. Is there another way? > Yes, only using device tree Then how about using 'simple-mfd' and 'syscon'? Then you can omit this driver completely. > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > index 3c547ed575e6..85be799795aa 100644 > > > --- a/drivers/mfd/Kconfig > > > +++ b/drivers/mfd/Kconfig > > > @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C > > > help > > > Select this if your MC13xxx is connected via an I2C bus. > > > +config MFD_MP2629 > > > + tristate "Monolithic power system MP2629 ADC and Battery charger" > > > + depends on I2C > > > + select REGMAP_I2C > > > + help > > > + Select this option to enable support for monolithic power system > > > + battery charger. This provides ADC, thermal, battery charger power > > > + management functions on the systems.
Hi Lee, On 24/04/20 11:37 am, Lee Jones wrote: > On Fri, 24 Apr 2020, saravanan sekar wrote: > >> Hi Lee, >> >> On 24/04/20 9:18 am, Lee Jones wrote: >>> On Wed, 15 Apr 2020, Saravanan Sekar wrote: >>> >>>> mp2629 is a highly-integrated switching-mode battery charge management >>>> device for single-cell Li-ion or Li-polymer battery. >>>> >>>> Add MFD core enables chip access for ADC driver for battery readings, >>>> and a power supply battery-charger driver >>>> >>>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> >>>> --- >>>> drivers/mfd/Kconfig | 9 ++++ >>>> drivers/mfd/Makefile | 2 + >>>> drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ >>>> include/linux/mfd/mp2629.h | 19 +++++++++ >>>> 4 files changed, 116 insertions(+) >>>> create mode 100644 drivers/mfd/mp2629.c >>>> create mode 100644 include/linux/mfd/mp2629.h >>> How is this driver registered? >>> >>> Looks like it has device tree support. Is there another way? >> Yes, only using device tree > Then how about using 'simple-mfd' and 'syscon'? > > Then you can omit this driver completely. The exception is to support for non device tree platform as well, but I have tested only for ARM device tree platform. > >>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>> index 3c547ed575e6..85be799795aa 100644 >>>> --- a/drivers/mfd/Kconfig >>>> +++ b/drivers/mfd/Kconfig >>>> @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C >>>> help >>>> Select this if your MC13xxx is connected via an I2C bus. >>>> +config MFD_MP2629 >>>> + tristate "Monolithic power system MP2629 ADC and Battery charger" >>>> + depends on I2C >>>> + select REGMAP_I2C >>>> + help >>>> + Select this option to enable support for monolithic power system >>>> + battery charger. This provides ADC, thermal, battery charger power >>>> + management functions on the systems.
On Fri, 24 Apr 2020, saravanan sekar wrote: > Hi Lee, > > On 24/04/20 11:37 am, Lee Jones wrote: > > On Fri, 24 Apr 2020, saravanan sekar wrote: > > > > > Hi Lee, > > > > > > On 24/04/20 9:18 am, Lee Jones wrote: > > > > On Wed, 15 Apr 2020, Saravanan Sekar wrote: > > > > > > > > > mp2629 is a highly-integrated switching-mode battery charge management > > > > > device for single-cell Li-ion or Li-polymer battery. > > > > > > > > > > Add MFD core enables chip access for ADC driver for battery readings, > > > > > and a power supply battery-charger driver > > > > > > > > > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> > > > > > --- > > > > > drivers/mfd/Kconfig | 9 ++++ > > > > > drivers/mfd/Makefile | 2 + > > > > > drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ > > > > > include/linux/mfd/mp2629.h | 19 +++++++++ > > > > > 4 files changed, 116 insertions(+) > > > > > create mode 100644 drivers/mfd/mp2629.c > > > > > create mode 100644 include/linux/mfd/mp2629.h > > > > How is this driver registered? > > > > > > > > Looks like it has device tree support. Is there another way? > > > Yes, only using device tree > > Then how about using 'simple-mfd' and 'syscon'? > > > > Then you can omit this driver completely. > > The exception is to support for non device tree platform as well, but I have > tested only for ARM device tree platform. Is that a reality though? How else do you see this realistically being registered? > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > > > index 3c547ed575e6..85be799795aa 100644 > > > > > --- a/drivers/mfd/Kconfig > > > > > +++ b/drivers/mfd/Kconfig > > > > > @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C > > > > > help > > > > > Select this if your MC13xxx is connected via an I2C bus. > > > > > +config MFD_MP2629 > > > > > + tristate "Monolithic power system MP2629 ADC and Battery charger" > > > > > + depends on I2C > > > > > + select REGMAP_I2C > > > > > + help > > > > > + Select this option to enable support for monolithic power system > > > > > + battery charger. This provides ADC, thermal, battery charger power > > > > > + management functions on the systems.
On 24/04/20 12:53 pm, Lee Jones wrote: > On Fri, 24 Apr 2020, saravanan sekar wrote: > >> Hi Lee, >> >> On 24/04/20 11:37 am, Lee Jones wrote: >>> On Fri, 24 Apr 2020, saravanan sekar wrote: >>> >>>> Hi Lee, >>>> >>>> On 24/04/20 9:18 am, Lee Jones wrote: >>>>> On Wed, 15 Apr 2020, Saravanan Sekar wrote: >>>>> >>>>>> mp2629 is a highly-integrated switching-mode battery charge management >>>>>> device for single-cell Li-ion or Li-polymer battery. >>>>>> >>>>>> Add MFD core enables chip access for ADC driver for battery readings, >>>>>> and a power supply battery-charger driver >>>>>> >>>>>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> >>>>>> --- >>>>>> drivers/mfd/Kconfig | 9 ++++ >>>>>> drivers/mfd/Makefile | 2 + >>>>>> drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ >>>>>> include/linux/mfd/mp2629.h | 19 +++++++++ >>>>>> 4 files changed, 116 insertions(+) >>>>>> create mode 100644 drivers/mfd/mp2629.c >>>>>> create mode 100644 include/linux/mfd/mp2629.h >>>>> How is this driver registered? >>>>> >>>>> Looks like it has device tree support. Is there another way? >>>> Yes, only using device tree >>> Then how about using 'simple-mfd' and 'syscon'? >>> >>> Then you can omit this driver completely. >> The exception is to support for non device tree platform as well, but I have >> tested only for ARM device tree platform. > Is that a reality though? > > How else do you see this realistically being registered? > I understand that acpi related device table are not covered here, well I don't have to platform to test so. If you ask me to cover acpi related table, I can do but hard to test. >>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>>>> index 3c547ed575e6..85be799795aa 100644 >>>>>> --- a/drivers/mfd/Kconfig >>>>>> +++ b/drivers/mfd/Kconfig >>>>>> @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C >>>>>> help >>>>>> Select this if your MC13xxx is connected via an I2C bus. >>>>>> +config MFD_MP2629 >>>>>> + tristate "Monolithic power system MP2629 ADC and Battery charger" >>>>>> + depends on I2C >>>>>> + select REGMAP_I2C >>>>>> + help >>>>>> + Select this option to enable support for monolithic power system >>>>>> + battery charger. This provides ADC, thermal, battery charger power >>>>>> + management functions on the systems.
On Wed, 15 Apr 2020, Saravanan Sekar wrote: > mp2629 is a highly-integrated switching-mode battery charge management > device for single-cell Li-ion or Li-polymer battery. > > Add MFD core enables chip access for ADC driver for battery readings, > and a power supply battery-charger driver > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> > --- > drivers/mfd/Kconfig | 9 ++++ > drivers/mfd/Makefile | 2 + > drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/mp2629.h | 19 +++++++++ > 4 files changed, 116 insertions(+) > create mode 100644 drivers/mfd/mp2629.c > create mode 100644 include/linux/mfd/mp2629.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 3c547ed575e6..85be799795aa 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C > help > Select this if your MC13xxx is connected via an I2C bus. > > +config MFD_MP2629 > + tristate "Monolithic power system MP2629 ADC and Battery charger" "Power Systems" > + depends on I2C > + select REGMAP_I2C > + help > + Select this option to enable support for monolithic power system "Monolithic Power Systems" > + battery charger. This provides ADC, thermal, battery charger power "thermal and battery" > + management functions on the systems. What does "on the systems" mean? I would just drop that part. > config MFD_MXS_LRADC > tristate "Freescale i.MX23/i.MX28 LRADC" > depends on ARCH_MXS || COMPILE_TEST > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index f935d10cbf0f..d6c210f96d02 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -170,6 +170,8 @@ obj-$(CONFIG_MFD_MAX8925) += max8925.o > obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o > obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o > > +obj-$(CONFIG_MFD_MP2629) += mp2629.o > + > pcf50633-objs := pcf50633-core.o pcf50633-irq.o > obj-$(CONFIG_MFD_PCF50633) += pcf50633.o > obj-$(CONFIG_PCF50633_ADC) += pcf50633-adc.o > diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c > new file mode 100644 > index 000000000000..46242b1cdf24 > --- /dev/null > +++ b/drivers/mfd/mp2629.c > @@ -0,0 +1,86 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * MP2629 parent driver for ADC and battery charger > + * > + * Copyright 2020 Monolithic Power Systems, Inc > + * > + * Author: Saravanan Sekar <sravanhome@gmail.com> > + */ > + > +#include <linux/i2c.h> > +#include <linux/kernel.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/mp2629.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +enum { > + MP2629_MFD_ADC, > + MP2629_MFD_CHARGER, > + MP2629_MFD_MAX > +}; Why do you need this? I think it can be dropped. > +static const struct mfd_cell mp2629mfd[] = { Drop the "mfd" part of "mp2629mfd". You can call it "mp2629_cell" if you like. > + [MP2629_MFD_ADC] = { > + .name = "mp2629_adc", > + .of_compatible = "mps,mp2629_adc", > + }, > + [MP2629_MFD_CHARGER] = { > + .name = "mp2629_charger", > + .of_compatible = "mps,mp2629_charger", > + } > +}; > + > +static const struct regmap_config mp2629_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0x17, Best to define the registers in an enum and call the last one MP2629_MAX_REG or similar. > +}; > + > +static int mp2629_probe(struct i2c_client *client) > +{ > + struct mp2629_info *ddata; Either change "info" to "ddata", or drop it completely. > + int ret; > + > + ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL); > + if (!ddata) > + return -ENOMEM; > + > + ddata->dev = &client->dev; > + i2c_set_clientdata(client, ddata); > + > + ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config); > + if (IS_ERR(ddata->regmap)) { > + dev_err(ddata->dev, "Failed to allocate regmap!\n"); No need for the exclamation mark. > + return PTR_ERR(ddata->regmap); > + } > + > + ret = devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_NONE, mp2629mfd, Why NONE over AUTO? > + ARRAY_SIZE(mp2629mfd), NULL, > + 0, NULL); > + if (ret) > + dev_err(ddata->dev, "Failed to register sub-devices %d\n", ret); > + > + return ret; > +} > + > +static const struct of_device_id mp2629_of_match[] = { > + { .compatible = "mps,mp2629"}, > + { } > +}; > +MODULE_DEVICE_TABLE(of, mp2629_of_match); > + > +static struct i2c_driver mp2629_driver = { > + .driver = { > + .name = "mp2629", > + .of_match_table = mp2629_of_match, > + }, > + .probe_new = mp2629_probe, > +}; > +module_i2c_driver(mp2629_driver); > + > +MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>"); > +MODULE_DESCRIPTION("MP2629 Battery charger parent driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h > new file mode 100644 > index 000000000000..06985e41fc6f > --- /dev/null > +++ b/include/linux/mfd/mp2629.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * register definitions for MP2629 charger "Register" Although there aren't any register definitions in here? > + * Copyright 2020 Monolithic Power Systems, Inc > + */ > + > +#ifndef __MP2629_H__ > +#define __MP2629_H__ > + > +#include <linux/device.h> > +#include <linux/regmap.h> > + > +struct mp2629_info { > + struct device *dev; > + struct regmap *regmap; > +}; > + > +#endif
On Fri, 24 Apr 2020, saravanan sekar wrote: > > On 24/04/20 12:53 pm, Lee Jones wrote: > > On Fri, 24 Apr 2020, saravanan sekar wrote: > > > > > Hi Lee, > > > > > > On 24/04/20 11:37 am, Lee Jones wrote: > > > > On Fri, 24 Apr 2020, saravanan sekar wrote: > > > > > > > > > Hi Lee, > > > > > > > > > > On 24/04/20 9:18 am, Lee Jones wrote: > > > > > > On Wed, 15 Apr 2020, Saravanan Sekar wrote: > > > > > > > > > > > > > mp2629 is a highly-integrated switching-mode battery charge management > > > > > > > device for single-cell Li-ion or Li-polymer battery. > > > > > > > > > > > > > > Add MFD core enables chip access for ADC driver for battery readings, > > > > > > > and a power supply battery-charger driver > > > > > > > > > > > > > > Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> > > > > > > > --- > > > > > > > drivers/mfd/Kconfig | 9 ++++ > > > > > > > drivers/mfd/Makefile | 2 + > > > > > > > drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ > > > > > > > include/linux/mfd/mp2629.h | 19 +++++++++ > > > > > > > 4 files changed, 116 insertions(+) > > > > > > > create mode 100644 drivers/mfd/mp2629.c > > > > > > > create mode 100644 include/linux/mfd/mp2629.h > > > > > > How is this driver registered? > > > > > > > > > > > > Looks like it has device tree support. Is there another way? > > > > > Yes, only using device tree > > > > Then how about using 'simple-mfd' and 'syscon'? > > > > > > > > Then you can omit this driver completely. > > > The exception is to support for non device tree platform as well, but I have > > > tested only for ARM device tree platform. > > Is that a reality though? > > > > How else do you see this realistically being registered? > > > I understand that acpi related device table are not covered here, well I > don't have to platform to test so. > If you ask me to cover acpi related table, I can do but hard to test. I don't know of any reasons why syscon can't be used by ACPI. Please try to solve this issue using 'simple-mfd' and 'syscon'. > > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > > > > > index 3c547ed575e6..85be799795aa 100644 > > > > > > > --- a/drivers/mfd/Kconfig > > > > > > > +++ b/drivers/mfd/Kconfig > > > > > > > @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C > > > > > > > help > > > > > > > Select this if your MC13xxx is connected via an I2C bus. > > > > > > > +config MFD_MP2629 > > > > > > > + tristate "Monolithic power system MP2629 ADC and Battery charger" > > > > > > > + depends on I2C > > > > > > > + select REGMAP_I2C > > > > > > > + help > > > > > > > + Select this option to enable support for monolithic power system > > > > > > > + battery charger. This provides ADC, thermal, battery charger power > > > > > > > + management functions on the systems.
Hi Lee, On 27/04/20 10:51 am, Lee Jones wrote: > On Fri, 24 Apr 2020, saravanan sekar wrote: > >> On 24/04/20 12:53 pm, Lee Jones wrote: >>> On Fri, 24 Apr 2020, saravanan sekar wrote: >>> >>>> Hi Lee, >>>> >>>> On 24/04/20 11:37 am, Lee Jones wrote: >>>>> On Fri, 24 Apr 2020, saravanan sekar wrote: >>>>> >>>>>> Hi Lee, >>>>>> >>>>>> On 24/04/20 9:18 am, Lee Jones wrote: >>>>>>> On Wed, 15 Apr 2020, Saravanan Sekar wrote: >>>>>>> >>>>>>>> mp2629 is a highly-integrated switching-mode battery charge management >>>>>>>> device for single-cell Li-ion or Li-polymer battery. >>>>>>>> >>>>>>>> Add MFD core enables chip access for ADC driver for battery readings, >>>>>>>> and a power supply battery-charger driver >>>>>>>> >>>>>>>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> >>>>>>>> --- >>>>>>>> drivers/mfd/Kconfig | 9 ++++ >>>>>>>> drivers/mfd/Makefile | 2 + >>>>>>>> drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ >>>>>>>> include/linux/mfd/mp2629.h | 19 +++++++++ >>>>>>>> 4 files changed, 116 insertions(+) >>>>>>>> create mode 100644 drivers/mfd/mp2629.c >>>>>>>> create mode 100644 include/linux/mfd/mp2629.h >>>>>>> How is this driver registered? >>>>>>> >>>>>>> Looks like it has device tree support. Is there another way? >>>>>> Yes, only using device tree >>>>> Then how about using 'simple-mfd' and 'syscon'? >>>>> >>>>> Then you can omit this driver completely. >>>> The exception is to support for non device tree platform as well, but I have >>>> tested only for ARM device tree platform. >>> Is that a reality though? >>> >>> How else do you see this realistically being registered? >>> >> I understand that acpi related device table are not covered here, well I >> don't have to platform to test so. >> If you ask me to cover acpi related table, I can do but hard to test. > I don't know of any reasons why syscon can't be used by ACPI. > > Please try to solve this issue using 'simple-mfd' and 'syscon'. Well the simple-mfd and syscon topic recommended by you when Device tree alone is used. I wounder still I receive review/improvements comments for this mfd patch and also another to omit this driver using simple-mfd (rework the series). Confused, not sure which is valid and will be accepted at the end. > >>>>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>>>>>> index 3c547ed575e6..85be799795aa 100644 >>>>>>>> --- a/drivers/mfd/Kconfig >>>>>>>> +++ b/drivers/mfd/Kconfig >>>>>>>> @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C >>>>>>>> help >>>>>>>> Select this if your MC13xxx is connected via an I2C bus. >>>>>>>> +config MFD_MP2629 >>>>>>>> + tristate "Monolithic power system MP2629 ADC and Battery charger" >>>>>>>> + depends on I2C >>>>>>>> + select REGMAP_I2C >>>>>>>> + help >>>>>>>> + Select this option to enable support for monolithic power system >>>>>>>> + battery charger. This provides ADC, thermal, battery charger power >>>>>>>> + management functions on the systems.
Hi Lee, On 27/04/20 11:30 am, saravanan sekar wrote: > Hi Lee, > > On 27/04/20 10:51 am, Lee Jones wrote: >> On Fri, 24 Apr 2020, saravanan sekar wrote: >> >>> On 24/04/20 12:53 pm, Lee Jones wrote: >>>> On Fri, 24 Apr 2020, saravanan sekar wrote: >>>> >>>>> Hi Lee, >>>>> >>>>> On 24/04/20 11:37 am, Lee Jones wrote: >>>>>> On Fri, 24 Apr 2020, saravanan sekar wrote: >>>>>> >>>>>>> Hi Lee, >>>>>>> >>>>>>> On 24/04/20 9:18 am, Lee Jones wrote: >>>>>>>> On Wed, 15 Apr 2020, Saravanan Sekar wrote: >>>>>>>> >>>>>>>>> mp2629 is a highly-integrated switching-mode battery charge >>>>>>>>> management >>>>>>>>> device for single-cell Li-ion or Li-polymer battery. >>>>>>>>> >>>>>>>>> Add MFD core enables chip access for ADC driver for battery >>>>>>>>> readings, >>>>>>>>> and a power supply battery-charger driver >>>>>>>>> >>>>>>>>> Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> >>>>>>>>> --- >>>>>>>>> drivers/mfd/Kconfig | 9 ++++ >>>>>>>>> drivers/mfd/Makefile | 2 + >>>>>>>>> drivers/mfd/mp2629.c | 86 >>>>>>>>> ++++++++++++++++++++++++++++++++++++++ >>>>>>>>> include/linux/mfd/mp2629.h | 19 +++++++++ >>>>>>>>> 4 files changed, 116 insertions(+) >>>>>>>>> create mode 100644 drivers/mfd/mp2629.c >>>>>>>>> create mode 100644 include/linux/mfd/mp2629.h >>>>>>>> How is this driver registered? >>>>>>>> >>>>>>>> Looks like it has device tree support. Is there another way? >>>>>>> Yes, only using device tree >>>>>> Then how about using 'simple-mfd' and 'syscon'? >>>>>> >>>>>> Then you can omit this driver completely. >>>>> The exception is to support for non device tree platform as well, >>>>> but I have >>>>> tested only for ARM device tree platform. >>>> Is that a reality though? >>>> >>>> How else do you see this realistically being registered? >>>> >>> I understand that acpi related device table are not covered here, >>> well I >>> don't have to platform to test so. >>> If you ask me to cover acpi related table, I can do but hard to test. >> I don't know of any reasons why syscon can't be used by ACPI. >> >> Please try to solve this issue using 'simple-mfd' and 'syscon'. > Well the simple-mfd and syscon topic recommended by you when Device > tree alone is used. > I wounder still I receive review/improvements comments for this mfd > patch and also another > to omit this driver using simple-mfd (rework the series). > Confused, not sure which is valid and will be accepted at the end. I had look into syscon, as far my understanding syscon is supported only for memory mapped IO. MP2629 device is over I2C bus, could you share your thought about syscon for this device? Thanks, Saravanan
On Thu, Apr 30, 2020 at 1:39 PM saravanan sekar <sravanhome@gmail.com> wrote: > On 27/04/20 11:30 am, saravanan sekar wrote: > > On 27/04/20 10:51 am, Lee Jones wrote: > >> On Fri, 24 Apr 2020, saravanan sekar wrote: > >>> On 24/04/20 12:53 pm, Lee Jones wrote: > >>>> On Fri, 24 Apr 2020, saravanan sekar wrote: > >>>>> On 24/04/20 11:37 am, Lee Jones wrote: > >>>>>> On Fri, 24 Apr 2020, saravanan sekar wrote: ... > >>>>>> Then how about using 'simple-mfd' and 'syscon'? > >>>>>> > >>>>>> Then you can omit this driver completely. > >>>>> The exception is to support for non device tree platform as well, > >>>>> but I have > >>>>> tested only for ARM device tree platform. > >>>> Is that a reality though? > >>>> > >>>> How else do you see this realistically being registered? > >>>> > >>> I understand that acpi related device table are not covered here, > >>> well I > >>> don't have to platform to test so. > >>> If you ask me to cover acpi related table, I can do but hard to test. > >> I don't know of any reasons why syscon can't be used by ACPI. > >> > >> Please try to solve this issue using 'simple-mfd' and 'syscon'. > > Well the simple-mfd and syscon topic recommended by you when Device > > tree alone is used. > > I wounder still I receive review/improvements comments for this mfd > > patch and also another > > to omit this driver using simple-mfd (rework the series). > > Confused, not sure which is valid and will be accepted at the end. > > I had look into syscon, as far my understanding syscon is supported only > for memory mapped IO. MP2629 > device is over I2C bus, could you share your thought about syscon for > this device? My understanding too, but I didn't dug into it. Something like general regmap support is missing (or I am missing the way how to pass it to syscon).
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 3c547ed575e6..85be799795aa 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -434,6 +434,15 @@ config MFD_MC13XXX_I2C help Select this if your MC13xxx is connected via an I2C bus. +config MFD_MP2629 + tristate "Monolithic power system MP2629 ADC and Battery charger" + depends on I2C + select REGMAP_I2C + help + Select this option to enable support for monolithic power system + battery charger. This provides ADC, thermal, battery charger power + management functions on the systems. + config MFD_MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on ARCH_MXS || COMPILE_TEST diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index f935d10cbf0f..d6c210f96d02 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -170,6 +170,8 @@ obj-$(CONFIG_MFD_MAX8925) += max8925.o obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o +obj-$(CONFIG_MFD_MP2629) += mp2629.o + pcf50633-objs := pcf50633-core.o pcf50633-irq.o obj-$(CONFIG_MFD_PCF50633) += pcf50633.o obj-$(CONFIG_PCF50633_ADC) += pcf50633-adc.o diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c new file mode 100644 index 000000000000..46242b1cdf24 --- /dev/null +++ b/drivers/mfd/mp2629.c @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * MP2629 parent driver for ADC and battery charger + * + * Copyright 2020 Monolithic Power Systems, Inc + * + * Author: Saravanan Sekar <sravanhome@gmail.com> + */ + +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/mfd/core.h> +#include <linux/mfd/mp2629.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +enum { + MP2629_MFD_ADC, + MP2629_MFD_CHARGER, + MP2629_MFD_MAX +}; + +static const struct mfd_cell mp2629mfd[] = { + [MP2629_MFD_ADC] = { + .name = "mp2629_adc", + .of_compatible = "mps,mp2629_adc", + }, + [MP2629_MFD_CHARGER] = { + .name = "mp2629_charger", + .of_compatible = "mps,mp2629_charger", + } +}; + +static const struct regmap_config mp2629_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0x17, +}; + +static int mp2629_probe(struct i2c_client *client) +{ + struct mp2629_info *ddata; + int ret; + + ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL); + if (!ddata) + return -ENOMEM; + + ddata->dev = &client->dev; + i2c_set_clientdata(client, ddata); + + ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config); + if (IS_ERR(ddata->regmap)) { + dev_err(ddata->dev, "Failed to allocate regmap!\n"); + return PTR_ERR(ddata->regmap); + } + + ret = devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_NONE, mp2629mfd, + ARRAY_SIZE(mp2629mfd), NULL, + 0, NULL); + if (ret) + dev_err(ddata->dev, "Failed to register sub-devices %d\n", ret); + + return ret; +} + +static const struct of_device_id mp2629_of_match[] = { + { .compatible = "mps,mp2629"}, + { } +}; +MODULE_DEVICE_TABLE(of, mp2629_of_match); + +static struct i2c_driver mp2629_driver = { + .driver = { + .name = "mp2629", + .of_match_table = mp2629_of_match, + }, + .probe_new = mp2629_probe, +}; +module_i2c_driver(mp2629_driver); + +MODULE_AUTHOR("Saravanan Sekar <sravanhome@gmail.com>"); +MODULE_DESCRIPTION("MP2629 Battery charger parent driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h new file mode 100644 index 000000000000..06985e41fc6f --- /dev/null +++ b/include/linux/mfd/mp2629.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * register definitions for MP2629 charger + * + * Copyright 2020 Monolithic Power Systems, Inc + */ + +#ifndef __MP2629_H__ +#define __MP2629_H__ + +#include <linux/device.h> +#include <linux/regmap.h> + +struct mp2629_info { + struct device *dev; + struct regmap *regmap; +}; + +#endif
mp2629 is a highly-integrated switching-mode battery charge management device for single-cell Li-ion or Li-polymer battery. Add MFD core enables chip access for ADC driver for battery readings, and a power supply battery-charger driver Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> --- drivers/mfd/Kconfig | 9 ++++ drivers/mfd/Makefile | 2 + drivers/mfd/mp2629.c | 86 ++++++++++++++++++++++++++++++++++++++ include/linux/mfd/mp2629.h | 19 +++++++++ 4 files changed, 116 insertions(+) create mode 100644 drivers/mfd/mp2629.c create mode 100644 include/linux/mfd/mp2629.h