Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] usb: typec: don't disable sink or source on
 initialization
To:     Angus Ainslie <angus@akkea.ca>
Cc:     Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Guenter Roeck <groeck7@gmail.com>
References: <20180906192644.24587-1-angus@akkea.ca>
 <20180907103438.GI25121@kuha.fi.intel.com>
 <0adff11c-4836-4035-39a7-1cd2ea3ff74d@roeck-us.net>
 <00ed8e5a4ae0dd40181b8de534684960@www.akkea.ca>
 <690375f9-b721-51d5-7292-59acff44af7c@roeck-us.net>
 <363bd1128780d8de8e560b1b82ce361a@www.akkea.ca>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <e48e7ff0-751c-a559-138d-0bb9411e18b0@roeck-us.net>
Date:   Sun, 9 Sep 2018 07:43:44 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <363bd1128780d8de8e560b1b82ce361a@www.akkea.ca>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 09/09/2018 07:36 AM, Angus Ainslie wrote:
> On 2018-09-09 08:20, Guenter Roeck wrote:
>> On 09/09/2018 07:08 AM, Angus Ainslie wrote:
>>> Hi Guenter
>>>
>>> On 2018-09-07 06:55, Guenter Roeck wrote:
>>>> On 09/07/2018 03:34 AM, Heikki Krogerus wrote:
>>>>> +Guenter
>>>>>
>>>>> On Thu, Sep 06, 2018 at 01:26:44PM -0600, Angus Ainslie (Purism) wrote:
>>>>>> If the board is being powered by USB disabling the source and sink
>>>>>> can remove power from the board. Default to source and sink enabled.
>>>>>>
>>>>
>>>> Seems to me that might violate the specification, and cause trouble for systems
>>>> where vbus has to be off initially. It may be better to provide this kind of
>>>> information as configuration parameter instead of imposing it on everyone.
>>>>
>>>
>>> It felt like it would not be the correct thing to do either. I've tried re-arranging the code in tcpci.c to enable the sink before disabling the source but the only way I found to not disable the power was by setting both of those to true.
>>>
>>> What about adding some device tree entries for the initial vbus state and default to false if they are not present ?
>>>
>>> init-vbus-source and init-vbus-charge ?
>>>
>>
>> Yes, I think we should do something along that line. Another question is
>> if we could or should optionally pull the current state from the low level
>> driver, but that may be secondary.
>>
> 
> I thought of trying to do that but from reading the "PTN5110N PD PHY application programming guide" there didn't seem to be a direct way to read those parameters.
> 

Let's put that on the backburner (maybe it works for some other chips)
and go with the devicetree property for now.

Guenter


>> Thanks,
>> Guenter
>>
>>> Angus
>>>
>>>> Guenter
>>>>
>>>>>> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
>>>>>> ---
>>>>>>   drivers/usb/typec/tcpm.c | 8 +++++---
>>>>>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
>>>>>> index ca7bedb46f7f..a1b819cf31da 100644
>>>>>> --- a/drivers/usb/typec/tcpm.c
>>>>>> +++ b/drivers/usb/typec/tcpm.c
>>>>>> @@ -2462,9 +2462,11 @@ static int tcpm_init_vbus(struct tcpm_port *port)
>>>>>>   {
>>>>>>       int ret;
>>>>>>   -    ret = port->tcpc->set_vbus(port->tcpc, false, false);
>>>>>> -    port->vbus_source = false;
>>>>>> -    port->vbus_charge = false;
>>>>>> +    /* default to source and sink enabled in case USB is our only power
>>>>>> +     * source */
>>>>
>>>> I am personally in favor of standard multi-line comments.
>>>>
>>>>>> +    ret = port->tcpc->set_vbus(port->tcpc, true, true);
>>>>>> +    port->vbus_source = true;
>>>>>> +    port->vbus_charge = true;
>>>>>>       return ret;
>>>>>>   }
>>>>>
>>>
>>>
> 
>