[PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

The R-Car H2 and M2 SoCs come with an xHCI controller that requires
some specific initilization related to the firmware downloading and
some specific registers. This patch adds the support for this special
configuration as an xHCI quirk executed during probe and start.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/host/Kconfig     |    8 ++
 drivers/usb/host/Makefile    |    3 +
 drivers/usb/host/xhci-plat.c |   18 +++++
 drivers/usb/host/xhci-rcar.c |  182 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-rcar.h |   28 +++++++
 5 files changed, 239 insertions(+)
 create mode 100644 drivers/usb/host/xhci-rcar.c
 create mode 100644 drivers/usb/host/xhci-rcar.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 9247ad2..229e968 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -37,6 +37,14 @@ config USB_XHCI_MVEBU
 	  Say 'Y' to enable the support for the xHCI host controller
 	  found in Marvell Armada 375/38x ARM SOCs.

+config USB_XHCI_RCAR
+	tristate "xHCI support for Renesas R-Car SoCs"
+	select USB_XHCI_PLATFORM
+	depends on ARCH_SHMOBILE || COMPILE_TEST
+	---help---
+	  Say 'Y' to enable the support for the xHCI host controller
+	  found in Renesas R-Car ARM SoCs.
+
 endif # USB_XHCI_HCD

 config USB_EHCI_HCD
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 7c0886a..b59ca3c 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -22,6 +22,9 @@ ifneq ($(CONFIG_USB_XHCI_PLATFORM), )
 ifneq ($(CONFIG_USB_XHCI_MVEBU), )
 	xhci-hcd-y		+= xhci-mvebu.o
 endif
+ifneq ($(CONFIG_USB_XHCI_RCAR), )
+	xhci-hcd-y		+= xhci-rcar.o
+endif
 endif

 obj-$(CONFIG_USB_WHCI_HCD)	+= whci/
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 561d07e..3a2da1f 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -20,6 +20,7 @@

 #include "xhci.h"
 #include "xhci-mvebu.h"
+#include "xhci-rcar.h"

 static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
@@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)

 static int xhci_plat_start(struct usb_hcd *hcd)
 {
+	struct device_node *of_node = hcd->self.controller->of_node;
+
+	if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
+	    of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
+		xhci_rcar_start(hcd);
+
 	return xhci_run(hcd);
 }

@@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto unmap_registers;
 	}

+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "renesas,r8a7790-xhci") ||
+	    of_device_is_compatible(pdev->dev.of_node,
+				    "renesas,r8a7791-xhci")) {
+		ret = xhci_rcar_init_quirk(pdev);
+		if (ret)
+			goto disable_clk;
+	}
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto disable_clk;
@@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
 	{ .compatible = "xhci-platform" },
 	{ .compatible = "marvell,armada-375-xhci"},
 	{ .compatible = "marvell,armada-380-xhci"},
+	{ .compatible = "renesas,r8a7790-xhci"},
+	{ .compatible = "renesas,r8a7791-xhci"},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
new file mode 100644
index 0000000..3c0bfae
--- /dev/null
+++ b/drivers/usb/host/xhci-rcar.c
@@ -0,0 +1,182 @@
+/*
+ * xHCI host controller driver for R-Car SoCs
+ *
+ * Copyright (C) 2014 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/usb/phy.h>
+
+#include "xhci.h"
+#include "xhci-rcar.h"
+
+#define FIRMWARE_NAME		"r8a779x_usb3_v1.dlmem"
+
+/*** Register Offset ***/
+#define RCAR_USB3_INT_ENA	0x224	/* Interrupt Enable */
+#define RCAR_USB3_DL_CTRL	0x250	/* FW Download Control & Status */
+#define RCAR_USB3_FW_DATA0	0x258	/* FW Data0 */
+
+#define RCAR_USB3_LCLK		0xa44	/* LCLK Select */
+#define RCAR_USB3_CONF1		0xa48	/* USB3.0 Configuration1 */
+#define RCAR_USB3_CONF2		0xa5c	/* USB3.0 Configuration2 */
+#define RCAR_USB3_CONF3		0xaa8	/* USB3.0 Configuration3 */
+#define RCAR_USB3_RX_POL	0xab0	/* USB3.0 RX Polarity */
+#define RCAR_USB3_TX_POL	0xab8	/* USB3.0 TX Polarity */
+
+/*** Register Settings ***/
+/* Interrupt Enable */
+#define RCAR_USB3_INT_XHC_ENA	0x00000001
+#define RCAR_USB3_INT_PME_ENA	0x00000002
+#define RCAR_USB3_INT_HSE_ENA	0x00000004
+#define RCAR_USB3_INT_ENA_VAL	(RCAR_USB3_INT_XHC_ENA | \
+				RCAR_USB3_INT_PME_ENA | RCAR_USB3_INT_HSE_ENA)
+
+/* FW Download Control & Status */
+#define RCAR_USB3_DL_CTRL		0x250
+#define RCAR_USB3_DL_CTRL_ENABLE	0x00000001
+#define RCAR_USB3_DL_CTRL_FW_SUCCESS	0x00000010
+#define RCAR_USB3_DL_CTRL_FW_SET_DATA0	0x00000100
+
+/* LCLK Select */
+#define RCAR_USB3_LCLK_ENA_VAL	0x01030001
+
+/* USB3.0 Configuraion */
+#define RCAR_USB3_CONF1_VAL	0x00030204
+#define RCAR_USB3_CONF2_VAL	0x00030300
+#define RCAR_USB3_CONF3_VAL	0x13802007
+
+/* USB3.0 Polarity */
+#define RCAR_USB3_RX_POL_VAL	0x00020000
+#define RCAR_USB3_TX_POL_VAL	0x00000010
+
+int xhci_rcar_start(struct usb_hcd *hcd)
+{
+	if (hcd->regs != NULL) {
+		u32 temp;
+		/* Interrupt Enable */
+		temp = readl(hcd->regs + RCAR_USB3_INT_ENA);
+		temp |= RCAR_USB3_INT_ENA_VAL;
+		writel(temp, hcd->regs + RCAR_USB3_INT_ENA);
+		/* LCLK Select */
+		writel(RCAR_USB3_LCLK_ENA_VAL, hcd->regs + RCAR_USB3_LCLK);
+		/* USB3.0 Configuration */
+		writel(RCAR_USB3_CONF1_VAL, hcd->regs + RCAR_USB3_CONF1);
+		writel(RCAR_USB3_CONF2_VAL, hcd->regs + RCAR_USB3_CONF2);
+		writel(RCAR_USB3_CONF3_VAL, hcd->regs + RCAR_USB3_CONF3);
+		/* USB3.0 Polariy */
+		writel(RCAR_USB3_RX_POL_VAL, hcd->regs + RCAR_USB3_RX_POL);
+		writel(RCAR_USB3_TX_POL_VAL, hcd->regs + RCAR_USB3_TX_POL);
+	}
+
+	return 0;
+}
+
+static int xhci_rcar_download_firmware(struct device *dev, void __iomem *regs)
+{
+	const struct firmware *fw;
+	int retval, index, j, time;
+	int timeout = 10000;
+	u32 data, val, temp;
+
+	/* request R-Car USB3.0 firmware */
+	retval = request_firmware(&fw, FIRMWARE_NAME, dev);
+	if (retval)
+		return retval;
+
+	/* download R-Car USB3.0 firmware */
+	temp = readl(regs + RCAR_USB3_DL_CTRL);
+	temp |= RCAR_USB3_DL_CTRL_ENABLE;
+	writel(temp, regs + RCAR_USB3_DL_CTRL);
+
+	for (index = 0; index < fw->size; index += 4) {
+		for (data = 0, j = 3; j >= 0; j--) {
+			if ((j + index) >= fw->size)
+				continue;
+			data |= fw->data[index + j] << (8 * j);
+		}
+		writel(data, regs + RCAR_USB3_FW_DATA0);
+		temp = readl(regs + RCAR_USB3_DL_CTRL);
+		temp |= RCAR_USB3_DL_CTRL_FW_SET_DATA0;
+		writel(temp, regs + RCAR_USB3_DL_CTRL);
+
+		for (time = 0; time < timeout; time++) {
+			val = readl(regs + RCAR_USB3_DL_CTRL);
+			if ((val & RCAR_USB3_DL_CTRL_FW_SET_DATA0) == 0)
+				break;
+			udelay(1);
+		}
+		if (time == timeout) {
+			retval = -ETIMEDOUT;
+			break;
+		}
+	}
+
+	temp = readl(regs + RCAR_USB3_DL_CTRL);
+	temp &= ~RCAR_USB3_DL_CTRL_ENABLE;
+	writel(temp, regs + RCAR_USB3_DL_CTRL);
+
+	for (time = 0; time < timeout; time++) {
+		val = readl(regs + RCAR_USB3_DL_CTRL);
+		if (val & RCAR_USB3_DL_CTRL_FW_SUCCESS) {
+			retval = 0;
+			break;
+		}
+		udelay(1);
+	}
+	if (time == timeout)
+		retval = -ETIMEDOUT;
+
+	release_firmware(fw);
+
+	return retval;
+}
+
+int xhci_rcar_init_quirk(struct platform_device *pdev)
+{
+	struct usb_phy *phy = NULL;
+	struct resource *res;
+	void __iomem	*regs;
+	int		ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	regs = ioremap_nocache(res->start, resource_size(res));
+	if (!regs) {
+		dev_dbg(&pdev->dev, "error mapping memory\n");
+		return -EFAULT;
+	}
+
+	/* We have to initialize the "usb phy" to download the firmware here */
+	if (IS_ENABLED(CONFIG_USB_PHY)) {
+		phy = usb_get_phy_dev(&pdev->dev, 0);
+		if (IS_ERR(phy)) {
+			ret = PTR_ERR(phy);
+			goto out;
+		} else {
+			ret = usb_phy_init(phy);
+			if (ret) {
+				usb_put_phy(phy);
+				goto out;
+			}
+		}
+	}
+
+	ret = xhci_rcar_download_firmware(&pdev->dev, regs);
+
+	/* usb_add_hcd() calls usb_get_phy_dev() again */
+	usb_put_phy(phy);
+
+out:
+	iounmap(regs);
+
+	return ret;
+}
diff --git a/drivers/usb/host/xhci-rcar.h b/drivers/usb/host/xhci-rcar.h
new file mode 100644
index 0000000..40f9b20
--- /dev/null
+++ b/drivers/usb/host/xhci-rcar.h
@@ -0,0 +1,28 @@
+/*
+ * drivers/usb/host/xhci-rcar.h
+ *
+ * Copyright (C) 2014 Renesas Electronics Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#ifndef _XHCI_RCAR_H
+#define _XHCI_RCAR_H
+
+#if IS_ENABLED(CONFIG_USB_XHCI_RCAR)
+int xhci_rcar_start(struct usb_hcd *hcd);
+int xhci_rcar_init_quirk(struct platform_device *pdev);
+#else
+static inline int xhci_rcar_start(struct usb_hcd *hcd)
+{
+	return 0;
+}
+
+static inline int xhci_rcar_init_quirk(struct platform_device *pdev)
+{
+	return 0;
+}
+#endif
+#endif /* _XHCI_RCAR_H */
-- 
1.7.9.5

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Magnus Damm]

Hi Shimoda-san,

Thanks for your patches, I did however find one typo below:

On Mon, May 19, 2014 at 7:08 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> The R-Car H2 and M2 SoCs come with an xHCI controller that requires
> some specific initilization related to the firmware downloading and
> some specific registers. This patch adds the support for this special
> configuration as an xHCI quirk executed during probe and start.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/usb/host/Kconfig     |    8 ++
>  drivers/usb/host/Makefile    |    3 +
>  drivers/usb/host/xhci-plat.c |   18 +++++
>  drivers/usb/host/xhci-rcar.c |  182 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci-rcar.h |   28 +++++++
>  5 files changed, 239 insertions(+)
>  create mode 100644 drivers/usb/host/xhci-rcar.c
>  create mode 100644 drivers/usb/host/xhci-rcar.h
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 9247ad2..229e968 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -37,6 +37,14 @@ config USB_XHCI_MVEBU
>           Say 'Y' to enable the support for the xHCI host controller
>           found in Marvell Armada 375/38x ARM SOCs.
>
> +config USB_XHCI_RCAR
> +       tristate "xHCI support for Renesas R-Car SoCs"
> +       select USB_XHCI_PLATFORM
> +       depends on ARCH_SHMOBILE || COMPILE_TEST
> +       ---help---
> +         Say 'Y' to enable the support for the xHCI host controller
> +         found in Renesas R-Car ARM SoCs.
> +
>  endif # USB_XHCI_HCD
>
>  config USB_EHCI_HCD
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 7c0886a..b59ca3c 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -22,6 +22,9 @@ ifneq ($(CONFIG_USB_XHCI_PLATFORM), )
>  ifneq ($(CONFIG_USB_XHCI_MVEBU), )
>         xhci-hcd-y              += xhci-mvebu.o
>  endif
> +ifneq ($(CONFIG_USB_XHCI_RCAR), )
> +       xhci-hcd-y              += xhci-rcar.o
> +endif
>  endif
>
>  obj-$(CONFIG_USB_WHCI_HCD)     += whci/
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 561d07e..3a2da1f 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -20,6 +20,7 @@
>
>  #include "xhci.h"
>  #include "xhci-mvebu.h"
> +#include "xhci-rcar.h"
>
>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> +       struct device_node *of_node = hcd->self.controller->of_node;
> +
> +       if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> +           of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> +               xhci_rcar_start(hcd);
> +

This is most likely a typo - I believe this is supposed to be r8a7790
and r8a7791?

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Geert Uytterhoeven]

Hi Shimoda-san,

On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> The R-Car H2 and M2 SoCs come with an xHCI controller that requires
> some specific initilization related to the firmware downloading and
> some specific registers. This patch adds the support for this special
> configuration as an xHCI quirk executed during probe and start.

Thanks for your patch!

> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 9247ad2..229e968 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -37,6 +37,14 @@ config USB_XHCI_MVEBU
>           Say 'Y' to enable the support for the xHCI host controller
>           found in Marvell Armada 375/38x ARM SOCs.
>
> +config USB_XHCI_RCAR
> +       tristate "xHCI support for Renesas R-Car SoCs"
> +       select USB_XHCI_PLATFORM
> +       depends on ARCH_SHMOBILE || COMPILE_TEST
> +       ---help---
> +         Say 'Y' to enable the support for the xHCI host controller
> +         found in Renesas R-Car ARM SoCs.

Does R-Car Gen1 also have xHCI, and is it compatible?
If not, you may want to call this driver USB_XHCI_RCAR2.

> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 561d07e..3a2da1f 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c

> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> +       struct device_node *of_node = hcd->self.controller->of_node;
> +
> +       if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> +           of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))

r8a7791, as Magnus already pointed out.

> +               xhci_rcar_start(hcd);

If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy
function, but the of_device_is_compatible() checks will still be compiled in.

Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here,
possibly combined with inclusion of a C-source file, like is done in
drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this,
though.

> +
>         return xhci_run(hcd);
>  }
>
> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>                         goto unmap_registers;
>         }
>
> +       if (of_device_is_compatible(pdev->dev.of_node,
> +                                   "renesas,r8a7790-xhci") ||
> +           of_device_is_compatible(pdev->dev.of_node,
> +                                   "renesas,r8a7791-xhci")) {
> +               ret = xhci_rcar_init_quirk(pdev);

Same here.

> +               if (ret)
> +                       goto disable_clk;
> +       }
> +
>         ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>         if (ret)
>                 goto disable_clk;

> --- /dev/null
> +++ b/drivers/usb/host/xhci-rcar.c

> +/* USB3.0 Configuraion */

Configuration

> +static int xhci_rcar_download_firmware(struct device *dev, void __iomem *regs)
> +{
> +       const struct firmware *fw;
> +       int retval, index, j, time;
> +       int timeout = 10000;
> +       u32 data, val, temp;
> +
> +       /* request R-Car USB3.0 firmware */
> +       retval = request_firmware(&fw, FIRMWARE_NAME, dev);
> +       if (retval)
> +               return retval;
> +
> +       /* download R-Car USB3.0 firmware */
> +       temp = readl(regs + RCAR_USB3_DL_CTRL);
> +       temp |= RCAR_USB3_DL_CTRL_ENABLE;
> +       writel(temp, regs + RCAR_USB3_DL_CTRL);
> +
> +       for (index = 0; index < fw->size; index += 4) {
> +               for (data = 0, j = 3; j >= 0; j--) {
> +                       if ((j + index) >= fw->size)
> +                               continue;
> +                       data |= fw->data[index + j] << (8 * j);
> +               }

This is your custom get_unaligned_le32(), to avoid reading beyond the end
of the buffer if its size is not a multiple of 4 bytes?
Is there some way to just use get_unaligned_le32()?

If you want to keep it, I would rewrite it as

              for (data = 0, j = 3; j >= 0; j--) {
                      if ((j + index) < fw->size)
                              data |= fw->data[index + j] << (8 * j);
              }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torval

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Sergei Shtylyov]

Hello.

On 19-05-2014 14:08, Yoshihiro Shimoda wrote:

> The R-Car H2 and M2 SoCs come with an xHCI controller that requires
> some specific initilization related to the firmware downloading and
> some specific registers. This patch adds the support for this special
> configuration as an xHCI quirk executed during probe and start.

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
[...]

> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 561d07e..3a2da1f 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -20,6 +20,7 @@
>
>   #include "xhci.h"
>   #include "xhci-mvebu.h"
> +#include "xhci-rcar.h"
>
>   static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>   {
> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>
>   static int xhci_plat_start(struct usb_hcd *hcd)
>   {
> +	struct device_node *of_node = hcd->self.controller->of_node;
> +
> +	if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> +	    of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))

    Perhaps "renesas,r8a7791-xhci"?

> +		xhci_rcar_start(hcd);
> +
>   	return xhci_run(hcd);
>   }
>
[...]
> diff --git a/drivers/usb/host/xhci-rcar.c b/drivers/usb/host/xhci-rcar.c
> new file mode 100644
> index 0000000..3c0bfae
> --- /dev/null
> +++ b/drivers/usb/host/xhci-rcar.c
> @@ -0,0 +1,182 @@
> +/*
> + * xHCI host controller driver for R-Car SoCs
> + *
> + * Copyright (C) 2014 Renesas Electronics Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/usb/phy.h>
> +
> +#include "xhci.h"
> +#include "xhci-rcar.h"
> +
> +#define FIRMWARE_NAME		"r8a779x_usb3_v1.dlmem"
> +
> +/*** Register Offset ***/
> +#define RCAR_USB3_INT_ENA	0x224	/* Interrupt Enable */
> +#define RCAR_USB3_DL_CTRL	0x250	/* FW Download Control & Status */
> +#define RCAR_USB3_FW_DATA0	0x258	/* FW Data0 */
> +
> +#define RCAR_USB3_LCLK		0xa44	/* LCLK Select */
> +#define RCAR_USB3_CONF1		0xa48	/* USB3.0 Configuration1 */
> +#define RCAR_USB3_CONF2		0xa5c	/* USB3.0 Configuration2 */
> +#define RCAR_USB3_CONF3		0xaa8	/* USB3.0 Configuration3 */
> +#define RCAR_USB3_RX_POL	0xab0	/* USB3.0 RX Polarity */
> +#define RCAR_USB3_TX_POL	0xab8	/* USB3.0 TX Polarity */
> +
> +/*** Register Settings ***/
> +/* Interrupt Enable */
> +#define RCAR_USB3_INT_XHC_ENA	0x00000001
> +#define RCAR_USB3_INT_PME_ENA	0x00000002
> +#define RCAR_USB3_INT_HSE_ENA	0x00000004
> +#define RCAR_USB3_INT_ENA_VAL	(RCAR_USB3_INT_XHC_ENA | \
> +				RCAR_USB3_INT_PME_ENA | RCAR_USB3_INT_HSE_ENA)
> +
> +/* FW Download Control & Status */
> +#define RCAR_USB3_DL_CTRL		0x250

    Already #define'd.

> +/* USB3.0 Configuraion */

    Configuration.

