Re: [Bug ?] usb :typec :tcpm :fusb302

[Markus Reichl <m.reichl@fivetechno.de>]

Hi Guenter,

Am 20.01.20 um 21:26 schrieb Guenter Roeck:
> On 1/20/20 12:14 PM, Markus Reichl wrote:
>> Hi Guenter,
>>
>> Am 20.01.20 um 17:04 schrieb Guenter Roeck:
>>> On 1/20/20 6:34 AM, Markus Reichl wrote:
>>>> Hi Guenter,
>>>>
>>>> Am 20.01.20 um 15:21 schrieb Guenter Roeck:
>>>>> On 1/20/20 3:58 AM, Heikki Krogerus wrote:
>>>>>> Hi Markus,
>>>>>>
>>>>>> On Thu, Jan 09, 2020 at 05:29:07PM +0100, Markus Reichl wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I'm working with a ROC-RK3399-PC arm64 board from firefly, circuit sheet [1].
>>>>>>> The board is powered from an USB-C type connector via an FUSB302 PD controller.
>>>>>>> With measured 15W+ power consumption it should use higher voltage PD modes than
>>>>>>> the standard 5V USB-C mode.
>>>>>>>
>>>>>>> When I add the related connector node in DTS [2] the FUSB302 initializes
>>>>>>> the right PD mode (e.g. 15V/3A).
>>>>>>>
>>>>>>> But during initialisation the PD is switched off shortly and the board has a blackout.
>>>>>>> When I inject a backup supply voltage behind the FUSB302 (e.g. at SYS_12V line) during boot
>>>>>>> I can remove the backup after succesfull setting up the PD and the board will run fine.
>>>>>>>
>>>>>>> Is it possible to change the behaviour of the fusb302 driver to not power down the PD supply
>>>>>>> during init?
>>>>>>
>>>>>> I guess it's also possible that the problem is with tcpm.c instead of
>>>>>> fusb302.c. tcpm.c provides the USB PD state matchines. Guenter! Can
>>>>>> you take a look at this?
>>>>>>
>>>>>
>>>>> There was always a problem with handoff from the bootloader. tcpm_init() calls
>>>>> tcpm_reset_port() which turns vbus and vconn off, which I imagine can
>>>>> trigger the situation.
>>>>>
>>>>> Unfortunately I was never able to solve the puzzle. The Type-C protocol does
>>>>> not support any kind of "hand-off" from one component in the system to another.
>>>>> If the state machine doesn't start from a clean state, there is pretty
>>>>> much no guarantee that it ever synchronizes.
>>>>>
>>>>> Maybe someone can find a better solution, but when I wrote the code I just
>>>>> could not get it to work reliably without resetting everything during
>>>>> registration.
>>>>>
>>>>> Note that v4.4 did not include the upstream tcpm code, suggesting the
>>>>> code in the vendor kernel was possibly using a different or backported
>>>>> state machine. Impossible to say what was done there without access
>>>>> to the code.
>>>>
>>>> The vendor code for fusb302 is here:
>>>> https://github.com/FireflyTeam/kernel/tree/rk3399/firefly/drivers/mfd
>>>>
>>>
>>> AFAICS the vendor code don't reset VBUS, and selectively (only) resets the
>>> PD state machine in the fusb302 on startup. The tcpm state machine is embedded
>>> in the fusb302 driver, making this easier to control.
>>>
>>> The fusb302 Linux kernel driver, on the other side, resets the entire fusb302
>>> on initialization, not just PD (bit 0 of the reset register). Question is if
>>> that can be changed to just reset PD (bit 1 of the reset register).
>>> Maybe that would already fix the problem. Can you give it a try ?
>>>
>>> Guenter
>>
>> I tried
>>
>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
>> index ed8655c6af8c..6e15e7b22064 100644
>> --- a/drivers/usb/typec/tcpm/fusb302.c
>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>> @@ -334,11 +334,11 @@ static int fusb302_sw_reset(struct fusb302_chip *chip)
>>          int ret = 0;
>>            ret = fusb302_i2c_write(chip, FUSB_REG_RESET,
>> -                               FUSB_REG_RESET_SW_RESET);
>> +                               FUSB_REG_RESET_PD_RESET);
>>          if (ret < 0)
>> -               fusb302_log(chip, "cannot sw reset the chip, ret=%d", ret);
>> +               fusb302_log(chip, "cannot pd reset the chip, ret=%d", ret);
>>          else
>> -               fusb302_log(chip, "sw reset");
>> +               fusb302_log(chip, "pd reset");
>>            return ret;
>>   }
>>
>> but did not help, after mmc and ehci initializing the PD-supply gets switched off at 1.95s.
> 
> Next step to try would be to skip vbus initialization - drop tcpm_init_vbus()
> from tcpm_reset_port(). Can you do that as well ?
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index f3087ef8265c..db2a75d67bc7 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2698,7 +2698,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
        port->rx_msgid = -1;
 
        port->tcpc->set_pd_rx(port->tcpc, false);
-       tcpm_init_vbus(port);   /* also disables charging */
+//     tcpm_init_vbus(port);   /* also disables charging */
        tcpm_init_vconn(port);
        tcpm_set_current_limit(port, 0, 0);
        tcpm_set_polarity(port, TYPEC_POLARITY_CC1);

Did not help, but instead of switching off, reboots now most of the time.
I watch with an USB-C-Power meter (V and A).
> 
> Thanks,
> Guenter