[PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings

Jihed Chaibi posted 3 patches 4 months, 4 weeks ago
There is a newer version of this series
.../devicetree/bindings/mfd/ti,twl.yaml       | 232 +++++++++++++++++-
.../devicetree/bindings/mfd/twl4030-power.txt |  48 ----
.../devicetree/bindings/pwm/ti,twl-pwm.txt    |  17 --
.../devicetree/bindings/pwm/ti,twl-pwmled.txt |  17 --
arch/arm/boot/dts/ti/omap/omap3-beagle-xm.dts |   2 +-
arch/arm/boot/dts/ti/omap/omap3-n900.dts      |   2 +-
6 files changed, 223 insertions(+), 95 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt
delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwm.txt
delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwmled.txt
[PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings
Posted by Jihed Chaibi 4 months, 4 weeks ago
This version addresses a final piece of feedback from Andreas to make
the twl4030/twl6030-specific child nodes (audio, usb, keypad etc.)
conditional by moving them out of the common block, which now only
contains common properties (rtc, charger, pwm, pwmled..) ensuring
the schema is fully accurate.

The complete dtbs_check for this binding is clean except for two
warnings originating from pre-existing bugs in the OMAP DTS files,
for which fixes have already been submitted separately [1][2].

Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Uwe Kleine-König <ukleinek@kernel.org>

---
Changes in v7:
  - (1/3): Moved twl4030/twl6030-specific child node definitions (audio,
    usb etc.) into the conditional 'if/then' block to improve schema
    accuracy.
  - (2/3 & 3/3): No changes.

Changes in v6:
  - Refactored the ti,twl4030-power compatible schema to be much stricter,
    removing obsolete board-specific compatibles (-n900, -beagleboard-xm),
    that were added in v5. The schema now only permits specific, valid
    fallback combinations.
  - This series presents two new patches (2/3) & (3/3), which update the
    affected DTS files by removing obsolete entries.
  - Enforced the presence of the compatible property on all relevant
    sub-nodes by adding 'required: - compatible', closing a key validation
    loophole.
  - Applied various formatting cleanups for readability and correctness.

Changes in v5:
  - Restructured the entire binding to define properties at the top
    level instead of if/then blocks, per maintainer feedback.
  - Added specific compatible enums for new child nodes instead of a
    generic 'compatible: true'.
  - Set 'unevaluatedProperties: false' for 'pwm' and 'pwmled' nodes to
    enforce strict validation.
  - Expanded 'power' node compatible enum to include all board-specific
    compatible strings (used in existing device trees, e.g. OMAP3-based
    boards) for more complete coverage.
  - Corrected the schema for the 'power' node compatible to properly
    handle single and fallback entries.

Changes in v4:
  - Reworked binding to be independent and bisectable per maintainer
    feedback by using 'additionalProperties: true' for child nodes.
  - Added board-specific compatibles to the 'power' node enum.
  - Added definitions for 'clocks' and 'clock-names' properties.
  - Renamed 'twl6030-usb' child node to 'usb-comparator' to match
    existing Device Tree usage (twl6030.dtsi).
  - Fixed some spelling/grammar erros in the description.

Changes in v3:
  - New patch to consolidate simple bindings (power, pwm) and add
    definitions for all child nodes to fix dtbs_check validation
    errors found in v2.

Changes in v2:
  - This patch is split from larger series [3] per maintainer feedback.
  - Added missing sub-node definitions, resolving dtbs_check errors.

[1] https://lore.kernel.org/all/20250822222530.113520-1-jihed.chaibi.dev@gmail.com/
[2] https://lore.kernel.org/all/20250822225052.136919-1-jihed.chaibi.dev@gmail.com/
[3] https://lore.kernel.org/all/20250816021523.167049-1-jihed.chaibi.dev@gmail.com/

Jihed Chaibi (3):
  dt-bindings: mfd: twl: Add missing sub-nodes for TWL4030 & TWL603x
  ARM: dts: omap3: beagle-xm: Correct obsolete TWL4030 power compatible
  ARM: dts: omap3: n900: Correct obsolete TWL4030 power compatible

 .../devicetree/bindings/mfd/ti,twl.yaml       | 232 +++++++++++++++++-
 .../devicetree/bindings/mfd/twl4030-power.txt |  48 ----
 .../devicetree/bindings/pwm/ti,twl-pwm.txt    |  17 --
 .../devicetree/bindings/pwm/ti,twl-pwmled.txt |  17 --
 arch/arm/boot/dts/ti/omap/omap3-beagle-xm.dts |   2 +-
 arch/arm/boot/dts/ti/omap/omap3-n900.dts      |   2 +-
 6 files changed, 223 insertions(+), 95 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt
 delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwm.txt
 delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwmled.txt

-- 
2.39.5

Re: [PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings
Posted by Krzysztof Kozlowski 4 months, 4 weeks ago
On Wed, Sep 10, 2025 at 06:07:01PM +0200, Jihed Chaibi wrote:
> This version addresses a final piece of feedback from Andreas to make
> the twl4030/twl6030-specific child nodes (audio, usb, keypad etc.)
> conditional by moving them out of the common block, which now only
> contains common properties (rtc, charger, pwm, pwmled..) ensuring
> the schema is fully accurate.
> 
> The complete dtbs_check for this binding is clean except for two
> warnings originating from pre-existing bugs in the OMAP DTS files,
> for which fixes have already been submitted separately [1][2].
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
> 
> ---
> Changes in v7:
>   - (1/3): Moved twl4030/twl6030-specific child node definitions (audio,
>     usb etc.) into the conditional 'if/then' block to improve schema
>     accuracy.

Who asked for this? It's wrong code.

Best regards,
Krzysztof
Re: [PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings
Posted by Andreas Kemnade 4 months, 4 weeks ago
Am Thu, 11 Sep 2025 08:35:32 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:

> On Wed, Sep 10, 2025 at 06:07:01PM +0200, Jihed Chaibi wrote:
> > This version addresses a final piece of feedback from Andreas to make
> > the twl4030/twl6030-specific child nodes (audio, usb, keypad etc.)
> > conditional by moving them out of the common block, which now only
> > contains common properties (rtc, charger, pwm, pwmled..) ensuring
> > the schema is fully accurate.
> > 
> > The complete dtbs_check for this binding is clean except for two
> > warnings originating from pre-existing bugs in the OMAP DTS files,
> > for which fixes have already been submitted separately [1][2].
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
> > 
> > ---
> > Changes in v7:
> >   - (1/3): Moved twl4030/twl6030-specific child node definitions (audio,
> >     usb etc.) into the conditional 'if/then' block to improve schema
> >     accuracy.  
> 
> Who asked for this? It's wrong code.
> 
maybe I was not clear there. That was not was I meant. As far as I
understand, the correct pattern is to define things outside of the
if/then block and
then disable it with property-name: false in the if/then block
Example: Handling of regulator-initial-mode property.

Sorry, for the confusion.

Regards,
Andreas
Re: [PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings
Posted by Krzysztof Kozlowski 4 months, 4 weeks ago
On 11/09/2025 08:43, Andreas Kemnade wrote:
> Am Thu, 11 Sep 2025 08:35:32 +0200
> schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> 
>> On Wed, Sep 10, 2025 at 06:07:01PM +0200, Jihed Chaibi wrote:
>>> This version addresses a final piece of feedback from Andreas to make
>>> the twl4030/twl6030-specific child nodes (audio, usb, keypad etc.)
>>> conditional by moving them out of the common block, which now only
>>> contains common properties (rtc, charger, pwm, pwmled..) ensuring
>>> the schema is fully accurate.
>>>
>>> The complete dtbs_check for this binding is clean except for two
>>> warnings originating from pre-existing bugs in the OMAP DTS files,
>>> for which fixes have already been submitted separately [1][2].
>>>
>>> Reviewed-by: Rob Herring <robh@kernel.org>
>>> Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
>>>
>>> ---
>>> Changes in v7:
>>>   - (1/3): Moved twl4030/twl6030-specific child node definitions (audio,
>>>     usb etc.) into the conditional 'if/then' block to improve schema
>>>     accuracy.  
>>
>> Who asked for this? It's wrong code.
>>
> maybe I was not clear there. That was not was I meant. As far as I
> understand, the correct pattern is to define things outside of the
> if/then block and
> then disable it with property-name: false in the if/then block
> Example: Handling of regulator-initial-mode property.

Yes, I read your comment afterwards and that is how I would understand
it as well.

But the patch here is done differently.


Best regards,
Krzysztof
Re: [PATCH v7 0/3] dt-bindings: mfd: twl: Consolidate and fix TI TWL family bindings
Posted by Jihed Chaibi 4 months, 4 weeks ago
On Thu, Sep 11, 2025 at 9:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 11/09/2025 08:43, Andreas Kemnade wrote:
> > Am Thu, 11 Sep 2025 08:35:32 +0200
> > schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> >
> >> On Wed, Sep 10, 2025 at 06:07:01PM +0200, Jihed Chaibi wrote:
> >>> This version addresses a final piece of feedback from Andreas to make
> >>> the twl4030/twl6030-specific child nodes (audio, usb, keypad etc.)
> >>> conditional by moving them out of the common block, which now only
> >>> contains common properties (rtc, charger, pwm, pwmled..) ensuring
> >>> the schema is fully accurate.
> >>>
> >>> The complete dtbs_check for this binding is clean except for two
> >>> warnings originating from pre-existing bugs in the OMAP DTS files,
> >>> for which fixes have already been submitted separately [1][2].
> >>>
> >>> Reviewed-by: Rob Herring <robh@kernel.org>
> >>> Acked-by: Uwe Kleine-König <ukleinek@kernel.org>
> >>>
> >>> ---
> >>> Changes in v7:
> >>>   - (1/3): Moved twl4030/twl6030-specific child node definitions (audio,
> >>>     usb etc.) into the conditional 'if/then' block to improve schema
> >>>     accuracy.
> >>
> >> Who asked for this? It's wrong code.
> >>
> > maybe I was not clear there. That was not was I meant. As far as I
> > understand, the correct pattern is to define things outside of the
> > if/then block and
> > then disable it with property-name: false in the if/then block
> > Example: Handling of regulator-initial-mode property.
>
> Yes, I read your comment afterwards and that is how I would understand
> it as well.
>
> But the patch here is done differently.
>
>
> Best regards,
> Krzysztof

Hi Krzysztof, Andreas,

Thank you for the clarification. my apologies, I
misunderstood the correct pattern.

I was following the existing structure in the original yaml
file, where several board-specific sub-nodes like 'madc',
'pwrbutton', and 'gpadc' etc. were already defined inside
the 'if/then' blocks. I assumed that was the correct
convention and that the main properties block was only
for shared nodes (like 'regulator' & 'pwm'..) which is
not the case.

I had (mis)interpreted Krzysztof's earlier feedback about
top-level definitions as applying only to properties
that were common to all variants (like 'pwm').

I will send a v8 implementing this "define then disable"
pattern for all sub-nodes. This will be a good
opportunity to clean up the pre-existing definitions
to make the entire binding fully consistent.

Thanks,
Jihed