Hi Shimoda-San,
Thanks for the feedback.
> Subject: RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
>
> Hi Biju-san,
>
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@xxxxxxxxxxxxxx>
> > ---
> > V1-->V2
> > * Driver uses usb role clas for handling dual role switch and handling
> > connect/disconnect events instead of extcon.
> > ---
>
> Thank you for the patch!
>
> > drivers/usb/gadget/udc/renesas_usb3.c | 125
> > ++++++++++++++++++++++++++++------
> > 1 file changed, 103 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..359a92b 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -351,6 +351,10 @@ struct renesas_usb3 {
> > int disabled_count;
> >
> > struct usb_request *ep0_req;
> > +
> > + struct usb_role_switch *typec_role_sw;
>
> This typec_role_sw is not used.
OK. Will remove this.
>
> > + enum usb_role connection_state;
> > +
> > u16 test_mode;
> > u8 ep0_buf[USB3_EP0_BUF_SIZE];
> > bool softconnect;
> > @@ -359,6 +363,7 @@ struct renesas_usb3 {
> > bool extcon_usb; /* check vbus and set EXTCON_USB
> */
> > bool forced_b_device;
> > bool start_to_connect;
> > + bool usb_role_switch_property;
> > };
> >
> > #define gadget_to_renesas_usb3(_gadget) \
> > @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3
> *usb3)
> > usb3->driver->disconnect(&usb3->gadget);
> > }
> >
> > -static void usb3_check_vbus(struct renesas_usb3 *usb3) -{
> > - if (usb3->workaround_for_vbus) {
> > - usb3_connect(usb3);
> > - } else {
> > - usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> > -
> USB_STA_VBUS_STA);
> > - if (usb3->extcon_usb)
> > - usb3_connect(usb3);
> > - else
> > - usb3_disconnect(usb3);
> > -
> > - schedule_work(&usb3->extcon_work);
> > - }
> > -}
> > -
> > static void renesas_usb3_role_work(struct work_struct *work) {
> > struct renesas_usb3 *usb3 =
> > @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&usb3->lock, flags);
> > - usb3_set_mode_by_role_sw(usb3, host);
> > - usb3_vbus_out(usb3, a_dev);
> > + if ((!usb3->usb_role_switch_property) ||
> > + (usb3->connection_state !=
> USB_ROLE_NONE)) {
>
> We can modify the code as following:
> if (!usb3->usb_role_switch_property ||
> usb3->connection_state != USB_ROLE_NONE) {
OK. Will change this.
> <snip>
> > @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
> > EXTCON_NONE,
> > };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> > .set = renesas_usb3_role_switch_set,
> > .get = renesas_usb3_role_switch_get,
> > .allow_userspace_control = true,
> > @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> > if (ret < 0)
> > goto err_dev_create;
> >
> > + if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> > + usb3->usb_role_switch_property = true;
> > + renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
>
> This causes build error because struct usb_role_switch_desc doesn't have
> fwnode.
I have compiled this code with linux-next and the commit
09aa11cfda9d8186046b ("device connection: Add fwnode member to struct device_connection")
defines the property fwnode. The HD3SS3220 port controller driver uses usb-role-switch property to
get role-switch handle using graph api's.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4
regards,
Biju
