Peter Chen <peter.chen@xxxxxxxxxxxxx> 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.chen@xxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
