[PATCH v2 00/10] A3700 USB S2RAM support

[PATCH v2 00/10] A3700 USB S2RAM support

[Miquel Raynal]

Hello,

As part of an effort to bring suspend to RAM support to Armada 3700
SoCs (main target: ESPRESSObin), this series handles the work around
the USB2 and USB32 IPs.

First, a change in the core adds support for the new PHY framework by
following the phy_set_mode()/phy_power_on() sequence (patch 1). The
second change needed is to update the xHCI MVEBU driver (patch 2) with
the XHCI_RESET_ON_RESUME quirk that will do most of the
reconfiguration work when resuming.

Then, because of the asynchronous suspend feature implemented lately,
the xHCI driver was blocking during the S2RAM operation, probably due
to a register access while the clock was not enabled. A first patch
has been written for that, adding a new quirk to ignore the
asynchronous setting [1] which has become useless once clock links
with consumer have been contributed [2] (series not merged yet).

Then, the OHCI Orion (USB2 host controller) driver is updated to avoid
doing twice the PHY management (patch 3) and upgraded with traditional
S2RAM callbacks (patch 4).

The last missing peace is a UTMI PHY driver for the USB2 part of each
controller, which is added in patch 5 (see patch 6 for the bindings).

Finally, the A3700 device tree is updated (patch 7, 8, 9) to reference
the PHYs. xHCI bindings already document the PHY so no update is needed
on this regard.

[1] http://code.bulix.org/s2ccd4-511198
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-January/623885.html

Thanks,
Miquèl

Changes since v1:
=================
* Add UTMI PHY driver/bindings/DT changes to bring S2RAM support to
  both USB ports available on the A3700 SoC.
* Updates to the OHCI Orion driver to avoid doing the PHY
  initialization twice.
* Upgrade of the OHCI Orion driver with S2RAM callbacks.
* Added a reference to the A3700 SoC in the USB Host Kconfig prompt
  (not only in the help section).
* Rebased on top of v5.0-rc1.


Miquel Raynal (9):
  usb: core: comply to PHY framework
  usb: ehci-orion: avoid double PHY initialization
  usb: ehci-orion: add S2RAM support
  phy: add A3700 UTMI PHY driver
  dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings
  MAINTAINERS: phy: fill Armada 3700 PHY drivers entry
  ARM64: dts: marvell: armada-37xx: fix USB2 memory region
  ARM64: dts: marvell: armada-37xx: declare USB2 UTMI PHYs
  ARM64: dts: marvell: armada-37xx: link USB hosts with their PHYs

Ofer Heifetz (1):
  usb: host: xhci: mvebu: add reset on resume quirk

 .../bindings/phy/phy-mvebu-utmi.txt           |  37 +++
 MAINTAINERS                                   |   4 +-
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  32 +-
 drivers/phy/marvell/Kconfig                   |   9 +
 drivers/phy/marvell/Makefile                  |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-utmi.c    | 297 ++++++++++++++++++
 drivers/usb/core/hcd.c                        |   5 +
 drivers/usb/core/phy.c                        |  28 ++
 drivers/usb/core/phy.h                        |   2 +
 drivers/usb/host/Kconfig                      |   4 +-
 drivers/usb/host/ehci-orion.c                 |  51 +--
 drivers/usb/host/xhci-mvebu.c                 |  11 +
 drivers/usb/host/xhci-mvebu.h                 |   6 +
 drivers/usb/host/xhci-plat.c                  |   7 +
 14 files changed, 467 insertions(+), 27 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c

-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 01/10] usb: core: comply to PHY framework

[Miquel Raynal]

Current implementation of the USB core does not take into account the
new PHY framework. Correct the situation by adding a call to
phy_set_mode() before phy_power_on().

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/usb/core/hcd.c |  5 +++++
 drivers/usb/core/phy.c | 28 ++++++++++++++++++++++++++++
 drivers/usb/core/phy.h |  2 ++
 3 files changed, 35 insertions(+)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 015b126ce455..86f39e44f98a 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2736,6 +2736,11 @@ int usb_add_hcd(struct usb_hcd *hcd,
 		if (retval)
 			return retval;
 
+		retval = usb_phy_roothub_set_mode(hcd->phy_roothub,
+						  PHY_MODE_USB_HOST_SS);
+		if (retval)
+			goto err_usb_phy_roothub_power_on;
+
 		retval = usb_phy_roothub_power_on(hcd->phy_roothub);
 		if (retval)
 			goto err_usb_phy_roothub_power_on;
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index 38b2c776c4b4..7580493b867a 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -123,6 +123,34 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub)
 }
 EXPORT_SYMBOL_GPL(usb_phy_roothub_exit);
 
+int usb_phy_roothub_set_mode(struct usb_phy_roothub *phy_roothub,
+			     enum phy_mode mode)
+{
+	struct usb_phy_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	if (!phy_roothub)
+		return 0;
+
+	head = &phy_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_set_mode(roothub_entry->phy, mode);
+		if (err)
+			goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list)
+		phy_power_off(roothub_entry->phy);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_set_mode);
+
 int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub)
 {
 	struct usb_phy_roothub *roothub_entry;
diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
index 88a3c037e9df..dad564e2d2d4 100644
--- a/drivers/usb/core/phy.h
+++ b/drivers/usb/core/phy.h
@@ -16,6 +16,8 @@ struct usb_phy_roothub *usb_phy_roothub_alloc(struct device *dev);
 int usb_phy_roothub_init(struct usb_phy_roothub *phy_roothub);
 int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
 
+int usb_phy_roothub_set_mode(struct usb_phy_roothub *phy_roothub,
+			     enum phy_mode mode);
 int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
 void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
 
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 02/10] usb: host: xhci: mvebu: add reset on resume quirk

[Miquel Raynal]

From: Ofer Heifetz <oferh@marvell.com>

The mvebu xHCI host driver does not have suspend/resume support. Use of
the XHCI_RESET_ON_RESUME quirk is mandatory in order to avoid failures
after resume. This will work only if no USB device is plugged-in.

While at it, mention in the Kconfig file that this IP is also present
on the A3700 SoC.

Signed-off-by: Ofer Heifetz <oferh@marvell.com>
[miquel.raynal@bootlin.com: Reword the commit message]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/usb/host/Kconfig      |  4 ++--
 drivers/usb/host/xhci-mvebu.c | 11 +++++++++++
 drivers/usb/host/xhci-mvebu.h |  6 ++++++
 drivers/usb/host/xhci-plat.c  |  7 +++++++
 4 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 16758b12a5e9..13d59be6a6aa 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -69,13 +69,13 @@ config USB_XHCI_MTK
 	  If unsure, say N.
 
 config USB_XHCI_MVEBU
-	tristate "xHCI support for Marvell Armada 375/38x"
+	tristate "xHCI support for Marvell Armada 375/38x/3700"
 	select USB_XHCI_PLATFORM
 	depends on HAS_IOMEM
 	depends on ARCH_MVEBU || COMPILE_TEST
 	---help---
 	  Say 'Y' to enable the support for the xHCI host controller
-	  found in Marvell Armada 375/38x ARM SOCs.
+	  found in Marvell Armada 375/38x/3700 ARM SOCs.
 
 config USB_XHCI_RCAR
 	tristate "xHCI support for Renesas R-Car SoCs"
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
index 32e158568788..60651a50770f 100644
--- a/drivers/usb/host/xhci-mvebu.c
+++ b/drivers/usb/host/xhci-mvebu.c
@@ -13,6 +13,7 @@
 #include <linux/usb/hcd.h>
 
 #include "xhci-mvebu.h"
+#include "xhci.h"
 
 #define USB3_MAX_WINDOWS	4
 #define USB3_WIN_CTRL(w)	(0x0 + ((w) * 8))
@@ -72,3 +73,13 @@ int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 
 	return 0;
 }
+
+int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+
+	/* Without reset on resume, the HC won't work at all */
+	xhci->quirks |= XHCI_RESET_ON_RESUME;
+
+	return 0;
+}
diff --git a/drivers/usb/host/xhci-mvebu.h b/drivers/usb/host/xhci-mvebu.h
index 09791df2cec0..ca0a3a5721dd 100644
--- a/drivers/usb/host/xhci-mvebu.h
+++ b/drivers/usb/host/xhci-mvebu.h
@@ -12,10 +12,16 @@ struct usb_hcd;
 
 #if IS_ENABLED(CONFIG_USB_XHCI_MVEBU)
 int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd);
+int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd);
 #else
 static inline int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
 {
 	return 0;
 }
+
+static inline int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd)
+{
+	return 0;
+}
 #endif
 #endif /* __LINUX_XHCI_MVEBU_H */
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ef09cb06212f..0ac4ec975547 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -98,6 +98,10 @@ static const struct xhci_plat_priv xhci_plat_marvell_armada = {
 	.init_quirk = xhci_mvebu_mbus_init_quirk,
 };
 
+static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
+	.init_quirk = xhci_mvebu_a3700_init_quirk,
+};
+
 static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
 	.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
 	.init_quirk = xhci_rcar_init_quirk,
@@ -123,6 +127,9 @@ static const struct of_device_id usb_xhci_of_match[] = {
 	}, {
 		.compatible = "marvell,armada-380-xhci",
 		.data = &xhci_plat_marvell_armada,
+	}, {
+		.compatible = "marvell,armada3700-xhci",
+		.data = &xhci_plat_marvell_armada3700,
 	}, {
 		.compatible = "renesas,xhci-r8a7790",
 		.data = &xhci_plat_renesas_rcar_gen2,
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/10] usb: host: xhci: mvebu: add reset on resume quirk

[Gregory CLEMENT]

Hi Miquel,
 
 On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> From: Ofer Heifetz <oferh@marvell.com>
>
> The mvebu xHCI host driver does not have suspend/resume support. Use of
> the XHCI_RESET_ON_RESUME quirk is mandatory in order to avoid failures
> after resume. This will work only if no USB device is plugged-in.
>
> While at it, mention in the Kconfig file that this IP is also present
> on the A3700 SoC.
>
> Signed-off-by: Ofer Heifetz <oferh@marvell.com>
> [miquel.raynal@bootlin.com: Reword the commit message]
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/usb/host/Kconfig      |  4 ++--
>  drivers/usb/host/xhci-mvebu.c | 11 +++++++++++
>  drivers/usb/host/xhci-mvebu.h |  6 ++++++
>  drivers/usb/host/xhci-plat.c  |  7 +++++++
>  4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 16758b12a5e9..13d59be6a6aa 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -69,13 +69,13 @@ config USB_XHCI_MTK
>  	  If unsure, say N.
>  
>  config USB_XHCI_MVEBU
> -	tristate "xHCI support for Marvell Armada 375/38x"
> +	tristate "xHCI support for Marvell Armada 375/38x/3700"

