* [RFC 1/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-12 10:51 ` Moritz Fischer
2019-02-14 9:23 ` Lee Jones
2019-02-12 1:01 ` [RFC 2/6] " Virendra Kakade
` (4 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Document bindings for E31x device PMU MFD driver.
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/e31x-pmu.txt
diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
new file mode 100644
index 000000000000..ebb5625f6c74
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
@@ -0,0 +1,14 @@
+Ettus Research/National Instruments E31x PMU MFD driver
+
+Required properties:
+- compatible : Must be "ni,e31x-pmu"
+- regmap : Must be <®mapnode>
+
+Example:
+
+pmu {
+ compatible = "ni,e31x-pmu";
+ regmap = <&devctrl>;
+ status = "okay";
+};
+
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 1/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-12 1:01 ` [RFC 1/6] mfd: Support " Virendra Kakade
@ 2019-02-12 10:51 ` Moritz Fischer
2019-02-14 9:23 ` Lee Jones
1 sibling, 0 replies; 17+ messages in thread
From: Moritz Fischer @ 2019-02-12 10:51 UTC (permalink / raw)
To: Virendra Kakade
Cc: Devicetree List, Linux Kernel Mailing List, Linux PM, Lee Jones,
Rob Herring, Mark Rutland, Sebastian Reichel
Hi Virendra,
On Mon, Feb 11, 2019 at 5:02 PM Virendra Kakade <virendra.kakade@ni.com> wrote:
>
> Document bindings for E31x device PMU MFD driver.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/e31x-pmu.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> new file mode 100644
> index 000000000000..ebb5625f6c74
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> @@ -0,0 +1,14 @@
> +Ettus Research/National Instruments E31x PMU MFD driver
> +
> +Required properties:
> +- compatible : Must be "ni,e31x-pmu"
> +- regmap : Must be <®mapnode>
> +
> +Example:
> +
> +pmu {
> + compatible = "ni,e31x-pmu";
> + regmap = <&devctrl>;
> + status = "okay";
Please drop this.
>
> +};
> +
> --
> 2.17.1
>
Your binding patches should have "dt-bindings: ... " in the commit message.
Please fix that when you resubmit your series.
Cheers,
Moritz
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC 1/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-12 1:01 ` [RFC 1/6] mfd: Support " Virendra Kakade
2019-02-12 10:51 ` Moritz Fischer
@ 2019-02-14 9:23 ` Lee Jones
1 sibling, 0 replies; 17+ messages in thread
From: Lee Jones @ 2019-02-14 9:23 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, robh+dt, mark.rutland, sre,
moritz.fischer
On Mon, 11 Feb 2019, Virendra Kakade wrote:
> Document bindings for E31x device PMU MFD driver.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/e31x-pmu.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> new file mode 100644
> index 000000000000..ebb5625f6c74
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> @@ -0,0 +1,14 @@
> +Ettus Research/National Instruments E31x PMU MFD driver
Please expand on what PMU is.
MFD is a Linuxisum and has no place in the description.
You can supplement it with "parent device" or some such.
> +Required properties:
> +- compatible : Must be "ni,e31x-pmu"
> +- regmap : Must be <®mapnode>
> +
> +Example:
> +
> +pmu {
> + compatible = "ni,e31x-pmu";
No address?
> + regmap = <&devctrl>;
> + status = "okay";
Not required in DT documentation.
> +};
> +
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC 2/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
2019-02-12 1:01 ` [RFC 1/6] mfd: Support " Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-14 9:34 ` Lee Jones
2019-02-12 1:01 ` [RFC 3/6] power: supply: Ettus Research E31x charger driver Virendra Kakade
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
drivers/mfd/Kconfig | 7 +++
drivers/mfd/Makefile | 2 +-
drivers/mfd/e31x-pmu.c | 89 ++++++++++++++++++++++++++++++++++++
include/linux/mfd/e31x-pmu.h | 20 ++++++++
4 files changed, 117 insertions(+), 1 deletion(-)
create mode 100644 drivers/mfd/e31x-pmu.c
create mode 100644 include/linux/mfd/e31x-pmu.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8c5dfdce4326..6c4c0747f2a5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1916,4 +1916,11 @@ config RAVE_SP_CORE
device found on several devices in RAVE line of hardware.
endmenu
+
+config MFD_E31X_PMU
+ tristate "E31X PMU driver"
+ select MFD_SYSCON
+ help
+ Select this to get support for the Ettus Research E31x devices.
+
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 12980a4ad460..e37c2057134b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -241,4 +241,4 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
-
+obj-$(CONFIG_MFD_E31X_PMU) += e31x-pmu.o
diff --git a/drivers/mfd/e31x-pmu.c b/drivers/mfd/e31x-pmu.c
new file mode 100644
index 000000000000..383df68a538f
--- /dev/null
+++ b/drivers/mfd/e31x-pmu.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 National Instruments Corp
+ * Author: Virendra Kakade <virendra.kakade@ni.com>
+ *
+ * Ettus Research E31x PMU MFD driver
+ */
+
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/of_device.h>
+#include <linux/mfd/e31x-pmu.h>
+#include <linux/platform_device.h>
+
+#define E31X_PMU_MISC_IRQ_MASK BIT(8)
+#define E31X_PMU_MISC_IRQ_SHIFT 8
+#define E31X_PMU_MISC_VERSION_MIN_MASK GENMASK(3, 0)
+#define E31X_PMU_MISC_VERSION_MIN_SHIFT 0
+#define E31X_PMU_MISC_VERSION_MAJ_MASK GENMASK(7, 4)
+#define E31X_PMU_MISC_VERSION_MAJ_SHIFT 4
+
+struct e31x_pmu {
+ struct regmap *regmap;
+};
+
+static int e31x_pmu_check_version(struct e31x_pmu *pmu)
+{
+ int timeout = 100;
+ u32 misc, maj;
+ int err;
+ /* we need to wait a bit for firmware to populate the fields */
+ while (timeout--) {
+ err = regmap_read(pmu->regmap, E31X_PMU_REG_MISC, &misc);
+ if (err)
+ return err;
+ if (misc)
+ break;
+
+ usleep_range(2500, 5000);
+ }
+
+ /* only firmware versions above 2.0 are supported */
+ maj = E31X_PMU_GET_FIELD(MISC_VERSION_MAJ, misc);
+ if (maj < 2)
+ return -ENOTSUPP;
+ return 0;
+}
+
+static int e31x_pmu_probe(struct platform_device *pdev)
+{
+ struct e31x_pmu *pmu;
+
+ pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
+ if (!pmu)
+ return -ENOMEM;
+ pmu->regmap = syscon_regmap_lookup_by_phandle(\
+ pdev->dev.of_node, "regmap");
+
+ if (IS_ERR(pmu->regmap))
+ return PTR_ERR(pmu->regmap);
+ if (e31x_pmu_check_version(pmu))
+ return -ENOTSUPP;
+ return devm_of_platform_populate(&pdev->dev);
+}
+
+static const struct of_device_id e31x_pmu_id[] = {
+ { .compatible = "ni,e31x-pmu" },
+ {},
+};
+
+static struct platform_driver e31x_pmu_driver = {
+ .driver = {
+ .name = "e31x-pmu",
+ .of_match_table = e31x_pmu_id,
+ },
+ .probe = e31x_pmu_probe,
+};
+module_platform_driver(e31x_pmu_driver);
+
+MODULE_DESCRIPTION("E31x PMU driver");
+MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
new file mode 100644
index 000000000000..c57d5a8c1aad
--- /dev/null
+++ b/include/linux/mfd/e31x-pmu.h
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 National Instruments Corp
+ * Author: Virendra Kakade <virendra.kakade@ni.com>
+ *
+ * Ettus Research E31x PMU constants
+ */
+
+#ifndef MFD_E31X_PMU_H
+#define MFD_E31X_PMU_H
+
+#include <linux/bitops.h>
+
+#define E31X_PMU_REG_MISC 0x04
+
+#define E31X_PMU_GET_FIELD(name, reg) \
+ (((reg) & E31X_PMU_## name ##_MASK) >> \
+ E31X_PMU_## name ##_SHIFT)
+
+#endif /* MFD_E31X_PMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 2/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-12 1:01 ` [RFC 2/6] " Virendra Kakade
@ 2019-02-14 9:34 ` Lee Jones
2019-02-17 16:37 ` Moritz Fischer
0 siblings, 1 reply; 17+ messages in thread
From: Lee Jones @ 2019-02-14 9:34 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, robh+dt, mark.rutland, sre,
moritz.fischer
On Mon, 11 Feb 2019, Virendra Kakade wrote:
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> drivers/mfd/Kconfig | 7 +++
> drivers/mfd/Makefile | 2 +-
> drivers/mfd/e31x-pmu.c | 89 ++++++++++++++++++++++++++++++++++++
> include/linux/mfd/e31x-pmu.h | 20 ++++++++
> 4 files changed, 117 insertions(+), 1 deletion(-)
> create mode 100644 drivers/mfd/e31x-pmu.c
> create mode 100644 include/linux/mfd/e31x-pmu.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8c5dfdce4326..6c4c0747f2a5 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1916,4 +1916,11 @@ config RAVE_SP_CORE
> device found on several devices in RAVE line of hardware.
>
> endmenu
> +
> +config MFD_E31X_PMU
> + tristate "E31X PMU driver"
> + select MFD_SYSCON
> + help
> + Select this to get support for the Ettus Research E31x devices.
> +
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 12980a4ad460..e37c2057134b 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -241,4 +241,4 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> -
> +obj-$(CONFIG_MFD_E31X_PMU) += e31x-pmu.o
> diff --git a/drivers/mfd/e31x-pmu.c b/drivers/mfd/e31x-pmu.c
> new file mode 100644
> index 000000000000..383df68a538f
> --- /dev/null
> +++ b/drivers/mfd/e31x-pmu.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 National Instruments Corp
This is out of date.
'\n' here.
> + * Author: Virendra Kakade<virendra.kakade@ni.com>
You need a space after your name.
> + * Ettus Research E31x PMU MFD driver
Please expand PMU.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/core.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/e31x-pmu.h>
> +#include <linux/platform_device.h>
Alphabetical.
> +#define E31X_PMU_MISC_IRQ_MASK BIT(8)
> +#define E31X_PMU_MISC_IRQ_SHIFT 8
> +#define E31X_PMU_MISC_VERSION_MIN_MASK GENMASK(3, 0)
> +#define E31X_PMU_MISC_VERSION_MIN_SHIFT 0
> +#define E31X_PMU_MISC_VERSION_MAJ_MASK GENMASK(7, 4)
> +#define E31X_PMU_MISC_VERSION_MAJ_SHIFT 4
> +
> +struct e31x_pmu {
> + struct regmap *regmap;
> +};
A whole struct for one attribute?
> +static int e31x_pmu_check_version(struct e31x_pmu *pmu)
> +{
> + int timeout = 100;
Where does 100 come from?
> + u32 misc, maj;
> + int err;
'ret' is more common.
> + /* we need to wait a bit for firmware to populate the fields */
"a bit" ?
What does the datasheet say?
Please use proper grammar. Capital letters.
> + while (timeout--) {
> + err = regmap_read(pmu->regmap, E31X_PMU_REG_MISC, &misc);
> + if (err)
> + return err;
> + if (misc)
> + break;
> +
> + usleep_range(2500, 5000);
Formatting.
> + }
> +
> + /* only firmware versions above 2.0 are supported */
"Only supports firmware version 2.0 and above"
> + maj = E31X_PMU_GET_FIELD(MISC_VERSION_MAJ, misc);
> + if (maj < 2)
> + return -ENOTSUPP;
'\n' here.
> + return 0;
> +}
> +
> +static int e31x_pmu_probe(struct platform_device *pdev)
> +{
> + struct e31x_pmu *pmu;
> +
> + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
'\n' here.
> + pmu->regmap = syscon_regmap_lookup_by_phandle(\
Why are you storing regmap?
Just pass it directly to e31x_pmu_check_version().
> + pdev->dev.of_node, "regmap");
> +
Remove this line.
> + if (IS_ERR(pmu->regmap))
> + return PTR_ERR(pmu->regmap);
'\n' here.
> + if (e31x_pmu_check_version(pmu))
Please split out the function from the if.
Use a variable 'ret' to feed into and check that.
> + return -ENOTSUPP;
'\n' here.
> + return devm_of_platform_populate(&pdev->dev);
> +}
> +
> +static const struct of_device_id e31x_pmu_id[] = {
> + { .compatible = "ni,e31x-pmu" },
> + {},
> +};
> +
> +static struct platform_driver e31x_pmu_driver = {
> + .driver = {
> + .name = "e31x-pmu",
> + .of_match_table = e31x_pmu_id,
These 2 lines need indenting.
> + },
> + .probe = e31x_pmu_probe,
> +};
> +module_platform_driver(e31x_pmu_driver);
> +
> +MODULE_DESCRIPTION("E31x PMU driver");
> +MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
> +MODULE_LICENSE("GPL");
So the whole purpose of this driver is to do a version check.
Seems like over-kill!
Probably better to do the version check in an inline function stored
in a header file, then call it from each of the children's .probe()
function.
> diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
> new file mode 100644
> index 000000000000..c57d5a8c1aad
> --- /dev/null
> +++ b/include/linux/mfd/e31x-pmu.h
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 National Instruments Corp
Out of date.
'\n' here.
> + * Author: Virendra Kakade <virendra.kakade@ni.com>
> + *
> + * Ettus Research E31x PMU constants
> + */
> +
> +#ifndef MFD_E31X_PMU_H
> +#define MFD_E31X_PMU_H
> +
> +#include <linux/bitops.h>
> +
> +#define E31X_PMU_REG_MISC 0x04
> +
> +#define E31X_PMU_GET_FIELD(name, reg) \
> + (((reg) & E31X_PMU_## name ##_MASK) >> \
> + E31X_PMU_## name ##_SHIFT)
> +
> +#endif /* MFD_E31X_PMU_H */
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC 2/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-14 9:34 ` Lee Jones
@ 2019-02-17 16:37 ` Moritz Fischer
2019-02-18 8:56 ` Lee Jones
0 siblings, 1 reply; 17+ messages in thread
From: Moritz Fischer @ 2019-02-17 16:37 UTC (permalink / raw)
To: Lee Jones
Cc: Virendra Kakade, Devicetree List, Linux Kernel Mailing List,
Linux PM, Rob Herring, Mark Rutland, Sebastian Reichel
Hi Lee,
thanks for your feedback!
On Thu, Feb 14, 2019 at 1:34 AM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Mon, 11 Feb 2019, Virendra Kakade wrote:
>
> > Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> > ---
> > drivers/mfd/Kconfig | 7 +++
> > drivers/mfd/Makefile | 2 +-
> > drivers/mfd/e31x-pmu.c | 89 ++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/e31x-pmu.h | 20 ++++++++
> > 4 files changed, 117 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/mfd/e31x-pmu.c
> > create mode 100644 include/linux/mfd/e31x-pmu.h
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 8c5dfdce4326..6c4c0747f2a5 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1916,4 +1916,11 @@ config RAVE_SP_CORE
> > device found on several devices in RAVE line of hardware.
> >
> > endmenu
> > +
> > +config MFD_E31X_PMU
> > + tristate "E31X PMU driver"
> > + select MFD_SYSCON
> > + help
> > + Select this to get support for the Ettus Research E31x devices.
> > +
> > endif
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 12980a4ad460..e37c2057134b 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -241,4 +241,4 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> > -
> > +obj-$(CONFIG_MFD_E31X_PMU) += e31x-pmu.o
> > diff --git a/drivers/mfd/e31x-pmu.c b/drivers/mfd/e31x-pmu.c
> > new file mode 100644
> > index 000000000000..383df68a538f
> > --- /dev/null
> > +++ b/drivers/mfd/e31x-pmu.c
> > @@ -0,0 +1,89 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 National Instruments Corp
>
> This is out of date.
>
> '\n' here.
>
> > + * Author: Virendra Kakade<virendra.kakade@ni.com>
>
> You need a space after your name.
>
> > + * Ettus Research E31x PMU MFD driver
>
> Please expand PMU.
>
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/delay.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/of_device.h>
> > +#include <linux/mfd/e31x-pmu.h>
> > +#include <linux/platform_device.h>
>
> Alphabetical.
>
> > +#define E31X_PMU_MISC_IRQ_MASK BIT(8)
> > +#define E31X_PMU_MISC_IRQ_SHIFT 8
> > +#define E31X_PMU_MISC_VERSION_MIN_MASK GENMASK(3, 0)
> > +#define E31X_PMU_MISC_VERSION_MIN_SHIFT 0
> > +#define E31X_PMU_MISC_VERSION_MAJ_MASK GENMASK(7, 4)
> > +#define E31X_PMU_MISC_VERSION_MAJ_SHIFT 4
> > +
> > +struct e31x_pmu {
> > + struct regmap *regmap;
> > +};
>
> A whole struct for one attribute?
>
> > +static int e31x_pmu_check_version(struct e31x_pmu *pmu)
> > +{
> > + int timeout = 100;
>
> Where does 100 come from?
>
> > + u32 misc, maj;
> > + int err;
>
> 'ret' is more common.
>
> > + /* we need to wait a bit for firmware to populate the fields */
>
> "a bit" ?
>
> What does the datasheet say?
>
> Please use proper grammar. Capital letters.
>
> > + while (timeout--) {
> > + err = regmap_read(pmu->regmap, E31X_PMU_REG_MISC, &misc);
> > + if (err)
> > + return err;
> > + if (misc)
> > + break;
> > +
> > + usleep_range(2500, 5000);
>
> Formatting.
>
> > + }
> > +
> > + /* only firmware versions above 2.0 are supported */
>
> "Only supports firmware version 2.0 and above"
>
> > + maj = E31X_PMU_GET_FIELD(MISC_VERSION_MAJ, misc);
> > + if (maj < 2)
> > + return -ENOTSUPP;
>
> '\n' here.
>
> > + return 0;
> > +}
> > +
> > +static int e31x_pmu_probe(struct platform_device *pdev)
> > +{
> > + struct e31x_pmu *pmu;
> > +
> > + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> > + if (!pmu)
> > + return -ENOMEM;
>
> '\n' here.
>
> > + pmu->regmap = syscon_regmap_lookup_by_phandle(\
>
> Why are you storing regmap?
See my comment below.
>
> Just pass it directly to e31x_pmu_check_version().
>
> > + pdev->dev.of_node, "regmap");
> > +
>
> Remove this line.
>
> > + if (IS_ERR(pmu->regmap))
> > + return PTR_ERR(pmu->regmap);
>
> '\n' here.
>
> > + if (e31x_pmu_check_version(pmu))
>
> Please split out the function from the if.
>
> Use a variable 'ret' to feed into and check that.
>
> > + return -ENOTSUPP;
>
> '\n' here.
>
> > + return devm_of_platform_populate(&pdev->dev);
> > +}
> > +
> > +static const struct of_device_id e31x_pmu_id[] = {
> > + { .compatible = "ni,e31x-pmu" },
> > + {},
> > +};
> > +
> > +static struct platform_driver e31x_pmu_driver = {
> > + .driver = {
> > + .name = "e31x-pmu",
> > + .of_match_table = e31x_pmu_id,
>
> These 2 lines need indenting.
>
> > + },
> > + .probe = e31x_pmu_probe,
> > +};
> > +module_platform_driver(e31x_pmu_driver);
> > +
> > +MODULE_DESCRIPTION("E31x PMU driver");
> > +MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
> > +MODULE_LICENSE("GPL");
>
> So the whole purpose of this driver is to do a version check.
>
> Seems like over-kill!
>
> Probably better to do the version check in an inline function stored
> in a header file, then call it from each of the children's .probe()
> function.
A bit of context here, this is based on an in-house driver that we had that does
a bunch of other stuff (e.g. it controls a flag on whether the device auto-boots
on power being plugged in etc.). These functions will use the regmap.
I had advised Virendra to submit a base version and follow up with patches
that would add these functions one by one as he figures out how to expose them
in a proper way to the kernel.
Cheers,
Moritz
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC 2/6] mfd: Support for Ettus Research E31x devices PMU
2019-02-17 16:37 ` Moritz Fischer
@ 2019-02-18 8:56 ` Lee Jones
0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2019-02-18 8:56 UTC (permalink / raw)
To: Moritz Fischer
Cc: Virendra Kakade, Devicetree List, Linux Kernel Mailing List,
Linux PM, Rob Herring, Mark Rutland, Sebastian Reichel
On Sun, 17 Feb 2019, Moritz Fischer wrote:
> On Thu, Feb 14, 2019 at 1:34 AM Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 11 Feb 2019, Virendra Kakade wrote:
> >
> > > Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> > > ---
> > > drivers/mfd/Kconfig | 7 +++
> > > drivers/mfd/Makefile | 2 +-
> > > drivers/mfd/e31x-pmu.c | 89 ++++++++++++++++++++++++++++++++++++
> > > include/linux/mfd/e31x-pmu.h | 20 ++++++++
> > > 4 files changed, 117 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/mfd/e31x-pmu.c
> > > create mode 100644 include/linux/mfd/e31x-pmu.h
[...]
Try not to quote too many unnecessary lines when replying please.
> > So the whole purpose of this driver is to do a version check.
> >
> > Seems like over-kill!
> >
> > Probably better to do the version check in an inline function stored
> > in a header file, then call it from each of the children's .probe()
> > function.
>
> A bit of context here, this is based on an in-house driver that we had that does
> a bunch of other stuff (e.g. it controls a flag on whether the device auto-boots
> on power being plugged in etc.). These functions will use the regmap.
>
> I had advised Virendra to submit a base version and follow up with patches
> that would add these functions one by one as he figures out how to expose them
> in a proper way to the kernel.
Probably best to wait until you have something a little more
featureful before attempting to upstream then. I couldn't tell you
how many times I've had contributors tell me that they plan to follow
up with additional patches and (for their own perfectly decent reasons
I'm sure) fail to do so.
As it stands, this driver over-kill for what you're trying to achieve
and therefore isn't overly suitable for upstreaming at the moment.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC 3/6] power: supply: Ettus Research E31x charger driver
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
2019-02-12 1:01 ` [RFC 1/6] mfd: Support " Virendra Kakade
2019-02-12 1:01 ` [RFC 2/6] " Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-14 9:38 ` Lee Jones
2019-02-25 23:38 ` Rob Herring
2019-02-12 1:01 ` [RFC 4/6] " Virendra Kakade
` (2 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Document bindings for the E31x devices charger.
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 8 ++++++++
.../bindings/power/supply/e31x-charger.txt | 14 ++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/e31x-charger.txt
diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
index ebb5625f6c74..3798886578ba 100644
--- a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
+++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
@@ -4,11 +4,19 @@ Required properties:
- compatible : Must be "ni,e31x-pmu"
- regmap : Must be <®mapnode>
+e31x-pmu has a sub-device called e31x-charger.
+
+For the e31x-charger properties please refer to:
+Documentation/device-tree/bindings/power/supply/e31x-charger.txt
+
Example:
pmu {
compatible = "ni,e31x-pmu";
regmap = <&devctrl>;
status = "okay";
+ charger: charger@0{
+ compatible = "ni,e31x-charger";
+ };
};
diff --git a/Documentation/devicetree/bindings/power/supply/e31x-charger.txt b/Documentation/devicetree/bindings/power/supply/e31x-charger.txt
new file mode 100644
index 000000000000..ae9fedcbf1b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/e31x-charger.txt
@@ -0,0 +1,14 @@
+Ettus Research/National Instruments E31x Devices Charger
+
+All E31x devices have to be connected to an external power supply
+charger to charge the in-built battery or supply power.
+Some capability is provided to monitor this charger.
+
+Required properties:
+- compatible: Must be "ni,e31x-charger"
+
+Example:
+
+charger: charger@0{
+ compatible = "ni,e31x-charger";
+};
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 3/6] power: supply: Ettus Research E31x charger driver
2019-02-12 1:01 ` [RFC 3/6] power: supply: Ettus Research E31x charger driver Virendra Kakade
@ 2019-02-14 9:38 ` Lee Jones
2019-02-25 23:38 ` Rob Herring
1 sibling, 0 replies; 17+ messages in thread
From: Lee Jones @ 2019-02-14 9:38 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, robh+dt, mark.rutland, sre,
moritz.fischer
On Mon, 11 Feb 2019, Virendra Kakade wrote:
> Document bindings for the E31x devices charger.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 8 ++++++++
> .../bindings/power/supply/e31x-charger.txt | 14 ++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/e31x-charger.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> index ebb5625f6c74..3798886578ba 100644
> --- a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> @@ -4,11 +4,19 @@ Required properties:
> - compatible : Must be "ni,e31x-pmu"
> - regmap : Must be <®mapnode>
>
> +e31x-pmu has a sub-device called e31x-charger.
> +
> +For the e31x-charger properties please refer to:
> +Documentation/device-tree/bindings/power/supply/e31x-charger.txt
Use relative path, so:
../power/supply/e31x-charger.txt
> Example:
>
> pmu {
> compatible = "ni,e31x-pmu";
> regmap = <&devctrl>;
> status = "okay";
'\n' here.
> + charger: charger@0{
> + compatible = "ni,e31x-charger";
> + };
> };
>
> diff --git a/Documentation/devicetree/bindings/power/supply/e31x-charger.txt b/Documentation/devicetree/bindings/power/supply/e31x-charger.txt
> new file mode 100644
> index 000000000000..ae9fedcbf1b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/e31x-charger.txt
> @@ -0,0 +1,14 @@
> +Ettus Research/National Instruments E31x Devices Charger
Too many spaces.
> +All E31x devices have to be connected to an external power supply
> +charger to charge the in-built battery or supply power.
> +Some capability is provided to monitor this charger.
Why the linefeed here? Take "Some" up to the previous line.
> +Required properties:
> +- compatible: Must be "ni,e31x-charger"
> +
> +Example:
> +
> +charger: charger@0{
> + compatible = "ni,e31x-charger";
> +};
Not sure this really requires documentation. Seems kinda simple.
I'll leave that up to Rob et al.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [RFC 3/6] power: supply: Ettus Research E31x charger driver
2019-02-12 1:01 ` [RFC 3/6] power: supply: Ettus Research E31x charger driver Virendra Kakade
2019-02-14 9:38 ` Lee Jones
@ 2019-02-25 23:38 ` Rob Herring
1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2019-02-25 23:38 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, lee.jones, mark.rutland, sre,
moritz.fischer
On Mon, Feb 11, 2019 at 07:01:40PM -0600, Virendra Kakade wrote:
> Document bindings for the E31x devices charger.
Fix your subject. It is the same as patch 4.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 8 ++++++++
> .../bindings/power/supply/e31x-charger.txt | 14 ++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/supply/e31x-charger.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> index ebb5625f6c74..3798886578ba 100644
> --- a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> @@ -4,11 +4,19 @@ Required properties:
> - compatible : Must be "ni,e31x-pmu"
> - regmap : Must be <®mapnode>
>
> +e31x-pmu has a sub-device called e31x-charger.
> +
> +For the e31x-charger properties please refer to:
> +Documentation/device-tree/bindings/power/supply/e31x-charger.txt
> +
> Example:
>
> pmu {
> compatible = "ni,e31x-pmu";
> regmap = <&devctrl>;
regmap is a linux thing. How is the h/w addressed?
> status = "okay";
> + charger: charger@0{
unit-address without
> + compatible = "ni,e31x-charger";
No other properties or DT resources? Then you don't need a child node in
DT. The driver for the parent node can instantiate child devices like a
charger.
> + };
> };
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC 4/6] power: supply: Ettus Research E31x charger driver
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
` (2 preceding siblings ...)
2019-02-12 1:01 ` [RFC 3/6] power: supply: Ettus Research E31x charger driver Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-12 21:46 ` Sebastian Reichel
2019-02-12 1:01 ` [RFC 5/6] power: supply: Ettus Research E31x battery driver Virendra Kakade
2019-02-12 1:01 ` [RFC 6/6] " Virendra Kakade
5 siblings, 1 reply; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Add support for E31x devices charger which is a sub-device of E31X-PMU.
Enables query of charger properties from device tree.
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
drivers/power/supply/Kconfig | 6 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/e31x-charger.c | 190 ++++++++++++++++++++++++++++
include/linux/mfd/e31x-pmu.h | 4 +
4 files changed, 201 insertions(+)
create mode 100644 drivers/power/supply/e31x-charger.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index e901b9879e7e..3d043b8fd49c 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -660,4 +660,10 @@ config FUEL_GAUGE_SC27XX
Say Y here to enable support for fuel gauge with SC27XX
PMIC chips.
+config E31X_CHARGER
+ tristate "Ettus Research E31x charger support"
+ depends on MFD_E31X_PMU
+ help
+ This adds support for the battery charger in Ettus Research E31x PMU.
+
endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index b731c2a9b695..6f4f8ca5484e 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
+obj-$(CONFIG_E31X_CHARGER) += e31x-charger.o
diff --git a/drivers/power/supply/e31x-charger.c b/drivers/power/supply/e31x-charger.c
new file mode 100644
index 000000000000..f154482bf95e
--- /dev/null
+++ b/drivers/power/supply/e31x-charger.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 National Instruments Corp
+ * Author: Virendra Kakade <virendra.kakade@ni.com>
+ *
+ * Ettus Research E31x charger driver
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/power_supply.h>
+#include <linux/delay.h>
+#include <linux/mfd/e31x-pmu.h>
+
+#define E31X_PMU_CHARGER_HEALTH_MASK GENMASK(1, 0)
+#define E31X_PMU_CHARGER_HEALTH_SHIFT 0
+#define E31X_PMU_CHARGER_CHARGE_TYPE_MASK GENMASK(4, 3)
+#define E31X_PMU_CHARGER_CHARGE_TYPE_SHIFT 3
+
+struct e31x_charger_dev {
+ struct regmap *regmap;
+ struct power_supply *supply;
+};
+
+static int e31x_charger_get_health(struct e31x_charger_dev *charger,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(CHARGER_HEALTH, value);
+ switch (value) {
+ case 0x0:
+ val->intval = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x1:
+ val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+ break;
+ case 0x2:
+ val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ break;
+ case 0x3:
+ val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+ break;
+ default:
+ val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+ break;
+ };
+
+ return 0;
+}
+
+static int e31x_charger_get_type(struct e31x_charger_dev *charger,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(CHARGER_CHARGE_TYPE, value);
+ switch (value) {
+ case 0x0:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ case 0x1:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ case 0x2:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ break;
+ default:
+ val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ break;
+ };
+ return 0;
+}
+
+static int e31x_charger_get_online(struct e31x_charger_dev *charger,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(CHARGER_ONLINE, value);
+ val->intval = !!value;
+
+ return 0;
+}
+
+static int e31x_charger_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct e31x_charger_dev *charger = power_supply_get_drvdata(psy);
+ int err = -EINVAL;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_HEALTH:
+ return e31x_charger_get_health(charger, val);
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ return e31x_charger_get_type(charger, val);
+ case POWER_SUPPLY_PROP_ONLINE:
+ return e31x_charger_get_online(charger, val);
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
+ return 0;
+ default:
+ break;
+ }
+
+ return err;
+}
+
+static enum power_supply_property e31x_charger_properties[] = {
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_SCOPE,
+};
+
+static const struct power_supply_desc e31x_charger_desc = {
+ .name = "e31x-charger",
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .properties = e31x_charger_properties,
+ .num_properties = ARRAY_SIZE(e31x_charger_properties),
+ .get_property = e31x_charger_get_property,
+};
+
+static const struct of_device_id e31x_charger_id[] = {
+ { .compatible = "ni,e31x-charger" },
+ { },
+};
+
+static int e31x_charger_probe(struct platform_device *pdev)
+{
+ struct power_supply_config psy_cfg = {};
+ struct e31x_charger_dev *charger;
+
+ if (!of_device_is_available(pdev->dev.of_node))
+ return -ENODEV;
+
+ charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ charger->regmap = syscon_regmap_lookup_by_phandle(\
+ pdev->dev.parent->of_node, "regmap");
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = charger;
+
+ charger->supply = devm_power_supply_register(&pdev->dev,
+ &e31x_charger_desc,
+ &psy_cfg);
+
+ if (IS_ERR(charger->supply))
+ return PTR_ERR(charger->supply);
+ return 0;
+}
+
+static struct platform_driver e31x_charger_driver = {
+ .driver = {
+ .name = "e31x-charger",
+ .of_match_table = e31x_charger_id,
+ },
+ .probe = e31x_charger_probe,
+};
+module_platform_driver(e31x_charger_driver);
+
+MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
+MODULE_DESCRIPTION("E31x charger driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
index c57d5a8c1aad..e0649845fe4d 100644
--- a/include/linux/mfd/e31x-pmu.h
+++ b/include/linux/mfd/e31x-pmu.h
@@ -12,9 +12,13 @@
#include <linux/bitops.h>
#define E31X_PMU_REG_MISC 0x04
+#define E31X_PMU_REG_CHARGER 0x0c
#define E31X_PMU_GET_FIELD(name, reg) \
(((reg) & E31X_PMU_## name ##_MASK) >> \
E31X_PMU_## name ##_SHIFT)
+#define E31X_PMU_CHARGER_ONLINE_MASK BIT(2)
+#define E31X_PMU_CHARGER_ONLINE_SHIFT 2
+
#endif /* MFD_E31X_PMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 4/6] power: supply: Ettus Research E31x charger driver
2019-02-12 1:01 ` [RFC 4/6] " Virendra Kakade
@ 2019-02-12 21:46 ` Sebastian Reichel
0 siblings, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2019-02-12 21:46 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, lee.jones, robh+dt,
mark.rutland, moritz.fischer
[-- Attachment #1: Type: text/plain, Size: 7973 bytes --]
Hi,
On Mon, Feb 11, 2019 at 07:01:41PM -0600, Virendra Kakade wrote:
> Add support for E31x devices charger which is a sub-device of E31X-PMU.
> Enables query of charger properties from device tree.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> drivers/power/supply/Kconfig | 6 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/e31x-charger.c | 190 ++++++++++++++++++++++++++++
> include/linux/mfd/e31x-pmu.h | 4 +
> 4 files changed, 201 insertions(+)
> create mode 100644 drivers/power/supply/e31x-charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..3d043b8fd49c 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -660,4 +660,10 @@ config FUEL_GAUGE_SC27XX
> Say Y here to enable support for fuel gauge with SC27XX
> PMIC chips.
>
> +config E31X_CHARGER
> + tristate "Ettus Research E31x charger support"
> + depends on MFD_E31X_PMU
> + help
> + This adds support for the battery charger in Ettus Research E31x PMU.
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..6f4f8ca5484e 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
> obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
> obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
> obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
> +obj-$(CONFIG_E31X_CHARGER) += e31x-charger.o
> diff --git a/drivers/power/supply/e31x-charger.c b/drivers/power/supply/e31x-charger.c
> new file mode 100644
> index 000000000000..f154482bf95e
> --- /dev/null
> +++ b/drivers/power/supply/e31x-charger.c
> @@ -0,0 +1,190 @@
> +// SPDX-License-Identifier: GPL-2.0
This specifies GPLv2 only, but MODULE_LICENSE("GPL") specifies GPL2
or later. Please fix one of them.
> +/*
> + * Copyright (c) 2018 National Instruments Corp
> + * Author: Virendra Kakade <virendra.kakade@ni.com>
> + *
> + * Ettus Research E31x charger driver
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/power_supply.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/e31x-pmu.h>
Please check the includes. Quite a few seem to be unused.
> +
> +#define E31X_PMU_CHARGER_HEALTH_MASK GENMASK(1, 0)
> +#define E31X_PMU_CHARGER_HEALTH_SHIFT 0
> +#define E31X_PMU_CHARGER_CHARGE_TYPE_MASK GENMASK(4, 3)
> +#define E31X_PMU_CHARGER_CHARGE_TYPE_SHIFT 3
> +
> +struct e31x_charger_dev {
> + struct regmap *regmap;
> + struct power_supply *supply;
> +};
> +
> +static int e31x_charger_get_health(struct e31x_charger_dev *charger,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(CHARGER_HEALTH, value);
> + switch (value) {
> + case 0x0:
> + val->intval = POWER_SUPPLY_HEALTH_GOOD;
> + break;
> + case 0x1:
> + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> + break;
> + case 0x2:
> + val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + break;
> + case 0x3:
> + val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
> + break;
> + default:
> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + };
> +
> + return 0;
> +}
> +
> +static int e31x_charger_get_type(struct e31x_charger_dev *charger,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(CHARGER_CHARGE_TYPE, value);
> + switch (value) {
> + case 0x0:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + break;
> + case 0x1:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + case 0x2:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + default:
> + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + break;
> + };
> + return 0;
> +}
> +
> +static int e31x_charger_get_online(struct e31x_charger_dev *charger,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(charger->regmap, E31X_PMU_REG_CHARGER, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(CHARGER_ONLINE, value);
> + val->intval = !!value;
> +
> + return 0;
> +}
> +
> +static int e31x_charger_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct e31x_charger_dev *charger = power_supply_get_drvdata(psy);
> + int err = -EINVAL;
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_HEALTH:
> + return e31x_charger_get_health(charger, val);
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + return e31x_charger_get_type(charger, val);
> + case POWER_SUPPLY_PROP_ONLINE:
> + return e31x_charger_get_online(charger, val);
> + case POWER_SUPPLY_PROP_SCOPE:
> + val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
> + return 0;
> + default:
> + break;
> + }
> +
> + return err;
> +}
> +
> +static enum power_supply_property e31x_charger_properties[] = {
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_SCOPE,
> +};
> +
> +static const struct power_supply_desc e31x_charger_desc = {
> + .name = "e31x-charger",
> + .type = POWER_SUPPLY_TYPE_MAINS,
> + .properties = e31x_charger_properties,
> + .num_properties = ARRAY_SIZE(e31x_charger_properties),
> + .get_property = e31x_charger_get_property,
> +};
> +
> +static const struct of_device_id e31x_charger_id[] = {
> + { .compatible = "ni,e31x-charger" },
> + { },
> +};
> +
> +static int e31x_charger_probe(struct platform_device *pdev)
> +{
> + struct power_supply_config psy_cfg = {};
> + struct e31x_charger_dev *charger;
> +
> + if (!of_device_is_available(pdev->dev.of_node))
> + return -ENODEV;
> +
> + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + charger->regmap = syscon_regmap_lookup_by_phandle(\
useless \
> + pdev->dev.parent->of_node, "regmap");
> +
> + psy_cfg.of_node = pdev->dev.of_node;
> + psy_cfg.drv_data = charger;
> +
> + charger->supply = devm_power_supply_register(&pdev->dev,
> + &e31x_charger_desc,
> + &psy_cfg);
> +
> + if (IS_ERR(charger->supply))
> + return PTR_ERR(charger->supply);
> + return 0;
> +}
> +
> +static struct platform_driver e31x_charger_driver = {
> + .driver = {
> + .name = "e31x-charger",
> + .of_match_table = e31x_charger_id,
> + },
> + .probe = e31x_charger_probe,
> +};
> +module_platform_driver(e31x_charger_driver);
> +
> +MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
> +MODULE_DESCRIPTION("E31x charger driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
> index c57d5a8c1aad..e0649845fe4d 100644
> --- a/include/linux/mfd/e31x-pmu.h
> +++ b/include/linux/mfd/e31x-pmu.h
> @@ -12,9 +12,13 @@
> #include <linux/bitops.h>
>
> #define E31X_PMU_REG_MISC 0x04
> +#define E31X_PMU_REG_CHARGER 0x0c
>
> #define E31X_PMU_GET_FIELD(name, reg) \
> (((reg) & E31X_PMU_## name ##_MASK) >> \
> E31X_PMU_## name ##_SHIFT)
>
> +#define E31X_PMU_CHARGER_ONLINE_MASK BIT(2)
> +#define E31X_PMU_CHARGER_ONLINE_SHIFT 2
> +
> #endif /* MFD_E31X_PMU_H */
Otherwise this looks good.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC 5/6] power: supply: Ettus Research E31x battery driver
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
` (3 preceding siblings ...)
2019-02-12 1:01 ` [RFC 4/6] " Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-14 9:39 ` Lee Jones
2019-02-12 1:01 ` [RFC 6/6] " Virendra Kakade
5 siblings, 1 reply; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Document bindings for the E31x devices battery.
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 8 +++++++-
.../bindings/power/supply/e31x-battery.txt | 14 ++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/power/supply/e31x-battery.txt
diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
index 3798886578ba..223ab9e0d680 100644
--- a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
+++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
@@ -4,11 +4,14 @@ Required properties:
- compatible : Must be "ni,e31x-pmu"
- regmap : Must be <®mapnode>
-e31x-pmu has a sub-device called e31x-charger.
+e31x-pmu consists of two sub-devices, e31x-charger and e31x-battery.
For the e31x-charger properties please refer to:
Documentation/device-tree/bindings/power/supply/e31x-charger.txt
+For the e31x-battery properties please refer to:
+Documentation/device-tree/bindings/power/supply/e31x-battery.txt
+
Example:
pmu {
@@ -18,5 +21,8 @@ pmu {
charger: charger@0{
compatible = "ni,e31x-charger";
};
+ battery: battery@1{
+ compatible = "ni,e31x-battery";
+ };
};
diff --git a/Documentation/devicetree/bindings/power/supply/e31x-battery.txt b/Documentation/devicetree/bindings/power/supply/e31x-battery.txt
new file mode 100644
index 000000000000..e09cd92ef82e
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/e31x-battery.txt
@@ -0,0 +1,14 @@
+Ettus Research/National Instruments E31x Devices Battery
+
+Some variants of the E31x devices have an in-built Li-ion
+battery. This driver provides capability to monitor
+the battery.
+
+Required properties:
+- compatible: Must be "ni,e31x-battery"
+
+Example:
+
+battery: battery@0{
+ compatible = "ni,e31x-battery";
+};
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 5/6] power: supply: Ettus Research E31x battery driver
2019-02-12 1:01 ` [RFC 5/6] power: supply: Ettus Research E31x battery driver Virendra Kakade
@ 2019-02-14 9:39 ` Lee Jones
0 siblings, 0 replies; 17+ messages in thread
From: Lee Jones @ 2019-02-14 9:39 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, robh+dt, mark.rutland, sre,
moritz.fischer
On Mon, 11 Feb 2019, Virendra Kakade wrote:
> Document bindings for the E31x devices battery.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> Documentation/devicetree/bindings/mfd/e31x-pmu.txt | 8 +++++++-
> .../bindings/power/supply/e31x-battery.txt | 14 ++++++++++++++
> 2 files changed, 21 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/power/supply/e31x-battery.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> index 3798886578ba..223ab9e0d680 100644
> --- a/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/e31x-pmu.txt
> @@ -4,11 +4,14 @@ Required properties:
> - compatible : Must be "ni,e31x-pmu"
> - regmap : Must be <®mapnode>
>
> -e31x-pmu has a sub-device called e31x-charger.
> +e31x-pmu consists of two sub-devices, e31x-charger and e31x-battery.
>
> For the e31x-charger properties please refer to:
> Documentation/device-tree/bindings/power/supply/e31x-charger.txt
>
> +For the e31x-battery properties please refer to:
> +Documentation/device-tree/bindings/power/supply/e31x-battery.txt
Same comment as I provided in the Charger patch.
> Example:
>
> pmu {
> @@ -18,5 +21,8 @@ pmu {
> charger: charger@0{
> compatible = "ni,e31x-charger";
> };
> + battery: battery@1{
> + compatible = "ni,e31x-battery";
> + };
> };
Still seems kinda simple.
> diff --git a/Documentation/devicetree/bindings/power/supply/e31x-battery.txt b/Documentation/devicetree/bindings/power/supply/e31x-battery.txt
> new file mode 100644
> index 000000000000..e09cd92ef82e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/e31x-battery.txt
> @@ -0,0 +1,14 @@
> +Ettus Research/National Instruments E31x Devices Battery
> +
> +Some variants of the E31x devices have an in-built Li-ion
> +battery. This driver provides capability to monitor
> +the battery.
> +
> +Required properties:
> +- compatible: Must be "ni,e31x-battery"
> +
> +Example:
> +
> +battery: battery@0{
> + compatible = "ni,e31x-battery";
> +};
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC 6/6] power: supply: Ettus Research E31x battery driver
2019-02-12 1:01 [RFC 0/6] Add support for Ettus Research E31x devices PMU Virendra Kakade
` (4 preceding siblings ...)
2019-02-12 1:01 ` [RFC 5/6] power: supply: Ettus Research E31x battery driver Virendra Kakade
@ 2019-02-12 1:01 ` Virendra Kakade
2019-02-12 22:26 ` Sebastian Reichel
5 siblings, 1 reply; 17+ messages in thread
From: Virendra Kakade @ 2019-02-12 1:01 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-pm
Cc: lee.jones, robh+dt, mark.rutland, sre, moritz.fischer,
Virendra Kakade
Add support for E31x devices battery which is a sub-device of E31X-PMU.
Enables query of battery properties from device tree.
Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
---
drivers/power/supply/Kconfig | 6 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/e31x-battery.c | 357 ++++++++++++++++++++++++++++
include/linux/mfd/e31x-pmu.h | 15 +-
4 files changed, 377 insertions(+), 2 deletions(-)
create mode 100644 drivers/power/supply/e31x-battery.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 3d043b8fd49c..e05e02dffc49 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -666,4 +666,10 @@ config E31X_CHARGER
help
This adds support for the battery charger in Ettus Research E31x PMU.
+config E31X_BATTERY
+ tristate "Ettus Research E31x battery support"
+ depends on MFD_E31X_PMU
+ help
+ This adds support for the battery in Ettus Research E31x PMU.
+
endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 6f4f8ca5484e..3850d08db942 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -88,3 +88,4 @@ obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
obj-$(CONFIG_E31X_CHARGER) += e31x-charger.o
+obj-$(CONFIG_E31X_BATTERY) += e31x-battery.o
diff --git a/drivers/power/supply/e31x-battery.c b/drivers/power/supply/e31x-battery.c
new file mode 100644
index 000000000000..d44a639e8bc2
--- /dev/null
+++ b/drivers/power/supply/e31x-battery.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 National Instruments Corp
+ * Author: Virendra Kakade <virendra.kakade@ni.com>
+ *
+ * Ettus Research E31x battery driver
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/power_supply.h>
+#include <linux/delay.h>
+#include <linux/mfd/e31x-pmu.h>
+
+
+#define E31X_PMU_BATTERY_VOLTAGE_MASK GENMASK(23, 8)
+#define E31X_PMU_BATTERY_VOLTAGE_SHIFT 8
+#define E31X_PMU_BATTERY_TEMP_ALERT_MASK GENMASK(7, 6)
+#define E31X_PMU_BATTERY_TEMP_ALERT_SHIFT 6
+#define E31X_PMU_BATTERY_ONLINE_MASK BIT(5)
+#define E31X_PMU_BATTERY_ONLINE_SHIFT 5
+#define E31X_PMU_BATTERY_HEALTH_MASK GENMASK(4, 2)
+#define E31X_PMU_BATTERY_HEALTH_SHIFT 2
+#define E31X_PMU_BATTERY_STATUS_MASK GENMASK(1, 0)
+#define E31X_PMU_BATTERY_STATUS_SHIFT 0
+
+#define E31X_PMU_GAUGE_TEMP_MASK GENMASK(31, 16)
+#define E31X_PMU_GAUGE_TEMP_SHIFT 16
+#define E31X_PMU_GAUGE_CHARGE_MASK GENMASK(15, 0)
+#define E31X_PMU_GAUGE_CHARGE_SHIFT 0
+
+#define E31X_PMU_GAUGE_VOLTAGE_MASK GENMASK(15, 0)
+#define E31X_PMU_GAUGE_VOLTAGE_SHIFT 0
+
+#define E31X_PMU_GAUGE_CHARGE_LAST_FULL_MASK GENMASK(15, 0)
+#define E31X_PMU_GAUGE_CHARGE_LAST_FULL_SHIFT 0
+
+#define E31X_BATTERY_CHARGE_DESIGN_FULL (3200000)
+#define E31X_PMU_VSENSE (6000)
+
+struct e31x_battery_dev {
+ struct regmap *regmap;
+ struct power_supply *supply;
+};
+
+static int e31x_battery_set_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ return 0;
+}
+
+static int e31x_battery_get_status(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_CHARGER, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(CHARGER_ONLINE, value);
+
+ /* if charger is offline, we're discharging, period */
+ if (!value) {
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ return 0;
+ }
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
+ if (err)
+ return err;
+
+ value &= E31X_PMU_BATTERY_STATUS_MASK;
+
+ switch (value) {
+ case 0x0:
+ val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ break;
+ case 0x1:
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ break;
+ case 0x2:
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ break;
+ case 0x3:
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ break;
+ default:
+ return -EIO;
+ };
+
+ return 0;
+}
+
+static int e31x_battery_get_health(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(BATTERY_HEALTH, value);
+
+ switch (value) {
+ case 0x00:
+ val->intval = POWER_SUPPLY_HEALTH_GOOD;
+ break;
+ case 0x01:
+ val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+ break;
+ case 0x02:
+ val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ break;
+ case 0x03:
+ val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+ break;
+ case 0x04:
+ val->intval = POWER_SUPPLY_HEALTH_COLD;
+ break;
+ default:
+ val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+ break;
+ }
+
+ return 0;
+}
+
+static int e31x_battery_get_online(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
+ if (err)
+ return err;
+
+ val->intval = !!(value & E31X_PMU_BATTERY_ONLINE_MASK);
+
+ return 0;
+}
+
+static int e31x_battery_get_voltage_now(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
+
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(BATTERY_VOLTAGE, value);
+
+ val->intval = 10000 * (value * E31X_PMU_VSENSE / GENMASK(15, 0));
+
+ return 0;
+}
+
+#define E31X_PMU_BIN_TO_UAH(x) ((x) * 53)
+#define E31X_PMU_UAH_TO_BIN(x) ((x) / 53)
+
+static int e31x_battery_get_charge_now(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_GAUGE, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(GAUGE_CHARGE, value);
+ val->intval = E31X_PMU_BIN_TO_UAH(value);
+
+ return 0;
+}
+
+static int e31x_battery_get_temp(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_GAUGE, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(GAUGE_TEMP, value);
+ val->intval = 10 * (((600 * value) / 0xffff) - 273);
+
+ return 0;
+}
+
+static int e31x_battery_get_charge_full(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_LAST, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(GAUGE_CHARGE_LAST_FULL, value);
+ val->intval = E31X_PMU_BIN_TO_UAH(value);
+
+ return 0;
+}
+
+static const int e31x_pmu_temp_values[] = {
+ 600, 800, 1000, 1200
+};
+
+static int e31x_battery_get_temp_alert_max(struct e31x_battery_dev *bat,
+ union power_supply_propval *val)
+{
+ u32 value;
+ int err;
+ int i;
+
+ err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
+ if (err)
+ return err;
+
+ value = E31X_PMU_GET_FIELD(BATTERY_TEMP_ALERT, value);
+ for (i = 1; i < ARRAY_SIZE(e31x_pmu_temp_values); i++)
+ if (e31x_pmu_temp_values[i] > value)
+ break;
+
+ val->intval = e31x_pmu_temp_values[i - 1];
+
+ return 0;
+}
+
+static int e31x_battery_get_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct e31x_battery_dev *battery = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_STATUS:
+ return e31x_battery_get_status(battery, val);
+ case POWER_SUPPLY_PROP_HEALTH:
+ return e31x_battery_get_health(battery, val);
+ case POWER_SUPPLY_PROP_ONLINE:
+ return e31x_battery_get_online(battery, val);
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
+ case POWER_SUPPLY_PROP_TEMP:
+ return e31x_battery_get_temp(battery, val);
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ return e31x_battery_get_voltage_now(battery, val);
+ case POWER_SUPPLY_PROP_CHARGE_NOW:
+ return e31x_battery_get_charge_now(battery, val);
+ case POWER_SUPPLY_PROP_CHARGE_FULL:
+ return e31x_battery_get_charge_full(battery, val);
+ case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+ val->intval = E31X_BATTERY_CHARGE_DESIGN_FULL;
+ return 0;
+ case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+ return e31x_battery_get_temp_alert_max(battery, val);
+ default:
+ break;
+ };
+
+ return 0;
+}
+
+static enum power_supply_property e31x_battery_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+ POWER_SUPPLY_PROP_TEMP,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CHARGE_NOW,
+ POWER_SUPPLY_PROP_CHARGE_FULL,
+ POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+ POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
+};
+
+static int e31x_battery_prop_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return psp == POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ||
+ psp == POWER_SUPPLY_PROP_SCOPE;
+}
+
+static const struct power_supply_desc e31x_battery_desc = {
+ .name = "e31x-battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = e31x_battery_props,
+ .num_properties = ARRAY_SIZE(e31x_battery_props),
+ .property_is_writeable = e31x_battery_prop_writeable,
+ .get_property = e31x_battery_get_prop,
+ .set_property = e31x_battery_set_prop,
+};
+
+static const struct of_device_id e31x_battery_id[] = {
+ { .compatible = "ni,e31x-battery" },
+ { },
+};
+
+static int e31x_battery_probe(struct platform_device *pdev)
+{
+ struct power_supply_config psy_cfg = {};
+ struct e31x_battery_dev *battery;
+
+ if (!of_device_is_available(pdev->dev.of_node))
+ return -ENODEV;
+
+ battery = devm_kzalloc(&pdev->dev, sizeof(*battery), GFP_KERNEL);
+ if (!battery)
+ return -ENOMEM;
+
+ battery->regmap = syscon_regmap_lookup_by_phandle(\
+ pdev->dev.parent->of_node, "regmap");
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = battery;
+
+ battery->supply = devm_power_supply_register(&pdev->dev,
+ &e31x_battery_desc,
+ &psy_cfg);
+ if (IS_ERR(battery->supply))
+ return PTR_ERR(battery->supply);
+ return 0;
+}
+
+static struct platform_driver e31x_battery_driver = {
+ .driver = {
+ .name = "e31x-battery",
+ .of_match_table = e31x_battery_id,
+ },
+ .probe = e31x_battery_probe,
+};
+module_platform_driver(e31x_battery_driver);
+
+MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
+MODULE_DESCRIPTION("E31x battery driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
index e0649845fe4d..0441f8f896ca 100644
--- a/include/linux/mfd/e31x-pmu.h
+++ b/include/linux/mfd/e31x-pmu.h
@@ -11,13 +11,24 @@
#include <linux/bitops.h>
-#define E31X_PMU_REG_MISC 0x04
-#define E31X_PMU_REG_CHARGER 0x0c
+#define E31X_PMU_REG_MISC 0x04
+#define E31X_PMU_REG_BATTERY 0x08
+#define E31X_PMU_REG_CHARGER 0x0c
+#define E31X_PMU_REG_GAUGE 0x10
+#define E31X_PMU_REG_STATUS 0x14
+#define E31X_PMU_REG_LAST 0x18
+#define E31X_PMU_REG_EEPROM 0x1c
#define E31X_PMU_GET_FIELD(name, reg) \
(((reg) & E31X_PMU_## name ##_MASK) >> \
E31X_PMU_## name ##_SHIFT)
+/* the eeprom register */
+static const u32 E31X_PMU_EEPROM_AUTOBOOT_MASK = BIT(0);
+static const u32 E31X_PMU_EEPROM_AUTOBOOT_SHIFT;
+static const u32 E31X_PMU_EEPROM_DB_POWER_MASK = BIT(1);
+static const u32 E31X_PMU_EEPROM_DB_POWER_SHIFT = 1;
+
#define E31X_PMU_CHARGER_ONLINE_MASK BIT(2)
#define E31X_PMU_CHARGER_ONLINE_SHIFT 2
--
2.17.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [RFC 6/6] power: supply: Ettus Research E31x battery driver
2019-02-12 1:01 ` [RFC 6/6] " Virendra Kakade
@ 2019-02-12 22:26 ` Sebastian Reichel
0 siblings, 0 replies; 17+ messages in thread
From: Sebastian Reichel @ 2019-02-12 22:26 UTC (permalink / raw)
To: Virendra Kakade
Cc: devicetree, linux-kernel, linux-pm, lee.jones, robh+dt,
mark.rutland, moritz.fischer
[-- Attachment #1: Type: text/plain, Size: 13480 bytes --]
Hi,
On Mon, Feb 11, 2019 at 07:01:43PM -0600, Virendra Kakade wrote:
> Add support for E31x devices battery which is a sub-device of E31X-PMU.
> Enables query of battery properties from device tree.
>
> Signed-off-by: Virendra Kakade <virendra.kakade@ni.com>
> ---
> drivers/power/supply/Kconfig | 6 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/e31x-battery.c | 357 ++++++++++++++++++++++++++++
> include/linux/mfd/e31x-pmu.h | 15 +-
> 4 files changed, 377 insertions(+), 2 deletions(-)
> create mode 100644 drivers/power/supply/e31x-battery.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 3d043b8fd49c..e05e02dffc49 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -666,4 +666,10 @@ config E31X_CHARGER
> help
> This adds support for the battery charger in Ettus Research E31x PMU.
>
> +config E31X_BATTERY
> + tristate "Ettus Research E31x battery support"
> + depends on MFD_E31X_PMU
> + help
> + This adds support for the battery in Ettus Research E31x PMU.
> +
> endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 6f4f8ca5484e..3850d08db942 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -88,3 +88,4 @@ obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
> obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
> obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
> obj-$(CONFIG_E31X_CHARGER) += e31x-charger.o
> +obj-$(CONFIG_E31X_BATTERY) += e31x-battery.o
> diff --git a/drivers/power/supply/e31x-battery.c b/drivers/power/supply/e31x-battery.c
> new file mode 100644
> index 000000000000..d44a639e8bc2
> --- /dev/null
> +++ b/drivers/power/supply/e31x-battery.c
> @@ -0,0 +1,357 @@
> +// SPDX-License-Identifier: GPL-2.0
same issue as in charger driver.
> +/*
> + * Copyright (c) 2018 National Instruments Corp
> + * Author: Virendra Kakade <virendra.kakade@ni.com>
> + *
> + * Ettus Research E31x battery driver
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/power_supply.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/e31x-pmu.h>
I also believe a few of those includes are unused.
> +#define E31X_PMU_BATTERY_VOLTAGE_MASK GENMASK(23, 8)
> +#define E31X_PMU_BATTERY_VOLTAGE_SHIFT 8
> +#define E31X_PMU_BATTERY_TEMP_ALERT_MASK GENMASK(7, 6)
> +#define E31X_PMU_BATTERY_TEMP_ALERT_SHIFT 6
> +#define E31X_PMU_BATTERY_ONLINE_MASK BIT(5)
> +#define E31X_PMU_BATTERY_ONLINE_SHIFT 5
> +#define E31X_PMU_BATTERY_HEALTH_MASK GENMASK(4, 2)
> +#define E31X_PMU_BATTERY_HEALTH_SHIFT 2
> +#define E31X_PMU_BATTERY_STATUS_MASK GENMASK(1, 0)
> +#define E31X_PMU_BATTERY_STATUS_SHIFT 0
> +
> +#define E31X_PMU_GAUGE_TEMP_MASK GENMASK(31, 16)
> +#define E31X_PMU_GAUGE_TEMP_SHIFT 16
> +#define E31X_PMU_GAUGE_CHARGE_MASK GENMASK(15, 0)
> +#define E31X_PMU_GAUGE_CHARGE_SHIFT 0
> +
> +#define E31X_PMU_GAUGE_VOLTAGE_MASK GENMASK(15, 0)
> +#define E31X_PMU_GAUGE_VOLTAGE_SHIFT 0
> +
> +#define E31X_PMU_GAUGE_CHARGE_LAST_FULL_MASK GENMASK(15, 0)
> +#define E31X_PMU_GAUGE_CHARGE_LAST_FULL_SHIFT 0
> +
> +#define E31X_BATTERY_CHARGE_DESIGN_FULL (3200000)
> +#define E31X_PMU_VSENSE (6000)
> +
> +struct e31x_battery_dev {
> + struct regmap *regmap;
> + struct power_supply *supply;
> +};
> +
> +static int e31x_battery_set_prop(struct power_supply *psy,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + return 0;
> +}
If you don't have writable properties you do not have to
specify any handler.
> +static int e31x_battery_get_status(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_CHARGER, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(CHARGER_ONLINE, value);
> +
> + /* if charger is offline, we're discharging, period */
> + if (!value) {
> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + return 0;
> + }
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
> + if (err)
> + return err;
> +
> + value &= E31X_PMU_BATTERY_STATUS_MASK;
> +
> + switch (value) {
> + case 0x0:
> + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + break;
> + case 0x1:
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + break;
> + case 0x2:
> + val->intval = POWER_SUPPLY_STATUS_FULL;
> + break;
> + case 0x3:
> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + break;
> + default:
> + return -EIO;
> + };
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_health(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(BATTERY_HEALTH, value);
> +
> + switch (value) {
> + case 0x00:
> + val->intval = POWER_SUPPLY_HEALTH_GOOD;
> + break;
> + case 0x01:
> + val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> + break;
> + case 0x02:
> + val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + break;
> + case 0x03:
> + val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
> + break;
> + case 0x04:
> + val->intval = POWER_SUPPLY_HEALTH_COLD;
> + break;
> + default:
> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_online(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
> + if (err)
> + return err;
> +
> + val->intval = !!(value & E31X_PMU_BATTERY_ONLINE_MASK);
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_voltage_now(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
> +
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(BATTERY_VOLTAGE, value);
> +
> + val->intval = 10000 * (value * E31X_PMU_VSENSE / GENMASK(15, 0));
> +
> + return 0;
> +}
> +
> +#define E31X_PMU_BIN_TO_UAH(x) ((x) * 53)
> +#define E31X_PMU_UAH_TO_BIN(x) ((x) / 53)
> +
> +static int e31x_battery_get_charge_now(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_GAUGE, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(GAUGE_CHARGE, value);
> + val->intval = E31X_PMU_BIN_TO_UAH(value);
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_temp(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_GAUGE, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(GAUGE_TEMP, value);
> + val->intval = 10 * (((600 * value) / 0xffff) - 273);
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_charge_full(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_LAST, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(GAUGE_CHARGE_LAST_FULL, value);
> + val->intval = E31X_PMU_BIN_TO_UAH(value);
> +
> + return 0;
> +}
> +
> +static const int e31x_pmu_temp_values[] = {
> + 600, 800, 1000, 1200
> +};
> +
> +static int e31x_battery_get_temp_alert_max(struct e31x_battery_dev *bat,
> + union power_supply_propval *val)
> +{
> + u32 value;
> + int err;
> + int i;
> +
> + err = regmap_read(bat->regmap, E31X_PMU_REG_BATTERY, &value);
> + if (err)
> + return err;
> +
> + value = E31X_PMU_GET_FIELD(BATTERY_TEMP_ALERT, value);
> + for (i = 1; i < ARRAY_SIZE(e31x_pmu_temp_values); i++)
> + if (e31x_pmu_temp_values[i] > value)
> + break;
> +
> + val->intval = e31x_pmu_temp_values[i - 1];
> +
> + return 0;
> +}
> +
> +static int e31x_battery_get_prop(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct e31x_battery_dev *battery = power_supply_get_drvdata(psy);
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_STATUS:
> + return e31x_battery_get_status(battery, val);
> + case POWER_SUPPLY_PROP_HEALTH:
> + return e31x_battery_get_health(battery, val);
> + case POWER_SUPPLY_PROP_ONLINE:
> + return e31x_battery_get_online(battery, val);
> + case POWER_SUPPLY_PROP_TECHNOLOGY:
> + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> + break;
> + case POWER_SUPPLY_PROP_TEMP:
> + return e31x_battery_get_temp(battery, val);
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + return e31x_battery_get_voltage_now(battery, val);
> + case POWER_SUPPLY_PROP_CHARGE_NOW:
> + return e31x_battery_get_charge_now(battery, val);
> + case POWER_SUPPLY_PROP_CHARGE_FULL:
> + return e31x_battery_get_charge_full(battery, val);
> + case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> + val->intval = E31X_BATTERY_CHARGE_DESIGN_FULL;
> + return 0;
> + case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
> + return e31x_battery_get_temp_alert_max(battery, val);
> + default:
> + break;
> + };
> +
> + return 0;
> +}
> +
> +static enum power_supply_property e31x_battery_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_TECHNOLOGY,
> + POWER_SUPPLY_PROP_TEMP,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CHARGE_NOW,
> + POWER_SUPPLY_PROP_CHARGE_FULL,
> + POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> + POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
> +};
> +
> +static int e31x_battery_prop_writeable(struct power_supply *psy,
> + enum power_supply_property psp)
> +{
> + return psp == POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ||
> + psp == POWER_SUPPLY_PROP_SCOPE;
> +}
You'r e31x_battery_set_prop() is empty and you do not even
expose POWER_SUPPLY_PROP_SCOPE.
> +static const struct power_supply_desc e31x_battery_desc = {
> + .name = "e31x-battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .properties = e31x_battery_props,
> + .num_properties = ARRAY_SIZE(e31x_battery_props),
> + .property_is_writeable = e31x_battery_prop_writeable,
> + .get_property = e31x_battery_get_prop,
> + .set_property = e31x_battery_set_prop,
> +};
> +
> +static const struct of_device_id e31x_battery_id[] = {
> + { .compatible = "ni,e31x-battery" },
> + { },
> +};
> +
> +static int e31x_battery_probe(struct platform_device *pdev)
> +{
> + struct power_supply_config psy_cfg = {};
> + struct e31x_battery_dev *battery;
> +
> + if (!of_device_is_available(pdev->dev.of_node))
> + return -ENODEV;
Why is this needed? I would expect, that the driver is not being
probed for a disabled node.
> + battery = devm_kzalloc(&pdev->dev, sizeof(*battery), GFP_KERNEL);
> + if (!battery)
> + return -ENOMEM;
> +
> + battery->regmap = syscon_regmap_lookup_by_phandle(\
useless \
> + pdev->dev.parent->of_node, "regmap");
> +
> + psy_cfg.of_node = pdev->dev.of_node;
> + psy_cfg.drv_data = battery;
> +
> + battery->supply = devm_power_supply_register(&pdev->dev,
> + &e31x_battery_desc,
> + &psy_cfg);
> + if (IS_ERR(battery->supply))
> + return PTR_ERR(battery->supply);
> + return 0;
> +}
> +
> +static struct platform_driver e31x_battery_driver = {
> + .driver = {
> + .name = "e31x-battery",
> + .of_match_table = e31x_battery_id,
> + },
> + .probe = e31x_battery_probe,
> +};
> +module_platform_driver(e31x_battery_driver);
> +
> +MODULE_AUTHOR("Virendra Kakade <virendra.kakade@ni.com>");
> +MODULE_DESCRIPTION("E31x battery driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/e31x-pmu.h b/include/linux/mfd/e31x-pmu.h
> index e0649845fe4d..0441f8f896ca 100644
> --- a/include/linux/mfd/e31x-pmu.h
> +++ b/include/linux/mfd/e31x-pmu.h
> @@ -11,13 +11,24 @@
>
> #include <linux/bitops.h>
>
> -#define E31X_PMU_REG_MISC 0x04
> -#define E31X_PMU_REG_CHARGER 0x0c
> +#define E31X_PMU_REG_MISC 0x04
> +#define E31X_PMU_REG_BATTERY 0x08
> +#define E31X_PMU_REG_CHARGER 0x0c
> +#define E31X_PMU_REG_GAUGE 0x10
> +#define E31X_PMU_REG_STATUS 0x14
> +#define E31X_PMU_REG_LAST 0x18
> +#define E31X_PMU_REG_EEPROM 0x1c
>
> #define E31X_PMU_GET_FIELD(name, reg) \
> (((reg) & E31X_PMU_## name ##_MASK) >> \
> E31X_PMU_## name ##_SHIFT)
>
> +/* the eeprom register */
> +static const u32 E31X_PMU_EEPROM_AUTOBOOT_MASK = BIT(0);
> +static const u32 E31X_PMU_EEPROM_AUTOBOOT_SHIFT;
> +static const u32 E31X_PMU_EEPROM_DB_POWER_MASK = BIT(1);
> +static const u32 E31X_PMU_EEPROM_DB_POWER_SHIFT = 1;
> +
> #define E31X_PMU_CHARGER_ONLINE_MASK BIT(2)
> #define E31X_PMU_CHARGER_ONLINE_SHIFT 2
Otherwise looks good.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread