diff mbox series

[v2,11/15] usb: renesas_usbhs: Add support for RZ/A2

Message ID 20190509201142.10543-12-chris.brandt@renesas.com (mailing list archive)
State Superseded
Headers show
Series usb: Add host and device support for RZ/A2 | expand

Commit Message

Chris Brandt May 9, 2019, 8:11 p.m. UTC
The RZ/A2 is similar to the R-Car Gen3 with some small differences.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * combined RZA1 and RZA2 for fifo setting
 * added braces to make code easier to read
 * fixed and clean up usbhs_rza2_power_ctrl()
---
 drivers/usb/renesas_usbhs/Makefile |  2 +-
 drivers/usb/renesas_usbhs/common.c | 12 +++++-
 drivers/usb/renesas_usbhs/rza.h    |  1 +
 drivers/usb/renesas_usbhs/rza2.c   | 79 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb/renesas_usbhs.h  |  1 +
 5 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 drivers/usb/renesas_usbhs/rza2.c

Comments

Chunfeng Yun (云春峰) May 10, 2019, 1:53 a.m. UTC | #1
On Thu, 2019-05-09 at 15:11 -0500, Chris Brandt wrote:
> The RZ/A2 is similar to the R-Car Gen3 with some small differences.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
>  * combined RZA1 and RZA2 for fifo setting
>  * added braces to make code easier to read
>  * fixed and clean up usbhs_rza2_power_ctrl()
> ---
>  drivers/usb/renesas_usbhs/Makefile |  2 +-
>  drivers/usb/renesas_usbhs/common.c | 12 +++++-
>  drivers/usb/renesas_usbhs/rza.h    |  1 +
>  drivers/usb/renesas_usbhs/rza2.c   | 79 ++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/renesas_usbhs.h  |  1 +
>  5 files changed, 93 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/usb/renesas_usbhs/rza2.c
> 
> diff --git a/drivers/usb/renesas_usbhs/Makefile b/drivers/usb/renesas_usbhs/Makefile
> index 5c5b51bb48ef..a1fed56b0957 100644
> --- a/drivers/usb/renesas_usbhs/Makefile
> +++ b/drivers/usb/renesas_usbhs/Makefile
> @@ -5,7 +5,7 @@
>  
>  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs.o
>  
> -renesas_usbhs-y			:= common.o mod.o pipe.o fifo.o rcar2.o rcar3.o rza.o
> +renesas_usbhs-y			:= common.o mod.o pipe.o fifo.o rcar2.o rcar3.o rza.o rza2.o
>  
>  ifneq ($(CONFIG_USB_RENESAS_USBHS_HCD),)
>  	renesas_usbhs-y		+= mod_host.o
> diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
> index 820636fc4dc9..35d2298c03a0 100644
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -582,6 +582,10 @@ static const struct of_device_id usbhs_of_match[] = {
>  		.compatible = "renesas,rza1-usbhs",
>  		.data = (void *)USBHS_TYPE_RZA1,
>  	},
> +	{
> +		.compatible = "renesas,rza2-usbhs",
> +		.data = (void *)USBHS_TYPE_RZA2,
> +	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, usbhs_of_match);
> @@ -614,7 +618,8 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
>  		dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
>  	}
>  
> -	if (dparam->type == USBHS_TYPE_RZA1) {
> +	if (dparam->type == USBHS_TYPE_RZA1 ||
> +	    dparam->type == USBHS_TYPE_RZA2) {
>  		dparam->pipe_configs = usbhsc_new_pipe;
>  		dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
>  	}
> @@ -688,6 +693,11 @@ static int usbhs_probe(struct platform_device *pdev)
>  	case USBHS_TYPE_RZA1:
>  		priv->pfunc = usbhs_rza1_ops;
>  		break;
> +	case USBHS_TYPE_RZA2:
> +		priv->pfunc = usbhs_rza2_ops;
> +		usbhsc_flags_set(priv, USBHSF_HAS_CNEN);
> +		usbhsc_flags_set(priv, USBHSF_CFIFO_BYTE_ADDR);
> +		break;
>  	default:
>  		if (!info->platform_callback.get_id) {
>  			dev_err(&pdev->dev, "no platform callbacks");
> diff --git a/drivers/usb/renesas_usbhs/rza.h b/drivers/usb/renesas_usbhs/rza.h
> index ca917ca54f6d..073a53d1d442 100644
> --- a/drivers/usb/renesas_usbhs/rza.h
> +++ b/drivers/usb/renesas_usbhs/rza.h
> @@ -2,3 +2,4 @@
>  #include "common.h"
>  
>  extern const struct renesas_usbhs_platform_callback usbhs_rza1_ops;
> +extern const struct renesas_usbhs_platform_callback usbhs_rza2_ops;
> diff --git a/drivers/usb/renesas_usbhs/rza2.c b/drivers/usb/renesas_usbhs/rza2.c
> new file mode 100644
> index 000000000000..a1d9eb2d40cf
> --- /dev/null
> +++ b/drivers/usb/renesas_usbhs/rza2.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas USB driver RZ/A2 initialization and power control
> + *
> + * Copyright (C) 2019 Chris Brandt
> + * Copyright (C) 2019 Renesas Electronics Corporation
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include "common.h"
> +#include "rza.h"
> +
> +
> +static int usbhs_rza2_hardware_init(struct platform_device *pdev)
> +{
> +	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
> +
> +	if (IS_ENABLED(CONFIG_GENERIC_PHY)) {
no need check it, if it's not enabled, phy_get() will return an error
number.

> +		struct phy *phy = phy_get(&pdev->dev, "usb");
use devm_phy_get??
> +
> +		if (IS_ERR(phy))
> +			return PTR_ERR(phy);
> +
> +		priv->phy = phy;
> +		return 0;
> +	}
> +	return -ENXIO;
> +}
> +
> +static int usbhs_rza2_hardware_exit(struct platform_device *pdev)
> +{
> +	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
> +
> +	if (priv->phy) {
> +		phy_put(priv->phy);
> +		priv->phy = NULL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int usbhs_rza2_power_ctrl(struct platform_device *pdev,
> +				void __iomem *base, int enable)
> +{
> +	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
> +	int retval = 0;
> +
> +	if (!priv->phy)
> +		return -ENODEV;
> +
> +	if (enable) {
> +		retval = phy_init(priv->phy);
> +		usbhs_bset(priv, SUSPMODE, SUSPM, SUSPM);
> +		udelay(100);	/* Wait for PLL to become stable */
> +		if (!retval)
> +			retval = phy_power_on(priv->phy);
> +	} else {
> +		usbhs_bset(priv, SUSPMODE, SUSPM, 0);
> +		phy_power_off(priv->phy);
> +		phy_exit(priv->phy);
> +	}
> +
> +	return retval;
> +}
> +
> +static int usbhs_rza2_get_id(struct platform_device *pdev)
> +{
> +	return USBHS_GADGET;
> +}
> +
> +const struct renesas_usbhs_platform_callback usbhs_rza2_ops = {
> +	.hardware_init = usbhs_rza2_hardware_init,
> +	.hardware_exit = usbhs_rza2_hardware_exit,
> +	.power_ctrl = usbhs_rza2_power_ctrl,
> +	.get_id = usbhs_rza2_get_id,
> +};
> diff --git a/include/linux/usb/renesas_usbhs.h b/include/linux/usb/renesas_usbhs.h
> index 53924f8e840c..39604c8b1eed 100644
> --- a/include/linux/usb/renesas_usbhs.h
> +++ b/include/linux/usb/renesas_usbhs.h
> @@ -196,6 +196,7 @@ struct renesas_usbhs_driver_param {
>  #define USBHS_TYPE_RCAR_GEN3		2
>  #define USBHS_TYPE_RCAR_GEN3_WITH_PLL	3
>  #define USBHS_TYPE_RZA1			4
> +#define USBHS_TYPE_RZA2			5
>  
>  /*
>   * option:
Geert Uytterhoeven May 10, 2019, 7:07 a.m. UTC | #2
Hi Chris,

On Thu, May 9, 2019 at 10:14 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> The RZ/A2 is similar to the R-Car Gen3 with some small differences.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Thanks for your patch!

> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c

> @@ -614,7 +618,8 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
>                 dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
>         }
>
> -       if (dparam->type == USBHS_TYPE_RZA1) {
> +       if (dparam->type == USBHS_TYPE_RZA1 ||
> +           dparam->type == USBHS_TYPE_RZA2) {
>                 dparam->pipe_configs = usbhsc_new_pipe;
>                 dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);

The above two lines are also present in the block for R-Car Gen2/3 above.
Perhaps they can just be moved out, and executed unconditionally?
That leaves R-Car with just the has_usb_dmac feature flag.

BTW, this driver uses a mix of feature checking using USBHS_TYPE_*
enums, and a parameter block/callback struct
(renesas_usbhs_platform_callback).  Perhaps the feature flags can just
be moved to the struct, and the various structs referenced from
of_device_id.data?

Gr{oetje,eeting}s,

                        Geert
Yoshihiro Shimoda May 10, 2019, 8:16 a.m. UTC | #3
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Friday, May 10, 2019 4:07 PM
> 
> Hi Chris,
> 
> On Thu, May 9, 2019 at 10:14 PM Chris Brandt <chris.brandt@renesas.com> wrote:
<snip>
> BTW, this driver uses a mix of feature checking using USBHS_TYPE_*
> enums, and a parameter block/callback struct
> (renesas_usbhs_platform_callback).  Perhaps the feature flags can just
> be moved to the struct, and the various structs referenced from
> of_device_id.data?

Thank you for your comment! I think so. So, I'll make such a patch later.

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
Yoshihiro Shimoda May 10, 2019, 11 a.m. UTC | #4
Hi Geert-san, Chris-san,

> From: Yoshihiro Shimoda, Sent: Friday, May 10, 2019 5:16 PM
> 
> Hi Geert-san,
> 
> > From: Geert Uytterhoeven, Sent: Friday, May 10, 2019 4:07 PM
> >
> > Hi Chris,
> >
> > On Thu, May 9, 2019 at 10:14 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> <snip>
> > BTW, this driver uses a mix of feature checking using USBHS_TYPE_*
> > enums, and a parameter block/callback struct
> > (renesas_usbhs_platform_callback).  Perhaps the feature flags can just
> > be moved to the struct, and the various structs referenced from
> > of_device_id.data?
> 
> Thank you for your comment! I think so. So, I'll make such a patch later.

I have submitted such a patch as following:
https://patchwork.kernel.org/patch/10938575/

Since usbhsc_is_multi_clks() uses the type member, each struct also has the type like previous code.

About SoC parameters, I think it is better to add members into struct renesas_usbhs_driver_param like
has_usb_dmac instead of USBHSF_* definitions. In other words, we don't need the patch 08/15 and
patch 09/15 and 10/15 should add each member for it. Chris-san, what do you think?

Best regards,
Yoshihiro Shimoda

> 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
Chris Brandt May 10, 2019, 1:10 p.m. UTC | #5
On Thu, May 09, 2019, Chunfeng Yun wrote:
> > +	if (IS_ENABLED(CONFIG_GENERIC_PHY)) {
> no need check it, if it's not enabled, phy_get() will return an error
> number.

Good point.
Thank you.

Chris
Chris Brandt May 10, 2019, 2:20 p.m. UTC | #6
Hi Shimodaさん

> From: Yoshihiro Shimoda
> Sent: Friday, May 10, 2019 7:00 AM

> I have submitted such a patch as following:
> https://patchwork.kernel.org/patch/10938575/

OK.
I will rebase my patches on top of your patch.
I will say my patch series depends on your patch.

> About SoC parameters, I think it is better to add members into struct
> renesas_usbhs_driver_param like
> has_usb_dmac instead of USBHSF_* definitions. In other words, we don't
> need the patch 08/15 and
> patch 09/15 and 10/15 should add each member for it. Chris-san, what do
> you think?

I think that is good.

New Patch 08/15:
 * Add to struct renesas_usbhs_driver_param:
      u32 has_runtime_pwctrl:1;
 * Remove USBHSF_*
 * Remove usbhsc_flags_*

New Patch 09/15:
* Add to struct renesas_usbhs_driver_param:
     u32 has_cnen:1;

New Patch 10/15:
* Add to struct renesas_usbhs_driver_param:
     u32 cfifo_byte_addr:1;


Chris
diff mbox series

Patch

diff --git a/drivers/usb/renesas_usbhs/Makefile b/drivers/usb/renesas_usbhs/Makefile
index 5c5b51bb48ef..a1fed56b0957 100644
--- a/drivers/usb/renesas_usbhs/Makefile
+++ b/drivers/usb/renesas_usbhs/Makefile
@@ -5,7 +5,7 @@ 
 
 obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs.o
 
-renesas_usbhs-y			:= common.o mod.o pipe.o fifo.o rcar2.o rcar3.o rza.o
+renesas_usbhs-y			:= common.o mod.o pipe.o fifo.o rcar2.o rcar3.o rza.o rza2.o
 
 ifneq ($(CONFIG_USB_RENESAS_USBHS_HCD),)
 	renesas_usbhs-y		+= mod_host.o
diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index 820636fc4dc9..35d2298c03a0 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -582,6 +582,10 @@  static const struct of_device_id usbhs_of_match[] = {
 		.compatible = "renesas,rza1-usbhs",
 		.data = (void *)USBHS_TYPE_RZA1,
 	},
+	{
+		.compatible = "renesas,rza2-usbhs",
+		.data = (void *)USBHS_TYPE_RZA2,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, usbhs_of_match);
@@ -614,7 +618,8 @@  static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
 		dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
 	}
 
-	if (dparam->type == USBHS_TYPE_RZA1) {
+	if (dparam->type == USBHS_TYPE_RZA1 ||
+	    dparam->type == USBHS_TYPE_RZA2) {
 		dparam->pipe_configs = usbhsc_new_pipe;
 		dparam->pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
 	}
@@ -688,6 +693,11 @@  static int usbhs_probe(struct platform_device *pdev)
 	case USBHS_TYPE_RZA1:
 		priv->pfunc = usbhs_rza1_ops;
 		break;
+	case USBHS_TYPE_RZA2:
+		priv->pfunc = usbhs_rza2_ops;
+		usbhsc_flags_set(priv, USBHSF_HAS_CNEN);
+		usbhsc_flags_set(priv, USBHSF_CFIFO_BYTE_ADDR);
+		break;
 	default:
 		if (!info->platform_callback.get_id) {
 			dev_err(&pdev->dev, "no platform callbacks");
diff --git a/drivers/usb/renesas_usbhs/rza.h b/drivers/usb/renesas_usbhs/rza.h
index ca917ca54f6d..073a53d1d442 100644
--- a/drivers/usb/renesas_usbhs/rza.h
+++ b/drivers/usb/renesas_usbhs/rza.h
@@ -2,3 +2,4 @@ 
 #include "common.h"
 
 extern const struct renesas_usbhs_platform_callback usbhs_rza1_ops;
+extern const struct renesas_usbhs_platform_callback usbhs_rza2_ops;
diff --git a/drivers/usb/renesas_usbhs/rza2.c b/drivers/usb/renesas_usbhs/rza2.c
new file mode 100644
index 000000000000..a1d9eb2d40cf
--- /dev/null
+++ b/drivers/usb/renesas_usbhs/rza2.c
@@ -0,0 +1,79 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas USB driver RZ/A2 initialization and power control
+ *
+ * Copyright (C) 2019 Chris Brandt
+ * Copyright (C) 2019 Renesas Electronics Corporation
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include "common.h"
+#include "rza.h"
+
+
+static int usbhs_rza2_hardware_init(struct platform_device *pdev)
+{
+	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+
+	if (IS_ENABLED(CONFIG_GENERIC_PHY)) {
+		struct phy *phy = phy_get(&pdev->dev, "usb");
+
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		priv->phy = phy;
+		return 0;
+	}
+	return -ENXIO;
+}
+
+static int usbhs_rza2_hardware_exit(struct platform_device *pdev)
+{
+	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+
+	if (priv->phy) {
+		phy_put(priv->phy);
+		priv->phy = NULL;
+	}
+
+	return 0;
+}
+
+static int usbhs_rza2_power_ctrl(struct platform_device *pdev,
+				void __iomem *base, int enable)
+{
+	struct usbhs_priv *priv = usbhs_pdev_to_priv(pdev);
+	int retval = 0;
+
+	if (!priv->phy)
+		return -ENODEV;
+
+	if (enable) {
+		retval = phy_init(priv->phy);
+		usbhs_bset(priv, SUSPMODE, SUSPM, SUSPM);
+		udelay(100);	/* Wait for PLL to become stable */
+		if (!retval)
+			retval = phy_power_on(priv->phy);
+	} else {
+		usbhs_bset(priv, SUSPMODE, SUSPM, 0);
+		phy_power_off(priv->phy);
+		phy_exit(priv->phy);
+	}
+
+	return retval;
+}
+
+static int usbhs_rza2_get_id(struct platform_device *pdev)
+{
+	return USBHS_GADGET;
+}
+
+const struct renesas_usbhs_platform_callback usbhs_rza2_ops = {
+	.hardware_init = usbhs_rza2_hardware_init,
+	.hardware_exit = usbhs_rza2_hardware_exit,
+	.power_ctrl = usbhs_rza2_power_ctrl,
+	.get_id = usbhs_rza2_get_id,
+};
diff --git a/include/linux/usb/renesas_usbhs.h b/include/linux/usb/renesas_usbhs.h
index 53924f8e840c..39604c8b1eed 100644
--- a/include/linux/usb/renesas_usbhs.h
+++ b/include/linux/usb/renesas_usbhs.h
@@ -196,6 +196,7 @@  struct renesas_usbhs_driver_param {
 #define USBHS_TYPE_RCAR_GEN3		2
 #define USBHS_TYPE_RCAR_GEN3_WITH_PLL	3
 #define USBHS_TYPE_RZA1			4
+#define USBHS_TYPE_RZA2			5
 
 /*
  * option: