mbox series

[v10,0/8] Add support for Maxim Integrated MAX77705 PMIC

Message ID 20241204-starqltechn_integration_upstream-v10-0-7de85e48e562@gmail.com
Headers show
Series Add support for Maxim Integrated MAX77705 PMIC | expand

Message

Dzmitry Sankouski Dec. 4, 2024, 8:09 p.m. UTC
The Maxim MAX77705 is a Companion Power Management and Type-C
interface IC which includes charger, fuelgauge, LED, haptic motor driver and
Type-C management IC. It's used in Samsung S series smart phones
starting from S9 model.

Add features:
  - charger
  - fuelgauge
  - haptic
  - led

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
Changes in v10:
- drop NACKed 'dt-bindings: power: supply: max17042: remove reg from
  required' patch
- review fixes
- use dev_err_probe for errors in probe functions
- Link to v9: https://lore.kernel.org/r/20241202-starqltechn_integration_upstream-v9-0-a1adc3bae2b8@gmail.com

Changes in v9:
- fuel gauge: use max17042 driver instead of separate max77705
- fix kernel bot error
- charger: enable interrupt after power supply registration
- add dependency on max17042 patch series
- Link to v8: https://lore.kernel.org/r/20241031-starqltechn_integration_upstream-v8-0-2fa666c2330e@gmail.com

Changes in v8:
- Fix comment style
- join line where possible to fit in 100 chars
- Link to v7: https://lore.kernel.org/r/20241023-starqltechn_integration_upstream-v7-0-9bfaa3f4a1a0@gmail.com

Changes in v7:
- Fix review comments
- Link to v6: https://lore.kernel.org/r/20241007-starqltechn_integration_upstream-v6-0-0d38b5090c57@gmail.com

Changes in v6:
- fix binding review comments
- update trailers
- Link to v5: https://lore.kernel.org/r/20240617-starqltechn_integration_upstream-v5-0-e0033f141d17@gmail.com

Changes in v5:
- Split patchset per subsystem
- Link to v4: https://lore.kernel.org/r/20240913-starqltechn_integration_upstream-v4-0-2d2efd5c5877@gmail.com

Changes in v4:
- Rewrite max77705, max77705_charger, max77705_fuel_gauge from scratch
- Reorder patches:
  - squash max77705 subdevice bindings in core file because
    no resources there
  - split device tree changes
- Use _ as space for filenames in power/supply like the majority
- Link to v3: https://lore.kernel.org/r/20240618-starqltechn_integration_upstream-v3-0-e3f6662017ac@gmail.com

---
Dzmitry Sankouski (8):
      power: supply: add undervoltage health status property
      dt-bindings: power: supply: max17042: add max77705 support
      dt-bindings: mfd: add maxim,max77705
      power: supply: max17042: add max77705 fuel gauge support
      mfd: Add new driver for MAX77705 PMIC
      input: max77693: add max77705 haptic support
      power: supply: max77705: Add charger driver for Maxim 77705
      leds: max77705: Add LEDs support

 Documentation/ABI/testing/sysfs-class-power                        |   2 +-
 Documentation/devicetree/bindings/mfd/maxim,max77705.yaml          | 192 +++++++++++++++++++++++++++++++++++++
 Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml |   1 +
 MAINTAINERS                                                        |   4 +
 drivers/input/misc/Kconfig                                         |   4 +-
 drivers/input/misc/Makefile                                        |   1 +
 drivers/input/misc/max77693-haptic.c                               |  15 ++-
 drivers/leds/Kconfig                                               |   6 ++
 drivers/leds/Makefile                                              |   1 +
 drivers/leds/leds-max77705.c                                       | 267 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig                                                |  12 +++
 drivers/mfd/Makefile                                               |   2 +
 drivers/mfd/max77705.c                                             | 233 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/power/supply/Kconfig                                       |   6 ++
 drivers/power/supply/Makefile                                      |   1 +
 drivers/power/supply/max17042_battery.c                            |   3 +
 drivers/power/supply/max77705_charger.c                            | 590 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/power/supply/power_supply_sysfs.c                          |   1 +
 include/linux/mfd/max77693-common.h                                |   4 +-
 include/linux/mfd/max77705-private.h                               | 194 +++++++++++++++++++++++++++++++++++++
 include/linux/power/max77705_charger.h                             | 194 +++++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h                                       |   1 +
 22 files changed, 1729 insertions(+), 5 deletions(-)
---
base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
change-id: 20240617-starqltechn_integration_upstream-bc86850b2fe3
prerequisite-change-id: 20241108-b4-max17042-9306fc75afae:v4
prerequisite-patch-id: a78c51c4a1b48756c00cbc3d56b9e019577e4a6b
prerequisite-patch-id: 735d52c3137c5e474f3601adf010f9fe2f3f7036

Best regards,

Comments

