diff mbox series

[v3,2/7] dt-bindings: mfd: brcm,bcm59056: Add compatible for BCM59054

Message ID 20250131-bcm59054-v3-2-bbac52a84787@gmail.com (mailing list archive)
State New
Headers show
Series mfd: bcm590xx: Add support for BCM59054 | expand

Commit Message

Artur Weber Jan. 31, 2025, 6:13 p.m. UTC
The BCM59054 MFD is fairly similar to the BCM59056, and will use
the same driver. Add compatible and specify the allowed regulator
nodes.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changes in v3:
- Split regulator node into separate file
- Removed quotes around compatible
---
 .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 26 +++++++++-
 .../bindings/regulator/brcm,bcm59054.yaml          | 55 ++++++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) Jan. 31, 2025, 7:41 p.m. UTC | #1
On Fri, 31 Jan 2025 19:13:50 +0100, Artur Weber wrote:
> The BCM59054 MFD is fairly similar to the BCM59056, and will use
> the same driver. Add compatible and specify the allowed regulator
> nodes.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
> Changes in v3:
> - Split regulator node into separate file
> - Removed quotes around compatible
> ---
>  .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 26 +++++++++-
>  .../bindings/regulator/brcm,bcm59054.yaml          | 55 ++++++++++++++++++++++
>  2 files changed, 79 insertions(+), 2 deletions(-)
> 

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/regulator/brcm,bcm59054.yaml: ^(cam|sim|mmc)ldo[1-2]$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: ^(rf|sd|sdx|aud|mic|usb|vib|tcx)ldo$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: ^(c|mm|v)sr$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: ^(io|sd)sr[1-2]$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: ^gpldo[1-3]$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: ^lvldo[1-2]$: Missing additionalProperties/unevaluatedProperties constraint

/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml: vbus: Missing additionalProperties/unevaluatedProperties constraint

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250131-bcm59054-v3-2-bbac52a84787@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Stanislav Jakubek Feb. 2, 2025, 10:08 a.m. UTC | #2
On Fri, Jan 31, 2025 at 07:13:50PM +0100, Artur Weber wrote:
> The BCM59054 MFD is fairly similar to the BCM59056, and will use
> the same driver. Add compatible and specify the allowed regulator
> nodes.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>

[snip]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: brcm,bcm59054
> +    then:
> +      properties:
> +        regulators:
> +          $ref: ../regulator/brcm,bcm59054.yaml

Full path.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: brcm,bcm59056
> +    then:
> +      properties:
> +        regulators:
> +          $ref: ../regulator/brcm,bcm59056.yaml

Full path.

> +
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..719621c7f0e71cd9368f4d7243c79aaa97ca7255
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/brcm,bcm59054.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM59054 Power Management IC regulators
> +
> +description: |
> +  This is a part of device tree bindings for the BCM590XX family of power
> +  management ICs.

Same comment as patch 1.

> +
> +  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
> +  additional information and example.
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +# The valid regulator node names for BCM59054 are:
> +#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
> +#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
> +#   csr, iosr1, iosr2, mmsr, sdsr1, sdsr2, vsr,
> +#   gpldo1, gpldo2, gpldo3, tcxldo, lvldo1, lvldo2

Same comment as patch 1.

> +
> +patternProperties:
> +  "^(cam|sim|mmc)ldo[1-2]$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#

As Rob's bot pointed out, you're missing unevaluatedProperties: false in
all of these.

Regards,
Stanislav
Krzysztof Kozlowski Feb. 2, 2025, 1:24 p.m. UTC | #3
On Fri, Jan 31, 2025 at 07:13:50PM +0100, Artur Weber wrote:
> The BCM59054 MFD is fairly similar to the BCM59056, and will use
> the same driver. Add compatible and specify the allowed regulator
> nodes.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
> Changes in v3:
> - Split regulator node into separate file
> - Removed quotes around compatible
> ---
>  .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 26 +++++++++-
>  .../bindings/regulator/brcm,bcm59054.yaml          | 55 ++++++++++++++++++++++
>  2 files changed, 79 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
> index 3296799eb452fca2a4b03699fcb5aa27005a8e8d..87d663416ed9e7f5ec4aa25c1aa2d9e650c42e2c 100644
> --- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
> +++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
> @@ -11,7 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: brcm,bcm59056
> +    enum:
> +      - brcm,bcm59054
> +      - brcm,bcm59056
>  
>    reg:
>      maxItems: 1
> @@ -22,7 +24,6 @@ properties:
>    regulators:
>      type: object
>      description: Container node for regulators.
> -    $ref: ../regulator/brcm,bcm59056.yaml

