| Message ID | 1524844982-4714-5-git-send-email-pawel.mikolaj.chmiel@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | mfd: max8998: Device Tree support fixes | expand |
On Fri, 27 Apr 2018, Paweł Chmiel wrote: > This patch adds devicetree bindings documentation for > battery charging controller as the subnode of MAX8998 PMIC. > It's based on current behavior of driver. > > Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") Why is this here? This patch doesn't look like a fix to me. > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> > --- > Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt > index 23a3650ff2a2..f95610afb57f 100644 > --- a/Documentation/devicetree/bindings/mfd/max8998.txt > +++ b/Documentation/devicetree/bindings/mfd/max8998.txt > @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: > - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts > for buck2 regulator that can be selected using dvs gpio. > > +Charger: Configuration for battery charging controller should be added > +inside a child node named 'charger'. > + Required properties: > + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: 0, 10 - 45 > + > + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: -1, 0, 100, 150, 200 > + > + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: -1, 0, 5, 6, 7 > + > Regulators: All the regulators of MAX8998 to be instantiated shall be > listed in a child node named 'regulators'. Each regulator is represented > by a child node of the 'regulators' node. > @@ -99,6 +114,13 @@ Example: > max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */ > max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>; > > + /* Charger configuration */ > + charger { > + max8998,charge-eoc = <0>; > + max8998,charge-restart = <(-1)>; > + max8998,charge-timeout = <7>; > + }; > + > /* Regulators to instantiate */ > regulators { > ldo2_reg: LDO2 {
On Monday, April 30, 2018 1:30:36 PM CEST Lee Jones wrote: > On Fri, 27 Apr 2018, Paweł Chmiel wrote: > > > This patch adds devicetree bindings documentation for > > battery charging controller as the subnode of MAX8998 PMIC. > > It's based on current behavior of driver. > > > > Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") > > Why is this here? This patch doesn't look like a fix to me. Hi I though that if previous patch, which is adding missing device tree parsing into charger driver, has fixes tag, this patch (which is documenting it), should also contain that tag. If it shouldn't be here, i can fix this in v2. Thanks for feedback (this is my second patchset for Linux Kernel) > > > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> > > --- > > Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt > > index 23a3650ff2a2..f95610afb57f 100644 > > --- a/Documentation/devicetree/bindings/mfd/max8998.txt > > +++ b/Documentation/devicetree/bindings/mfd/max8998.txt > > @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: > > - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts > > for buck2 regulator that can be selected using dvs gpio. > > > > +Charger: Configuration for battery charging controller should be added > > +inside a child node named 'charger'. > > + Required properties: > > + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: 0, 10 - 45 > > + > > + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: -1, 0, 100, 150, 200 > > + > > + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: -1, 0, 5, 6, 7 > > + > > Regulators: All the regulators of MAX8998 to be instantiated shall be > > listed in a child node named 'regulators'. Each regulator is represented > > by a child node of the 'regulators' node. > > @@ -99,6 +114,13 @@ Example: > > max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */ > > max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>; > > > > + /* Charger configuration */ > > + charger { > > + max8998,charge-eoc = <0>; > > + max8998,charge-restart = <(-1)>; > > + max8998,charge-timeout = <7>; > > + }; > > + > > /* Regulators to instantiate */ > > regulators { > > ldo2_reg: LDO2 { > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Apr 27, 2018 at 06:03:02PM +0200, Paweł Chmiel wrote: > This patch adds devicetree bindings documentation for > battery charging controller as the subnode of MAX8998 PMIC. > It's based on current behavior of driver. > > Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> > --- > Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt > index 23a3650ff2a2..f95610afb57f 100644 > --- a/Documentation/devicetree/bindings/mfd/max8998.txt > +++ b/Documentation/devicetree/bindings/mfd/max8998.txt > @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: > - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts > for buck2 regulator that can be selected using dvs gpio. > > +Charger: Configuration for battery charging controller should be added > +inside a child node named 'charger'. > + Required properties: > + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: 0, 10 - 45 > + > + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: -1, 0, 100, 150, 200 > + > + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, > + remain value set from bootloader or default value will be used. > + Valid values: -1, 0, 5, 6, 7 What are those values? seconds? > + > Regulators: All the regulators of MAX8998 to be instantiated shall be > listed in a child node named 'regulators'. Each regulator is represented > by a child node of the 'regulators' node. > @@ -99,6 +114,13 @@ Example: > max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */ > max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>; > > + /* Charger configuration */ > + charger { > + max8998,charge-eoc = <0>; > + max8998,charge-restart = <(-1)>; > + max8998,charge-timeout = <7>; > + }; > + > /* Regulators to instantiate */ > regulators { > ldo2_reg: LDO2 { > -- > 2.7.4 >
On Tuesday, May 1, 2018 2:45:40 PM CEST Sebastian Reichel wrote: > Hi, > > On Fri, Apr 27, 2018 at 06:03:02PM +0200, Paweł Chmiel wrote: > > This patch adds devicetree bindings documentation for > > battery charging controller as the subnode of MAX8998 PMIC. > > It's based on current behavior of driver. > > > > Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") > > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> > > --- > > Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt > > index 23a3650ff2a2..f95610afb57f 100644 > > --- a/Documentation/devicetree/bindings/mfd/max8998.txt > > +++ b/Documentation/devicetree/bindings/mfd/max8998.txt > > @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: > > - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts > > for buck2 regulator that can be selected using dvs gpio. > > > > +Charger: Configuration for battery charging controller should be added > > +inside a child node named 'charger'. > > + Required properties: > > + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: 0, 10 - 45 > > + > > + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: -1, 0, 100, 150, 200 > > + > > + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, > > + remain value set from bootloader or default value will be used. > > + Valid values: -1, 0, 5, 6, 7 > > What are those values? seconds? > Honestly i don't know. I've just documented values accepted currently by charger driver, so we can use it from devicetree. I couldn't find any max8998 datasheet with this information (units, possible values etc for those properties). If this is not acceptable, i can drop this patch. > > + > > Regulators: All the regulators of MAX8998 to be instantiated shall be > > listed in a child node named 'regulators'. Each regulator is represented > > by a child node of the 'regulators' node. > > @@ -99,6 +114,13 @@ Example: > > max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */ > > max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>; > > > > + /* Charger configuration */ > > + charger { > > + max8998,charge-eoc = <0>; > > + max8998,charge-restart = <(-1)>; > > + max8998,charge-timeout = <7>; > > + }; > > + > > /* Regulators to instantiate */ > > regulators { > > ldo2_reg: LDO2 { > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 5/1/18 16:43, Paweł Chmiel wrote: > On Tuesday, May 1, 2018 2:45:40 PM CEST Sebastian Reichel wrote: >> On Fri, Apr 27, 2018 at 06:03:02PM +0200, Paweł Chmiel wrote: >>> This patch adds devicetree bindings documentation for >>> battery charging controller as the subnode of MAX8998 PMIC. >>> It's based on current behavior of driver. >>> >>> Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") >>> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> >>> --- >>> Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> --- a/Documentation/devicetree/bindings/mfd/max8998.txt >>> +++ b/Documentation/devicetree/bindings/mfd/max8998.txt >>> @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: >>> - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts >>> for buck2 regulator that can be selected using dvs gpio. >>> >>> +Charger: Configuration for battery charging controller should be added >>> +inside a child node named 'charger'. >>> + Required properties: >>> + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, >>> + remain value set from bootloader or default value will be used. >>> + Valid values: 0, 10 - 45 >>> + >>> + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, >>> + remain value set from bootloader or default value will be used. >>> + Valid values: -1, 0, 100, 150, 200 Perhaps change the property name to max8998,charge-restart-threshold, in include/linux/mfd/max8998.h we have: * @restart: Restart Level in mV: 100, 150, 200, and -1 for disable. * If it equals 0, leave it unchanged. Then we could make it an optional property: - max8998,charge-restart-threshold: Charge restart threshold in millivolts. Valid values are: 0, 100, 150, 200. If the value equals 0 the charger restart will be disabled. If the property is missing the charger restart threshold configuration would be left unchanged. >>> + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, >>> + remain value set from bootloader or default value will be used. >>> + Valid values: -1, 0, 5, 6, 7 >> >> What are those values? seconds? >> > Honestly i don't know. I've just documented values accepted currently > by charger driver, so we can use it from devicetree. > I couldn't find any max8998 datasheet with this information (units, possible > values etc for those properties). The charge timeout is in hours, as described in include/linux/mfd/max8998.h: "* @timeout: Full Timeout in hours: 5, 6, 7, and -1 for disable. * If it equals 0, leave it unchanged. * Otherwise, leave it unchanged." We could change description of the property to something along the lines of: Optional properties: - max8998,charge-timeout: Charge timeout in hours. Valid values are: 0, 5, 6, 7. If the value is 0 the charge timer will be disabled. Then if the property is missing the driver will leave charge timer configuration unchanged.
Hi On Thursday, 10 January 2019 18:47:11 CET Sylwester Nawrocki wrote: > Hi, > > On 5/1/18 16:43, Paweł Chmiel wrote: > > On Tuesday, May 1, 2018 2:45:40 PM CEST Sebastian Reichel wrote: > >> On Fri, Apr 27, 2018 at 06:03:02PM +0200, Paweł Chmiel wrote: > >>> This patch adds devicetree bindings documentation for > >>> battery charging controller as the subnode of MAX8998 PMIC. > >>> It's based on current behavior of driver. > >>> > >>> Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") > >>> Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> > >>> --- > >>> Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ > >>> 1 file changed, 22 insertions(+) > > >>> --- a/Documentation/devicetree/bindings/mfd/max8998.txt > >>> +++ b/Documentation/devicetree/bindings/mfd/max8998.txt > >>> @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: > >>> - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts > >>> for buck2 regulator that can be selected using dvs gpio. > >>> > >>> +Charger: Configuration for battery charging controller should be added > >>> +inside a child node named 'charger'. > >>> + Required properties: > >>> + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, > >>> + remain value set from bootloader or default value will be used. > >>> + Valid values: 0, 10 - 45 > >>> + > >>> + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, > >>> + remain value set from bootloader or default value will be used. > >>> + Valid values: -1, 0, 100, 150, 200 > > Perhaps change the property name to max8998,charge-restart-threshold, > in include/linux/mfd/max8998.h we have: > > * @restart: Restart Level in mV: 100, 150, 200, and -1 for disable. > * If it equals 0, leave it unchanged. > > Then we could make it an optional property: > > - max8998,charge-restart-threshold: Charge restart threshold in millivolts. > Valid values are: 0, 100, 150, 200. If the value equals 0 the charger > restart will be disabled. > > If the property is missing the charger restart threshold configuration would > be left unchanged. > Strange, i've looked at max8998 and didn't saw information that those values are in mV (maybe it was from vendor sources). > >>> + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, > >>> + remain value set from bootloader or default value will be used. > >>> + Valid values: -1, 0, 5, 6, 7 > >> > >> What are those values? seconds? > >> > > Honestly i don't know. I've just documented values accepted currently > > by charger driver, so we can use it from devicetree. > > I couldn't find any max8998 datasheet with this information (units, possible > > values etc for those properties). > > The charge timeout is in hours, as described in include/linux/mfd/max8998.h: > > > "* @timeout: Full Timeout in hours: 5, 6, 7, and -1 for disable. > * If it equals 0, leave it unchanged. > * Otherwise, leave it unchanged." > > We could change description of the property to something along the lines of: > > Optional properties: > > - max8998,charge-timeout: Charge timeout in hours. Valid values are: > 0, 5, 6, 7. If the value is 0 the charge timer will be disabled. > > Then if the property is missing the driver will leave charge timer configuration > unchanged. > > Thanks Sylwester Nawrocki for all those hints (mostly regarding units of those magic numbers/values). I'll prepare new version of those patches with all hints included.
diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt index 23a3650ff2a2..f95610afb57f 100644 --- a/Documentation/devicetree/bindings/mfd/max8998.txt +++ b/Documentation/devicetree/bindings/mfd/max8998.txt @@ -50,6 +50,21 @@ Additional properties required if max8998,pmic-buck2-dvs-gpio is defined: - max8998,pmic-buck2-dvs-voltage: An array of 2 voltage values in microvolts for buck2 regulator that can be selected using dvs gpio. +Charger: Configuration for battery charging controller should be added +inside a child node named 'charger'. + Required properties: + - max8998,charge-eoc: Setup "End of Charge". If value equals 0, + remain value set from bootloader or default value will be used. + Valid values: 0, 10 - 45 + + - max8998,charge-restart: Setup "Charge Restart Level". If value equals 0, + remain value set from bootloader or default value will be used. + Valid values: -1, 0, 100, 150, 200 + + - max8998,charge-timeout: Setup "Charge Full Timeout". If value equals 0, + remain value set from bootloader or default value will be used. + Valid values: -1, 0, 5, 6, 7 + Regulators: All the regulators of MAX8998 to be instantiated shall be listed in a child node named 'regulators'. Each regulator is represented by a child node of the 'regulators' node. @@ -99,6 +114,13 @@ Example: max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */ max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>; + /* Charger configuration */ + charger { + max8998,charge-eoc = <0>; + max8998,charge-restart = <(-1)>; + max8998,charge-timeout = <7>; + }; + /* Regulators to instantiate */ regulators { ldo2_reg: LDO2 {
This patch adds devicetree bindings documentation for battery charging controller as the subnode of MAX8998 PMIC. It's based on current behavior of driver. Fixes: ee999fb3f17f ("mfd: max8998: Add support for Device Tree") Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com> --- Documentation/devicetree/bindings/mfd/max8998.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)