Re: [PATCH v7] usb: common: usb-conn-gpio: Set last role to unknown before initial detection

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Wed, Jun 14, 2023 at 09:55:10AM +0530, Prashanth K wrote:
> 
> 
> On 13-06-23 04:40 pm, Heikki Krogerus wrote:
> > Hi,
> > 
> > On Wed, May 31, 2023 at 08:11:14PM +0530, Prashanth K wrote:
> > > 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, added a new constant in enum usb_role, last_role
> > > is set to USB_ROLE_UNKNOWN before performing initial detection.
> > 
> > So why can't you fix this by just always setting the role
> > unconditionally to USB_ROLE_NONE in your probe function before the
> > initial detection?
> > 
> Hi Heikki, thats exactly what we are doing here.
> 
> +	/* Set last role to unknown before performing the initial detection */
> +	info->last_role = USB_ROLE_UNKNOWN;

No, I'm asking why can't you just call set_role(USB_ROLE_NONE)
(together with any other steps that you need to take in order to fix
you issue) directly from your probe function?

That USB_ROLE_UNKNOWN as a global is not acceptable - there is no
difference between USB_ROLE_UNKNOWN and USB_ROLE_NONE from the role
switch PoW. So if you want to use something like that, you have to
confine it to your driver.

But I honestly don't think you need it at all. You should be able to
refactor your driver in order to solve the issue described in the
commit message without any need for it.

Note! I just realised that you are not modifying
drivers/usb/roles/class.c, so this patch is actually broken.

thanks,

-- 
heikki