Hi > -----Original Message----- > From: Angus Ainslie <angus@xxxxxxxx> > Sent: 2018年9月13日 19:10 > To: Peter Chen <hzpeterchen@xxxxxxxxx> > Cc: linux@xxxxxxxxxxxx; Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>; Greg > Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; lkml > <linux-kernel@xxxxxxxxxxxxxxx>; Peter Chen <peter.chen@xxxxxxx>; Jun Li > <jun.li@xxxxxxx>; Guenter Roeck <linux@xxxxxxxxxxxx> > Subject: Re: [PATCH v3] usb: typec: get the vbus source and charge values from the > devicetree > > On 2018-09-13 01:27, Peter Chen wrote: > > On Thu, Sep 13, 2018 at 12:35 AM Guenter Roeck <linux@xxxxxxxxxxxx> > > wrote: > >> > >> On Wed, Sep 12, 2018 at 10:08:58AM -0600, Angus Ainslie wrote: > >> > On 2018-09-11 09:33, Guenter Roeck wrote: > >> > >I cant put my finger on it but this seems wrong. As i said both > >> > >src and sink should never be true at the same time. I also din’t > >> > >understand why turning off src should power off your board. > >> > >Ultimately my concern is that we may be just painting over the > >> > >real problem, and that would be really bad to do with dt properties. > >> > > > >> > > >> > I agree that this doesn't seem like the correct way of solving the problem. > >> > On this HW (Emcraft iMX8M BSB) I think the PTN5110 chip has been > >> > connected correctly so I'm assuming that it is some quirk of the PTN5110. > >> > > >> > I didn't design the HW or the chip. This is a workaround for "quirky" > >> > hardware and there may be others that don't behave exactly as expected. > >> > > >> > >> I wouldn't be that sure about that. It may as well be that the tcpc > >> driver and/or the tcpm driver are doing something wrong when > >> initializing. > >> > >> I didn't really understand the logs you sent out earlier. It looked > >> like the system would loose power if the TCPC_CMD_DISABLE_SRC_VBUS > >> command is sent. That doesn't really make sense to me since it > >> indicates that the chip sources power to the remote, and turning that > >> off should not result in a local loss of power. > >> > >> Note that the chip is supposed to be able to report if it is sourcing > >> vbus and if VBUS is present, in the POWER_STATUS register. Another > >> question is the content of the ROLE_CONTROL register when the system > >> boots, and the DEVICE_CAPABILITIES settings. > >> > >> Overall I suspect that we don't handle startup for your system > >> correctly in the tcpc driver. The ideal solution would be to find a > >> solution which does not require any devicetree properties, but to do > >> that we'll need to get a better understanding about your system's > >> requirements. > >> > >> Guenter > > > > Hi Angus, > > > > Would you please check if below patch can fix your issue? > > > > staging: typec: don't do vbus source disable for dead battery > > > > In PTN5110 design, DisableSourceVBUS command also disables the sink > > enable signal because the EN_SNK can be used to source higher voltage, > > and, there is only one TCPC command to disable sourcing voltage > > without telling whether to disable 5V or the high voltage, and to keep > > the design simple they designed the PTN5110 to disable both. with this > > fact, we use the flag drive_vbus to check if the source vbus enable > > was issued, if yes we then do vbus source disable, in dead battery > > case, we never did vbus source enable, so will not issue vbus source > > disable command. > > > > Thanks Peter, this sounds like the missing piece of information and I think some form of > the code below will fix that. > > There is still the issue that my board will need some way of controlling the initial state of > vbus-sink. > > @Guenter: would my initial patch be acceptable to set the default state of vbus-source and > vbus-sink. Would you like some code to sanity check that both were not enabled at the > same time ? I agree Guenter using a dt property is not the proper way here(maybe valid for your HW with only typec power supply), I think a right way is to change tcpm to handle this kind case(like the existing vbus_never_low flag) so we will have a common handling if (vbus_on && CC_state_is_RP) dead_battery = true; bypass tcpm_init_vbus or only enable sink.