Refs should rather stay here, so I don't think keeping these devices in
one binding makes it simpler.

Simpler - drop ref and add properties compatible with enum for your
regulator compatibles.

...

> +$id: http://devicetree.org/schemas/regulator/brcm,bcm59054.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM59054 Power Management IC regulators
> +
> +description: |
> +  This is a part of device tree bindings for the BCM590XX family of power
> +  management ICs.
> +
> +  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
> +  additional information and example.
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +# The valid regulator node names for BCM59054 are:
> +#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
> +#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
> +#   csr, iosr1, iosr2, mmsr, sdsr1, sdsr2, vsr,
> +#   gpldo1, gpldo2, gpldo3, tcxldo, lvldo1, lvldo2
> +
> +patternProperties:
> +  "^(cam|sim|mmc)ldo[1-2]$":
> +    type: object

Missin unevaluatedProperties everywhere. Look how other bindings do it.

> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +  "^(rf|sd|sdx|aud|mic|usb|vib|tcx)ldo$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +  "^(c|mm|v)sr$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +  "^(io|sd)sr[1-2]$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +  "^gpldo[1-3]$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +  "^lvldo[1-2]$":
> +    type: object
> +    $ref: /schemas/regulator/regulator.yaml#
> +
> +properties:
> +  "vbus":

No quotes.

Best regards,
Krzysztof
Artur Weber Feb. 5, 2025, 5:58 p.m. UTC | #4
On 2.02.2025 14:24, Krzysztof Kozlowski wrote:
> On Fri, Jan 31, 2025 at 07:13:50PM +0100, Artur Weber wrote:
>> ...
>> @@ -22,7 +24,6 @@ properties:
>>     regulators:
>>       type: object
>>       description: Container node for regulators.
>> -    $ref: ../regulator/brcm,bcm59056.yaml
> 
> Refs should rather stay here, so I don't think keeping these devices in
> one binding makes it simpler.
> 
> Simpler - drop ref and add properties compatible with enum for your
> regulator compatibles.
> 

The only problem with that is that the regulator nodes have no
compatibles. I see two ways out of it: either we add a compatible then
use the compatible enum (might break existing users who don't have the
compatible, unless we make the driver ignore it), or if we just want to
simplify but not add compatibles, list all the refs under an oneOf
condition (like it's done in
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml[1]).

Personally I'm in favor of keeping this as-is, though, for the benefit
of the DT linter preventing accidental mixing of bindings for different
device types (e.g. BCM59056 MFD node with BCM59054 regulators). There's
at least some precedent for this method[2]. But if making it simple is
what's considered better, I'll go with one of the aforementioned
options.

Best regards
Artur

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml#n147
[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml#n69
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
index 3296799eb452fca2a4b03699fcb5aa27005a8e8d..87d663416ed9e7f5ec4aa25c1aa2d9e650c42e2c 100644
--- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
@@ -11,7 +11,9 @@  maintainers:
 
 properties:
   compatible:
-    const: brcm,bcm59056
+    enum:
+      - brcm,bcm59054
+      - brcm,bcm59056
 
   reg:
     maxItems: 1
@@ -22,7 +24,6 @@  properties:
   regulators:
     type: object
     description: Container node for regulators.
-    $ref: ../regulator/brcm,bcm59056.yaml
 
 required:
   - compatible
@@ -31,6 +32,27 @@  required:
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm59054
+    then:
+      properties:
+        regulators:
+          $ref: ../regulator/brcm,bcm59054.yaml
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm59056
+    then:
+      properties:
+        regulators:
+          $ref: ../regulator/brcm,bcm59056.yaml
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..719621c7f0e71cd9368f4d7243c79aaa97ca7255
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59054.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/brcm,bcm59054.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM59054 Power Management IC regulators
+
+description: |
+  This is a part of device tree bindings for the BCM590XX family of power
+  management ICs.
+
+  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
+  additional information and example.
+
+maintainers:
+  - Artur Weber <aweber.kernel@gmail.com>
+
+# The valid regulator node names for BCM59054 are:
+#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
+#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
+#   csr, iosr1, iosr2, mmsr, sdsr1, sdsr2, vsr,
+#   gpldo1, gpldo2, gpldo3, tcxldo, lvldo1, lvldo2
+
+patternProperties:
+  "^(cam|sim|mmc)ldo[1-2]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+  "^(rf|sd|sdx|aud|mic|usb|vib|tcx)ldo$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+  "^(c|mm|v)sr$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+  "^(io|sd)sr[1-2]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+  "^gpldo[1-3]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+  "^lvldo[1-2]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+properties:
+  "vbus":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+
+additionalProperties: false