Re: [PATCH 1/6] dt-bindings: connector: add power-opmode optional property to usb-connector

[Amelie DELAUNAY]

Hi Rob,

On 7/15/20 8:19 PM, Rob Herring wrote:
> On Mon, Jun 15, 2020 at 06:15:07PM +0200, Amelie Delaunay wrote:
> > Power operation mode may depends on hardware design, so, add the optional
> > property power-opmode for usb-c connector to select the power operation
> > mode capability.
> >
> > Signed-off-by: Amelie Delaunay <amelie.delaunay@xxxxxx>
> > ---
> >  .../devicetree/bindings/connector/usb-connector.yaml | 11 +++++++++++
> > Â 1 file changed, 11 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 9bd52e63c935..cd7feb2d4984 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -88,6 +88,17 @@ properties:
> > ÂÂÂÂÂÂÂ - device
> > ÂÂÂÂÂÂÂ - dual
> >  
> > +Â power-opmode:
> > +ÂÂÂ description: Determines the power operation mode that the Type C connector
> > +ÂÂÂÂÂ will support.
> > +ÂÂÂ allOf:
> > +ÂÂÂÂÂ - $ref: /schemas/types.yaml#definitions/string
> > +ÂÂÂ enum:
> > +ÂÂÂÂÂ - default
>
> Wouldn't this just be no property?

No because the aim of this optional property is to re-configure the 
Type-C controller power operation mode. So, when using power-opmode = 
"default", the use can re-configure it to the 'default' USB power 
operation mode - 500mA for USB2 or 900mA for USB3.

> > +ÂÂÂÂÂ - 1.5A
> > +ÂÂÂÂÂ - 3.0A
>
> You'll need to explain these better.

Ok I'll add a description in V2.

> > +ÂÂÂÂÂ - usb_power_delivery
>
> I would have thought 'default' would be USB-PD. Though I thought Type-C
> was always USB-PD.

In fact, the DT property matches the typec_pwr_opmode enum and 
typec_pwr_opmodes strings tab:
static const char * const typec_pwr_opmodes[] = {
	[TYPEC_PWR_MODE_USB]	= "default",
	[TYPEC_PWR_MODE_1_5A]	= "1.5A",
	[TYPEC_PWR_MODE_3_0A]	= "3.0A",
	[TYPEC_PWR_MODE_PD]	= "usb_power_delivery",
};
The same is done for data-role and power-role properties.

So, I would keep "default", "1.5A", "3.0A" and "usb_power_delivery", and 
I'll add a description for each. Is it OK for you ?

Regards,
Amelie

> > +
> > ÂÂÂ # The following are optional properties for "usb-c-connector" with power
> > ÂÂÂ # delivery support.
> > ÂÂÂ source-pdos:
> > --
> > 2.17.1