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

On Thu, Sep 11, 2025 at 9:07 AM Krzysztof Kozlowski [off-list ref] wrote:

On 11/09/2025 08:43, Andreas Kemnade wrote:

quoted

Am Thu, 11 Sep 2025 08:35:32 +0200
schrieb Krzysztof Kozlowski [off-list ref]:

quoted

On Wed, Sep 10, 2025 at 06:07:01PM +0200, Jihed Chaibi wrote:

quoted

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