Peter Chen <peter.c...@freescale.com> writes:

> Since we can't load/unload gadget module on the fly, we can't
> de-init gadget structure during the otg switch.
> We have to create gadget at the probe function no matter current
> role is the host or device, of cource, the gadget will not
> be created for host-only port.

No, this is much worse than the previous version.

>
> Signed-off-by: Peter Chen <peter.c...@freescale.com>
> ---
>  drivers/usb/chipidea/ci.h   |   10 +++++++---
>  drivers/usb/chipidea/core.c |    7 ++++++-
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index d32f932..c704aa7 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -188,9 +188,13 @@ static inline int ci_role_start(struct ci13xxx *ci, enum 
> ci_role role)
>       if (!ci->roles[role])
>               return -ENXIO;
>  
> -     ret = ci->roles[role]->start(ci);
> -     if (!ret)
> -             ci->role = role;
> +     /* Only host role is supported at this function*/
> +     if (role == CI_ROLE_HOST) {
> +             ret = ci->roles[role]->start(ci);
> +             if (!ret)
> +                     ci->role = role;
> +     }
> +
>       return ret;
>  }

This is bad in a number of ways. What we need is add a "destroy" method
to the role and not add

  if (ci->role == CI_ROLE_HOST)
    hackity_hackity();

to ten different places in the driver. This is really bad, even worse
than the fact that with this patch you're returning an uninitialized
'ret' in case of non-host, which (correct me if I'm wrong) should
explode straight away, which in turn makes me wonder how is this
patchset fully tested as it claims to be.

Regards,
--
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to