> +int xhci_rcar_start(struct usb_hcd *hcd)
> +{
> +	if (hcd->regs != NULL) {
> +		u32 temp;

    Need empty line here... and should perhaps return error if hcd->regs NULL?

> +		/* Interrupt Enable */
> +		temp = readl(hcd->regs + RCAR_USB3_INT_ENA);
> +		temp |= RCAR_USB3_INT_ENA_VAL;
> +		writel(temp, hcd->regs + RCAR_USB3_INT_ENA);
> +		/* LCLK Select */
> +		writel(RCAR_USB3_LCLK_ENA_VAL, hcd->regs + RCAR_USB3_LCLK);
> +		/* USB3.0 Configuration */
> +		writel(RCAR_USB3_CONF1_VAL, hcd->regs + RCAR_USB3_CONF1);
> +		writel(RCAR_USB3_CONF2_VAL, hcd->regs + RCAR_USB3_CONF2);
> +		writel(RCAR_USB3_CONF3_VAL, hcd->regs + RCAR_USB3_CONF3);
> +		/* USB3.0 Polariy */
> +		writel(RCAR_USB3_RX_POL_VAL, hcd->regs + RCAR_USB3_RX_POL);
> +		writel(RCAR_USB3_TX_POL_VAL, hcd->regs + RCAR_USB3_TX_POL);
> +	}
> +
> +	return 0;
> +}
[...]

WBR, Sergei

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

Hi Magnus-san,

(2014/05/19 19:21), Magnus Damm wrote:
> Hi Shimoda-san,
> 
> Thanks for your patches, I did however find one typo below:
> 
> On Mon, May 19, 2014 at 7:08 PM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
< snip >
>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>  {
>> +       struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> +       if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> +           of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>> +               xhci_rcar_start(hcd);
>> +
> 
> This is most likely a typo - I believe this is supposed to be r8a7790
> and r8a7791?

Thank you for the review! Yes, this is a typo... I will correct this.

Best regards,
Yoshihiro Shimoda

> Cheers,
> 
> / magnus
> 

-- 
Yoshihiro Shimoda
EC No.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

Hi Geert-san,

(2014/05/19 20:58), Geert Uytterhoeven wrote:
> Hi Shimoda-san,
> 
> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
< snip >
>> +config USB_XHCI_RCAR
>> +       tristate "xHCI support for Renesas R-Car SoCs"
>> +       select USB_XHCI_PLATFORM
>> +       depends on ARCH_SHMOBILE || COMPILE_TEST
>> +       ---help---
>> +         Say 'Y' to enable the support for the xHCI host controller
>> +         found in Renesas R-Car ARM SoCs.
> 
> Does R-Car Gen1 also have xHCI, and is it compatible?
> If not, you may want to call this driver USB_XHCI_RCAR2.

R-Car Gen1 doesn't have xHCI.
However, next generation of R-Car may have xHCI. (But, I don't know it is compatible.)
If we call this driver "USB_XHCI_RCAR2", should we also change filename to "xhci-rcar2.[ch]"?

< snip >
>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>  {
>> +       struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> +       if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> +           of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> 
> r8a7791, as Magnus already pointed out.

Yes, I will correct this.

>> +               xhci_rcar_start(hcd);
> 
> If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy
> function, but the of_device_is_compatible() checks will still be compiled in.
> 
> Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here,
> possibly combined with inclusion of a C-source file, like is done in
> drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this,
> though.

This implementation is similar with the following patch. And the patch already got
"Acked-by" from Mathias Nyman of USB XHCI DRIVER's maintainer.

	http://marc.info/?l=linux-usb&m=140014933101775&w=2

< snip >
>> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>                         goto unmap_registers;
>>         }
>>
>> +       if (of_device_is_compatible(pdev->dev.of_node,
>> +                                   "renesas,r8a7790-xhci") ||
>> +           of_device_is_compatible(pdev->dev.of_node,
>> +                                   "renesas,r8a7791-xhci")) {
>> +               ret = xhci_rcar_init_quirk(pdev);
> 
> Same here.
> 

Same above.

< snip >
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-rcar.c
> 
>> +/* USB3.0 Configuraion */
> 
> Configuration

I ran the "aspell -c" command, and I found other 2 typos. ("Initilization" and "Porariy")
So, I will correct these typos.

< snip >
>> +       for (index = 0; index < fw->size; index += 4) {
>> +               for (data = 0, j = 3; j >= 0; j--) {
>> +                       if ((j + index) >= fw->size)
>> +                               continue;
>> +                       data |= fw->data[index + j] << (8 * j);
>> +               }
> 
> This is your custom get_unaligned_le32(), to avoid reading beyond the end
> of the buffer if its size is not a multiple of 4 bytes?

Yes, I would like to avoid it.

> Is there some way to just use get_unaligned_le32()?

Yes, I will remove the custom get_unaligned_le32() and add the following code.
Do you think that this code is good?

	int i;
	u32 data;
	u8 buf[4];
< snip >
	for (i = 0; i < fw->size; i += 4) {
		memset(buf, 0, sizeof(buf));
		memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i));
		data = get_unaligned_le32(buf);

Best regards,
Yoshihiro Shimoda

> If you want to keep it, I would rewrite it as
> 
>               for (data = 0, j = 3; j >= 0; j--) {
>                       if ((j + index) < fw->size)
>                               data |= fw->data[index + j] << (8 * j);
>               }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torval
> 

-- 
Yoshihiro Shimoda
EC No.

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

Hello,

(2014/05/19 21:14), Sergei Shtylyov wrote:
> Hello.
> 
> On 19-05-2014 14:08, Yoshihiro Shimoda wrote:
< snip >
>>   static int xhci_plat_start(struct usb_hcd *hcd)
>>   {
>> +	struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> +	if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> +	    of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> 
>     Perhaps "renesas,r8a7791-xhci"?

Yes. I will correct this.

< snip >
>> +/* FW Download Control & Status */
>> +#define RCAR_USB3_DL_CTRL		0x250
> 
>     Already #define'd.

Thank you for the point! I will remove this.

>> +/* USB3.0 Configuraion */
> 
>     Configuration.

I will correct this.

>> +int xhci_rcar_start(struct usb_hcd *hcd)
>> +{
>> +	if (hcd->regs != NULL) {
>> +		u32 temp;
> 
>     Need empty line here... and should perhaps return error if hcd->regs NULL?

I will add empty line.
If this function returns error and xhci_plat_start() also returns error,
the xhci driver was not able to work.
So, I will change the prototype of this function to "void".

Best regards,
Yoshihiro Shimoda

>> +		/* Interrupt Enable */
>> +		temp = readl(hcd->regs + RCAR_USB3_INT_ENA);
>> +		temp |= RCAR_USB3_INT_ENA_VAL;
>> +		writel(temp, hcd->regs + RCAR_USB3_INT_ENA);
>> +		/* LCLK Select */
>> +		writel(RCAR_USB3_LCLK_ENA_VAL, hcd->regs + RCAR_USB3_LCLK);
>> +		/* USB3.0 Configuration */
>> +		writel(RCAR_USB3_CONF1_VAL, hcd->regs + RCAR_USB3_CONF1);
>> +		writel(RCAR_USB3_CONF2_VAL, hcd->regs + RCAR_USB3_CONF2);
>> +		writel(RCAR_USB3_CONF3_VAL, hcd->regs + RCAR_USB3_CONF3);
>> +		/* USB3.0 Polariy */
>> +		writel(RCAR_USB3_RX_POL_VAL, hcd->regs + RCAR_USB3_RX_POL);
>> +		writel(RCAR_USB3_TX_POL_VAL, hcd->regs + RCAR_USB3_TX_POL);
>> +	}
>> +
>> +	return 0;
>> +}
> [...]
> 
> WBR, Sergei
> 

-- 
Yoshihiro Shimoda
EC No.

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Arnd Bergmann]

On Monday 19 May 2014 19:08:05 Yoshihiro Shimoda wrote:
> 
>  #include "xhci.h"
>  #include "xhci-mvebu.h"
> +#include "xhci-rcar.h"
> 
>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>  {
> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
> 
>  static int xhci_plat_start(struct usb_hcd *hcd)
>  {
> +	struct device_node *of_node = hcd->self.controller->of_node;
> +
> +	if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> +	    of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> +		xhci_rcar_start(hcd);
> +
>  	return xhci_run(hcd);
>  }
> 
> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  			goto unmap_registers;
>  	}
> 
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "renesas,r8a7790-xhci") ||
> +	    of_device_is_compatible(pdev->dev.of_node,
> +				    "renesas,r8a7791-xhci")) {
> +		ret = xhci_rcar_init_quirk(pdev);
> +		if (ret)
> +			goto disable_clk;
> +	}
> +
>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (ret)
>  		goto disable_clk;
> @@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
>  	{ .compatible = "xhci-platform" },
>  	{ .compatible = "marvell,armada-375-xhci"},
>  	{ .compatible = "marvell,armada-380-xhci"},
> +	{ .compatible = "renesas,r8a7790-xhci"},
> +	{ .compatible = "renesas,r8a7791-xhci"},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, usb_xhci_of_match);