Dzmitry Sankouski Dec. 4, 2024, 9:42 p.m. UTC | #1
чт, 5 дек. 2024 г. в 00:30, Rob Herring (Arm) <robh@kernel.org>:
>
>
> On Wed, 04 Dec 2024 23:09:53 +0300, Dzmitry Sankouski wrote:
> > Add maxim,max77705 binding.
> >
(...)
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/maxim,max77705.example.dtb: pmic@66: fuel-gauge@36:compatible:0: 'maxim,max77705-battery' is not one of ['maxim,max17042', 'maxim,max17047', 'maxim,max17050', 'maxim,max17055', 'maxim,max77849-battery']
>         from schema $id: http://devicetree.org/schemas/mfd/maxim,max77705.yaml#
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/maxim,max77705.example.dtb: pmic@66: fuel-gauge@36: Unevaluated properties are not allowed ('compatible' was unexpected)
>         from schema $id: http://devicetree.org/schemas/mfd/maxim,max77705.yaml#
> Documentation/devicetree/bindings/mfd/maxim,max77705.example.dtb: /example-0/i2c/pmic@66/fuel-gauge@36: failed to match any schema with compatible: ['maxim,max77705-battery']
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241204-starqltechn_integration_upstream-v10-3-7de85e48e562@gmail.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.

Hmm, I added 'maxim,max77705-battery' in previous patch of the series:
https://lore.kernel.org/all/20241204-starqltechn_integration_upstream-v10-2-7de85e48e562@gmail.com/

Does your bot run checks patch-by-patch, not the whole series?
Krzysztof Kozlowski Dec. 5, 2024, 7:48 a.m. UTC | #2
On Wed, Dec 04, 2024 at 11:09:53PM +0300, Dzmitry Sankouski wrote:
> Add maxim,max77705 binding.
> 
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
> Changes in v10:
> - leds: replace label with color and function properties
> - leds: add support for leds-class-multicolor
> - move fuelgauge node to patternProperties "^fuel-gauge@[0-9a-f]+$"
>   to comply with max17042 binding

You silently added significant change - new address to fuel-gauge.

> 
> Changes in v9:
> - replace max77705 fuel gauge with max17042
> - remove monitored battery because not supported by max17042
> 
> Changes in v8:
> - fix leds compatible
> 
> Changes in v6:
> - unevaluatedProperties must be false
> - drop excessive sentence from description,
>   just describe the device
> - change leds compatible to maxim,max77705-rgb

Your changelog should mention you added review here.

> 
> Changes in v5:
> - formatting changes
> - add unevaluatedProperties: false for nodes referencing
>   common schemas
> - remove additionalProperties on nodes with
>   unevaluatedProperties: false
> - add min and max to led index
> Changes in v4:
> - change dts example intendation from tabs
>  to spaces
> - remove interrupt-names property
> - remove obvious reg description
> - split long(>80) lines
> ---
>  Documentation/devicetree/bindings/mfd/maxim,max77705.yaml | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  MAINTAINERS                                               |   1 +
>  2 files changed, 193 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77705.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77705.yaml
> new file mode 100644
> index 000000000000..1bc026a01337
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77705.yaml
> @@ -0,0 +1,192 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/maxim,max77705.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX77705 Companion Power Management IC and USB Type-C interface IC
> +
> +maintainers:
> +  - Dzmitry Sankouski <dsankouski@gmail.com>
> +
> +description: |
> +  The Maxim MAX77705 is a Companion Power Management and Type-C
> +  interface IC which includes charger, fuelgauge, LED, haptic motor driver and
> +  Type-C management IC.
> +
> +properties:
> +  compatible:
> +    const: maxim,max77705
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

Nope, none of children are supposed to have addresses. All nodes are
unit-less.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1

...

> +    properties:
> +      compatible:
> +        const: maxim,max77705-rgb
> +
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      multi-led:
> +        type: object
> +        $ref: /schemas/leds/leds-class-multicolor.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          "#address-cells":
> +            const: 1
> +
> +          "#size-cells":
> +            const: 0
> +
> +        patternProperties:
> +          "^led@[0-3]$":
> +            type: object
> +            $ref: /schemas/leds/common.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              reg:
> +                maxItems: 1
> +
> +            required:
> +              - reg
> +
> +    required:
> +      - compatible
> +
> +patternProperties:
> +  "^fuel-gauge@[0-9a-f]+$":


How unit address appeared here? It was absolutely never reviewed. This
is significant change, so drop the review tag.

It's also a no, drop unit address.

Best regards,
Krzysztof
Krzysztof Kozlowski Dec. 5, 2024, 7:54 a.m. UTC | #3
On 05/12/2024 08:48, Krzysztof Kozlowski wrote:
>> +        patternProperties:
>> +          "^led@[0-3]$":
>> +            type: object
>> +            $ref: /schemas/leds/common.yaml#
>> +            unevaluatedProperties: false
>> +
>> +            properties:
>> +              reg:
>> +                maxItems: 1
>> +
>> +            required:
>> +              - reg
>> +
>> +    required:
>> +      - compatible
>> +
>> +patternProperties:
>> +  "^fuel-gauge@[0-9a-f]+$":
> 
> 
> How unit address appeared here? It was absolutely never reviewed. This
> is significant change, so drop the review tag.
> 
> It's also a no, drop unit address.

To explain more:
In the few Maxim PMIC/MUIC charger designs known to me, the fuel gauge
is a separate device. In all implementations it is even on separate bus,
although does not have to. You claim that this is not a separate device
and you even claim that this *MUST* be on the same bus.

Both claims sound just wrong, IMO. This needs explanation. It seems you
develop bindings just to match your drivers and that is not the correct
process. Bindings must match the hardware, not your driver architecture.


Best regards,
Krzysztof