mbox series

[0/3] power: supply: twl6030/32 charger

Message ID 20240918084132.928295-1-andreas@kemnade.info
Headers show
Series power: supply: twl6030/32 charger | expand

Message

Andreas Kemnade Sept. 18, 2024, 8:41 a.m. UTC 
Add basic support for the charger in the TWL6030/32. Supported is the USB
path. AC path is not handled yet, also there is no entry yet
in /sys/class/power_supply with type battery yet.

Without this series, devices will happily drain battery when running
on mainline.

Andreas Kemnade (3):
  dt-bindings: power: supply: Add TI TWL603X charger
  dt-bindings: mfd: twl: add charger node also for TWL603x
  power: supply: initial support for TWL6030/32

 .../devicetree/bindings/mfd/ti,twl.yaml       |  18 +
 .../power/supply/ti,twl6030-charger.yaml      |  62 ++
 drivers/power/supply/Kconfig                  |  10 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/twl6030_charger.c        | 566 ++++++++++++++++++
 5 files changed, 657 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml
 create mode 100644 drivers/power/supply/twl6030_charger.c

Comments

Andreas Kemnade Sept. 18, 2024, 11:35 a.m. UTC | #1 
Am Wed, 18 Sep 2024 12:47:22 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:

> On 18/09/2024 10:41, Andreas Kemnade wrote:
> > Also the TWL603X devices have a charger, so allow to specify it
> > here.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  .../devicetree/bindings/mfd/ti,twl.yaml        | 18
> > ++++++++++++++++++ 1 file changed, 18 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml index
> > e94b0fd7af0f8..4064a228cb0fc 100644 ---
> > a/Documentation/devicetree/bindings/mfd/ti,twl.yaml +++
> > b/Documentation/devicetree/bindings/mfd/ti,twl.yaml @@ -105,6
> > +105,11 @@ allOf: regulator-initial-mode: false
> >  
> >        properties:
> > +        bci:  
> 
> charger
> 
> > +          type: object  
> 
> additionalProperties: true
> 
> Each node must end with additionalProperties or unevaluated. I think
> you never tested it, because dtschema reports this.
> 
I did run it, no complaints:

andi@aktux:~/kernel$ touch Documentation/devicetree/bindings/mfd/ti,twl.yaml 
andi@aktux:~/kernel$ touch Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.yaml 
andi@aktux:~/kernel$ make ARCH=arm dt_binding_check DT_SCHEMA_FILES=twl
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/home/andi/kernel/Documentation/devicetree/bindings/net/snps,dwmac.yaml: mac-mode: missing type definition
  CHKDT   Documentation/devicetree/bindings
  LINT    Documentation/devicetree/bindings
  DTC [C] Documentation/devicetree/bindings/power/supply/twl4030-charger.example.dtb
  DTEX    Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.example.dts
  DTC [C] Documentation/devicetree/bindings/power/supply/ti,twl6030-charger.example.dtb
  DTC [C] Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.example.dtb
  DTC [C] Documentation/devicetree/bindings/iio/adc/ti,twl4030-madc.example.dtb
  DTEX    Documentation/devicetree/bindings/mfd/ti,twl.example.dts
  DTC [C] Documentation/devicetree/bindings/mfd/ti,twl.example.dtb
andi@aktux:~/kernel$

Regards,
Andreas
Krzysztof Kozlowski Sept. 18, 2024, 12:53 p.m. UTC | #2 
On 18/09/2024 14:43, Andreas Kemnade wrote:
> Am Wed, 18 Sep 2024 12:43:01 +0200
> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> 
> [...]
>> Drop {}, see checkpatch.
>>
>>> +		return dev_err_probe(&pdev->dev, ret,
>>> +				     "could not request irq %d\n",
>>> +				     charger->irq_chg);
>>> +	}
>>> +
> 
> Apparently checkpatch only moans about {} around single *lines*
> not single *statements*, even with --strict.
> 
> Coding-style says single statements,  so maybe checkpatch should be
> fixed?
> 
> Same for other appearance of this pattern.

Hm, could be. I think this still should be without {}, regardless of
checkpatch.

> 
>>> +	/* turing to charging to configure things */
>>> +	twl6030_charger_write(CONTROLLER_CTRL1, 0);
>>> +	twl6030_charger_interrupt(0, charger);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id twl_charger_of_match[]
>>> __maybe_unused = {
>>> +	{.compatible = "ti,twl6030-charger", },
>>> +	{.compatible = "ti,twl6032-charger", },  
>>
>> So they are compatible? Why two entries in such case?
>>
> There is one device_is_compatible() in the file.

Ah, you should rather use match data. Compatibles inside the code do not
scale.



> 
> Regrads,
> Andreas

Best regards,
Krzysztof