I would name it 37xx to be consistent with 38x

>  	select USB_XHCI_PLATFORM
>  	depends on HAS_IOMEM
>  	depends on ARCH_MVEBU || COMPILE_TEST
>  	---help---
>  	  Say 'Y' to enable the support for the xHCI host controller
> -	  found in Marvell Armada 375/38x ARM SOCs.
> +	  found in Marvell Armada 375/38x/3700 ARM SOCs.
same here


>  
>  config USB_XHCI_RCAR
>  	tristate "xHCI support for Renesas R-Car SoCs"
> diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c
> index 32e158568788..60651a50770f 100644
> --- a/drivers/usb/host/xhci-mvebu.c
> +++ b/drivers/usb/host/xhci-mvebu.c
> @@ -13,6 +13,7 @@
>  #include <linux/usb/hcd.h>
>  
>  #include "xhci-mvebu.h"
> +#include "xhci.h"
>  
>  #define USB3_MAX_WINDOWS	4
>  #define USB3_WIN_CTRL(w)	(0x0 + ((w) * 8))
> @@ -72,3 +73,13 @@ int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>  
>  	return 0;
>  }
> +
> +int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd)
> +{
> +	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +
> +	/* Without reset on resume, the HC won't work at all */
> +	xhci->quirks |= XHCI_RESET_ON_RESUME;
> +
> +	return 0;
> +}
> diff --git a/drivers/usb/host/xhci-mvebu.h b/drivers/usb/host/xhci-mvebu.h
> index 09791df2cec0..ca0a3a5721dd 100644
> --- a/drivers/usb/host/xhci-mvebu.h
> +++ b/drivers/usb/host/xhci-mvebu.h
> @@ -12,10 +12,16 @@ struct usb_hcd;
>  
>  #if IS_ENABLED(CONFIG_USB_XHCI_MVEBU)
>  int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd);
> +int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd);
>  #else
>  static inline int xhci_mvebu_mbus_init_quirk(struct usb_hcd *hcd)
>  {
>  	return 0;
>  }
> +
> +static inline int xhci_mvebu_a3700_init_quirk(struct usb_hcd *hcd)
> +{
> +	return 0;
> +}
>  #endif
>  #endif /* __LINUX_XHCI_MVEBU_H */
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ef09cb06212f..0ac4ec975547 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -98,6 +98,10 @@ static const struct xhci_plat_priv xhci_plat_marvell_armada = {
>  	.init_quirk = xhci_mvebu_mbus_init_quirk,
>  };
>  
> +static const struct xhci_plat_priv xhci_plat_marvell_armada3700 = {
> +	.init_quirk = xhci_mvebu_a3700_init_quirk,
> +};
> +
>  static const struct xhci_plat_priv xhci_plat_renesas_rcar_gen2 = {
>  	.firmware_name = XHCI_RCAR_FIRMWARE_NAME_V1,
>  	.init_quirk = xhci_rcar_init_quirk,
> @@ -123,6 +127,9 @@ static const struct of_device_id usb_xhci_of_match[] = {
>  	}, {
>  		.compatible = "marvell,armada-380-xhci",
>  		.data = &xhci_plat_marvell_armada,
> +	}, {
> +		.compatible = "marvell,armada3700-xhci",
> +		.data = &xhci_plat_marvell_armada3700,


Reviewed-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/10] usb: host: xhci: mvebu: add reset on resume quirk

[Miquel Raynal]

Hi Gregory,

Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Fri, 18 Jan 2019
17:04:01 +0100:

> Hi Miquel,
>  
>  On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > From: Ofer Heifetz <oferh@marvell.com>
> >
> > The mvebu xHCI host driver does not have suspend/resume support. Use of
> > the XHCI_RESET_ON_RESUME quirk is mandatory in order to avoid failures
> > after resume. This will work only if no USB device is plugged-in.
> >
> > While at it, mention in the Kconfig file that this IP is also present
> > on the A3700 SoC.
> >
> > Signed-off-by: Ofer Heifetz <oferh@marvell.com>
> > [miquel.raynal@bootlin.com: Reword the commit message]
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/usb/host/Kconfig      |  4 ++--
> >  drivers/usb/host/xhci-mvebu.c | 11 +++++++++++
> >  drivers/usb/host/xhci-mvebu.h |  6 ++++++
> >  drivers/usb/host/xhci-plat.c  |  7 +++++++
> >  4 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > index 16758b12a5e9..13d59be6a6aa 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -69,13 +69,13 @@ config USB_XHCI_MTK
> >  	  If unsure, say N.
> >  
> >  config USB_XHCI_MVEBU
> > -	tristate "xHCI support for Marvell Armada 375/38x"
> > +	tristate "xHCI support for Marvell Armada 375/38x/3700"  
> 
> I would name it 37xx to be consistent with 38x
> 
> >  	select USB_XHCI_PLATFORM
> >  	depends on HAS_IOMEM
> >  	depends on ARCH_MVEBU || COMPILE_TEST
> >  	---help---
> >  	  Say 'Y' to enable the support for the xHCI host controller
> > -	  found in Marvell Armada 375/38x ARM SOCs.
> > +	  found in Marvell Armada 375/38x/3700 ARM SOCs.  
> same here
> 

Sure!


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 03/10] usb: ehci-orion: avoid double PHY initialization

[Miquel Raynal]

