diff mbox series

[v12,02/11] dt-bindings: power: supply: max17042: split on 2 files

Message ID 20241217-starqltechn_integration_upstream-v12-2-ed840944f948@gmail.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Add support for Maxim Integrated MAX77705 PMIC | expand

Commit Message

Dzmitry Sankouski Dec. 17, 2024, 5:30 p.m. UTC
Move max17042 common binding part to separate file, to
reuse it for MFDs with platform driver version.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>

Changes on v12:
- add addtionalProperties: true on common file
- rename *-base file to *-common
- remove compatibles from shared shema
- move required properties to final schema
- remove max77705 compatible from binding - it will be used in
  mfd77705 binding
---
 Documentation/devicetree/bindings/power/supply/maxim,max17042-common.yaml | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml        | 43 +++----------------------------------------
 MAINTAINERS                                                               |  2 +-
 3 files changed, 58 insertions(+), 41 deletions(-)

Comments

Krzysztof Kozlowski Dec. 18, 2024, 8:28 a.m. UTC | #1
On Tue, Dec 17, 2024 at 08:30:00PM +0300, Dzmitry Sankouski wrote:
> Move max17042 common binding part to separate file, to
> reuse it for MFDs with platform driver version.
> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> 
> Changes on v12:

Malformed patch.

> - add addtionalProperties: true on common file
> - rename *-base file to *-common
> - remove compatibles from shared shema
> - move required properties to final schema
> - remove max77705 compatible from binding - it will be used in
>   mfd77705 binding

Sorry, all this is somehow complicated effort of not calling the fuel
gauge what it really is: separate device with its own I2C address, just
like all previous designs in that family from Maxim.

I keep repeating this and you keep going that way, maybe because it fits
your drivers, but that's not the way.

Best regards,
Krzysztof
Dzmitry Sankouski Dec. 18, 2024, 11:25 a.m. UTC | #2
ср, 18 дек. 2024 г. в 11:28, Krzysztof Kozlowski <krzk@kernel.org>:
>
> On Tue, Dec 17, 2024 at 08:30:00PM +0300, Dzmitry Sankouski wrote:
> > Move max17042 common binding part to separate file, to
> > reuse it for MFDs with platform driver version.
> >
> > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> >
> > Changes on v12:
>
> Malformed patch.
>
> > - add addtionalProperties: true on common file
> > - rename *-base file to *-common
> > - remove compatibles from shared shema
> > - move required properties to final schema
> > - remove max77705 compatible from binding - it will be used in
> >   mfd77705 binding
>
> Sorry, all this is somehow complicated effort of not calling the fuel
> gauge what it really is: separate device with its own I2C address, just
> like all previous designs in that family from Maxim.
>
> I keep repeating this and you keep going that way, maybe because it fits
> your drivers, but that's not the way.
>
> Best regards,
> Krzysztof

Fuel gauge ICs designed to sit between battery and charger, or even in the
battery pack itself, with a goal to track and protect the battery.
Given powering diagram:

----------              ---------      ------------      --------------
|usb port|<--[input]--> |charger| <--> |fuel gauge| <--> |battery pack|
----------              ---------      ------------      --------------
                            |
                            |
                            |---> [system bus]

There's no fuel gauge ICs with input and system bus measurements on the market.

This device indeed has its own I2C address, but that's not enough to
say it should be
a separate device, because we have MFD's with its goal to share
resources like a single
i2c address for devices with separate functions.

