diff mbox series

[v8,05/11] dt-bindings: connector: Add property to set initial current cap for FRS

Message ID 20200921195555.1050731-5-badhri@google.com
State Superseded
Headers show
Series [v8,01/11] usb: typec: tcpci: Add a getter method to retrieve tcpm_port reference | expand

Commit Message

Badhri Jagan Sridharan Sept. 21, 2020, 7:55 p.m. UTC 
This change adds frs-typec-current which allows setting the initial current
capability of the new source when vSafe5V is applied during PD3.0
sink Fast Role Swap.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
Changes since v1:
- Changing patch version to v6 to fix version number confusion.

Changes since v6:
- Removed the redundant usb-connector.txt that I created by mistake.
- Moved to yaml.

Changes since v7:
- Rebase 
---
 .../devicetree/bindings/connector/usb-connector.yaml   |  8 ++++++++
 include/dt-bindings/usb/pd.h                           | 10 ++++++++++
 2 files changed, 18 insertions(+)

Comments

Badhri Jagan Sridharan Sept. 24, 2020, 10:09 a.m. UTC | #1 
Hi Jun,

Thanks for the feedback !
The sink PDO from current source reflects the current source's(i.e.
transmitter of the FRS signal) power requirement during fr swap.
The current sink (i.e. receiver of the FRS signal) should check if it
will be able to satisfy the current source's
requirement during frswap before enabling the frs signal reception.
The property in this patch refers to maximum current capability
that the current sink can satisfy. Perhaps, I should name it
sink-frs-typec-current. Does that make sense to you ?

Thanks,
Badhri

On Wed, Sep 23, 2020 at 3:43 AM Jun Li <lijun.kernel@gmail.com> wrote:
>

> Badhri Jagan Sridharan <badhri@google.com> 于2020年9月22日周二 上午3:57写道：

> >

> > This change adds frs-typec-current which allows setting the initial current

> > capability of the new source when vSafe5V is applied during PD3.0

> > sink Fast Role Swap.

> >

> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>

> > ---

> > Changes since v1:

> > - Changing patch version to v6 to fix version number confusion.

> >

> > Changes since v6:

> > - Removed the redundant usb-connector.txt that I created by mistake.

> > - Moved to yaml.

> >

> > Changes since v7:

> > - Rebase

> > ---

> >  .../devicetree/bindings/connector/usb-connector.yaml   |  8 ++++++++

> >  include/dt-bindings/usb/pd.h                           | 10 ++++++++++

> >  2 files changed, 18 insertions(+)

> >

> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml

> > index 9bd52e63c935..1ca8e6a337e5 100644

> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml

> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml

> > @@ -142,6 +142,14 @@ properties:

> >      required:

> >        - port@0

> >

> > +  frs-typec-current:

> > +    description: Initial current capability of the new source when vSafe5V

> > +      is applied during PD3.0 Fast Role Swap. "Table 6-14 Fixed Supply PDO - Sink"

> > +      of "USB Power Delivery Specification Revision 3.0, Version 1.2" provides the

> > +      different power levels and "6.4.1.3.1.6 Fast Role Swap USB Type-C Current"

> > +      provides a detailed description of the field.

> > +    $ref: /schemas/types.yaml#/definitions/uint32

>

> If it's a part of sink PDO, I think you don't need a new property for this, just

> define it directly into sink-pdos by adding a new PDO define for PD 3.0,

> something like:

>

> sink-pdos = <PDO_FIXED_v3(5000, 3000, PDO_FIXED_USB_COMM, FRS_CURRENT_1P5A)>;

>

> Li Jun

> > +

> >  required:

> >    - compatible

> >

> > diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h

> > index 985f2bbd4d24..db1ad4532197 100644

> > --- a/include/dt-bindings/usb/pd.h

> > +++ b/include/dt-bindings/usb/pd.h

> > @@ -35,6 +35,16 @@

> >

> >  #define VSAFE5V 5000 /* mv units */

> >

> > +/*

> > + * Based on "Table 6-14 Fixed Supply PDO - Sink" of "USB Power Delivery Specification Revision 3.0,

> > + * Version 1.2"

> > + * Initial current capability of the new source when vSafe5V is applied.

> > + */

> > +#define FRS_NOT_SUPPORTED      0

> > +#define FRS_DEFAULT_POWER      1

> > +#define FRS_5V_1P5A            2

> > +#define FRS_5V_3A              3

> > +

