Re: [PATCH 1/2] dt-bindings: regulator: Add pf8x00 regulator

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2013 bytes --]

On Sun, Dec 06, 2020 at 01:26:28AM +0100, Adrien Grassein wrote:
> Add dt-bindings for the pf8x00 driver.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +  compatible:
> +    enum:
> +      - nxp,pf8x00

Compatible strings should be for specific devices not wildcards.

> +          nxp,hw-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. Used to enable or disable ld02.

I don't understand what this is documenting - what is "hw-en" and how is
it used to enable or disable LDO2?

> +          nxp,vselect-en:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              Only available for ldo2. When specified, use the VSELECT pin
> +              of the chip to control the output voltage of the ldo02 regulator.

Shouldn't there be a GPIO specified somewhere or something so that the
VSELECT pin can be controlled?  

> +          nxp,ilim-ma:
> +            $ref: /schemas/types.yaml#definitions/uint32
> +            minimum: 2100
> +            maximum: 4500
> +            default: 2100
> +            enum: [ 2100, 2600, 3000, 4500 ]
> +            description: |
> +              Defines the maximum current delivered by the regulator (in mA).

Is this not a fixed property of the regulator?

> +          nxp,quad-phase:
> +            $ref: /schemas/types.yaml#definitions/flag
> +            description: |
> +              This allow regulators  sw1 and sw2, or sw3 and sw4 or sw4 and sw5
> +              to work together to deliver a maximum 10A current.

Presumably this must be set on both the regulators being grouped
together?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]