linux-kernel - Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL411-qmQ23=hfXX3s5Zo100rSASrByMWR70qLZ8cWqt9hPgSg@mail.gmail.com>
Date:   Thu, 13 Sep 2018 15:27:28 +0800
From:   Peter Chen <hzpeterchen@...il.com>
To:     linux@...ck-us.net
Cc:     angus@...ea.ca, Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
        peter.chen@....com, jun.li@....com
Subject: Re: [PATCH v3] usb: typec: get the vbus source and charge values from
 the devicetree

On Thu, Sep 13, 2018 at 12:35 AM Guenter Roeck <linux@...ck-us.net> 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.

Acked-by: Peter Chen <peter.chen@....com>
Signed-off-by: Li Jun <jun.li@....com>
---
 drivers/staging/typec/tcpci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 2d4fbb8aac5e..7352207224b5 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -381,9 +381,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
bool source, bool sink)
  struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
  int ret;

- /* Disable both source and sink first before enabling anything */
-
- if (!source) {
+ /* Only disable source if it was enabled */
+ if (!source && tcpci->drive_vbus) {
  ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
     TCPC_CMD_DISABLE_SRC_VBUS);
  if (ret < 0)