RE: [PATCH v6] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Felipe-san,
Would you review this patch?
Best regards,
Yoshihiro Shimoda
> From: Heikki Krogerus, Sent: Monday, June 11, 2018 11:09 PM
>
> On Thu, May 31, 2018 at 03:11:33PM +0900, Yoshihiro Shimoda wrote:
> > This patch adds role switch support for R-Car SoCs into the USB 3.0
> > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0
> > dual-role device controller which has the USB 3.0 xHCI host and
> > Renesas USB 3.0 peripheral.
> >
> > Unfortunately, the mode change register (DRD_CON) contains
> > the USB 3.0 peripheral controller side only. So, this renesas_usb3
> > driver manages the DRD_CON now. However, in peripheral mode, the host
> > should stop. Also the host hardware needs to reinitialize its own
> > registers when the mode changes from peripheral to host mode.
> > Otherwise, the host cannot work correctly (e.g. detect a device
> > as high-speed).
> >
> > To achieve this reinitialization by a driver, this driver also
> > registers a role switch driver to manage the DRD_CON and get
> > a device pointer of usb 3.0 host from "companion" property of OF.
> > Then, when the usb role is changed, renesas_usb3_role_switch_set()
> > will attach/release the xhci-plat driver to reinitialize the host
> > hardware.
> >
> > Signed-off-by: Yoshihiro Shimoda
>
> Reviewed-by: Heikki Krogerus
>
> > ---
> > This patch set is based on Felipe's usb.git / testing/next branch
> > (commit id = 47265c067c0d129f3a0e94bc221293a780af9d78).
> >
> > Changes from v5 (https://patchwork.kernel.org/patch/10426483/):
> > - Add a new function "usb3_set_mode_by_role_sw()" instead of reusing
> >usb3_set_mode() and add _usb3_set_mode().
> > - Change a coding style of container_of.
> > - Revise an error message in renesas_usb3_role_switch_set().
> > - Add "const" for "renesas_usb3_role_switch_desc".
> > - Simplify a condition check of usb_of_get_companion_dev().
> >
> > Changes from RFC v4 (https://patchwork.kernel.org/patch/10418265/):
> > - Use "companion" device tree property simply instead of device_connection
> >APIs with OF graph.
> > - Merge patch 2 and 3 to one.
> > - Revise the commit log (I should had revised this on RFC v4 though).
> >
> > Changes from RFC v3:
> > - Rebase latest usb.git / testing/next branch.
> > - Add graph parse into device_connection_find_match().
> > - Use workqueue to call _usb3_set_mode() in patch 3.
> >(I realized renesas_usb3_role_switch_set() cannot run on atomic because
> > device_attach() might sleep.)
> >
> > Changes from RFC v2:
> > - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
> >because hardware resource (a register) is shared and remove individual
> >usb role switch driver/dt-bindings for R-Car.
> > - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
> >doesn't need such API now.
> >
> > Changes from RFC:
> > - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
> > - Remove drivers/of code.
> > - Add a new API for find the connection by using graph on devcon.c and
> > roles.c.
> > - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
> > - Update the dtsi file for r8a7795.
> >
> >
> > drivers/usb/gadget/udc/Kconfig| 1 +
> > drivers/usb/gadget/udc/renesas_usb3.c | 84
> > ++-
> > 2 files changed, 84 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> > index b838cae..78823cd 100644
> > --- a/drivers/usb/gadget/udc/Kconfig
> > +++ b/drivers/usb/gadget/udc/Kconfig
> > @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> > tristate 'Renesas USB3.0 Peripheral controller'
> > depends on ARCH_RENESAS || COMPILE_TEST
> > depends on EXTCON && HAS_DMA
> > + select USB_ROLE_SWITCH
> > help
> >Renesas USB3.0 Peripheral controller is a USB peripheral controller
> >that supports super, high, and full speed USB 3.0 data transfers.
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 5caf78b..f44a4b1 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -23,6 +23,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > /* register definitions */
> > #define USB3_AXI_INT_STA 0x008
> > @@ -335,6 +337,11 @@ struct renesas_usb3 {
> > struct phy *phy;
> > struct dentry *dentry;
> >
> > + struct usb_role_switch *role_sw;
> > + struct device *host_dev;
> > + struct work_struct role_work;
> > + enum usb_role role;
> > +
> > struct renesas_usb3_ep *usb3_ep;
> > int num_usb3_eps;
> >
> > @@ -651,6 +658,14 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
> > }
> > }
> >
> > +static void renesas_usb3_role_work(struct work_struct *work)
> > +{
> > + struct renesas_usb3 *usb3 =
> > + container_of(work, struct renesas_usb3,
Re: [PATCH v6] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Thu, May 31, 2018 at 03:11:33PM +0900, Yoshihiro Shimoda wrote:
> This patch adds role switch support for R-Car SoCs into the USB 3.0
> peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0
> dual-role device controller which has the USB 3.0 xHCI host and
> Renesas USB 3.0 peripheral.
>
> Unfortunately, the mode change register (DRD_CON) contains
> the USB 3.0 peripheral controller side only. So, this renesas_usb3
> driver manages the DRD_CON now. However, in peripheral mode, the host
> should stop. Also the host hardware needs to reinitialize its own
> registers when the mode changes from peripheral to host mode.
> Otherwise, the host cannot work correctly (e.g. detect a device
> as high-speed).
>
> To achieve this reinitialization by a driver, this driver also
> registers a role switch driver to manage the DRD_CON and get
> a device pointer of usb 3.0 host from "companion" property of OF.
> Then, when the usb role is changed, renesas_usb3_role_switch_set()
> will attach/release the xhci-plat driver to reinitialize the host
> hardware.
>
> Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Heikki Krogerus
> ---
> This patch set is based on Felipe's usb.git / testing/next branch
> (commit id = 47265c067c0d129f3a0e94bc221293a780af9d78).
>
> Changes from v5 (https://patchwork.kernel.org/patch/10426483/):
> - Add a new function "usb3_set_mode_by_role_sw()" instead of reusing
>usb3_set_mode() and add _usb3_set_mode().
> - Change a coding style of container_of.
> - Revise an error message in renesas_usb3_role_switch_set().
> - Add "const" for "renesas_usb3_role_switch_desc".
> - Simplify a condition check of usb_of_get_companion_dev().
>
> Changes from RFC v4 (https://patchwork.kernel.org/patch/10418265/):
> - Use "companion" device tree property simply instead of device_connection
>APIs with OF graph.
> - Merge patch 2 and 3 to one.
> - Revise the commit log (I should had revised this on RFC v4 though).
>
> Changes from RFC v3:
> - Rebase latest usb.git / testing/next branch.
> - Add graph parse into device_connection_find_match().
> - Use workqueue to call _usb3_set_mode() in patch 3.
>(I realized renesas_usb3_role_switch_set() cannot run on atomic because
> device_attach() might sleep.)
>
> Changes from RFC v2:
> - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
>because hardware resource (a register) is shared and remove individual
>usb role switch driver/dt-bindings for R-Car.
> - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
>doesn't need such API now.
>
> Changes from RFC:
> - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
> - Remove drivers/of code.
> - Add a new API for find the connection by using graph on devcon.c and
> roles.c.
> - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
> - Update the dtsi file for r8a7795.
>
>
> drivers/usb/gadget/udc/Kconfig| 1 +
> drivers/usb/gadget/udc/renesas_usb3.c | 84
> ++-
> 2 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index b838cae..78823cd 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -193,6 +193,7 @@ config USB_RENESAS_USB3
> tristate 'Renesas USB3.0 Peripheral controller'
> depends on ARCH_RENESAS || COMPILE_TEST
> depends on EXTCON && HAS_DMA
> + select USB_ROLE_SWITCH
> help
> Renesas USB3.0 Peripheral controller is a USB peripheral controller
> that supports super, high, and full speed USB 3.0 data transfers.
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> b/drivers/usb/gadget/udc/renesas_usb3.c
> index 5caf78b..f44a4b1 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -23,6 +23,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> /* register definitions */
> #define USB3_AXI_INT_STA 0x008
> @@ -335,6 +337,11 @@ struct renesas_usb3 {
> struct phy *phy;
> struct dentry *dentry;
>
> + struct usb_role_switch *role_sw;
> + struct device *host_dev;
> + struct work_struct role_work;
> + enum usb_role role;
> +
> struct renesas_usb3_ep *usb3_ep;
> int num_usb3_eps;
>
> @@ -651,6 +658,14 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
> }
> }
>
> +static void renesas_usb3_role_work(struct work_struct *work)
> +{
> + struct renesas_usb3 *usb3 =
> + container_of(work, struct renesas_usb3, role_work);
> +
> + usb_role_switch_set_role(usb3->role_sw, usb3->role);
> +}
> +
> static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
> {
> if (host)
> @@ -659,6 +674,16 @@ static void usb3_set_mode(struct renesas_usb3 *usb3,
> bool host)
> usb3_set_bit(usb3, DRD_CON_PERI_CON,