No need to initialize the PHY from the driver's probe. It is done by
the core automatically and doing it twice would increment the
phy->powercount counter to 2 instead of 1. During later suspend
operation, the counter will be decremented to one, no phy->power_off()
will occur and worst than that, the following phy->power_on() at
resume time will be also skipped, failing the whole S2RAM operation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/usb/host/ehci-orion.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 1ad72647a069..3109f082949e 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -257,15 +257,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->phy)) {
 		err = PTR_ERR(priv->phy);
 		if (err != -ENOSYS)
-			goto err_phy_get;
-	} else {
-		err = phy_init(priv->phy);
-		if (err)
-			goto err_phy_init;
-
-		err = phy_power_on(priv->phy);
-		if (err)
-			goto err_phy_power_on;
+			goto err_dis_clk;
 	}
 
 	/*
@@ -297,19 +289,12 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
 
 	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (err)
-		goto err_add_hcd;
+		goto err_dis_clk;
 
 	device_wakeup_enable(hcd->self.controller);
 	return 0;
 
-err_add_hcd:
-	if (!IS_ERR(priv->phy))
-		phy_power_off(priv->phy);
-err_phy_power_on:
-	if (!IS_ERR(priv->phy))
-		phy_exit(priv->phy);
-err_phy_init:
-err_phy_get:
+err_dis_clk:
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
 	usb_put_hcd(hcd);
@@ -327,11 +312,6 @@ static int ehci_orion_drv_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(hcd);
 
-	if (!IS_ERR(priv->phy)) {
-		phy_power_off(priv->phy);
-		phy_exit(priv->phy);
-	}
-
 	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
 
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] usb: ehci-orion: avoid double PHY initialization

[Sergei Shtylyov]

Hello!

On 01/11/2019 04:31 PM, Miquel Raynal wrote:

> No need to initialize the PHY from the driver's probe. It is done by
> the core automatically and doing it twice would increment the
> phy->powercount counter to 2 instead of 1. During later suspend
> operation, the counter will be decremented to one, no phy->power_off()
> will occur and worst than that, the following phy->power_on() at

   Worse.

> resume time will be also skipped, failing the whole S2RAM operation.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/usb/host/ehci-orion.c | 26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
> index 1ad72647a069..3109f082949e 100644
> --- a/drivers/usb/host/ehci-orion.c
> +++ b/drivers/usb/host/ehci-orion.c
> @@ -257,15 +257,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->phy)) {
>  		err = PTR_ERR(priv->phy);
>  		if (err != -ENOSYS)
> -			goto err_phy_get;
> -	} else {
> -		err = phy_init(priv->phy);
> -		if (err)
> -			goto err_phy_init;
> -
> -		err = phy_power_on(priv->phy);
> -		if (err)
> -			goto err_phy_power_on;
> +			goto err_dis_clk;

   Familiar code in unfamiliar place. Somebody must have blindly copied it... :-)

[...]

MBR, Sergei

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] usb: ehci-orion: avoid double PHY initialization

[Miquel Raynal]

Hi Sergei,

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote on Fri, 11
Jan 2019 21:03:01 +0300:

> Hello!
> 
> On 01/11/2019 04:31 PM, Miquel Raynal wrote:
> 
> > No need to initialize the PHY from the driver's probe. It is done by
> > the core automatically and doing it twice would increment the
> > phy->powercount counter to 2 instead of 1. During later suspend
> > operation, the counter will be decremented to one, no phy->power_off()
> > will occur and worst than that, the following phy->power_on() at  
> 
>    Worse.

Noted

> 
> > resume time will be also skipped, failing the whole S2RAM operation.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/usb/host/ehci-orion.c | 26 +++-----------------------
> >  1 file changed, 3 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
> > index 1ad72647a069..3109f082949e 100644
> > --- a/drivers/usb/host/ehci-orion.c
> > +++ b/drivers/usb/host/ehci-orion.c
> > @@ -257,15 +257,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
> >  	if (IS_ERR(priv->phy)) {
> >  		err = PTR_ERR(priv->phy);
> >  		if (err != -ENOSYS)
> > -			goto err_phy_get;
> > -	} else {
> > -		err = phy_init(priv->phy);
> > -		if (err)
> > -			goto err_phy_init;
> > -
> > -		err = phy_power_on(priv->phy);
> > -		if (err)
> > -			goto err_phy_power_on;
> > +			goto err_dis_clk;  
> 
>    Familiar code in unfamiliar place. Somebody must have blindly copied it... :-)

Actually a git blame shows that this code is there since 2014, 4 years
before the HCD core supported PHYs. This driver was probably forgotten
during the process. 


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] usb: ehci-orion: avoid double PHY initialization

[Gregory CLEMENT]

Hi Miquel,
 
 On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> No need to initialize the PHY from the driver's probe. It is done by
> the core automatically and doing it twice would increment the

Do you know exactly which core take care of it?

When the phy support was added to this driver there was not such
feature. I made some research and found that recently (less than one
year ago) a series was added to initialize PHYs at HCD level[1]. I think
that our platform was forgotten in the conversion.

Could you check that we are now aligned with the requirement of this
series?

Thanks,

Gregory

[1]:https://www.spinics.net/lists/linux-usb/msg166281.html


> phy->powercount counter to 2 instead of 1. During later suspend
> operation, the counter will be decremented to one, no phy->power_off()
> will occur and worst than that, the following phy->power_on() at
> resume time will be also skipped, failing the whole S2RAM operation.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/usb/host/ehci-orion.c | 26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
> index 1ad72647a069..3109f082949e 100644
> --- a/drivers/usb/host/ehci-orion.c
> +++ b/drivers/usb/host/ehci-orion.c
> @@ -257,15 +257,7 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->phy)) {
>  		err = PTR_ERR(priv->phy);
>  		if (err != -ENOSYS)
> -			goto err_phy_get;
> -	} else {
> -		err = phy_init(priv->phy);
> -		if (err)
> -			goto err_phy_init;
> -
> -		err = phy_power_on(priv->phy);
> -		if (err)
> -			goto err_phy_power_on;
> +			goto err_dis_clk;
>  	}
>  
>  	/*
> @@ -297,19 +289,12 @@ static int ehci_orion_drv_probe(struct platform_device *pdev)
>  
>  	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (err)
> -		goto err_add_hcd;
> +		goto err_dis_clk;
>  
>  	device_wakeup_enable(hcd->self.controller);
>  	return 0;
>  
> -err_add_hcd:
> -	if (!IS_ERR(priv->phy))
> -		phy_power_off(priv->phy);
> -err_phy_power_on:
> -	if (!IS_ERR(priv->phy))
> -		phy_exit(priv->phy);
> -err_phy_init:
> -err_phy_get:
> +err_dis_clk:
>  	if (!IS_ERR(priv->clk))
>  		clk_disable_unprepare(priv->clk);
>  	usb_put_hcd(hcd);
> @@ -327,11 +312,6 @@ static int ehci_orion_drv_remove(struct platform_device *pdev)
>  
>  	usb_remove_hcd(hcd);
>  
> -	if (!IS_ERR(priv->phy)) {
> -		phy_power_off(priv->phy);
> -		phy_exit(priv->phy);
> -	}
> -
>  	if (!IS_ERR(priv->clk))
>  		clk_disable_unprepare(priv->clk);
>  
> -- 
> 2.19.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/10] usb: ehci-orion: avoid double PHY initialization

[Miquel Raynal]

Hi Gregory,

Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Fri, 18 Jan 2019
17:25:30 +0100:

> Hi Miquel,
>  
>  On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > No need to initialize the PHY from the driver's probe. It is done by
> > the core automatically and doing it twice would increment the  
> 
> Do you know exactly which core take care of it?

The Orion's probe function calls usb_add_hcd() which handles PHY
initialization (init, set mode, power on).

> 
> When the phy support was added to this driver there was not such
> feature. I made some research and found that recently (less than one
> year ago) a series was added to initialize PHYs at HCD level[1]. I think
> that our platform was forgotten in the conversion.

AFAICS the conversion was applied to ehci-platform.c but not to other
drivers so yes, this one was left aside but there are maybe others in
the same situation.

> 
> Could you check that we are now aligned with the requirement of this
> series?

There has been quite a few evolutions since this series but for what I
understand, yes we are.


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 04/10] usb: ehci-orion: add S2RAM support

[Miquel Raynal]

Add suspend/resume callbacks to reset the host controller properly
during S2RAM operation.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/usb/host/ehci-orion.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 3109f082949e..dab22aa57c0b 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -182,6 +182,30 @@ static int ehci_orion_drv_reset(struct usb_hcd *hcd)
 	return ret;
 }
 
+static int __maybe_unused ehci_orion_drv_suspend(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+
+	ehci_prepare_ports_for_controller_suspend(ehci,
+						  device_may_wakeup(dev));
+
+	return ehci_suspend(hcd, false);
+}
+
+static int __maybe_unused ehci_orion_drv_resume(struct device *dev)
+{
+	struct usb_hcd *hcd = dev_get_drvdata(dev);
+	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+
+	ehci_prepare_ports_for_controller_resume(ehci);
+
+	return ehci_resume(hcd, false);
+}
+
+static SIMPLE_DEV_PM_OPS(ehci_orion_pm_ops, ehci_orion_drv_suspend,
+			 ehci_orion_drv_resume);
+
 static const struct ehci_driver_overrides orion_overrides __initconst = {
 	.extra_priv_size =	sizeof(struct orion_ehci_hcd),
 	.reset = ehci_orion_drv_reset,
@@ -334,6 +358,7 @@ static struct platform_driver ehci_orion_driver = {
 	.driver = {
 		.name	= "orion-ehci",
 		.of_match_table = ehci_orion_dt_ids,
+		.pm = &ehci_orion_pm_ops,
 	},
 };
 
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Miquel Raynal]

Marvell Armada 3700 SoC has two USB controllers, each of them being
wired to an internal UTMI PHY. Add a driver to control them.

Igal Liberman worked on supporting the PHY, I took the while 'register
configuration' from his work and rewrote almost entirely the
driver/bindings around it.

Co-developed-by: Igal Liberman <igall@marvell.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Igal Liberman <igall@marvell.com>
---
 drivers/phy/marvell/Kconfig                |   9 +
 drivers/phy/marvell/Makefile               |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
 3 files changed, 307 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 9c90c0408ea3..b8e9dd38ad0d 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
 	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
 	  used by various controllers: Ethernet, SATA, USB3, PCIe.
 
+config PHY_MVEBU_A3700_UTMI
+	tristate "Marvell A3700 UTMI driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	default y
+	select GENERIC_PHY
+	help
+	  Enable this to support Marvell A3700 UTMI PHY driver.
+
 config PHY_MVEBU_CP110_COMPHY
 	tristate "Marvell CP110 comphy driver"
 	depends on ARCH_MVEBU || COMPILE_TEST
diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
index c13a0c8ab6f0..82f291cf59ee 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
+obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
 obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
 obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
 obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
new file mode 100644
index 000000000000..97d8235d661d
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell
+ *
+ * Authors:
+ *   Evan Wang <xswang@marvell.com>
+ *   Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Marvell A3700 UTMI PHY driver
+ */
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* Armada 3700 UTMI PHY registers */
+#define USB2_PHY_PLL_CTRL_REG0			0x0
+#define   PLL_REF_DIV_OFF			0
+#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
+#define   PLL_REF_DIV_5				0x5
+#define   PLL_FB_DIV_OFF			16
+#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
+#define   PLL_FB_DIV_96				96
+#define   PLL_SEL_LPFR_OFF			28
+#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
+#define   PLL_READY				BIT(31)
+#define USB2_PHY_CAL_CTRL			0x8
+#define   PHY_PLLCAL_DONE			BIT(31)
+#define   PHY_IMPCAL_DONE			BIT(23)
+#define USB2_RX_CHAN_CTRL1			0x18
+#define   USB2PHY_SQCAL_DONE			BIT(31)
+#define USB2_PHY_OTG_CTRL			0x34
+#define   PHY_PU_OTG				BIT(4)
+#define USB2_PHY_CHRGR_DETECT			0x38
+#define   PHY_CDP_EN				BIT(2)
+#define   PHY_DCP_EN				BIT(3)
+#define   PHY_PD_EN				BIT(4)
+#define   PHY_PU_CHRG_DTC			BIT(5)
+#define   PHY_CDP_DM_AUTO			BIT(7)
+#define   PHY_ENSWITCH_DP			BIT(12)
+#define   PHY_ENSWITCH_DM			BIT(13)
+
+/* Armada 3700 USB miscellaneous registers */
+#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
+#define   RB_USB2PHY_PU				BIT(0)
+#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
+#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
+#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
+
+#define PLL_LOCK_DELAY_US			10000
+#define PLL_LOCK_TIMEOUT_US			1000000
+
+/**
+ * struct mvebu_a3700_utmi_caps - PHY capabilities
+ *
+ * @usb32: Flag indicating which PHY is in use (impacts the register map):
+ *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
+ *           - The UTMI PHY wired to the USB2 controller (host only)
+ * @ops: PHY operations
+ */
+struct mvebu_a3700_utmi_caps {
+	int usb32;
+	const struct phy_ops *ops;
+};
+
+/**
+ * struct mvebu_a3700_utmi - PHY driver data
+ *
+ * @regs: PHY registers
+ * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
+ * @caps: PHY capabilities
+ * @phy: PHY handle
+ */
+struct mvebu_a3700_utmi {
+	void __iomem *regs;
+	struct regmap *usb_misc;
+	const struct mvebu_a3700_utmi_caps *caps;
+	struct phy *phy;
+};
+
+static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
+{
+	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
+	struct device *dev = &phy->dev;
+	int usb32 = utmi->caps->usb32;
+	int ret = 0;
+	u32 reg;
+
+	if (!utmi)
+		return -ENODEV;
+
+	/*
+	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
+	 * See "PLL Settings for Typical REFCLK" table.
+	 */
+	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
+	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
+	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
+		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
+	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
+
+	/* Enable PHY pull up and disable USB2 suspend */
+	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
+			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
+			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
+
+	if (usb32) {
+		/* Power up OTG module */
+		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
+		reg |= PHY_PU_OTG;
+		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
+
+		/* Disable PHY charger detection */
+		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
+		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
+			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
+		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
+
+		/* Disable PHY DP/DM pull-down (used for device mode) */
+		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
+				   USB2_DP_PULLDN_DEV_MODE |
+				   USB2_DM_PULLDN_DEV_MODE, 0);
+	}
+
+	/* Wait for PLL calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
+				 reg & PHY_PLLCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 PLL calibration\n");
+		return ret;
+	}
+
+	/* Wait for impedance calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
+				 reg & PHY_IMPCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 impedance calibration\n");
+		return ret;
+	}
+
+	/* Wait for squetch calibration */
+	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
+				 reg & USB2PHY_SQCAL_DONE,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret) {
+		dev_err(dev, "Failed to end USB2 unknown calibration\n");
+		return ret;
+	}
+
+	/* Wait for PLL to be locked */
+	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
+				 reg & PLL_READY,
+				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
+	if (ret)
+		dev_err(dev, "Failed to lock USB2 PLL\n");
+
+	return ret;
+}
+
+static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
+{
+	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
+	int usb32 = utmi->caps->usb32;
+	u32 reg;
+
+	if (!utmi)
+		return -ENODEV;
+
+	/* Disable PHY pull-up and enable USB2 suspend */
+	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
+	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
+	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
+
+	/* Power down OTG module */
+	if (usb32) {
+		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
+		reg &= ~PHY_PU_OTG;
+		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
+	}
+
+	return 0;
+}
+
+static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
+	.power_on = mvebu_a3700_utmi_phy_power_on,
+	.power_off = mvebu_a3700_utmi_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
+	.usb32 = true,
+	.ops = &mvebu_a3700_utmi_phy_ops,
+};
+
+static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
+	.usb32 = false,
+	.ops = &mvebu_a3700_utmi_phy_ops,
+};
+
+static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
+	{
+		.compatible = "marvell,a3700-utmi-otg-phy",
+		.data = &mvebu_a3700_utmi_otg_phy_caps,
+	},
+	{
+		.compatible = "marvell,a3700-utmi-host-phy",
+		.data = &mvebu_a3700_utmi_host_phy_caps,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
+
+static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *dev_id;
+	struct device *dev = &pdev->dev;
+	struct mvebu_a3700_utmi *utmi;
+	struct phy_provider *provider;
+	const struct phy_ops *ops;
+	struct resource *res;
+
+	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
+	if (!utmi)
+		return -ENOMEM;
+
+	/* Get UTMI memory region */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "Missing UTMI PHY memory resource\n");
+		return -ENODEV;
+	}
+
+	utmi->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(utmi->regs))
+		return PTR_ERR(utmi->regs);
+
+	/* Get miscellaneous Host/PHY region */
+	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
+							 "marvell,usb-misc-reg");
+	if (IS_ERR(utmi->usb_misc)) {
+		dev_err(dev,
+			"Missing USB misc purpose system controller\n");
+		return PTR_ERR(utmi->usb_misc);
+	}
+
+	/* Retrieve PHY capabilities */
+	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
+	if (!dev_id)
+		return -EINVAL;
+
+	utmi->caps = dev_id->data;
+	ops = utmi->caps->ops;
+
+	/* Instantiate the PHY */
+	utmi->phy = devm_phy_create(dev, NULL, ops);
+	if (IS_ERR(utmi->phy)) {
+		dev_err(dev, "Failed to create the UTMI PHY\n");
+		return PTR_ERR(utmi->phy);
+	}
+
+	phy_set_drvdata(utmi->phy, utmi);
+
+	/* Ensure the PHY is powered off */
+	ops->power_off(utmi->phy);
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mvebu_a3700_utmi_driver = {
+	.probe	= mvebu_a3700_utmi_phy_probe,
+	.remove = mvebu_a3700_utmi_phy_remove,
+	.driver	= {
+		.name		= "mvebu-a3700-utmi-phy",
+		.owner		= THIS_MODULE,
+		.of_match_table	= mvebu_a3700_utmi_of_match,
+	 },
+};
+module_platform_driver(mvebu_a3700_utmi_driver);
+
+MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Chunfeng Yun]