Like the drivers before, this is way more than a quirk, and deserves to
be its own driver. It would be better to have an abstract way to split
out soc specific xhci front-ends and export functions from the xhci-platform
code.

	Arnd

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Geert Uytterhoeven]

Hi Shimoda-san,

On Tue, May 20, 2014 at 11:35 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> (2014/05/19 20:58), Geert Uytterhoeven wrote:
>> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> < snip >
>>> +config USB_XHCI_RCAR
>>> +       tristate "xHCI support for Renesas R-Car SoCs"
>>> +       select USB_XHCI_PLATFORM
>>> +       depends on ARCH_SHMOBILE || COMPILE_TEST
>>> +       ---help---
>>> +         Say 'Y' to enable the support for the xHCI host controller
>>> +         found in Renesas R-Car ARM SoCs.
>>
>> Does R-Car Gen1 also have xHCI, and is it compatible?
>> If not, you may want to call this driver USB_XHCI_RCAR2.
>
> R-Car Gen1 doesn't have xHCI.
> However, next generation of R-Car may have xHCI. (But, I don't know it is compatible.)
> If we call this driver "USB_XHCI_RCAR2", should we also change filename to "xhci-rcar2.[ch]"?

Iff you change the config symbol, please also change the filename.

But given the uncertainty about future version, you can leave it like it is.

>>> +               xhci_rcar_start(hcd);
>>
>> If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy
>> function, but the of_device_is_compatible() checks will still be compiled in.
>>
>> Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here,
>> possibly combined with inclusion of a C-source file, like is done in
>> drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this,
>> though.
>
> This implementation is similar with the following patch. And the patch already got
> "Acked-by" from Mathias Nyman of USB XHCI DRIVER's maintainer.
>
>         http://marc.info/?l=linux-usb&m=140014933101775&w=2

Fine. It can be fixed later by the maintainer, when the driver has gained too
many compatible checks ;-)