To me it's more like Maxim put its fuel gauge together with some hwmon
solution on the
single i2c client logic.
Krzysztof Kozlowski Dec. 18, 2024, 11:34 a.m. UTC | #3
On Wed, Dec 18, 2024 at 02:25:31PM +0300, Dzmitry Sankouski wrote:
> ср, 18 дек. 2024 г. в 11:28, Krzysztof Kozlowski <krzk@kernel.org>:
> >
> > On Tue, Dec 17, 2024 at 08:30:00PM +0300, Dzmitry Sankouski wrote:
> > > Move max17042 common binding part to separate file, to
> > > reuse it for MFDs with platform driver version.
> > >
> > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > >
> > > Changes on v12:
> >
> > Malformed patch.
> >
> > > - add addtionalProperties: true on common file
> > > - rename *-base file to *-common
> > > - remove compatibles from shared shema
> > > - move required properties to final schema
> > > - remove max77705 compatible from binding - it will be used in
> > >   mfd77705 binding
> >
> > Sorry, all this is somehow complicated effort of not calling the fuel
> > gauge what it really is: separate device with its own I2C address, just
> > like all previous designs in that family from Maxim.
> >
> > I keep repeating this and you keep going that way, maybe because it fits
> > your drivers, but that's not the way.
> >
> > Best regards,
> > Krzysztof
> 
> Fuel gauge ICs designed to sit between battery and charger, or even in the
> battery pack itself, with a goal to track and protect the battery.
> Given powering diagram:
> 
> ----------              ---------      ------------      --------------
> |usb port|<--[input]--> |charger| <--> |fuel gauge| <--> |battery pack|
> ----------              ---------      ------------      --------------
>                             |
>                             |
>                             |---> [system bus]
> 
> There's no fuel gauge ICs with input and system bus measurements on the market.

OK, good point, assuming that this is the input not for example the
charge on battery. But even if the diagram is correct, we represent here
programming model exposed by device, not physical components of entire
PMIC. Therefore you could have more components there yet still it is
one device: fuel gauge with its I2C addres.


> 
> This device indeed has its own I2C address, but that's not enough to
> say it should be
> a separate device, because we have MFD's with its goal to share
> resources like a single

There is no such thing as "MFD" device in terms of hardware. MFD is a
Linux construct.

> i2c address for devices with separate functions.

> 
> To me it's more like Maxim put its fuel gauge together with some hwmon
> solution on the
> single i2c client logic.

Which still makes it one device, unless you are capable of re-using this
other sensor-part on its own or in other devices.

Best regards,
Krzysztof
Dzmitry Sankouski Dec. 18, 2024, 6:24 p.m. UTC | #4
ср, 18 дек. 2024 г. в 14:34, Krzysztof Kozlowski <krzk@kernel.org>:
>
> On Wed, Dec 18, 2024 at 02:25:31PM +0300, Dzmitry Sankouski wrote:
> > ср, 18 дек. 2024 г. в 11:28, Krzysztof Kozlowski <krzk@kernel.org>:
> > >
> > > On Tue, Dec 17, 2024 at 08:30:00PM +0300, Dzmitry Sankouski wrote:
> > > > Move max17042 common binding part to separate file, to
> > > > reuse it for MFDs with platform driver version.
> > > >
> > > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> > > >
> > > > Changes on v12:
> > >
> > > Malformed patch.
> > >
> > > > - add addtionalProperties: true on common file
> > > > - rename *-base file to *-common
> > > > - remove compatibles from shared shema
> > > > - move required properties to final schema
> > > > - remove max77705 compatible from binding - it will be used in
> > > >   mfd77705 binding
> > >
> > > Sorry, all this is somehow complicated effort of not calling the fuel
> > > gauge what it really is: separate device with its own I2C address, just
> > > like all previous designs in that family from Maxim.
> > >
> > > I keep repeating this and you keep going that way, maybe because it fits
> > > your drivers, but that's not the way.
> > >
> > > Best regards,
> > > Krzysztof
> >
> > Fuel gauge ICs designed to sit between battery and charger, or even in the
> > battery pack itself, with a goal to track and protect the battery.
> > Given powering diagram:
> >
> > ----------              ---------      ------------      --------------
> > |usb port|<--[input]--> |charger| <--> |fuel gauge| <--> |battery pack|
> > ----------              ---------      ------------      --------------
> >                             |
> >                             |
> >                             |---> [system bus]
> >
> > There's no fuel gauge ICs with input and system bus measurements on the market.
>
> OK, good point, assuming that this is the input not for example the
> charge on battery. But even if the diagram is correct, we represent here
> programming model exposed by device, not physical components of entire
> PMIC. Therefore you could have more components there yet still it is
> one device: fuel gauge with its I2C addres.
>
>
> >
> > This device indeed has its own I2C address, but that's not enough to
> > say it should be
> > a separate device, because we have MFD's with its goal to share
> > resources like a single
>
> There is no such thing as "MFD" device in terms of hardware. MFD is a
> Linux construct.
>
> > i2c address for devices with separate functions.
>
> >
> > To me it's more like Maxim put its fuel gauge together with some hwmon
> > solution on the
> > single i2c client logic.
>
> Which still makes it one device, unless you are capable of re-using this
> other sensor-part on its own or in other devices.

I think I get it. There's no need for an MFD device node, because it's
just empty.
So in the device tree we'll only have a max17042 fuel gauge node. It'll get
matched with simple-mfd-i2c driver, which will create 2 sub devices -
fuel gauge and hwmon. Fuel gauge platform driver version will
get matched by platform id, and will take of_node from pdev dev parent
for setup.

Is that what you are thinking of?
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max17042-common.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max17042-common.yaml
new file mode 100644
index 000000000000..67e673ca7970
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/maxim,max17042-common.yaml
@@ -0,0 +1,54 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/maxim,max17042-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim 17042 fuel gauge series
+
+maintainers:
+  - Sebastian Reichel <sre@kernel.org>
+
+allOf:
+  - $ref: power-supply.yaml#
+
+properties:
+  interrupts:
+    maxItems: 1
+    description: |
+      The ALRT pin, an open-drain interrupt.
+
+  maxim,rsns-microohm:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Resistance of rsns resistor in micro Ohms (datasheet-recommended value is 10000).
+      Defining this property enables current-sense functionality.
+
+  maxim,cold-temp:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Temperature threshold to report battery as cold (in tenths of degree Celsius).
+      Default is not to report cold events.
+
+  maxim,over-heat-temp:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Temperature threshold to report battery as over heated (in tenths of degree Celsius).
+      Default is not to report over heating events.
+
+  maxim,dead-volt:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Voltage threshold to report battery as dead (in mV).
+      Default is not to report dead battery events.
+
+  maxim,over-volt:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Voltage threshold to report battery as over voltage (in mV).
+      Default is not to report over-voltage events.
+
+  power-supplies: true
+
+additionalProperties: true
+
diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml
index 085e2504d0dc..0832aa5f5eb0 100644
--- a/Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml
+++ b/Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml
@@ -10,7 +10,7 @@  maintainers:
   - Sebastian Reichel <sre@kernel.org>
 
 allOf:
-  - $ref: power-supply.yaml#
+  - $ref: maxim,max17042-common.yaml#
 
 properties:
   compatible:
@@ -24,48 +24,11 @@  properties:
   reg:
     maxItems: 1
 
-  interrupts:
-    maxItems: 1
-    description: |
-      The ALRT pin, an open-drain interrupt.
-
-  maxim,rsns-microohm:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Resistance of rsns resistor in micro Ohms (datasheet-recommended value is 10000).
-      Defining this property enables current-sense functionality.
-
-  maxim,cold-temp:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Temperature threshold to report battery as cold (in tenths of degree Celsius).
-      Default is not to report cold events.
-
-  maxim,over-heat-temp:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Temperature threshold to report battery as over heated (in tenths of degree Celsius).
-      Default is not to report over heating events.
-
-  maxim,dead-volt:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Voltage threshold to report battery as dead (in mV).
-      Default is not to report dead battery events.
-
-  maxim,over-volt:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Voltage threshold to report battery as over voltage (in mV).
-      Default is not to report over-voltage events.
-
-  power-supplies: true
-
 required:
-  - compatible
   - reg
+  - compatible
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/MAINTAINERS b/MAINTAINERS
index 81348dbce8ca..0816fe0f3c80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14164,7 +14164,7 @@  R:	Sebastian Krzyszkowiak <sebastian.krzyszkowiak@puri.sm>
 R:	Purism Kernel Team <kernel@puri.sm>
 L:	linux-pm@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/power/supply/maxim,max17042.yaml
+F:	Documentation/devicetree/bindings/power/supply/maxim,max17042*.yaml
 F:	drivers/power/supply/max17042_battery.c
 
 MAXIM MAX20086 CAMERA POWER PROTECTOR DRIVER