| Message ID | 20200921195555.1050731-5-badhri@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v8,01/11] usb: typec: tcpci: Add a getter method to retrieve tcpm_port reference | expand |
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/
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 >
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 > >
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 > > >
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 > > > >
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 --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 */
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(+)