>>> +       for (index = 0; index < fw->size; index += 4) {
>>> +               for (data = 0, j = 3; j >= 0; j--) {
>>> +                       if ((j + index) >= fw->size)
>>> +                               continue;
>>> +                       data |= fw->data[index + j] << (8 * j);
>>> +               }
>>
>> This is your custom get_unaligned_le32(), to avoid reading beyond the end
>> of the buffer if its size is not a multiple of 4 bytes?
>
> Yes, I would like to avoid it.
>
>> Is there some way to just use get_unaligned_le32()?
>
> Yes, I will remove the custom get_unaligned_le32() and add the following code.
> Do you think that this code is good?
>
>         int i;
>         u32 data;
>         u8 buf[4];
> < snip >
>         for (i = 0; i < fw->size; i += 4) {
>                 memset(buf, 0, sizeof(buf));
>                 memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i));
>                 data = get_unaligned_le32(buf);

I'm sorry, but IMHO this looks worse.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

Hi,

(2014/05/20 19:11), Arnd Bergmann wrote:
> On Monday 19 May 2014 19:08:05 Yoshihiro Shimoda wrote:
>>
>>  #include "xhci.h"
>>  #include "xhci-mvebu.h"
>> +#include "xhci-rcar.h"
>>
>>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>>  {
>> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>>
>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>  {
>> +	struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> +	if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> +	    of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>> +		xhci_rcar_start(hcd);
>> +
>>  	return xhci_run(hcd);
>>  }
>>
>> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  			goto unmap_registers;
>>  	}
>>
>> +	if (of_device_is_compatible(pdev->dev.of_node,
>> +				    "renesas,r8a7790-xhci") ||
>> +	    of_device_is_compatible(pdev->dev.of_node,
>> +				    "renesas,r8a7791-xhci")) {
>> +		ret = xhci_rcar_init_quirk(pdev);
>> +		if (ret)
>> +			goto disable_clk;
>> +	}
>> +
>>  	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>>  	if (ret)
>>  		goto disable_clk;
>> @@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
>>  	{ .compatible = "xhci-platform" },
>>  	{ .compatible = "marvell,armada-375-xhci"},
>>  	{ .compatible = "marvell,armada-380-xhci"},
>> +	{ .compatible = "renesas,r8a7790-xhci"},
>> +	{ .compatible = "renesas,r8a7791-xhci"},
>>  	{ },
>>  };
>>  MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> 
> Like the drivers before, this is way more than a quirk, and deserves to
> be its own driver. It would be better to have an abstract way to split
> out soc specific xhci front-ends and export functions from the xhci-platform
> code.

Thank you for your comment. But, I couldn't understand your comment...
Did you mean that xhci-rcar.c should call of_device_is_compatible(of_node, "renesas,...")?
If so, I will modify this patch.

Best regards,
Yoshihiro Shimoda

> 	Arnd
> 

-- 
Yoshihiro Shimoda
EC No.

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for the reply again.

(2014/05/20 19:14), Geert Uytterhoeven wrote:
> Hi Shimoda-san,
> 
> On Tue, May 20, 2014 at 11:35 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> (2014/05/19 20:58), Geert Uytterhoeven wrote:
>>> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
>>> <yoshihiro.shimoda.uh@renesas.com> wrote:
>> < snip >
>>>> +config USB_XHCI_RCAR
>>>> +       tristate "xHCI support for Renesas R-Car SoCs"
>>>> +       select USB_XHCI_PLATFORM
>>>> +       depends on ARCH_SHMOBILE || COMPILE_TEST
>>>> +       ---help---
>>>> +         Say 'Y' to enable the support for the xHCI host controller
>>>> +         found in Renesas R-Car ARM SoCs.
>>>
>>> Does R-Car Gen1 also have xHCI, and is it compatible?
>>> If not, you may want to call this driver USB_XHCI_RCAR2.
>>
>> R-Car Gen1 doesn't have xHCI.
>> However, next generation of R-Car may have xHCI. (But, I don't know it is compatible.)
>> If we call this driver "USB_XHCI_RCAR2", should we also change filename to "xhci-rcar2.[ch]"?
> 
> Iff you change the config symbol, please also change the filename.
> 
> But given the uncertainty about future version, you can leave it like it is.

I got it. I will leave the "USB_XHCI_RCAR".

< snip >
>>> Is there some way to just use get_unaligned_le32()?
>>
>> Yes, I will remove the custom get_unaligned_le32() and add the following code.
>> Do you think that this code is good?
>>
>>         int i;
>>         u32 data;
>>         u8 buf[4];
>> < snip >
>>         for (i = 0; i < fw->size; i += 4) {
>>                 memset(buf, 0, sizeof(buf));
>>                 memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i));
>>                 data = get_unaligned_le32(buf);
> 
> I'm sorry, but IMHO this looks worse.

Thank you for the review. :)
So, I will keep the following code with an additional comment.

              for (data = 0, j = 3; j >= 0; j--) {
                      if ((j + index) < fw->size)
                              data |= fw->data[index + j] << (8 * j);
              }

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Yoshihiro Shimoda
EC No.

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Arnd Bergmann]

On Wednesday 21 May 2014 16:54:00 Yoshihiro Shimoda wrote:
> 
> (2014/05/20 19:11), Arnd Bergmann wrote:
> > On Monday 19 May 2014 19:08:05 Yoshihiro Shimoda wrote:
> >>
> >>  #include "xhci.h"
> >>  #include "xhci-mvebu.h"
> >> +#include "xhci-rcar.h"
> >>
> >>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> >>  {
> >> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
> >>
> >>  static int xhci_plat_start(struct usb_hcd *hcd)
> >>  {
> >> +    struct device_node *of_node = hcd->self.controller->of_node;
> >> +
> >> +    if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> >> +        of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> >> +            xhci_rcar_start(hcd);
> >> +
> >>      return xhci_run(hcd);
> >>  }
> >>
> >> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
> >>                      goto unmap_registers;
> >>      }
> >>
> >> +    if (of_device_is_compatible(pdev->dev.of_node,
> >> +                                "renesas,r8a7790-xhci") ||
> >> +        of_device_is_compatible(pdev->dev.of_node,
> >> +                                "renesas,r8a7791-xhci")) {
> >> +            ret = xhci_rcar_init_quirk(pdev);
> >> +            if (ret)
> >> +                    goto disable_clk;
> >> +    }
> >> +
> >>      ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> >>      if (ret)
> >>              goto disable_clk;
> >> @@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
> >>      { .compatible = "xhci-platform" },
> >>      { .compatible = "marvell,armada-375-xhci"},
> >>      { .compatible = "marvell,armada-380-xhci"},
> >> +    { .compatible = "renesas,r8a7790-xhci"},
> >> +    { .compatible = "renesas,r8a7791-xhci"},
> >>      { },
> >>  };
> >>  MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> > 
> > Like the drivers before, this is way more than a quirk, and deserves to
> > be its own driver. It would be better to have an abstract way to split
> > out soc specific xhci front-ends and export functions from the xhci-platform
> > code.
> 
> Thank you for your comment. But, I couldn't understand your comment...
> Did you mean that xhci-rcar.c should call of_device_is_compatible(of_node, "renesas,...")?
> If so, I will modify this patch.


What I mean is that there should be a separate module that contains all the
renesas specific code, and that module should register a platform driver
that contains the match table for its own IDs.

Then instead of having a common xhci_plat_probe() that gets called as the
->probe() callback of the driver, you have a rcar_xhci_probe() function
that calls into common helper functions exported by the base driver, just
as we do things for all other drivers. See ehci or ahci for instance.


	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers

[Yoshihiro Shimoda]

(2014/05/21 17:04), Arnd Bergmann wrote:
> On Wednesday 21 May 2014 16:54:00 Yoshihiro Shimoda wrote:
>>
>> (2014/05/20 19:11), Arnd Bergmann wrote:
>>> On Monday 19 May 2014 19:08:05 Yoshihiro Shimoda wrote:
>>>>
>>>>  #include "xhci.h"
>>>>  #include "xhci-mvebu.h"
>>>> +#include "xhci-rcar.h"
>>>>
>>>>  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
>>>>  {
>>>> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>>>>
>>>>  static int xhci_plat_start(struct usb_hcd *hcd)
>>>>  {
>>>> +    struct device_node *of_node = hcd->self.controller->of_node;
>>>> +
>>>> +    if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>>>> +        of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>>>> +            xhci_rcar_start(hcd);
>>>> +
>>>>      return xhci_run(hcd);
>>>>  }
>>>>
>>>> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>>>                      goto unmap_registers;
>>>>      }
>>>>
>>>> +    if (of_device_is_compatible(pdev->dev.of_node,
>>>> +                                "renesas,r8a7790-xhci") ||
>>>> +        of_device_is_compatible(pdev->dev.of_node,
>>>> +                                "renesas,r8a7791-xhci")) {
>>>> +            ret = xhci_rcar_init_quirk(pdev);
>>>> +            if (ret)
>>>> +                    goto disable_clk;
>>>> +    }
>>>> +
>>>>      ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
>>>>      if (ret)
>>>>              goto disable_clk;
>>>> @@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
>>>>      { .compatible = "xhci-platform" },
>>>>      { .compatible = "marvell,armada-375-xhci"},
>>>>      { .compatible = "marvell,armada-380-xhci"},
>>>> +    { .compatible = "renesas,r8a7790-xhci"},
>>>> +    { .compatible = "renesas,r8a7791-xhci"},
>>>>      { },
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>>>
>>> Like the drivers before, this is way more than a quirk, and deserves to
>>> be its own driver. It would be better to have an abstract way to split
>>> out soc specific xhci front-ends and export functions from the xhci-platform
>>> code.
>>
>> Thank you for your comment. But, I couldn't understand your comment...
>> Did you mean that xhci-rcar.c should call of_device_is_compatible(of_node, "renesas,...")?
>> If so, I will modify this patch.
> 
> 
> What I mean is that there should be a separate module that contains all the
> renesas specific code, and that module should register a platform driver
> that contains the match table for its own IDs.
> 
> Then instead of having a common xhci_plat_probe() that gets called as the
> ->probe() callback of the driver, you have a rcar_xhci_probe() function
> that calls into common helper functions exported by the base driver, just
> as we do things for all other drivers. See ehci or ahci for instance.

Thank you very much for the prompt reply!
I will see ehci and ahci drivers.

Best regards,
Yoshihiro Shimoda

> 
> 	Arnd
> 

-- 
Yoshihiro Shimoda
EC No.