Il ven 8 ott 2021, 22:47 Guenter Roeck <linux@xxxxxxxxxxxx> ha scritto:
>
> On 10/8/21 1:14 PM, Gianni Pisetta wrote:
> > The data role check in tcpm.c cause the port manager to enter a loop with a
> > hard reset on some chargers.
> > By the spec in a GoodCRC Message the other end does not need to comply with
> > the data role, so ignore a data role mismatch in a request when the message
> > type is GoodCRC.
> >>From the USB PD spec:
> > "If a USB Type-C Port receive a Message with the Port Data Role field set
> > to the same Data Role, except for the GoodCRC Message, USB Type-C error
> > recovery..."
> >
> > Below are the log of a Pinebook Pro attached to a PinePower Desktop Supply
> > before the patch:
> >
> > [226057.970532] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES [rev3 NONE_AMS]
> > [226057.975891] pending state change SNK_WAIT_CAPABILITIES -> SNK_READY @ 310 ms [rev3 NONE_AMS]
> > [226058.134384] PD RX, header: 0x53a1 [1]
> > [226058.134389] Data role mismatch, initiating error recovery
> > [226058.134392] state change SNK_WAIT_CAPABILITIES -> ERROR_RECOVERY [rev3 NONE_AMS]
> > [226058.134404] state change ERROR_RECOVERY -> PORT_RESET [rev3 NONE_AMS]
> >
> > Fixes: f0690a25a140b
> > cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Gianni Pisetta <gianni.pisetta@xxxxxxxxx>
> >
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 5d05de666597..02ebf7e03c41 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -2887,11 +2887,11 @@ static void tcpm_pd_rx_handler(struct kthread_work *work)
> >
> > /*
> > * If both ends believe to be DFP/host, we have a data role
> > - * mismatch.
> > + * mismatch. Must perform error recovery actions, except for
> > + * a GoodCRC Message(USB PD standard, 6.2.1.6).
> > */
> > - if (!!(le16_to_cpu(msg->header) & PD_HEADER_DATA_ROLE) ==
> > - (port->data_role == TYPEC_HOST)) {
> > + if (!!((le16_to_cpu(msg->header) & PD_HEADER_DATA_ROLE) ==
> > + (port->data_role == TYPEC_HOST) && type != PD_CTRL_GOOD_CRC)) {
>
> Unless I am missing something, this could also be a PD_DATA_SOURCE_CAP
> or PD_EXT_SOURCE_CAP_EXT message.
>
> Guenter
Yes, you are definitely right about that. I thought I had checked.
Please discard this patch.
Would you accept a v2 with a proper check also on PD_HEADER_EXT_HDR
and pd_header_cnt?
Thanks,
Gianni