Hi,
On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:
> Marvell Armada 3700 SoC has two USB controllers, each of them being
> wired to an internal UTMI PHY. Add a driver to control them.
> 
> Igal Liberman worked on supporting the PHY, I took the while 'register
> configuration' from his work and rewrote almost entirely the
> driver/bindings around it.
> 
> Co-developed-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Igal Liberman <igall@marvell.com>
> ---
>  drivers/phy/marvell/Kconfig                |   9 +
>  drivers/phy/marvell/Makefile               |   1 +
>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
>  3 files changed, 307 insertions(+)
>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> 
> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> index 9c90c0408ea3..b8e9dd38ad0d 100644
> --- a/drivers/phy/marvell/Kconfig
> +++ b/drivers/phy/marvell/Kconfig
> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
>  
> +config PHY_MVEBU_A3700_UTMI
> +	tristate "Marvell A3700 UTMI driver"
> +	depends on ARCH_MVEBU || COMPILE_TEST
> +	depends on OF
> +	default y
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support Marvell A3700 UTMI PHY driver.
> +
>  config PHY_MVEBU_CP110_COMPHY
>  	tristate "Marvell CP110 comphy driver"
>  	depends on ARCH_MVEBU || COMPILE_TEST
> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> index c13a0c8ab6f0..82f291cf59ee 100644
> --- a/drivers/phy/marvell/Makefile
> +++ b/drivers/phy/marvell/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> new file mode 100644
> index 000000000000..97d8235d661d
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Marvell
> + *
> + * Authors:
> + *   Evan Wang <xswang@marvell.com>
> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
> + *
> + * Marvell A3700 UTMI PHY driver
> + */
> +
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/* Armada 3700 UTMI PHY registers */
> +#define USB2_PHY_PLL_CTRL_REG0			0x0
> +#define   PLL_REF_DIV_OFF			0
> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
use GENMASK?
> +#define   PLL_REF_DIV_5				0x5
> +#define   PLL_FB_DIV_OFF			16
> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
> +#define   PLL_FB_DIV_96				96
> +#define   PLL_SEL_LPFR_OFF			28
> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
> +#define   PLL_READY				BIT(31)
> +#define USB2_PHY_CAL_CTRL			0x8
> +#define   PHY_PLLCAL_DONE			BIT(31)
> +#define   PHY_IMPCAL_DONE			BIT(23)
> +#define USB2_RX_CHAN_CTRL1			0x18
> +#define   USB2PHY_SQCAL_DONE			BIT(31)
> +#define USB2_PHY_OTG_CTRL			0x34
> +#define   PHY_PU_OTG				BIT(4)
> +#define USB2_PHY_CHRGR_DETECT			0x38
> +#define   PHY_CDP_EN				BIT(2)
> +#define   PHY_DCP_EN				BIT(3)
> +#define   PHY_PD_EN				BIT(4)
> +#define   PHY_PU_CHRG_DTC			BIT(5)
> +#define   PHY_CDP_DM_AUTO			BIT(7)
> +#define   PHY_ENSWITCH_DP			BIT(12)
> +#define   PHY_ENSWITCH_DM			BIT(13)
> +
> +/* Armada 3700 USB miscellaneous registers */
> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
> +#define   RB_USB2PHY_PU				BIT(0)
> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
> +
> +#define PLL_LOCK_DELAY_US			10000
> +#define PLL_LOCK_TIMEOUT_US			1000000
> +
> +/**
> + * struct mvebu_a3700_utmi_caps - PHY capabilities
> + *
> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
> + *           - The UTMI PHY wired to the USB2 controller (host only)
> + * @ops: PHY operations
> + */
> +struct mvebu_a3700_utmi_caps {
> +	int usb32;
> +	const struct phy_ops *ops;
> +};
> +
> +/**
> + * struct mvebu_a3700_utmi - PHY driver data
> + *
> + * @regs: PHY registers
> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
> + * @caps: PHY capabilities
> + * @phy: PHY handle
> + */
> +struct mvebu_a3700_utmi {
> +	void __iomem *regs;
> +	struct regmap *usb_misc;
> +	const struct mvebu_a3700_utmi_caps *caps;
> +	struct phy *phy;
> +};
> +
> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	struct device *dev = &phy->dev;
> +	int usb32 = utmi->caps->usb32;
> +	int ret = 0;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
the check here may be not necessary
> +
> +	/*
> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
> +	 * See "PLL Settings for Typical REFCLK" table.
> +	 */
> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> +
> +	/* Enable PHY pull up and disable USB2 suspend */
> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
> +
> +	if (usb32) {
> +		/* Power up OTG module */
> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> +		reg |= PHY_PU_OTG;
> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> +
> +		/* Disable PHY charger detection */
> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
> +
> +		/* Disable PHY DP/DM pull-down (used for device mode) */
> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> +				   USB2_DP_PULLDN_DEV_MODE |
> +				   USB2_DM_PULLDN_DEV_MODE, 0);
> +	}
> +
> +	/* Wait for PLL calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> +				 reg & PHY_PLLCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for impedance calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> +				 reg & PHY_IMPCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for squetch calibration */
> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
> +				 reg & USB2PHY_SQCAL_DONE,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
> +		return ret;
> +	}
> +
> +	/* Wait for PLL to be locked */
> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
> +				 reg & PLL_READY,
> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> +	if (ret)
> +		dev_err(dev, "Failed to lock USB2 PLL\n");
> +
> +	return ret;
> +}
> +
> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	int usb32 = utmi->caps->usb32;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
> +
> +	/* Disable PHY pull-up and enable USB2 suspend */
> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
> +
> +	/* Power down OTG module */
> +	if (usb32) {
> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> +		reg &= ~PHY_PU_OTG;
> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
> +	.power_on = mvebu_a3700_utmi_phy_power_on,
> +	.power_off = mvebu_a3700_utmi_phy_power_off,
> +	.owner = THIS_MODULE,
> +};
> +
> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
> +	.usb32 = true,
> +	.ops = &mvebu_a3700_utmi_phy_ops,
> +};
> +
> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
> +	.usb32 = false,
> +	.ops = &mvebu_a3700_utmi_phy_ops,
> +};
> +
> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
> +	{
> +		.compatible = "marvell,a3700-utmi-otg-phy",
> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
> +	},
> +	{
> +		.compatible = "marvell,a3700-utmi-host-phy",
> +		.data = &mvebu_a3700_utmi_host_phy_caps,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
> +
> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *dev_id;
> +	struct device *dev = &pdev->dev;
> +	struct mvebu_a3700_utmi *utmi;
> +	struct phy_provider *provider;
> +	const struct phy_ops *ops;
> +	struct resource *res;
> +
> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
> +	if (!utmi)
> +		return -ENOMEM;
> +
> +	/* Get UTMI memory region */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
> +		return -ENODEV;
> +	}
> +
> +	utmi->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(utmi->regs))
> +		return PTR_ERR(utmi->regs);
> +
> +	/* Get miscellaneous Host/PHY region */
> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
> +							 "marvell,usb-misc-reg");
> +	if (IS_ERR(utmi->usb_misc)) {
> +		dev_err(dev,
> +			"Missing USB misc purpose system controller\n");
> +		return PTR_ERR(utmi->usb_misc);
> +	}
> +
> +	/* Retrieve PHY capabilities */
> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
> +	if (!dev_id)
> +		return -EINVAL;
> +
> +	utmi->caps = dev_id->data;
a useful API to get @data: of_device_get_match_data()

> +	ops = utmi->caps->ops;
> +
> +	/* Instantiate the PHY */
> +	utmi->phy = devm_phy_create(dev, NULL, ops);
> +	if (IS_ERR(utmi->phy)) {
> +		dev_err(dev, "Failed to create the UTMI PHY\n");
> +		return PTR_ERR(utmi->phy);
> +	}
> +
> +	phy_set_drvdata(utmi->phy, utmi);
> +
> +	/* Ensure the PHY is powered off */
> +	ops->power_off(utmi->phy);
> +
> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(provider);
> +}
> +
> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static struct platform_driver mvebu_a3700_utmi_driver = {
> +	.probe	= mvebu_a3700_utmi_phy_probe,
> +	.remove = mvebu_a3700_utmi_phy_remove,
> +	.driver	= {
> +		.name		= "mvebu-a3700-utmi-phy",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= mvebu_a3700_utmi_of_match,
> +	 },
> +};
> +module_platform_driver(mvebu_a3700_utmi_driver);
> +
> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
> +MODULE_LICENSE("GPL");



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Kishon Vijay Abraham I]

Hi,

On 15/01/19 8:10 AM, Chunfeng Yun wrote:
> Hi,
> On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:
>> Marvell Armada 3700 SoC has two USB controllers, each of them being
>> wired to an internal UTMI PHY. Add a driver to control them.
>>
>> Igal Liberman worked on supporting the PHY, I took the while 'register
>> configuration' from his work and rewrote almost entirely the
>> driver/bindings around it.
>>
>> Co-developed-by: Igal Liberman <igall@marvell.com>
>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> Signed-off-by: Igal Liberman <igall@marvell.com>
>> ---
>>  drivers/phy/marvell/Kconfig                |   9 +
>>  drivers/phy/marvell/Makefile               |   1 +
>>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
>>  3 files changed, 307 insertions(+)
>>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>>
>> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
>> index 9c90c0408ea3..b8e9dd38ad0d 100644
>> --- a/drivers/phy/marvell/Kconfig
>> +++ b/drivers/phy/marvell/Kconfig
>> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
>>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
>>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
>>  
>> +config PHY_MVEBU_A3700_UTMI
>> +	tristate "Marvell A3700 UTMI driver"
>> +	depends on ARCH_MVEBU || COMPILE_TEST
>> +	depends on OF
>> +	default y
>> +	select GENERIC_PHY
>> +	help
>> +	  Enable this to support Marvell A3700 UTMI PHY driver.
>> +
>>  config PHY_MVEBU_CP110_COMPHY
>>  	tristate "Marvell CP110 comphy driver"
>>  	depends on ARCH_MVEBU || COMPILE_TEST
>> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
>> index c13a0c8ab6f0..82f291cf59ee 100644
>> --- a/drivers/phy/marvell/Makefile
>> +++ b/drivers/phy/marvell/Makefile
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
>>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
>>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
>> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
>>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
>>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
>>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
>> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>> new file mode 100644
>> index 000000000000..97d8235d661d
>> --- /dev/null
>> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
>> @@ -0,0 +1,297 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Marvell
>> + *
>> + * Authors:
>> + *   Evan Wang <xswang@marvell.com>
>> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
>> + *
>> + * Marvell A3700 UTMI PHY driver
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +/* Armada 3700 UTMI PHY registers */
>> +#define USB2_PHY_PLL_CTRL_REG0			0x0
>> +#define   PLL_REF_DIV_OFF			0
>> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)
> use GENMASK?
>> +#define   PLL_REF_DIV_5				0x5
>> +#define   PLL_FB_DIV_OFF			16
>> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
>> +#define   PLL_FB_DIV_96				96
>> +#define   PLL_SEL_LPFR_OFF			28
>> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
>> +#define   PLL_READY				BIT(31)
>> +#define USB2_PHY_CAL_CTRL			0x8
>> +#define   PHY_PLLCAL_DONE			BIT(31)
>> +#define   PHY_IMPCAL_DONE			BIT(23)
>> +#define USB2_RX_CHAN_CTRL1			0x18
>> +#define   USB2PHY_SQCAL_DONE			BIT(31)
>> +#define USB2_PHY_OTG_CTRL			0x34
>> +#define   PHY_PU_OTG				BIT(4)
>> +#define USB2_PHY_CHRGR_DETECT			0x38
>> +#define   PHY_CDP_EN				BIT(2)
>> +#define   PHY_DCP_EN				BIT(3)
>> +#define   PHY_PD_EN				BIT(4)
>> +#define   PHY_PU_CHRG_DTC			BIT(5)
>> +#define   PHY_CDP_DM_AUTO			BIT(7)
>> +#define   PHY_ENSWITCH_DP			BIT(12)
>> +#define   PHY_ENSWITCH_DM			BIT(13)
>> +
>> +/* Armada 3700 USB miscellaneous registers */
>> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
>> +#define   RB_USB2PHY_PU				BIT(0)
>> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
>> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
>> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
>> +
>> +#define PLL_LOCK_DELAY_US			10000
>> +#define PLL_LOCK_TIMEOUT_US			1000000
>> +
>> +/**
>> + * struct mvebu_a3700_utmi_caps - PHY capabilities
>> + *
>> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
>> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
>> + *           - The UTMI PHY wired to the USB2 controller (host only)
>> + * @ops: PHY operations
>> + */
>> +struct mvebu_a3700_utmi_caps {
>> +	int usb32;
>> +	const struct phy_ops *ops;
>> +};
>> +
>> +/**
>> + * struct mvebu_a3700_utmi - PHY driver data
>> + *
>> + * @regs: PHY registers
>> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
>> + * @caps: PHY capabilities
>> + * @phy: PHY handle
>> + */
>> +struct mvebu_a3700_utmi {
>> +	void __iomem *regs;
>> +	struct regmap *usb_misc;
>> +	const struct mvebu_a3700_utmi_caps *caps;
>> +	struct phy *phy;
>> +};
>> +
>> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
>> +{
>> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
>> +	struct device *dev = &phy->dev;
>> +	int usb32 = utmi->caps->usb32;
>> +	int ret = 0;
>> +	u32 reg;
>> +
>> +	if (!utmi)
>> +		return -ENODEV;
> the check here may be not necessary

right. utmi is already referenced above.
>> +
>> +	/*
>> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
>> +	 * See "PLL Settings for Typical REFCLK" table.
>> +	 */
>> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
>> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
>> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
>> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
>> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
>> +
>> +	/* Enable PHY pull up and disable USB2 suspend */
>> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
>> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
>> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
>> +
>> +	if (usb32) {
>> +		/* Power up OTG module */
>> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
>> +		reg |= PHY_PU_OTG;
>> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
>> +
>> +		/* Disable PHY charger detection */
>> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
>> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
>> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
>> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
>> +
>> +		/* Disable PHY DP/DM pull-down (used for device mode) */
>> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
>> +				   USB2_DP_PULLDN_DEV_MODE |
>> +				   USB2_DM_PULLDN_DEV_MODE, 0);
>> +	}
>> +
>> +	/* Wait for PLL calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
>> +				 reg & PHY_PLLCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for impedance calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
>> +				 reg & PHY_IMPCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for squetch calibration */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
>> +				 reg & USB2PHY_SQCAL_DONE,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Wait for PLL to be locked */
>> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
>> +				 reg & PLL_READY,
>> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
>> +	if (ret)
>> +		dev_err(dev, "Failed to lock USB2 PLL\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
>> +{
>> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
>> +	int usb32 = utmi->caps->usb32;
>> +	u32 reg;
>> +
>> +	if (!utmi)
>> +		return -ENODEV;

here too, utmi is referenced above.
>> +
>> +	/* Disable PHY pull-up and enable USB2 suspend */
>> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
>> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
>> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
>> +
>> +	/* Power down OTG module */
>> +	if (usb32) {
>> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
>> +		reg &= ~PHY_PU_OTG;
>> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
>> +	.power_on = mvebu_a3700_utmi_phy_power_on,
>> +	.power_off = mvebu_a3700_utmi_phy_power_off,
>> +	.owner = THIS_MODULE,
>> +};
>> +
>> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
>> +	.usb32 = true,
>> +	.ops = &mvebu_a3700_utmi_phy_ops,
>> +};
>> +
>> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
>> +	.usb32 = false,
>> +	.ops = &mvebu_a3700_utmi_phy_ops,
>> +};
>> +
>> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
>> +	{
>> +		.compatible = "marvell,a3700-utmi-otg-phy",
>> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
>> +	},
>> +	{
>> +		.compatible = "marvell,a3700-utmi-host-phy",
>> +		.data = &mvebu_a3700_utmi_host_phy_caps,
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
>> +
>> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
>> +{
>> +	const struct of_device_id *dev_id;
>> +	struct device *dev = &pdev->dev;
>> +	struct mvebu_a3700_utmi *utmi;
>> +	struct phy_provider *provider;
>> +	const struct phy_ops *ops;
>> +	struct resource *res;
>> +
>> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>> +	if (!utmi)
>> +		return -ENOMEM;
>> +
>> +	/* Get UTMI memory region */
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res) {
>> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	utmi->regs = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(utmi->regs))
>> +		return PTR_ERR(utmi->regs);
>> +
>> +	/* Get miscellaneous Host/PHY region */
>> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
>> +							 "marvell,usb-misc-reg");
>> +	if (IS_ERR(utmi->usb_misc)) {
>> +		dev_err(dev,
>> +			"Missing USB misc purpose system controller\n");
>> +		return PTR_ERR(utmi->usb_misc);
>> +	}
>> +
>> +	/* Retrieve PHY capabilities */
>> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
>> +	if (!dev_id)
>> +		return -EINVAL;
>> +
>> +	utmi->caps = dev_id->data;
> a useful API to get @data: of_device_get_match_data()
> 
>> +	ops = utmi->caps->ops;
>> +
>> +	/* Instantiate the PHY */
>> +	utmi->phy = devm_phy_create(dev, NULL, ops);
>> +	if (IS_ERR(utmi->phy)) {
>> +		dev_err(dev, "Failed to create the UTMI PHY\n");
>> +		return PTR_ERR(utmi->phy);
>> +	}
>> +
>> +	phy_set_drvdata(utmi->phy, utmi);
>> +
>> +	/* Ensure the PHY is powered off */
>> +	ops->power_off(utmi->phy);
>> +
>> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> +	return PTR_ERR_OR_ZERO(provider);
>> +}
>> +
>> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
>> +{
>> +	return 0;
>> +}

This empty remove function can be removed.

Thanks
Kishon

>> +
>> +static struct platform_driver mvebu_a3700_utmi_driver = {
>> +	.probe	= mvebu_a3700_utmi_phy_probe,
>> +	.remove = mvebu_a3700_utmi_phy_remove,
>> +	.driver	= {
>> +		.name		= "mvebu-a3700-utmi-phy",
>> +		.owner		= THIS_MODULE,
>> +		.of_match_table	= mvebu_a3700_utmi_of_match,
>> +	 },
>> +};
>> +module_platform_driver(mvebu_a3700_utmi_driver);
>> +
>> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
>> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
>> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
>> +MODULE_LICENSE("GPL");
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Miquel Raynal]

Hi Kishon, Chunfeng,

Kishon Vijay Abraham I <kishon@ti.com> wrote on Wed, 16 Jan 2019
14:50:58 +0530:

> Hi,
> 
> On 15/01/19 8:10 AM, Chunfeng Yun wrote:
> > Hi,
> > On Fri, 2019-01-11 at 14:31 +0100, Miquel Raynal wrote:  
> >> Marvell Armada 3700 SoC has two USB controllers, each of them being
> >> wired to an internal UTMI PHY. Add a driver to control them.
> >>
> >> Igal Liberman worked on supporting the PHY, I took the while 'register
> >> configuration' from his work and rewrote almost entirely the
> >> driver/bindings around it.
> >>
> >> Co-developed-by: Igal Liberman <igall@marvell.com>
> >> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >> Signed-off-by: Igal Liberman <igall@marvell.com>
> >> ---
> >>  drivers/phy/marvell/Kconfig                |   9 +
> >>  drivers/phy/marvell/Makefile               |   1 +
> >>  drivers/phy/marvell/phy-mvebu-a3700-utmi.c | 297 +++++++++++++++++++++
> >>  3 files changed, 307 insertions(+)
> >>  create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >>
> >> diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
> >> index 9c90c0408ea3..b8e9dd38ad0d 100644
> >> --- a/drivers/phy/marvell/Kconfig
> >> +++ b/drivers/phy/marvell/Kconfig
> >> @@ -33,6 +33,15 @@ config PHY_MVEBU_A3700_COMPHY
> >>  	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
> >>  	  used by various controllers: Ethernet, SATA, USB3, PCIe.
> >>  
> >> +config PHY_MVEBU_A3700_UTMI
> >> +	tristate "Marvell A3700 UTMI driver"
> >> +	depends on ARCH_MVEBU || COMPILE_TEST
> >> +	depends on OF
> >> +	default y
> >> +	select GENERIC_PHY
> >> +	help
> >> +	  Enable this to support Marvell A3700 UTMI PHY driver.
> >> +
> >>  config PHY_MVEBU_CP110_COMPHY
> >>  	tristate "Marvell CP110 comphy driver"
> >>  	depends on ARCH_MVEBU || COMPILE_TEST
> >> diff --git a/drivers/phy/marvell/Makefile b/drivers/phy/marvell/Makefile
> >> index c13a0c8ab6f0..82f291cf59ee 100644
> >> --- a/drivers/phy/marvell/Makefile
> >> +++ b/drivers/phy/marvell/Makefile
> >> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
> >>  obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
> >>  obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
> >>  obj-$(CONFIG_PHY_MVEBU_A3700_COMPHY)	+= phy-mvebu-a3700-comphy.o
> >> +obj-$(CONFIG_PHY_MVEBU_A3700_UTMI)	+= phy-mvebu-a3700-utmi.o
> >>  obj-$(CONFIG_PHY_MVEBU_CP110_COMPHY)	+= phy-mvebu-cp110-comphy.o
> >>  obj-$(CONFIG_PHY_MVEBU_SATA)		+= phy-mvebu-sata.o
> >>  obj-$(CONFIG_PHY_PXA_28NM_HSIC)		+= phy-pxa-28nm-hsic.o
> >> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >> new file mode 100644
> >> index 000000000000..97d8235d661d
> >> --- /dev/null
> >> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> >> @@ -0,0 +1,297 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2018 Marvell
> >> + *
> >> + * Authors:
> >> + *   Evan Wang <xswang@marvell.com>
> >> + *   Miquèl Raynal <miquel.raynal@bootlin.com>
> >> + *
> >> + * Marvell A3700 UTMI PHY driver
> >> + */
> >> +
> >> +#include <linux/io.h>
> >> +#include <linux/iopoll.h>
> >> +#include <linux/mfd/syscon.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/phy/phy.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +/* Armada 3700 UTMI PHY registers */
> >> +#define USB2_PHY_PLL_CTRL_REG0			0x0
> >> +#define   PLL_REF_DIV_OFF			0
> >> +#define   PLL_REF_DIV_MASK			(0x7F << PLL_REF_DIV_OFF)  
> > use GENMASK?  

Ok

> >> +#define   PLL_REF_DIV_5				0x5
> >> +#define   PLL_FB_DIV_OFF			16
> >> +#define   PLL_FB_DIV_MASK			(0x1FF << PLL_FB_DIV_OFF)
> >> +#define   PLL_FB_DIV_96				96
> >> +#define   PLL_SEL_LPFR_OFF			28
> >> +#define   PLL_SEL_LPFR_MASK			(0x3 << PLL_SEL_LPFR_OFF)
> >> +#define   PLL_READY				BIT(31)
> >> +#define USB2_PHY_CAL_CTRL			0x8
> >> +#define   PHY_PLLCAL_DONE			BIT(31)
> >> +#define   PHY_IMPCAL_DONE			BIT(23)
> >> +#define USB2_RX_CHAN_CTRL1			0x18
> >> +#define   USB2PHY_SQCAL_DONE			BIT(31)
> >> +#define USB2_PHY_OTG_CTRL			0x34
> >> +#define   PHY_PU_OTG				BIT(4)
> >> +#define USB2_PHY_CHRGR_DETECT			0x38
> >> +#define   PHY_CDP_EN				BIT(2)
> >> +#define   PHY_DCP_EN				BIT(3)
> >> +#define   PHY_PD_EN				BIT(4)
> >> +#define   PHY_PU_CHRG_DTC			BIT(5)
> >> +#define   PHY_CDP_DM_AUTO			BIT(7)
> >> +#define   PHY_ENSWITCH_DP			BIT(12)
> >> +#define   PHY_ENSWITCH_DM			BIT(13)
> >> +
> >> +/* Armada 3700 USB miscellaneous registers */
> >> +#define USB2_PHY_CTRL(usb32)			(usb32 ? 0x20 : 0x4)
> >> +#define   RB_USB2PHY_PU				BIT(0)
> >> +#define   USB2_DP_PULLDN_DEV_MODE		BIT(5)
> >> +#define   USB2_DM_PULLDN_DEV_MODE		BIT(6)
> >> +#define   RB_USB2PHY_SUSPM(usb32)		(usb32 ? BIT(14) : BIT(7))
> >> +
> >> +#define PLL_LOCK_DELAY_US			10000
> >> +#define PLL_LOCK_TIMEOUT_US			1000000
> >> +
> >> +/**
> >> + * struct mvebu_a3700_utmi_caps - PHY capabilities
> >> + *
> >> + * @usb32: Flag indicating which PHY is in use (impacts the register map):
> >> + *           - The UTMI PHY wired to the USB3/USB2 controller (otg)
> >> + *           - The UTMI PHY wired to the USB2 controller (host only)
> >> + * @ops: PHY operations
> >> + */
> >> +struct mvebu_a3700_utmi_caps {
> >> +	int usb32;
> >> +	const struct phy_ops *ops;
> >> +};
> >> +
> >> +/**
> >> + * struct mvebu_a3700_utmi - PHY driver data
> >> + *
> >> + * @regs: PHY registers
> >> + * @usb_mis: Regmap with USB miscellaneous registers including PHY ones
> >> + * @caps: PHY capabilities
> >> + * @phy: PHY handle
> >> + */
> >> +struct mvebu_a3700_utmi {
> >> +	void __iomem *regs;
> >> +	struct regmap *usb_misc;
> >> +	const struct mvebu_a3700_utmi_caps *caps;
> >> +	struct phy *phy;
> >> +};
> >> +
> >> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> >> +{
> >> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> >> +	struct device *dev = &phy->dev;
> >> +	int usb32 = utmi->caps->usb32;
> >> +	int ret = 0;
> >> +	u32 reg;
> >> +
> >> +	if (!utmi)
> >> +		return -ENODEV;  
> > the check here may be not necessary  
> 
> right. utmi is already referenced above.

Removed.

> >> +
> >> +	/*
> >> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
> >> +	 * See "PLL Settings for Typical REFCLK" table.
> >> +	 */
> >> +	reg = readl(utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> >> +	reg &= ~(PLL_REF_DIV_MASK | PLL_FB_DIV_MASK | PLL_SEL_LPFR_MASK);
> >> +	reg |= (PLL_REF_DIV_5 << PLL_REF_DIV_OFF |
> >> +		PLL_FB_DIV_96 << PLL_FB_DIV_OFF);
> >> +	writel(reg, utmi->regs + USB2_PHY_PLL_CTRL_REG0);
> >> +
> >> +	/* Enable PHY pull up and disable USB2 suspend */
> >> +	regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> >> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU,
> >> +			   RB_USB2PHY_SUSPM(usb32) | RB_USB2PHY_PU);
> >> +
> >> +	if (usb32) {
> >> +		/* Power up OTG module */
> >> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> >> +		reg |= PHY_PU_OTG;
> >> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> >> +
> >> +		/* Disable PHY charger detection */
> >> +		reg = readl(utmi->regs + USB2_PHY_CHRGR_DETECT);
> >> +		reg &= ~(PHY_CDP_EN | PHY_DCP_EN | PHY_PD_EN | PHY_PU_CHRG_DTC |
> >> +			 PHY_CDP_DM_AUTO | PHY_ENSWITCH_DP | PHY_ENSWITCH_DM);
> >> +		writel(reg, utmi->regs + USB2_PHY_CHRGR_DETECT);
> >> +
> >> +		/* Disable PHY DP/DM pull-down (used for device mode) */
> >> +		regmap_update_bits(utmi->usb_misc, USB2_PHY_CTRL(usb32),
> >> +				   USB2_DP_PULLDN_DEV_MODE |
> >> +				   USB2_DM_PULLDN_DEV_MODE, 0);
> >> +	}
> >> +
> >> +	/* Wait for PLL calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> >> +				 reg & PHY_PLLCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 PLL calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for impedance calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_CAL_CTRL, reg,
> >> +				 reg & PHY_IMPCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 impedance calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for squetch calibration */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_RX_CHAN_CTRL1, reg,
> >> +				 reg & USB2PHY_SQCAL_DONE,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to end USB2 unknown calibration\n");
> >> +		return ret;
> >> +	}
> >> +
> >> +	/* Wait for PLL to be locked */
> >> +	ret = readl_poll_timeout(utmi->regs + USB2_PHY_PLL_CTRL_REG0, reg,
> >> +				 reg & PLL_READY,
> >> +				 PLL_LOCK_DELAY_US, PLL_LOCK_TIMEOUT_US);
> >> +	if (ret)
> >> +		dev_err(dev, "Failed to lock USB2 PLL\n");
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int mvebu_a3700_utmi_phy_power_off(struct phy *phy)
> >> +{
> >> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> >> +	int usb32 = utmi->caps->usb32;
> >> +	u32 reg;
> >> +
> >> +	if (!utmi)
> >> +		return -ENODEV;  
> 
> here too, utmi is referenced above.

Removed too.

> >> +
> >> +	/* Disable PHY pull-up and enable USB2 suspend */
> >> +	reg = readl(utmi->regs + USB2_PHY_CTRL(usb32));
> >> +	reg &= ~(RB_USB2PHY_PU | RB_USB2PHY_SUSPM(usb32));
> >> +	writel(reg, utmi->regs + USB2_PHY_CTRL(usb32));
> >> +
> >> +	/* Power down OTG module */
> >> +	if (usb32) {
> >> +		reg = readl(utmi->regs + USB2_PHY_OTG_CTRL);
> >> +		reg &= ~PHY_PU_OTG;
> >> +		writel(reg, utmi->regs + USB2_PHY_OTG_CTRL);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct phy_ops mvebu_a3700_utmi_phy_ops = {
> >> +	.power_on = mvebu_a3700_utmi_phy_power_on,
> >> +	.power_off = mvebu_a3700_utmi_phy_power_off,
> >> +	.owner = THIS_MODULE,
> >> +};
> >> +
> >> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_otg_phy_caps = {
> >> +	.usb32 = true,
> >> +	.ops = &mvebu_a3700_utmi_phy_ops,
> >> +};
> >> +
> >> +static const struct mvebu_a3700_utmi_caps mvebu_a3700_utmi_host_phy_caps = {
> >> +	.usb32 = false,
> >> +	.ops = &mvebu_a3700_utmi_phy_ops,
> >> +};
> >> +
> >> +static const struct of_device_id mvebu_a3700_utmi_of_match[] = {
> >> +	{
> >> +		.compatible = "marvell,a3700-utmi-otg-phy",
> >> +		.data = &mvebu_a3700_utmi_otg_phy_caps,
> >> +	},
> >> +	{
> >> +		.compatible = "marvell,a3700-utmi-host-phy",
> >> +		.data = &mvebu_a3700_utmi_host_phy_caps,
> >> +	},
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, mvebu_a3700_utmi_of_match);
> >> +
> >> +static int mvebu_a3700_utmi_phy_probe(struct platform_device *pdev)
> >> +{
> >> +	const struct of_device_id *dev_id;
> >> +	struct device *dev = &pdev->dev;
> >> +	struct mvebu_a3700_utmi *utmi;
> >> +	struct phy_provider *provider;
> >> +	const struct phy_ops *ops;
> >> +	struct resource *res;
> >> +
> >> +	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
> >> +	if (!utmi)
> >> +		return -ENOMEM;
> >> +
> >> +	/* Get UTMI memory region */
> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	if (!res) {
> >> +		dev_err(dev, "Missing UTMI PHY memory resource\n");
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	utmi->regs = devm_ioremap_resource(dev, res);
> >> +	if (IS_ERR(utmi->regs))
> >> +		return PTR_ERR(utmi->regs);
> >> +
> >> +	/* Get miscellaneous Host/PHY region */
> >> +	utmi->usb_misc = syscon_regmap_lookup_by_phandle(dev->of_node,
> >> +							 "marvell,usb-misc-reg");
> >> +	if (IS_ERR(utmi->usb_misc)) {
> >> +		dev_err(dev,
> >> +			"Missing USB misc purpose system controller\n");
> >> +		return PTR_ERR(utmi->usb_misc);
> >> +	}
> >> +
> >> +	/* Retrieve PHY capabilities */
> >> +	dev_id = of_match_device(mvebu_a3700_utmi_of_match, dev);
> >> +	if (!dev_id)
> >> +		return -EINVAL;
> >> +
> >> +	utmi->caps = dev_id->data;  
> > a useful API to get @data: of_device_get_match_data()

Nice one, I forgot about it.

> >   
> >> +	ops = utmi->caps->ops;
> >> +
> >> +	/* Instantiate the PHY */
> >> +	utmi->phy = devm_phy_create(dev, NULL, ops);
> >> +	if (IS_ERR(utmi->phy)) {
> >> +		dev_err(dev, "Failed to create the UTMI PHY\n");
> >> +		return PTR_ERR(utmi->phy);
> >> +	}
> >> +
> >> +	phy_set_drvdata(utmi->phy, utmi);
> >> +
> >> +	/* Ensure the PHY is powered off */
> >> +	ops->power_off(utmi->phy);
> >> +
> >> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >> +
> >> +	return PTR_ERR_OR_ZERO(provider);
> >> +}
> >> +
> >> +static int mvebu_a3700_utmi_phy_remove(struct platform_device *pdev)
> >> +{
> >> +	return 0;
> >> +}  
> 
> This empty remove function can be removed.

Sure.

> 
> Thanks
> Kishon
> 
> >> +
> >> +static struct platform_driver mvebu_a3700_utmi_driver = {
> >> +	.probe	= mvebu_a3700_utmi_phy_probe,
> >> +	.remove = mvebu_a3700_utmi_phy_remove,
> >> +	.driver	= {
> >> +		.name		= "mvebu-a3700-utmi-phy",
> >> +		.owner		= THIS_MODULE,
> >> +		.of_match_table	= mvebu_a3700_utmi_of_match,
> >> +	 },
> >> +};
> >> +module_platform_driver(mvebu_a3700_utmi_driver);
> >> +
> >> +MODULE_AUTHOR("Evan Wang <xswang@marvell.com>");
> >> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
> >> +MODULE_DESCRIPTION("Marvell EBU A3700 UTMI PHY driver");
> >> +MODULE_LICENSE("GPL");  
> > 
> >   

Thanks for reviewing!
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Gregory CLEMENT]

Hi Miquel,

I have only a few smallish remarks:

 On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Marvell Armada 3700 SoC has two USB controllers, each of them being
> wired to an internal UTMI PHY. Add a driver to control them.
>
> Igal Liberman worked on supporting the PHY, I took the while 'register
> configuration' from his work and rewrote almost entirely the
> driver/bindings around it.
>
> Co-developed-by: Igal Liberman <igall@marvell.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Igal Liberman <igall@marvell.com>
> ---
[...]

> diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> new file mode 100644
> index 000000000000..97d8235d661d
> --- /dev/null
> +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> @@ -0,0 +1,297 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Marvell
> + *
> + * Authors:
> + *   Evan Wang <xswang@marvell.com>
> + *   Miquèl Raynal <miquel.raynal@bootlin.com>

If it is co-developed with Igal Liberman, shouldn't be added here too?
[...]

> +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> +{
> +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> +	struct device *dev = &phy->dev;
> +	int usb32 = utmi->caps->usb32;
> +	int ret = 0;
> +	u32 reg;
> +
> +	if (!utmi)
> +		return -ENODEV;
> +
> +	/*
> +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.
                                                          bein?
> +	 * See "PLL Settings for Typical REFCLK" table.

[...]

> +
> +	/* Wait for squetch calibration */
                    squetch?

Gregory

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/10] phy: add A3700 UTMI PHY driver

[Miquel Raynal]

Hi Gregory,

Gregory CLEMENT <gregory.clement@bootlin.com> wrote on Fri, 18 Jan 2019
17:36:36 +0100:

> Hi Miquel,
> 
> I have only a few smallish remarks:
> 
>  On ven., janv. 11 2019, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Marvell Armada 3700 SoC has two USB controllers, each of them being
> > wired to an internal UTMI PHY. Add a driver to control them.
> >
> > Igal Liberman worked on supporting the PHY, I took the while 'register
> > configuration' from his work and rewrote almost entirely the
> > driver/bindings around it.
> >
> > Co-developed-by: Igal Liberman <igall@marvell.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Signed-off-by: Igal Liberman <igall@marvell.com>
> > ---  
> [...]
> 
> > diff --git a/drivers/phy/marvell/phy-mvebu-a3700-utmi.c b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > new file mode 100644
> > index 000000000000..97d8235d661d
> > --- /dev/null
> > +++ b/drivers/phy/marvell/phy-mvebu-a3700-utmi.c
> > @@ -0,0 +1,297 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Marvell
> > + *
> > + * Authors:
> > + *   Evan Wang <xswang@marvell.com>
> > + *   Miquèl Raynal <miquel.raynal@bootlin.com>  
> 
> If it is co-developed with Igal Liberman, shouldn't be added here too?

In the original patch, the author in the commit log was Igal and the
MODULE_AUTHOR macro was saying Evan. I think the macro is wrong, I will
keep Igal.

> [...]
> 
> > +static int mvebu_a3700_utmi_phy_power_on(struct phy *phy)
> > +{
> > +	struct mvebu_a3700_utmi *utmi = phy_get_drvdata(phy);
> > +	struct device *dev = &phy->dev;
> > +	int usb32 = utmi->caps->usb32;
> > +	int ret = 0;
> > +	u32 reg;
> > +
> > +	if (!utmi)
> > +		return -ENODEV;
> > +
> > +	/*
> > +	 * Setup PLL. 40MHz clock used to be the default, bein 25MHz now.  
>                                                           bein?
> > +	 * See "PLL Settings for Typical REFCLK" table.  
> 
> [...]
> 
> > +
> > +	/* Wait for squetch calibration */  
>                     squetch?

Good catch. It's written "squelch" in the datasheet, but I don't know
what it means exactly.


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings

[Miquel Raynal]

Add bindings for Marvell Armada 3700 USB2 UTMI+ PHY.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/phy/phy-mvebu-utmi.txt           | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
new file mode 100644
index 000000000000..4ade05ce5d51
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
@@ -0,0 +1,37 @@
+MVEBU A3700 UTMI PHY
+--------------------
+
+USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
+* Armada 3700
+
+On Armada 3700, there are two USB controllers, one is compatible with the USB2
+and USB3 specifications and supports OTG. The other one is USB2 compliant and
+only supports host mode. Both of these controllers come with a UTMI PHY.
+
+Required Properties:
+
+- compatible: Should be one of:
+	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
+	        the USB2 host-only controller.
+	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
+	        the USB3 and USB2 OTG capable controller.
+- reg: PHY IP register range.
+- marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
+			region covering registers related to both the host
+			controller and the PHY.
+- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
+
+
+Example:
+
+	usb2_utmi_host_phy: phy@5f000 {
+		compatible = "marvell,armada-3700-utmi-host-phy";
+		reg = <0x5f000 0x800>;
+		marvell,usb-misc-reg = <&usb2_syscon>;
+		#phy-cells = <0>;
+	};
+
+	usb2_syscon: system-controller@5f800 {
+		compatible = "syscon", "simple-mfd";
+		reg = <0x5f800 0x800>;
+	};
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings

[Rob Herring]

On Fri, Jan 11, 2019 at 02:31:29PM +0100, Miquel Raynal wrote:
> Add bindings for Marvell Armada 3700 USB2 UTMI+ PHY.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/phy/phy-mvebu-utmi.txt           | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> new file mode 100644
> index 000000000000..4ade05ce5d51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> @@ -0,0 +1,37 @@
> +MVEBU A3700 UTMI PHY
> +--------------------
> +
> +USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
> +* Armada 3700
> +
> +On Armada 3700, there are two USB controllers, one is compatible with the USB2
> +and USB3 specifications and supports OTG. The other one is USB2 compliant and
> +only supports host mode. Both of these controllers come with a UTMI PHY.

This paragraph says the controllers are different, but are the PHYs 
different?

> +
> +Required Properties:
> +
> +- compatible: Should be one of:
> +	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> +	        the USB2 host-only controller.
> +	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> +	        the USB3 and USB2 OTG capable controller.
> +- reg: PHY IP register range.
> +- marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
> +			region covering registers related to both the host
> +			controller and the PHY.
> +- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
> +
> +
> +Example:
> +
> +	usb2_utmi_host_phy: phy@5f000 {
> +		compatible = "marvell,armada-3700-utmi-host-phy";
> +		reg = <0x5f000 0x800>;
> +		marvell,usb-misc-reg = <&usb2_syscon>;
> +		#phy-cells = <0>;
> +	};
> +
> +	usb2_syscon: system-controller@5f800 {
> +		compatible = "syscon", "simple-mfd";

This is not valid because there's not a specific compatible string and 
'simple-mfd' implies there are child nodes.

> +		reg = <0x5f800 0x800>;
> +	};
> -- 
> 2.19.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings

[Miquel Raynal]

Hi Rob,

Rob Herring <robh@kernel.org> wrote on Tue, 15 Jan 2019 15:44:29 -0600:

> On Fri, Jan 11, 2019 at 02:31:29PM +0100, Miquel Raynal wrote:
> > Add bindings for Marvell Armada 3700 USB2 UTMI+ PHY.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../bindings/phy/phy-mvebu-utmi.txt           | 37 +++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > new file mode 100644
> > index 000000000000..4ade05ce5d51
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > @@ -0,0 +1,37 @@
> > +MVEBU A3700 UTMI PHY
> > +--------------------
> > +
> > +USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
> > +* Armada 3700
> > +
> > +On Armada 3700, there are two USB controllers, one is compatible with the USB2
> > +and USB3 specifications and supports OTG. The other one is USB2 compliant and
> > +only supports host mode. Both of these controllers come with a UTMI PHY.  
> 
> This paragraph says the controllers are different, but are the PHYs 
> different?

Yes they are. I will make it appear clearly in the v3.

> 
> > +
> > +Required Properties:
> > +
> > +- compatible: Should be one of:
> > +	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> > +	        the USB2 host-only controller.
> > +	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> > +	        the USB3 and USB2 OTG capable controller.
> > +- reg: PHY IP register range.
> > +- marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
> > +			region covering registers related to both the host
> > +			controller and the PHY.
> > +- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
> > +
> > +
> > +Example:
> > +
> > +	usb2_utmi_host_phy: phy@5f000 {
> > +		compatible = "marvell,armada-3700-utmi-host-phy";
> > +		reg = <0x5f000 0x800>;
> > +		marvell,usb-misc-reg = <&usb2_syscon>;
> > +		#phy-cells = <0>;
> > +	};
> > +
> > +	usb2_syscon: system-controller@5f800 {
> > +		compatible = "syscon", "simple-mfd";  
> 
> This is not valid because there's not a specific compatible string and 
> 'simple-mfd' implies there are child nodes.

What do you suggest? Is compatible = "syscon"; enough? This is just an
area with a few registers that do not belong to any specific IP. There
are registers that are related to the control of the USB controller,
and registers that are related to the PHY. It's quite challenging to
find a meaningful specific compatible string. Is it mandatory to add
one? If yes, would "marvell,armada-3700-usb-utmi-{host,otg}-misc" fit?


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings

[Rob Herring]

On Wed, Jan 16, 2019 at 02:20:21PM +0100, Miquel Raynal wrote:
> Hi Rob,
> 
> Rob Herring <robh@kernel.org> wrote on Tue, 15 Jan 2019 15:44:29 -0600:
> 
> > On Fri, Jan 11, 2019 at 02:31:29PM +0100, Miquel Raynal wrote:
> > > Add bindings for Marvell Armada 3700 USB2 UTMI+ PHY.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  .../bindings/phy/phy-mvebu-utmi.txt           | 37 +++++++++++++++++++
> > >  1 file changed, 37 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > > new file mode 100644
> > > index 000000000000..4ade05ce5d51
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
> > > @@ -0,0 +1,37 @@
> > > +MVEBU A3700 UTMI PHY
> > > +--------------------
> > > +
> > > +USB2 UTMI+ PHY controllers can be found on the following Marvell MVEBU SoCs:
> > > +* Armada 3700
> > > +
> > > +On Armada 3700, there are two USB controllers, one is compatible with the USB2
> > > +and USB3 specifications and supports OTG. The other one is USB2 compliant and
> > > +only supports host mode. Both of these controllers come with a UTMI PHY.  
> > 
> > This paragraph says the controllers are different, but are the PHYs 
> > different?
> 
> Yes they are. I will make it appear clearly in the v3.
> 
> > 
> > > +
> > > +Required Properties:
> > > +
> > > +- compatible: Should be one of:
> > > +	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> > > +	        the USB2 host-only controller.
> > > +	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> > > +	        the USB3 and USB2 OTG capable controller.
> > > +- reg: PHY IP register range.
> > > +- marvell,usb-misc-reg: handle on the "USB miscellaneous registers" shared
> > > +			region covering registers related to both the host
> > > +			controller and the PHY.
> > > +- #phy-cells: Standard property (Documentation: phy-bindings.txt) Should be 0.
> > > +
> > > +
> > > +Example:
> > > +
> > > +	usb2_utmi_host_phy: phy@5f000 {
> > > +		compatible = "marvell,armada-3700-utmi-host-phy";
> > > +		reg = <0x5f000 0x800>;
> > > +		marvell,usb-misc-reg = <&usb2_syscon>;
> > > +		#phy-cells = <0>;
> > > +	};
> > > +
> > > +	usb2_syscon: system-controller@5f800 {
> > > +		compatible = "syscon", "simple-mfd";  
> > 
> > This is not valid because there's not a specific compatible string and 
> > 'simple-mfd' implies there are child nodes.
> 
> What do you suggest? Is compatible = "syscon"; enough? 

No.

> This is just an
> area with a few registers that do not belong to any specific IP.

Must be some name for the registers in the datasheet?

> There
> are registers that are related to the control of the USB controller,
> and registers that are related to the PHY. It's quite challenging to
> find a meaningful specific compatible string. Is it mandatory to add
> one? If yes, would "marvell,armada-3700-usb-utmi-{host,otg}-misc" fit?

Yes, if there's not a name from the datasheet.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 07/10] MAINTAINERS: phy: fill Armada 3700 PHY drivers entry

[Miquel Raynal]

Update the Armada 3700 PHY drivers entry with the recently added UTMI
PHY driver.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 MAINTAINERS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index abfd80f21b24..2a08a097e107 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9080,11 +9080,13 @@ F:	drivers/gpu/drm/armada/
 F:	include/uapi/drm/armada_drm.h
 F:	Documentation/devicetree/bindings/display/armada/
 
-MARVELL ARMADA 3700 COMPHY DRIVER
+MARVELL ARMADA 3700 PHY DRIVERS
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
 S:	Maintained
 F:	drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+F:	drivers/phy/marvell/phy-mvebu-a3700-utmi.c
 F:	Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+F:	Documentation/devicetree/bindings/phy/phy-mvebu-utmi.txt
 
 MARVELL CRYPTO DRIVER
 M:	Boris Brezillon <bbrezillon@kernel.org>
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 08/10] ARM64: dts: marvell: armada-37xx: fix USB2 memory region

[Miquel Raynal]

The specification splits the USB2 memory region into three sections:
1/ 0xD005E000-0xD005EFFF: USB2 Host Controller Registers
2/ 0xD005F000-0xD005F7FF: USB2 UTMI PHY Registers
3/ 0xD005F800-0xD005FFFF: USB2 Host Miscellaneous Registers

Section 1/ belongs to the USB2 node but section 2/ belongs to the UTMI
PHY node. Section 3/ can be accessed by both the USB controller and
the PHY because of the miscaellaneous nature of the registers inside
so a specific node will be created to cover the area and a handle to
it will be added in both the USB controller and the PHY node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index d6e548618a95..5cfd1b920f31 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -351,7 +351,7 @@
 
 			usb2: usb@5e000 {
 				compatible = "marvell,armada-3700-ehci";
-				reg = <0x5e000 0x2000>;
+				reg = <0x5e000 0x1000>;
 				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 			};
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 09/10] ARM64: dts: marvell: armada-37xx: declare USB2 UTMI PHYs

[Miquel Raynal]

On Marvell Armada 3700 SoCs there are two USB2 UTMI PHYs. They are
both very similar but only one has OTG/charging capabilities.

Because there are USB host registers and PHY registers mixed in a
single area, a system controller is also created and referenced from
both the USB host node and the PHY node.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 5cfd1b920f31..d87db3d4cc3d 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -344,18 +344,44 @@
 				compatible = "marvell,armada3700-xhci",
 				"generic-xhci";
 				reg = <0x58000 0x4000>;
+				marvell,usb-misc-reg = <&usb32_syscon>;
 				interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&sb_periph_clk 12>;
 				status = "disabled";
 			};
 
+			usb2_utmi_otg_phy: phy@5d000 {
+				compatible = "marvell,a3700-utmi-otg-phy";
+				reg = <0x5d000 0x800>;
+				marvell,usb-misc-reg = <&usb32_syscon>;
+				#phy-cells = <0>;
+			};
+
+			usb32_syscon: system-controller@5d800 {
+				compatible = "syscon", "simple-mfd";
+				reg = <0x5d800 0x800>;
+			};
+
 			usb2: usb@5e000 {
 				compatible = "marvell,armada-3700-ehci";
 				reg = <0x5e000 0x1000>;
+				marvell,usb-misc-reg = <&usb2_syscon>;
 				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 			};
 
+			usb2_utmi_host_phy: phy@5f000 {
+				compatible = "marvell,a3700-utmi-host-phy";
+				reg = <0x5f000 0x800>;
+				marvell,usb-misc-reg = <&usb2_syscon>;
+				#phy-cells = <0>;
+			};
+
+			usb2_syscon: system-controller@5f800 {
+				compatible = "syscon", "simple-mfd";
+				reg = <0x5f800 0x800>;
+			};
+
 			xor@60900 {
 				compatible = "marvell,armada-3700-xor";
 				reg = <0x60900 0x100>,
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 10/10] ARM64: dts: marvell: armada-37xx: link USB hosts with their PHYs

[Miquel Raynal]

Reference the PHY nodes from the USB controller nodes.

The USB3 host controller is wired to:
  * the first PHY of the COMPHY IP
  * the OTG-capable UTMI PHY

The USB2 host controller is wired to:
  * the host-only UTMI PHY

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index d87db3d4cc3d..f32dd1d048e4 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -347,6 +347,8 @@
 				marvell,usb-misc-reg = <&usb32_syscon>;
 				interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&sb_periph_clk 12>;
+				phys = <&comphy0 0>, <&usb2_utmi_otg_phy>;
+				phy-names = "usb3-phy", "usb2-utmi-otg-phy";
 				status = "disabled";
 			};
 
@@ -367,6 +369,8 @@
 				reg = <0x5e000 0x1000>;
 				marvell,usb-misc-reg = <&usb2_syscon>;
 				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+				phys = <&usb2_utmi_host_phy>;
+				phy-names = "usb2-utmi-host-phy";
 				status = "disabled";
 			};
 
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel