2020-12-01 05:11:58

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: [PATCH v1 1/4] usb: typec: tcpm: Pass down negotiated rev to update retry count

nRetryCount was updated from 3 to 2 between PD2.0 and PD3.0 spec.
nRetryCount in "Table 6-34 Counter parameters" of the PD 2.0
spec is set to 3, whereas, nRetryCount in "Table 6-59 Counter
parameters" is set to 2.

Pass down negotiated rev in pd_transmit so that low level chip
drivers can update the retry count accordingly before attempting
packet transmission.

This helps in passing "TEST.PD.PORT.ALL.02" of the
"Power Delivery Merged" test suite which was initially failing
with "The UUT did not retransmit the message nReryCount times"

Signed-off-by: Badhri Jagan Sridharan <[email protected]>
---
drivers/usb/typec/tcpm/tcpm.c | 2 +-
include/linux/usb/tcpm.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3bbc1f10af49..c73bc3a8356a 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -667,7 +667,7 @@ static int tcpm_pd_transmit(struct tcpm_port *port,
tcpm_log(port, "PD TX, type: %#x", type);

reinit_completion(&port->tx_complete);
- ret = port->tcpc->pd_transmit(port->tcpc, type, msg);
+ ret = port->tcpc->pd_transmit(port->tcpc, type, msg, port->negotiated_rev);
if (ret < 0)
return ret;

diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index e68aaa12886f..efaedd7e8a18 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -121,7 +121,7 @@ struct tcpc_dev {
enum typec_cc_status cc);
int (*try_role)(struct tcpc_dev *dev, int role);
int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
- const struct pd_message *msg);
+ const struct pd_message *msg, unsigned int negotiated_rev);
int (*set_bist_data)(struct tcpc_dev *dev, bool on);
int (*enable_frs)(struct tcpc_dev *dev, bool enable);
void (*frs_sourcing_vbus)(struct tcpc_dev *dev);
--
2.29.2.454.gaff20da3a2-goog


2020-12-01 05:39:47

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: [PATCH v1 3/4] usb: typec: tcpci: Update retry count based on the negotiated rev

By default the driver sets the retry count to 3 (Default for PD 2.0).
Update this to 2, if the negotiated rev is PD 3.0.

Signed-off-by: Badhri Jagan Sridharan <[email protected]>
---
drivers/usb/typec/tcpm/tcpci.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 12d983a75510..98a2455f779f 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -18,7 +18,8 @@

#include "tcpci.h"

-#define PD_RETRY_COUNT 3
+#define PD_RETRY_COUNT_DEFAULT 3
+#define PD_RETRY_COUNT_3_0_OR_HIGHER 2
#define AUTO_DISCHARGE_DEFAULT_THRESHOLD_MV 3500
#define AUTO_DISCHARGE_PD_HEADROOM_MV 850
#define AUTO_DISCHARGE_PPS_HEADROOM_MV 1250
@@ -447,9 +448,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
return 0;
}

-static int tcpci_pd_transmit(struct tcpc_dev *tcpc,
- enum tcpm_transmit_type type,
- const struct pd_message *msg)
+static int tcpci_pd_transmit(struct tcpc_dev *tcpc, enum tcpm_transmit_type type,
+ const struct pd_message *msg, unsigned int negotiated_rev)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
u16 header = msg ? le16_to_cpu(msg->header) : 0;
@@ -497,7 +497,9 @@ static int tcpci_pd_transmit(struct tcpc_dev *tcpc,
}
}

- reg = (PD_RETRY_COUNT << TCPC_TRANSMIT_RETRY_SHIFT) | (type << TCPC_TRANSMIT_TYPE_SHIFT);
+ /* nRetryCount is 3 in PD2.0 spec where 2 in PD3.0 spec */
+ reg = ((negotiated_rev > PD_REV20 ? PD_RETRY_COUNT_3_0_OR_HIGHER : PD_RETRY_COUNT_DEFAULT)
+ << TCPC_TRANSMIT_RETRY_SHIFT) | (type << TCPC_TRANSMIT_TYPE_SHIFT);
ret = regmap_write(tcpci->regmap, TCPC_TRANSMIT, reg);
if (ret < 0)
return ret;
--
2.29.2.454.gaff20da3a2-goog

2020-12-01 22:33:33

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] usb: typec: tcpm: Pass down negotiated rev to update retry count

On Mon, Nov 30, 2020 at 08:22:34PM -0800, Badhri Jagan Sridharan wrote:
> nRetryCount was updated from 3 to 2 between PD2.0 and PD3.0 spec.
> nRetryCount in "Table 6-34 Counter parameters" of the PD 2.0
> spec is set to 3, whereas, nRetryCount in "Table 6-59 Counter
> parameters" is set to 2.
>
> Pass down negotiated rev in pd_transmit so that low level chip
> drivers can update the retry count accordingly before attempting
> packet transmission.
>
> This helps in passing "TEST.PD.PORT.ALL.02" of the
> "Power Delivery Merged" test suite which was initially failing
> with "The UUT did not retransmit the message nReryCount times"
>
> Signed-off-by: Badhri Jagan Sridharan <[email protected]>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 2 +-
> include/linux/usb/tcpm.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3bbc1f10af49..c73bc3a8356a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -667,7 +667,7 @@ static int tcpm_pd_transmit(struct tcpm_port *port,
> tcpm_log(port, "PD TX, type: %#x", type);
>
> reinit_completion(&port->tx_complete);
> - ret = port->tcpc->pd_transmit(port->tcpc, type, msg);
> + ret = port->tcpc->pd_transmit(port->tcpc, type, msg, port->negotiated_rev);
> if (ret < 0)
> return ret;
>
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index e68aaa12886f..efaedd7e8a18 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -121,7 +121,7 @@ struct tcpc_dev {
> enum typec_cc_status cc);
> int (*try_role)(struct tcpc_dev *dev, int role);
> int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
> - const struct pd_message *msg);
> + const struct pd_message *msg, unsigned int negotiated_rev);
> int (*set_bist_data)(struct tcpc_dev *dev, bool on);
> int (*enable_frs)(struct tcpc_dev *dev, bool enable);
> void (*frs_sourcing_vbus)(struct tcpc_dev *dev);

I think this will break bisectability. You need to change the users of
that at in the same commit.

thanks,

--
heikki

2020-12-02 03:22:25

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] usb: typec: tcpm: Pass down negotiated rev to update retry count

Sure. Done ! Just sent out v2 version of the patch.

Thanks,
Badhri.


On Tue, Dec 1, 2020 at 2:32 AM Heikki Krogerus
<[email protected]> wrote:
>
> On Mon, Nov 30, 2020 at 08:22:34PM -0800, Badhri Jagan Sridharan wrote:
> > nRetryCount was updated from 3 to 2 between PD2.0 and PD3.0 spec.
> > nRetryCount in "Table 6-34 Counter parameters" of the PD 2.0
> > spec is set to 3, whereas, nRetryCount in "Table 6-59 Counter
> > parameters" is set to 2.
> >
> > Pass down negotiated rev in pd_transmit so that low level chip
> > drivers can update the retry count accordingly before attempting
> > packet transmission.
> >
> > This helps in passing "TEST.PD.PORT.ALL.02" of the
> > "Power Delivery Merged" test suite which was initially failing
> > with "The UUT did not retransmit the message nReryCount times"
> >
> > Signed-off-by: Badhri Jagan Sridharan <[email protected]>
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 2 +-
> > include/linux/usb/tcpm.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 3bbc1f10af49..c73bc3a8356a 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -667,7 +667,7 @@ static int tcpm_pd_transmit(struct tcpm_port *port,
> > tcpm_log(port, "PD TX, type: %#x", type);
> >
> > reinit_completion(&port->tx_complete);
> > - ret = port->tcpc->pd_transmit(port->tcpc, type, msg);
> > + ret = port->tcpc->pd_transmit(port->tcpc, type, msg, port->negotiated_rev);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index e68aaa12886f..efaedd7e8a18 100644
> > --- a/include/linux/usb/tcpm.h
> > +++ b/include/linux/usb/tcpm.h
> > @@ -121,7 +121,7 @@ struct tcpc_dev {
> > enum typec_cc_status cc);
> > int (*try_role)(struct tcpc_dev *dev, int role);
> > int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
> > - const struct pd_message *msg);
> > + const struct pd_message *msg, unsigned int negotiated_rev);
> > int (*set_bist_data)(struct tcpc_dev *dev, bool on);
> > int (*enable_frs)(struct tcpc_dev *dev, bool enable);
> > void (*frs_sourcing_vbus)(struct tcpc_dev *dev);
>
> I think this will break bisectability. You need to change the users of
> that at in the same commit.
>
> thanks,
>
> --
> heikki