> >  #define PDO_BATT_MAX_VOLT_SHIFT        20      /* 50mV units */

> >  #define PDO_BATT_MIN_VOLT_SHIFT        10      /* 50mV units */

> >  #define PDO_BATT_MAX_PWR_SHIFT 0       /* 250mW units */

> > --

> > 2.28.0.681.g6f77f65b4e-goog

> >
Jun Li Sept. 28, 2020, 8:57 a.m. UTC | #2 
Badhri Jagan Sridharan <badhri@google.com> 于2020年9月24日周四 下午6:09写道：
>
> Hi Jun,
>
> Thanks for the feedback !
> The sink PDO from current source reflects the current source's(i.e.
> transmitter of the FRS signal) power requirement during fr swap.
> The current sink (i.e. receiver of the FRS signal) should check if it
> will be able to satisfy the current source's
> requirement during frswap before enabling the frs signal reception.
> The property in this patch refers to maximum current capability
> that the current sink can satisfy.

In this case I agree a new property is required.

Rob mentioned another similar property for typec[1], which is
for typec source(without power delivery) to define its power
capability to present its Rp, so a different usage.

[1]https://lore.kernel.org/linux-arm-kernel/20200902075707.9052-2-amelie.delaunay@st.com/

> Perhaps, I should name it
> sink-frs-typec-current. Does that make sense to you ?

it looks better, thanks.

Li Jun
>
> Thanks,
> Badhri
>
> On Wed, Sep 23, 2020 at 3:43 AM Jun Li <lijun.kernel@gmail.com> wrote:
> >
> > Badhri Jagan Sridharan <badhri@google.com> 于2020年9月22日周二 上午3:57写道：
> > >
> > > This change adds frs-typec-current which allows setting the initial current
> > > capability of the new source when vSafe5V is applied during PD3.0
> > > sink Fast Role Swap.
> > >
> > > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > > ---
> > > Changes since v1:
> > > - Changing patch version to v6 to fix version number confusion.
> > >
> > > Changes since v6:
> > > - Removed the redundant usb-connector.txt that I created by mistake.
> > > - Moved to yaml.
> > >
> > > Changes since v7:
> > > - Rebase
> > > ---
> > >  .../devicetree/bindings/connector/usb-connector.yaml   |  8 ++++++++
> > >  include/dt-bindings/usb/pd.h                           | 10 ++++++++++
> > >  2 files changed, 18 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > index 9bd52e63c935..1ca8e6a337e5 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > @@ -142,6 +142,14 @@ properties:
> > >      required:
> > >        - port@0
> > >
> > > +  frs-typec-current:
> > > +    description: Initial current capability of the new source when vSafe5V
> > > +      is applied during PD3.0 Fast Role Swap. "Table 6-14 Fixed Supply PDO - Sink"
> > > +      of "USB Power Delivery Specification Revision 3.0, Version 1.2" provides the
> > > +      different power levels and "6.4.1.3.1.6 Fast Role Swap USB Type-C Current"
> > > +      provides a detailed description of the field.
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> >
> > If it's a part of sink PDO, I think you don't need a new property for this, just
> > define it directly into sink-pdos by adding a new PDO define for PD 3.0,
> > something like:
> >
> > sink-pdos = <PDO_FIXED_v3(5000, 3000, PDO_FIXED_USB_COMM, FRS_CURRENT_1P5A)>;
> >
> > Li Jun
> > > +
> > >  required:
> > >    - compatible
> > >
> > > diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> > > index 985f2bbd4d24..db1ad4532197 100644
> > > --- a/include/dt-bindings/usb/pd.h
> > > +++ b/include/dt-bindings/usb/pd.h
> > > @@ -35,6 +35,16 @@
> > >
> > >  #define VSAFE5V 5000 /* mv units */
> > >
> > > +/*
> > > + * Based on "Table 6-14 Fixed Supply PDO - Sink" of "USB Power Delivery Specification Revision 3.0,
> > > + * Version 1.2"
> > > + * Initial current capability of the new source when vSafe5V is applied.
> > > + */
> > > +#define FRS_NOT_SUPPORTED      0
> > > +#define FRS_DEFAULT_POWER      1
> > > +#define FRS_5V_1P5A            2
> > > +#define FRS_5V_3A              3
> > > +
> > >  #define PDO_BATT_MAX_VOLT_SHIFT        20      /* 50mV units */
> > >  #define PDO_BATT_MIN_VOLT_SHIFT        10      /* 50mV units */
> > >  #define PDO_BATT_MAX_PWR_SHIFT 0       /* 250mW units */
> > > --
> > > 2.28.0.681.g6f77f65b4e-goog
> > >
Badhri Jagan Sridharan Sept. 29, 2020, 2:43 a.m. UTC | #3 
On Mon, Sep 28, 2020 at 1:57 AM Jun Li <lijun.kernel@gmail.com> wrote:
>
> Badhri Jagan Sridharan <badhri@google.com> 于2020年9月24日周四 下午6:09写道：
> >
> > Hi Jun,
> >
> > Thanks for the feedback !
> > The sink PDO from current source reflects the current source's(i.e.
> > transmitter of the FRS signal) power requirement during fr swap.
> > The current sink (i.e. receiver of the FRS signal) should check if it
> > will be able to satisfy the current source's
> > requirement during frswap before enabling the frs signal reception.
> > The property in this patch refers to maximum current capability
> > that the current sink can satisfy.
>
> In this case I agree a new property is required.
>
> Rob mentioned another similar property for typec[1], which is
> for typec source(without power delivery) to define its power
> capability to present its Rp, so a different usage.
>
> [1]https://lore.kernel.org/linux-arm-kernel/20200902075707.9052-2-amelie.delaunay@st.com/
>
> > Perhaps, I should name it
> > sink-frs-typec-current. Does that make sense to you ?
>
> it looks better, thanks.

Ended up naming it new-source-frs-type-ccurrent and have sent the
v9 version of the patch. Thanks for you reviews !

>
> Li Jun
> >
> > Thanks,
> > Badhri
> >
> > On Wed, Sep 23, 2020 at 3:43 AM Jun Li <lijun.kernel@gmail.com> wrote:
> > >
> > > Badhri Jagan Sridharan <badhri@google.com> 于2020年9月22日周二 上午3:57写道：
> > > >
> > > > This change adds frs-typec-current which allows setting the initial current
> > > > capability of the new source when vSafe5V is applied during PD3.0
> > > > sink Fast Role Swap.
> > > >
> > > > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > > > ---
> > > > Changes since v1:
> > > > - Changing patch version to v6 to fix version number confusion.
> > > >
> > > > Changes since v6:
> > > > - Removed the redundant usb-connector.txt that I created by mistake.
> > > > - Moved to yaml.
> > > >
> > > > Changes since v7:
> > > > - Rebase
> > > > ---
> > > >  .../devicetree/bindings/connector/usb-connector.yaml   |  8 ++++++++
> > > >  include/dt-bindings/usb/pd.h                           | 10 ++++++++++
> > > >  2 files changed, 18 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > index 9bd52e63c935..1ca8e6a337e5 100644
> > > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > @@ -142,6 +142,14 @@ properties:
> > > >      required:
> > > >        - port@0
> > > >
> > > > +  frs-typec-current:
> > > > +    description: Initial current capability of the new source when vSafe5V
> > > > +      is applied during PD3.0 Fast Role Swap. "Table 6-14 Fixed Supply PDO - Sink"
> > > > +      of "USB Power Delivery Specification Revision 3.0, Version 1.2" provides the
> > > > +      different power levels and "6.4.1.3.1.6 Fast Role Swap USB Type-C Current"
> > > > +      provides a detailed description of the field.
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > > If it's a part of sink PDO, I think you don't need a new property for this, just
> > > define it directly into sink-pdos by adding a new PDO define for PD 3.0,
> > > something like:
> > >
> > > sink-pdos = <PDO_FIXED_v3(5000, 3000, PDO_FIXED_USB_COMM, FRS_CURRENT_1P5A)>;
> > >
> > > Li Jun
> > > > +
> > > >  required:
> > > >    - compatible
> > > >
> > > > diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> > > > index 985f2bbd4d24..db1ad4532197 100644
> > > > --- a/include/dt-bindings/usb/pd.h
> > > > +++ b/include/dt-bindings/usb/pd.h
> > > > @@ -35,6 +35,16 @@
> > > >
> > > >  #define VSAFE5V 5000 /* mv units */
> > > >
> > > > +/*
> > > > + * Based on "Table 6-14 Fixed Supply PDO - Sink" of "USB Power Delivery Specification Revision 3.0,
> > > > + * Version 1.2"
> > > > + * Initial current capability of the new source when vSafe5V is applied.
> > > > + */
> > > > +#define FRS_NOT_SUPPORTED      0
> > > > +#define FRS_DEFAULT_POWER      1
> > > > +#define FRS_5V_1P5A            2
> > > > +#define FRS_5V_3A              3
> > > > +
> > > >  #define PDO_BATT_MAX_VOLT_SHIFT        20      /* 50mV units */
> > > >  #define PDO_BATT_MIN_VOLT_SHIFT        10      /* 50mV units */
> > > >  #define PDO_BATT_MAX_PWR_SHIFT 0       /* 250mW units */
> > > > --
> > > > 2.28.0.681.g6f77f65b4e-goog
> > > >
Badhri Jagan Sridharan Sept. 29, 2020, 2:50 a.m. UTC | #4 
On Tue, Sep 22, 2020 at 9:04 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Sep 21, 2020 at 12:55:49PM -0700, Badhri Jagan Sridharan wrote:
> > This change adds frs-typec-current which allows setting the initial current
> > capability of the new source when vSafe5V is applied during PD3.0
> > sink Fast Role Swap.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> > Changes since v1:
> > - Changing patch version to v6 to fix version number confusion.
> >
> > Changes since v6:
> > - Removed the redundant usb-connector.txt that I created by mistake.
> > - Moved to yaml.
> >
> > Changes since v7:
> > - Rebase
> > ---
> >  .../devicetree/bindings/connector/usb-connector.yaml   |  8 ++++++++
> >  include/dt-bindings/usb/pd.h                           | 10 ++++++++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 9bd52e63c935..1ca8e6a337e5 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -142,6 +142,14 @@ properties:
> >      required:
> >        - port@0
> >
> > +  frs-typec-current:
> > +    description: Initial current capability of the new source when vSafe5V
> > +      is applied during PD3.0 Fast Role Swap. "Table 6-14 Fixed Supply PDO - Sink"
> > +      of "USB Power Delivery Specification Revision 3.0, Version 1.2" provides the
> > +      different power levels and "6.4.1.3.1.6 Fast Role Swap USB Type-C Current"
> > +      provides a detailed description of the field.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> Looks the same/similar to this[1]. Please come up with a common
> approach to cover both.
>
> Rob
>
> https://lore.kernel.org/linux-arm-kernel/20200902075707.9052-2-amelie.delaunay@st.com/

Hi Rob,

The values will not be an exact overlap as it's a different
functionality. [1] introduces a property
for defining the power operation mode for the type-c connector.
However [2] introduces a property
to define the new-source's current capability during Fast role swap operation.
However, I modified [2] to use string based enums to follow the
pattern in [1] in v9 version of the
patch which I just sent out.

1. https://lore.kernel.org/linux-arm-kernel/20200902075707.9052-2-amelie.delaunay@st.com/
2. https://lore.kernel.org/patchwork/patch/1309792/

Thanks,
Badhri
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 9bd52e63c935..1ca8e6a337e5 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -142,6 +142,14 @@  properties:
     required:
       - port@0
 
+  frs-typec-current:
+    description: Initial current capability of the new source when vSafe5V
+      is applied during PD3.0 Fast Role Swap. "Table 6-14 Fixed Supply PDO - Sink"
+      of "USB Power Delivery Specification Revision 3.0, Version 1.2" provides the
+      different power levels and "6.4.1.3.1.6 Fast Role Swap USB Type-C Current"
+      provides a detailed description of the field.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 required:
   - compatible
 
diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
index 985f2bbd4d24..db1ad4532197 100644
--- a/include/dt-bindings/usb/pd.h
+++ b/include/dt-bindings/usb/pd.h
@@ -35,6 +35,16 @@ 
 
 #define VSAFE5V 5000 /* mv units */
 
+/*
+ * Based on "Table 6-14 Fixed Supply PDO - Sink" of "USB Power Delivery Specification Revision 3.0,
+ * Version 1.2"
+ * Initial current capability of the new source when vSafe5V is applied.
+ */
+#define FRS_NOT_SUPPORTED	0
+#define FRS_DEFAULT_POWER	1
+#define FRS_5V_1P5A		2
+#define FRS_5V_3A		3
+
 #define PDO_BATT_MAX_VOLT_SHIFT	20	/* 50mV units */
 #define PDO_BATT_MIN_VOLT_SHIFT	10	/* 50mV units */
 #define PDO_BATT_MAX_PWR_SHIFT	0	/* 250mW units */