Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <df3ddd18-f893-23d3-f035-d2f3572f1258@quicinc.com>
Date:   Wed, 24 May 2023 19:06:41 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] usb: common: usb-conn-gpio: Set last role to unknown
 before initial detection
Content-Language: en-US
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matthias Brugger <matthias.bgg@gmail.com>
CC:     <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <1684918441-14794-1-git-send-email-quic_prashk@quicinc.com>
 <ebbbee51-8792-6c35-c29b-adb7f83dda23@collabora.com>
From:   Prashanth K <quic_prashk@quicinc.com>
In-Reply-To: <ebbbee51-8792-6c35-c29b-adb7f83dda23@collabora.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 24-05-23 03:10 pm, AngeloGioacchino Del Regno wrote:
> Il 24/05/23 10:54, Prashanth K ha scritto:
>> Currently if we bootup a device without cable connected, then
>> usb-conn-gpio won't call set_role() since last_role is same as
>> current role. This happens because during probe last_role gets
>> initialised to zero.
>>
>> To avoid this, last_role is set to USB_ROLE_UNKNOWN before
>> performing the initial detection.
>>
>> Fixes: 4602f3bff266 ("usb: common: add USB GPIO based connection 
>> detection driver")
>> Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
>> ---
>>   drivers/usb/common/usb-conn-gpio.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/common/usb-conn-gpio.c 
>> b/drivers/usb/common/usb-conn-gpio.c
>> index e20874c..00af735 100644
>> --- a/drivers/usb/common/usb-conn-gpio.c
>> +++ b/drivers/usb/common/usb-conn-gpio.c
>> @@ -27,6 +27,8 @@
>>   #define USB_CONN_IRQF    \
>>       (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT)
>> +#define USB_ROLE_UNKNOWN    -1
> 
> I would add USB_ROLE_UNKNOWN to `enum usb_role` instead.
> 
> enum usb_role {
>      USB_ROLE_UNKNOWN = -1,
>      USB_ROLE_NONE,
>      ....
> };
> 
> Regards,
> Angelo

Thanks for the suggestion Agnelo. Will change it in next patch.

Regards,
Prashanth K