Re: [PATCH v3 1/4] dt-bindings: Add Rockchip rk817 battery charger support

From: Chris Morgan <macromorgan@hotmail.com>
To: Rob Herring <robh@kernel.org>
Cc: Chris Morgan <macroalpha82@gmail.com>,
	linux-pm@vger.kernel.org, linux-rockchip@lists.infradead.org,
	devicetree@vger.kernel.org, maccraft123mc@gmail.com,
	sre@kernel.org, heiko@sntech.de, lee.jones@linaro.org,
	pgwipeout@gmail.com
Subject: Re: [PATCH v3 1/4] dt-bindings: Add Rockchip rk817 battery charger support
Date: Wed, 18 Aug 2021 10:03:09 -0500	[thread overview]
Message-ID: <SN6PR06MB534295DCDF84CEA9D33535CEA5FF9@SN6PR06MB5342.namprd06.prod.outlook.com> (raw)
In-Reply-To: <YRwJ/riSIGfNu64B@robh.at.kernel.org>

On Tue, Aug 17, 2021 at 02:11:58PM -0500, Rob Herring wrote:
> On Mon, Aug 09, 2021 at 09:54:33PM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Create dt-binding documentation to document rk817 battery and charger
> > usage. New device-tree properties have been added.
> > 
> > - rockchip,resistor-sense-micro-ohms: The value in microohms of the
> > 				      sample resistor.
> > - rockchip,sleep-enter-current-microamp: The value in microamps of the
> > 					 sleep enter current.
> > - rockchip,sleep-filter-current: The value in microamps of the sleep
> > 				 filter current.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> > ---
> >  .../devicetree/bindings/mfd/rk808.txt         | 36 ++++++++++++++++++-
> >  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> Kind of a big change without converting this binding to DT schema first. 

I can try to convert the bindings to yaml, but I might need some help.
It’s a pretty big file and there’s lots of rules for each of the
different compatibles. Do you know of a good example where I can find
one with compatible specific rules? All of the mfd ones I looked at
don’t seem to have much (and my attempt is triggering yamllint about
duplicate if/then keys).

Thank you.

> 
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/rk808.txt b/Documentation/devicetree/bindings/mfd/rk808.txt
> > index 23a17a6663ec..89b9fa7e10b5 100644
> > --- a/Documentation/devicetree/bindings/mfd/rk808.txt
> > +++ b/Documentation/devicetree/bindings/mfd/rk808.txt
> > @@ -70,13 +70,40 @@ Optional RK817 properties:
> >  
> >  - vcc8-supply:  The input supply for BOOST
> >  - vcc9-supply:  The input supply for OTG_SWITCH
> > +
> >  - codec:	The child node for the codec to hold additional properties.
> >  		If no additional properties are required for the codec, this
> >  		node can be omitted.
> > -
> 
> Unrelated whitespace changes.

I thought the whitespace helped seperate the supplies from the codec,
and did the same thing with whitespace to seperate the battery from
the codec. I can change it back if need be.

> 
> >  - rockchip,mic-in-differential: Telling if the microphone uses differential
> >  				mode. Should be under the codec child node.
> >  
> > +- battery:	The child node for the charger to hold additional properties.
> > +		If a battery is not in use, this node can be omitted. If a
> > +		battery node is used, the following values are required:
> > +		rockchip,resistor-sense, rockchip,sleep-enter-current,
> > +		rockchip,sleep-filter-current, and a phandle to a
> 
> Property names are out of date...
> 

Good catch, sorry about that.

> > +		monitored-battery node that contains a valid value for
> > +		charge-full-design-microamp-hours,
> > +		charge-term-current-microamp,
> > +		constant-charge-current-max-microamp,
> > +		constant-charge-voltage-max-microvolt,
> > +		voltage-max-design-microvolt, voltage-min-design-microvolt,
> > +		and a valid ocv-capacity table.
> 
> Please list these 1 per line and make it clearer this is the list of 
> properties for the battery node.

Will do, thank you.

> 
> > +- rockchip,resistor-sense-micro-ohms: Value in microohms of the battery sense
> > +				      resistor. The PMIC only supports values
> > +				      of either 10000 or 20000. This value is
> > +				      used by the driver to set the correct
> > +				      divisor value to translate ADC readings
> > +				      into the proper units of measure.
> > +- rockchip,sleep-enter-current-microamp: Value in microamps of the sleep enter
> > +					 current for the charger. Value is used
> > +					 by the driver to calibrate the relax
> > +					 threshold.
> > +- rockchip,sleep-filter-current-microamp: Value in microamps of the sleep
> > +					  filter current for the charger.
> > +					  Value is used by the driver to derive
> > +					  the sleep sample current.
> > +
> >  Optional RK818 properties:
> >  - vcc1-supply:  The input supply for DCDC_REG1
> >  - vcc2-supply:  The input supply for DCDC_REG2
> > @@ -459,6 +486,13 @@ Example:
> >  			};
> >  		};
> >  
> > +		rk817_battery: battery {
> > +			monitored-battery = <&battery_cell>;
> > +			rockchip,resistor-sense-micro-ohms = <10000>;
> > +			rockchip,sleep-enter-current-microamp = <300000>;
> > +			rockchip,sleep-filter-current-microamp = <100000>;
> > +		};
> > +
> >  		rk817_codec: codec {
> >  			rockchip,mic-in-differential;
> >  		};
> > -- 
> > 2.25.1
> > 
> > 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip