[PATCH v2 0/8] Add Armada 3700 COMPHY support

[PATCH v2 0/8] Add Armada 3700 COMPHY support

[Miquel Raynal]

Hello,

This series adds a new driver to support Armada 3700 COMPHY IP.
The series has been tested on an ESPRESSObin with SATA, PCIe
and USB3 host. For this purpose, patch 1 enumerates the SATA PHY
mode. The SGMII PHY mode that is supported by the IP has been written
(uses SMC calls anyway) but could not be tested on this platform.

Three series will follow to add PHY support (or at least a PHY nodes
in the A3700/ESPRESSObin device trees) to the MVEBU AHCI driver, the
Aardvark PCI controller driver and the MVEBU xHCI driver. All this is
needed in order to achieve suspend to RAM on this platform.

Thanks,
Miquèl


Changes since v1:
=================
* Fix wrong check in ->xlate().
* Apply the same fix to the cp110 comphy driver from which the a3700
  driver is based.
* Added credit to Gregorz Jaszczyk for his work.
* Added Suggested-by tag to the patch adding the COMPHY DT node.


Grzegorz Jaszczyk (1):
  phy: enumerate SATA PHY mode

Miquel Raynal (7):
  phy: mvebu-cp110-comphy: fix spelling in structure name
  phy: mvebu-cp110-comphy: fix port check in ->xlate()
  phy: add A3700 COMPHY support
  dt-bindings: phy: mvebu-comphy: extend the file to describe a3700
    bindings
  MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver
  ARM64: dts: marvell: armada-37xx: fix SATA node scope
  ARM64: dts: marvell: armada-37xx: declare the COMPHY node

 .../bindings/phy/phy-mvebu-comphy.txt         |  65 ++++-
 MAINTAINERS                                   |   6 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  31 +-
 drivers/phy/marvell/Kconfig                   |  10 +
 drivers/phy/marvell/Makefile                  |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c  | 276 ++++++++++++++++++
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c  |   8 +-
 include/linux/phy/phy.h                       |   1 +
 8 files changed, 381 insertions(+), 17 deletions(-)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c

-- 
2.19.1

[PATCH v2 0/8] Add Armada 3700 COMPHY support

[Miquel Raynal]

Hello,

This series adds a new driver to support Armada 3700 COMPHY IP.
The series has been tested on an ESPRESSObin with SATA, PCIe
and USB3 host. For this purpose, patch 1 enumerates the SATA PHY
mode. The SGMII PHY mode that is supported by the IP has been written
(uses SMC calls anyway) but could not be tested on this platform.

Three series will follow to add PHY support (or at least a PHY nodes
in the A3700/ESPRESSObin device trees) to the MVEBU AHCI driver, the
Aardvark PCI controller driver and the MVEBU xHCI driver. All this is
needed in order to achieve suspend to RAM on this platform.

Thanks,
Miquèl


Changes since v1:
=================
* Fix wrong check in ->xlate().
* Apply the same fix to the cp110 comphy driver from which the a3700
  driver is based.
* Added credit to Gregorz Jaszczyk for his work.
* Added Suggested-by tag to the patch adding the COMPHY DT node.


Grzegorz Jaszczyk (1):
  phy: enumerate SATA PHY mode

Miquel Raynal (7):
  phy: mvebu-cp110-comphy: fix spelling in structure name
  phy: mvebu-cp110-comphy: fix port check in ->xlate()
  phy: add A3700 COMPHY support
  dt-bindings: phy: mvebu-comphy: extend the file to describe a3700
    bindings
  MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver
  ARM64: dts: marvell: armada-37xx: fix SATA node scope
  ARM64: dts: marvell: armada-37xx: declare the COMPHY node

 .../bindings/phy/phy-mvebu-comphy.txt         |  65 ++++-
 MAINTAINERS                                   |   6 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  31 +-
 drivers/phy/marvell/Kconfig                   |  10 +
 drivers/phy/marvell/Makefile                  |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c  | 276 ++++++++++++++++++
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c  |   8 +-
 include/linux/phy/phy.h                       |   1 +
 8 files changed, 381 insertions(+), 17 deletions(-)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c

-- 
2.19.1

[PATCH v2 0/8] Add Armada 3700 COMPHY support

[Miquel Raynal]

Hello,

This series adds a new driver to support Armada 3700 COMPHY IP.
The series has been tested on an ESPRESSObin with SATA, PCIe
and USB3 host. For this purpose, patch 1 enumerates the SATA PHY
mode. The SGMII PHY mode that is supported by the IP has been written
(uses SMC calls anyway) but could not be tested on this platform.

Three series will follow to add PHY support (or at least a PHY nodes
in the A3700/ESPRESSObin device trees) to the MVEBU AHCI driver, the
Aardvark PCI controller driver and the MVEBU xHCI driver. All this is
needed in order to achieve suspend to RAM on this platform.

Thanks,
Miquèl


Changes since v1:
=================
* Fix wrong check in ->xlate().
* Apply the same fix to the cp110 comphy driver from which the a3700
  driver is based.
* Added credit to Gregorz Jaszczyk for his work.
* Added Suggested-by tag to the patch adding the COMPHY DT node.


Grzegorz Jaszczyk (1):
  phy: enumerate SATA PHY mode

Miquel Raynal (7):
  phy: mvebu-cp110-comphy: fix spelling in structure name
  phy: mvebu-cp110-comphy: fix port check in ->xlate()
  phy: add A3700 COMPHY support
  dt-bindings: phy: mvebu-comphy: extend the file to describe a3700
    bindings
  MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver
  ARM64: dts: marvell: armada-37xx: fix SATA node scope
  ARM64: dts: marvell: armada-37xx: declare the COMPHY node

 .../bindings/phy/phy-mvebu-comphy.txt         |  65 ++++-
 MAINTAINERS                                   |   6 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi  |  31 +-
 drivers/phy/marvell/Kconfig                   |  10 +
 drivers/phy/marvell/Makefile                  |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c  | 276 ++++++++++++++++++
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c  |   8 +-
 include/linux/phy/phy.h                       |   1 +
 8 files changed, 381 insertions(+), 17 deletions(-)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.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 1/8] phy: mvebu-cp110-comphy: fix spelling in structure name

[Miquel Raynal]

Rename the mvebu_comhy_conf structure to be mvebu_comphy_conf, which is
probably what the original author meant.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 86a5f7b9448b..31b9a1c18345 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -114,7 +114,7 @@
 #define MVEBU_COMPHY_LANES	6
 #define MVEBU_COMPHY_PORTS	3
 
-struct mvebu_comhy_conf {
+struct mvebu_comphy_conf {
 	enum phy_mode mode;
 	unsigned lane;
 	unsigned port;
@@ -129,7 +129,7 @@ struct mvebu_comhy_conf {
 		.mux = _mux,				\
 	}
 
-static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
+static const struct mvebu_comphy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 0 */
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
-- 
2.19.1

[PATCH v2 1/8] phy: mvebu-cp110-comphy: fix spelling in structure name

[Miquel Raynal]

Rename the mvebu_comhy_conf structure to be mvebu_comphy_conf, which is
probably what the original author meant.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 86a5f7b9448b..31b9a1c18345 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -114,7 +114,7 @@
 #define MVEBU_COMPHY_LANES	6
 #define MVEBU_COMPHY_PORTS	3
 
-struct mvebu_comhy_conf {
+struct mvebu_comphy_conf {
 	enum phy_mode mode;
 	unsigned lane;
 	unsigned port;
@@ -129,7 +129,7 @@ struct mvebu_comhy_conf {
 		.mux = _mux,				\
 	}
 
-static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
+static const struct mvebu_comphy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 0 */
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
-- 
2.19.1

[PATCH v2 1/8] phy: mvebu-cp110-comphy: fix spelling in structure name

[Miquel Raynal]

Rename the mvebu_comhy_conf structure to be mvebu_comphy_conf, which is
probably what the original author meant.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 86a5f7b9448b..31b9a1c18345 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -114,7 +114,7 @@
 #define MVEBU_COMPHY_LANES	6
 #define MVEBU_COMPHY_PORTS	3
 
-struct mvebu_comhy_conf {
+struct mvebu_comphy_conf {
 	enum phy_mode mode;
 	unsigned lane;
 	unsigned port;
@@ -129,7 +129,7 @@ struct mvebu_comhy_conf {
 		.mux = _mux,				\
 	}
 
-static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
+static const struct mvebu_comphy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 0 */
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
 	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
-- 
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 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

So far the PHY ->xlate() callback was checking if the port was
"invalid" before continuing, meaning that the port has not been used
yet. This check is not correct as there is no opposite call to
->xlate() once the PHY is released by the user and the port will
remain "valid" after the first phy_get()/phy_put() calls. Hence, if
this driver is built as a module, inserted, removed and inserted
again, the PHY will appear busy and the second probe will fail.

To fix this, just drop the faulty check and instead verify that the
port number is valid (ie. in the possible range).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 31b9a1c18345..a40b876ff214 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
 		return phy;
 
 	lane = phy_get_drvdata(phy);
-	if (lane->port >= 0)
-		return ERR_PTR(-EBUSY);
 	lane->port = args->args[0];
+	if (lane->port >= MVEBU_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
 
 	return phy;
 }
-- 
2.19.1

[PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

So far the PHY ->xlate() callback was checking if the port was
"invalid" before continuing, meaning that the port has not been used
yet. This check is not correct as there is no opposite call to
->xlate() once the PHY is released by the user and the port will
remain "valid" after the first phy_get()/phy_put() calls. Hence, if
this driver is built as a module, inserted, removed and inserted
again, the PHY will appear busy and the second probe will fail.

To fix this, just drop the faulty check and instead verify that the
port number is valid (ie. in the possible range).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 31b9a1c18345..a40b876ff214 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
 		return phy;
 
 	lane = phy_get_drvdata(phy);
-	if (lane->port >= 0)
-		return ERR_PTR(-EBUSY);
 	lane->port = args->args[0];
+	if (lane->port >= MVEBU_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
 
 	return phy;
 }
-- 
2.19.1

[PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

So far the PHY ->xlate() callback was checking if the port was
"invalid" before continuing, meaning that the port has not been used
yet. This check is not correct as there is no opposite call to
->xlate() once the PHY is released by the user and the port will
remain "valid" after the first phy_get()/phy_put() calls. Hence, if
this driver is built as a module, inserted, removed and inserted
again, the PHY will appear busy and the second probe will fail.

To fix this, just drop the faulty check and instead verify that the
port number is valid (ie. in the possible range).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 31b9a1c18345..a40b876ff214 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
 		return phy;
 
 	lane = phy_get_drvdata(phy);
-	if (lane->port >= 0)
-		return ERR_PTR(-EBUSY);
 	lane->port = args->args[0];
+	if (lane->port >= MVEBU_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
 
 	return phy;
 }
-- 
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 3/8] phy: enumerate SATA PHY mode

[Miquel Raynal]

From: Grzegorz Jaszczyk <jaz@semihalf.com>

Add SATA mode to the PHY framework in preparation of upcoming PHYs
that will handle it. For instance, SATA mode will be used by the
Armada3700 COMPHY driver, which supports configuring SERDES lanes to
be used by various controllers: Ethernet, USB3, SATA and PCIe.

Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03b319f89a34..553d8520e37f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,6 +42,7 @@ enum phy_mode {
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
+	PHY_MODE_SATA,
 };
 
 /**
-- 
2.19.1

[PATCH v2 3/8] phy: enumerate SATA PHY mode

[Miquel Raynal]

From: Grzegorz Jaszczyk <jaz@semihalf.com>

Add SATA mode to the PHY framework in preparation of upcoming PHYs
that will handle it. For instance, SATA mode will be used by the
Armada3700 COMPHY driver, which supports configuring SERDES lanes to
be used by various controllers: Ethernet, USB3, SATA and PCIe.

Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03b319f89a34..553d8520e37f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,6 +42,7 @@ enum phy_mode {
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
+	PHY_MODE_SATA,
 };
 
 /**
-- 
2.19.1

[PATCH v2 3/8] phy: enumerate SATA PHY mode

[Miquel Raynal]

From: Grzegorz Jaszczyk <jaz@semihalf.com>

Add SATA mode to the PHY framework in preparation of upcoming PHYs
that will handle it. For instance, SATA mode will be used by the
Armada3700 COMPHY driver, which supports configuring SERDES lanes to
be used by various controllers: Ethernet, USB3, SATA and PCIe.

Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03b319f89a34..553d8520e37f 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,6 +42,7 @@ enum phy_mode {
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
+	PHY_MODE_SATA,
 };
 
 /**
-- 
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 4/8] phy: add A3700 COMPHY support

[Miquel Raynal]

Add a driver to support COMPHY, a hardware block providing shared
serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and
rely on having an up-to-date firmware.

SATA, PCie and USB3 host mode have been tested successfully with an
ESPRESSObin. SGMII mode cannot be tested with this platform.

Evan worked on the original driver structure and Grzegorz on the SMC
calls rework. The structure of this driver has been copied from
Antoine Tenart work on CP110 COMPHY driver.

Co-Developed-by: Evan Wang <xswang@marvell.com>
Co-Developed-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Evan Wang <xswang@marvell.com>
Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/Kconfig                  |  10 +
 drivers/phy/marvell/Makefile                 |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 6fb4b56e4c14..f0c39439f06e 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -21,6 +21,16 @@ config PHY_BERLIN_USB
 	help
 	  Enable this to support the USB PHY on Marvell Berlin SoCs.
 
+config PHY_MVEBU_A3700_COMPHY
+	tristate "Marvell A3700 comphy driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, a hardware block providing
+	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
+	  used by various controllers: Ethernet, SATA, USB3, PCIe.
+
 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 3975b144f8ec..c13a0c8ab6f0 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -2,6 +2,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_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-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
new file mode 100644
index 000000000000..4beb2e75f7ed
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell
+ *
+ * Authors:
+ *   Evan Wang <xswang@marvell.com>
+ *   Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Structure inspired from phy-mvebu-cp110-comphy.c written by Antoine Tenart.
+ * SMC call initial support done by Grzegorz Jaszczyk.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define MVEBU_A3700_COMPHY_LANES		3
+#define MVEBU_A3700_COMPHY_PORTS		2
+
+/* COMPHY Fast SMC function identifiers */
+#define COMPHY_SIP_POWER_ON			0x82000001
+#define COMPHY_SIP_POWER_OFF			0x82000002
+#define COMPHY_SIP_PLL_LOCK			0x82000003
+
+#define COMPHY_FW_MODE_SATA			0x1
+#define COMPHY_FW_MODE_SGMII			0x2
+#define COMPHY_FW_MODE_HS_SGMII			0x3
+#define COMPHY_FW_MODE_USB3H			0x4
+#define COMPHY_FW_MODE_USB3D			0x5
+#define COMPHY_FW_MODE_PCIE			0x6
+#define COMPHY_FW_MODE_RXAUI			0x7
+#define COMPHY_FW_MODE_XFI			0x8
+#define COMPHY_FW_MODE_SFI			0x9
+#define COMPHY_FW_MODE_USB3			0xa
+
+#define COMPHY_FW_SPEED_1_25G			0 /* SGMII 1G */
+#define COMPHY_FW_SPEED_2_5G			1
+#define COMPHY_FW_SPEED_3_125G			2 /* SGMII 2.5G */
+#define COMPHY_FW_SPEED_5G			3
+#define COMPHY_FW_SPEED_5_15625G		4 /* XFI 5G */
+#define COMPHY_FW_SPEED_6G			5
+#define COMPHY_FW_SPEED_10_3125G		6 /* XFI 10G */
+#define COMPHY_FW_SPEED_MAX			0x3F
+
+#define COMPHY_FW_MODE(mode)			((mode) << 12)
+#define COMPHY_FW_NET(mode, idx, speed)		(COMPHY_FW_MODE(mode) | \
+						 ((idx) << 8) |	\
+						 ((speed) << 2))
+#define COMPHY_FW_PCIE(mode, idx, speed, width)	(COMPHY_FW_NET(mode, idx, speed) | \
+						 ((width) << 18))
+
+struct mvebu_a3700_comphy_conf {
+	unsigned int lane;
+	enum phy_mode mode;
+	unsigned int port;
+	u32 fw_mode;
+};
+
+#define MVEBU_A3700_COMPHY_CONF(_lane, _mode, _port, _fw_mode)	\
+	{							\
+		.lane = _lane,					\
+		.mode = _mode,					\
+		.port = _port,					\
+		.fw_mode = _fw_mode,				\
+	}
+
+static const struct mvebu_a3700_comphy_conf mvebu_a3700_comphy_modes[] = {
+	/* lane 0 */
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_SGMII, 1, COMPHY_FW_MODE_SGMII),
+	/* lane 1 */
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_PCIE, 0, COMPHY_FW_MODE_PCIE),
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_SGMII, 0, COMPHY_FW_MODE_SGMII),
+	/* lane 2 */
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_SATA, 0, COMPHY_FW_MODE_SATA),
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+};
+
+struct mvebu_a3700_comphy_lane {
+	struct device *dev;
+	unsigned int id;
+	enum phy_mode mode;
+	int port;
+};
+
+static int mvebu_a3700_comphy_smc(unsigned long function, unsigned long lane,
+				  unsigned long mode)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(function, lane, mode, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int mvebu_a3700_comphy_get_fw_mode(int lane, int port,
+					  enum phy_mode mode)
+{
+	int i, n = ARRAY_SIZE(mvebu_a3700_comphy_modes);
+
+	/* Unused PHY mux value is 0x0 */
+	if (mode == PHY_MODE_INVALID)
+		return -EINVAL;
+
+	for (i = 0; i < n; i++) {
+		if (mvebu_a3700_comphy_modes[i].lane == lane &&
+		    mvebu_a3700_comphy_modes[i].port == port &&
+		    mvebu_a3700_comphy_modes[i].mode == mode)
+			break;
+	}
+
+	if (i == n)
+		return -EINVAL;
+
+	return mvebu_a3700_comphy_modes[i].fw_mode;
+}
+
+static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	/* Just remember the mode, ->power_on() will do the real setup */
+	lane->mode = mode;
+
+	return 0;
+}
+
+static int mvebu_a3700_comphy_power_on(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	u32 fw_param;
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port,
+						 lane->mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	switch (lane->mode) {
+	case PHY_MODE_USB_HOST_SS:
+		dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SATA:
+		dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SGMII:
+		dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id);
+		fw_param = COMPHY_FW_NET(fw_mode, lane->port,
+					 COMPHY_FW_SPEED_1_25G);
+		break;
+	case PHY_MODE_PCIE:
+		dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id);
+		fw_param = COMPHY_FW_PCIE(fw_mode, lane->port,
+					  COMPHY_FW_SPEED_5G,
+					  phy->attrs.bus_width);
+		break;
+	default:
+		dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode);
+		return -ENOTSUPP;
+	}
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param);
+}
+
+static int mvebu_a3700_comphy_power_off(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_OFF, lane->id, 0);
+}
+
+static const struct phy_ops mvebu_a3700_comphy_ops = {
+	.power_on	= mvebu_a3700_comphy_power_on,
+	.power_off	= mvebu_a3700_comphy_power_off,
+	.set_mode	= mvebu_a3700_comphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *mvebu_a3700_comphy_xlate(struct device *dev,
+					    struct of_phandle_args *args)
+{
+	struct mvebu_a3700_comphy_lane *lane;
+	struct phy *phy;
+
+	if (WARN_ON(args->args[0] >= MVEBU_A3700_COMPHY_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	phy = of_phy_simple_xlate(dev, args);
+	if (IS_ERR(phy))
+		return phy;
+
+	lane = phy_get_drvdata(phy);
+	lane->port = args->args[0];
+	if (lane->port >= MVEBU_A3700_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
+
+	return phy;
+}
+
+static int mvebu_a3700_comphy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *provider;
+	struct device_node *child;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct mvebu_a3700_comphy_lane *lane;
+		struct phy *phy;
+		int ret;
+		u32 lane_id;
+
+		ret = of_property_read_u32(child, "reg", &lane_id);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "missing 'reg' property (%d)\n",
+				ret);
+			continue;
+		}
+
+		if (lane_id >= MVEBU_A3700_COMPHY_LANES) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		lane = devm_kzalloc(&pdev->dev, sizeof(*lane), GFP_KERNEL);
+		if (!lane)
+			return -ENOMEM;
+
+		phy = devm_phy_create(&pdev->dev, child,
+				      &mvebu_a3700_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		lane->dev = &pdev->dev;
+		lane->mode = PHY_MODE_INVALID;
+		lane->id = lane_id;
+		lane->port = -1;
+		phy_set_drvdata(phy, lane);
+	}
+
+	provider = devm_of_phy_provider_register(&pdev->dev,
+						 mvebu_a3700_comphy_xlate);
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id mvebu_a3700_comphy_of_match_table[] = {
+	{ .compatible = "marvell,comphy-a3700" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mvebu_a3700_comphy_of_match_table);
+
+static struct platform_driver mvebu_a3700_comphy_driver = {
+	.probe	= mvebu_a3700_comphy_probe,
+	.driver	= {
+		.name = "mvebu-a3700-comphy",
+		.of_match_table = mvebu_a3700_comphy_of_match_table,
+	},
+};
+module_platform_driver(mvebu_a3700_comphy_driver);
+
+MODULE_AUTHOR("Miquèl Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Common PHY driver for A3700");
+MODULE_LICENSE("GPL v2");
-- 
2.19.1

[PATCH v2 4/8] phy: add A3700 COMPHY support

[Miquel Raynal]

Add a driver to support COMPHY, a hardware block providing shared
serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and
rely on having an up-to-date firmware.

SATA, PCie and USB3 host mode have been tested successfully with an
ESPRESSObin. SGMII mode cannot be tested with this platform.

Evan worked on the original driver structure and Grzegorz on the SMC
calls rework. The structure of this driver has been copied from
Antoine Tenart work on CP110 COMPHY driver.

Co-Developed-by: Evan Wang <xswang@marvell.com>
Co-Developed-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Evan Wang <xswang@marvell.com>
Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/Kconfig                  |  10 +
 drivers/phy/marvell/Makefile                 |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 6fb4b56e4c14..f0c39439f06e 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -21,6 +21,16 @@ config PHY_BERLIN_USB
 	help
 	  Enable this to support the USB PHY on Marvell Berlin SoCs.
 
+config PHY_MVEBU_A3700_COMPHY
+	tristate "Marvell A3700 comphy driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, a hardware block providing
+	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
+	  used by various controllers: Ethernet, SATA, USB3, PCIe.
+
 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 3975b144f8ec..c13a0c8ab6f0 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -2,6 +2,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_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-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
new file mode 100644
index 000000000000..4beb2e75f7ed
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell
+ *
+ * Authors:
+ *   Evan Wang <xswang@marvell.com>
+ *   Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Structure inspired from phy-mvebu-cp110-comphy.c written by Antoine Tenart.
+ * SMC call initial support done by Grzegorz Jaszczyk.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define MVEBU_A3700_COMPHY_LANES		3
+#define MVEBU_A3700_COMPHY_PORTS		2
+
+/* COMPHY Fast SMC function identifiers */
+#define COMPHY_SIP_POWER_ON			0x82000001
+#define COMPHY_SIP_POWER_OFF			0x82000002
+#define COMPHY_SIP_PLL_LOCK			0x82000003
+
+#define COMPHY_FW_MODE_SATA			0x1
+#define COMPHY_FW_MODE_SGMII			0x2
+#define COMPHY_FW_MODE_HS_SGMII			0x3
+#define COMPHY_FW_MODE_USB3H			0x4
+#define COMPHY_FW_MODE_USB3D			0x5
+#define COMPHY_FW_MODE_PCIE			0x6
+#define COMPHY_FW_MODE_RXAUI			0x7
+#define COMPHY_FW_MODE_XFI			0x8
+#define COMPHY_FW_MODE_SFI			0x9
+#define COMPHY_FW_MODE_USB3			0xa
+
+#define COMPHY_FW_SPEED_1_25G			0 /* SGMII 1G */
+#define COMPHY_FW_SPEED_2_5G			1
+#define COMPHY_FW_SPEED_3_125G			2 /* SGMII 2.5G */
+#define COMPHY_FW_SPEED_5G			3
+#define COMPHY_FW_SPEED_5_15625G		4 /* XFI 5G */
+#define COMPHY_FW_SPEED_6G			5
+#define COMPHY_FW_SPEED_10_3125G		6 /* XFI 10G */
+#define COMPHY_FW_SPEED_MAX			0x3F
+
+#define COMPHY_FW_MODE(mode)			((mode) << 12)
+#define COMPHY_FW_NET(mode, idx, speed)		(COMPHY_FW_MODE(mode) | \
+						 ((idx) << 8) |	\
+						 ((speed) << 2))
+#define COMPHY_FW_PCIE(mode, idx, speed, width)	(COMPHY_FW_NET(mode, idx, speed) | \
+						 ((width) << 18))
+
+struct mvebu_a3700_comphy_conf {
+	unsigned int lane;
+	enum phy_mode mode;
+	unsigned int port;
+	u32 fw_mode;
+};
+
+#define MVEBU_A3700_COMPHY_CONF(_lane, _mode, _port, _fw_mode)	\
+	{							\
+		.lane = _lane,					\
+		.mode = _mode,					\
+		.port = _port,					\
+		.fw_mode = _fw_mode,				\
+	}
+
+static const struct mvebu_a3700_comphy_conf mvebu_a3700_comphy_modes[] = {
+	/* lane 0 */
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_SGMII, 1, COMPHY_FW_MODE_SGMII),
+	/* lane 1 */
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_PCIE, 0, COMPHY_FW_MODE_PCIE),
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_SGMII, 0, COMPHY_FW_MODE_SGMII),
+	/* lane 2 */
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_SATA, 0, COMPHY_FW_MODE_SATA),
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+};
+
+struct mvebu_a3700_comphy_lane {
+	struct device *dev;
+	unsigned int id;
+	enum phy_mode mode;
+	int port;
+};
+
+static int mvebu_a3700_comphy_smc(unsigned long function, unsigned long lane,
+				  unsigned long mode)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(function, lane, mode, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int mvebu_a3700_comphy_get_fw_mode(int lane, int port,
+					  enum phy_mode mode)
+{
+	int i, n = ARRAY_SIZE(mvebu_a3700_comphy_modes);
+
+	/* Unused PHY mux value is 0x0 */
+	if (mode == PHY_MODE_INVALID)
+		return -EINVAL;
+
+	for (i = 0; i < n; i++) {
+		if (mvebu_a3700_comphy_modes[i].lane == lane &&
+		    mvebu_a3700_comphy_modes[i].port == port &&
+		    mvebu_a3700_comphy_modes[i].mode == mode)
+			break;
+	}
+
+	if (i == n)
+		return -EINVAL;
+
+	return mvebu_a3700_comphy_modes[i].fw_mode;
+}
+
+static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	/* Just remember the mode, ->power_on() will do the real setup */
+	lane->mode = mode;
+
+	return 0;
+}
+
+static int mvebu_a3700_comphy_power_on(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	u32 fw_param;
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port,
+						 lane->mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	switch (lane->mode) {
+	case PHY_MODE_USB_HOST_SS:
+		dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SATA:
+		dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SGMII:
+		dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id);
+		fw_param = COMPHY_FW_NET(fw_mode, lane->port,
+					 COMPHY_FW_SPEED_1_25G);
+		break;
+	case PHY_MODE_PCIE:
+		dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id);
+		fw_param = COMPHY_FW_PCIE(fw_mode, lane->port,
+					  COMPHY_FW_SPEED_5G,
+					  phy->attrs.bus_width);
+		break;
+	default:
+		dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode);
+		return -ENOTSUPP;
+	}
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param);
+}
+
+static int mvebu_a3700_comphy_power_off(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_OFF, lane->id, 0);
+}
+
+static const struct phy_ops mvebu_a3700_comphy_ops = {
+	.power_on	= mvebu_a3700_comphy_power_on,
+	.power_off	= mvebu_a3700_comphy_power_off,
+	.set_mode	= mvebu_a3700_comphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *mvebu_a3700_comphy_xlate(struct device *dev,
+					    struct of_phandle_args *args)
+{
+	struct mvebu_a3700_comphy_lane *lane;
+	struct phy *phy;
+
+	if (WARN_ON(args->args[0] >= MVEBU_A3700_COMPHY_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	phy = of_phy_simple_xlate(dev, args);
+	if (IS_ERR(phy))
+		return phy;
+
+	lane = phy_get_drvdata(phy);
+	lane->port = args->args[0];
+	if (lane->port >= MVEBU_A3700_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
+
+	return phy;
+}
+
+static int mvebu_a3700_comphy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *provider;
+	struct device_node *child;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct mvebu_a3700_comphy_lane *lane;
+		struct phy *phy;
+		int ret;
+		u32 lane_id;
+
+		ret = of_property_read_u32(child, "reg", &lane_id);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "missing 'reg' property (%d)\n",
+				ret);
+			continue;
+		}
+
+		if (lane_id >= MVEBU_A3700_COMPHY_LANES) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		lane = devm_kzalloc(&pdev->dev, sizeof(*lane), GFP_KERNEL);
+		if (!lane)
+			return -ENOMEM;
+
+		phy = devm_phy_create(&pdev->dev, child,
+				      &mvebu_a3700_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		lane->dev = &pdev->dev;
+		lane->mode = PHY_MODE_INVALID;
+		lane->id = lane_id;
+		lane->port = -1;
+		phy_set_drvdata(phy, lane);
+	}
+
+	provider = devm_of_phy_provider_register(&pdev->dev,
+						 mvebu_a3700_comphy_xlate);
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id mvebu_a3700_comphy_of_match_table[] = {
+	{ .compatible = "marvell,comphy-a3700" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mvebu_a3700_comphy_of_match_table);
+
+static struct platform_driver mvebu_a3700_comphy_driver = {
+	.probe	= mvebu_a3700_comphy_probe,
+	.driver	= {
+		.name = "mvebu-a3700-comphy",
+		.of_match_table = mvebu_a3700_comphy_of_match_table,
+	},
+};
+module_platform_driver(mvebu_a3700_comphy_driver);
+
+MODULE_AUTHOR("Miquèl Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Common PHY driver for A3700");
+MODULE_LICENSE("GPL v2");
-- 
2.19.1

[PATCH v2 4/8] phy: add A3700 COMPHY support

[Miquel Raynal]

Add a driver to support COMPHY, a hardware block providing shared
serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and
rely on having an up-to-date firmware.

SATA, PCie and USB3 host mode have been tested successfully with an
ESPRESSObin. SGMII mode cannot be tested with this platform.

Evan worked on the original driver structure and Grzegorz on the SMC
calls rework. The structure of this driver has been copied from
Antoine Tenart work on CP110 COMPHY driver.

Co-Developed-by: Evan Wang <xswang@marvell.com>
Co-Developed-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Evan Wang <xswang@marvell.com>
Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/phy/marvell/Kconfig                  |  10 +
 drivers/phy/marvell/Makefile                 |   1 +
 drivers/phy/marvell/phy-mvebu-a3700-comphy.c | 276 +++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/phy/marvell/phy-mvebu-a3700-comphy.c

diff --git a/drivers/phy/marvell/Kconfig b/drivers/phy/marvell/Kconfig
index 6fb4b56e4c14..f0c39439f06e 100644
--- a/drivers/phy/marvell/Kconfig
+++ b/drivers/phy/marvell/Kconfig
@@ -21,6 +21,16 @@ config PHY_BERLIN_USB
 	help
 	  Enable this to support the USB PHY on Marvell Berlin SoCs.
 
+config PHY_MVEBU_A3700_COMPHY
+	tristate "Marvell A3700 comphy driver"
+	depends on ARCH_MVEBU || COMPILE_TEST
+	depends on OF
+	select GENERIC_PHY
+	help
+	  This driver allows to control the comphy, a hardware block providing
+	  shared serdes PHYs on Marvell Armada 3700. Its serdes lanes can be
+	  used by various controllers: Ethernet, SATA, USB3, PCIe.
+
 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 3975b144f8ec..c13a0c8ab6f0 100644
--- a/drivers/phy/marvell/Makefile
+++ b/drivers/phy/marvell/Makefile
@@ -2,6 +2,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_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-comphy.c b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
new file mode 100644
index 000000000000..4beb2e75f7ed
--- /dev/null
+++ b/drivers/phy/marvell/phy-mvebu-a3700-comphy.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Marvell
+ *
+ * Authors:
+ *   Evan Wang <xswang@marvell.com>
+ *   Miquèl Raynal <miquel.raynal@bootlin.com>
+ *
+ * Structure inspired from phy-mvebu-cp110-comphy.c written by Antoine Tenart.
+ * SMC call initial support done by Grzegorz Jaszczyk.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+
+#define MVEBU_A3700_COMPHY_LANES		3
+#define MVEBU_A3700_COMPHY_PORTS		2
+
+/* COMPHY Fast SMC function identifiers */
+#define COMPHY_SIP_POWER_ON			0x82000001
+#define COMPHY_SIP_POWER_OFF			0x82000002
+#define COMPHY_SIP_PLL_LOCK			0x82000003
+
+#define COMPHY_FW_MODE_SATA			0x1
+#define COMPHY_FW_MODE_SGMII			0x2
+#define COMPHY_FW_MODE_HS_SGMII			0x3
+#define COMPHY_FW_MODE_USB3H			0x4
+#define COMPHY_FW_MODE_USB3D			0x5
+#define COMPHY_FW_MODE_PCIE			0x6
+#define COMPHY_FW_MODE_RXAUI			0x7
+#define COMPHY_FW_MODE_XFI			0x8
+#define COMPHY_FW_MODE_SFI			0x9
+#define COMPHY_FW_MODE_USB3			0xa
+
+#define COMPHY_FW_SPEED_1_25G			0 /* SGMII 1G */
+#define COMPHY_FW_SPEED_2_5G			1
+#define COMPHY_FW_SPEED_3_125G			2 /* SGMII 2.5G */
+#define COMPHY_FW_SPEED_5G			3
+#define COMPHY_FW_SPEED_5_15625G		4 /* XFI 5G */
+#define COMPHY_FW_SPEED_6G			5
+#define COMPHY_FW_SPEED_10_3125G		6 /* XFI 10G */
+#define COMPHY_FW_SPEED_MAX			0x3F
+
+#define COMPHY_FW_MODE(mode)			((mode) << 12)
+#define COMPHY_FW_NET(mode, idx, speed)		(COMPHY_FW_MODE(mode) | \
+						 ((idx) << 8) |	\
+						 ((speed) << 2))
+#define COMPHY_FW_PCIE(mode, idx, speed, width)	(COMPHY_FW_NET(mode, idx, speed) | \
+						 ((width) << 18))
+
+struct mvebu_a3700_comphy_conf {
+	unsigned int lane;
+	enum phy_mode mode;
+	unsigned int port;
+	u32 fw_mode;
+};
+
+#define MVEBU_A3700_COMPHY_CONF(_lane, _mode, _port, _fw_mode)	\
+	{							\
+		.lane = _lane,					\
+		.mode = _mode,					\
+		.port = _port,					\
+		.fw_mode = _fw_mode,				\
+	}
+
+static const struct mvebu_a3700_comphy_conf mvebu_a3700_comphy_modes[] = {
+	/* lane 0 */
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+	MVEBU_A3700_COMPHY_CONF(0, PHY_MODE_SGMII, 1, COMPHY_FW_MODE_SGMII),
+	/* lane 1 */
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_PCIE, 0, COMPHY_FW_MODE_PCIE),
+	MVEBU_A3700_COMPHY_CONF(1, PHY_MODE_SGMII, 0, COMPHY_FW_MODE_SGMII),
+	/* lane 2 */
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_SATA, 0, COMPHY_FW_MODE_SATA),
+	MVEBU_A3700_COMPHY_CONF(2, PHY_MODE_USB_HOST_SS, 0, COMPHY_FW_MODE_USB3H),
+};
+
+struct mvebu_a3700_comphy_lane {
+	struct device *dev;
+	unsigned int id;
+	enum phy_mode mode;
+	int port;
+};
+
+static int mvebu_a3700_comphy_smc(unsigned long function, unsigned long lane,
+				  unsigned long mode)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(function, lane, mode, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int mvebu_a3700_comphy_get_fw_mode(int lane, int port,
+					  enum phy_mode mode)
+{
+	int i, n = ARRAY_SIZE(mvebu_a3700_comphy_modes);
+
+	/* Unused PHY mux value is 0x0 */
+	if (mode == PHY_MODE_INVALID)
+		return -EINVAL;
+
+	for (i = 0; i < n; i++) {
+		if (mvebu_a3700_comphy_modes[i].lane == lane &&
+		    mvebu_a3700_comphy_modes[i].port == port &&
+		    mvebu_a3700_comphy_modes[i].mode == mode)
+			break;
+	}
+
+	if (i == n)
+		return -EINVAL;
+
+	return mvebu_a3700_comphy_modes[i].fw_mode;
+}
+
+static int mvebu_a3700_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port, mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	/* Just remember the mode, ->power_on() will do the real setup */
+	lane->mode = mode;
+
+	return 0;
+}
+
+static int mvebu_a3700_comphy_power_on(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+	u32 fw_param;
+	int fw_mode;
+
+	fw_mode = mvebu_a3700_comphy_get_fw_mode(lane->id, lane->port,
+						 lane->mode);
+	if (fw_mode < 0) {
+		dev_err(lane->dev, "invalid COMPHY mode\n");
+		return fw_mode;
+	}
+
+	switch (lane->mode) {
+	case PHY_MODE_USB_HOST_SS:
+		dev_dbg(lane->dev, "set lane %d to USB3 host mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SATA:
+		dev_dbg(lane->dev, "set lane %d to SATA mode\n", lane->id);
+		fw_param = COMPHY_FW_MODE(fw_mode);
+		break;
+	case PHY_MODE_SGMII:
+		dev_dbg(lane->dev, "set lane %d to SGMII mode\n", lane->id);
+		fw_param = COMPHY_FW_NET(fw_mode, lane->port,
+					 COMPHY_FW_SPEED_1_25G);
+		break;
+	case PHY_MODE_PCIE:
+		dev_dbg(lane->dev, "set lane %d to PCIe mode\n", lane->id);
+		fw_param = COMPHY_FW_PCIE(fw_mode, lane->port,
+					  COMPHY_FW_SPEED_5G,
+					  phy->attrs.bus_width);
+		break;
+	default:
+		dev_err(lane->dev, "unsupported PHY mode (%d)\n", lane->mode);
+		return -ENOTSUPP;
+	}
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_ON, lane->id, fw_param);
+}
+
+static int mvebu_a3700_comphy_power_off(struct phy *phy)
+{
+	struct mvebu_a3700_comphy_lane *lane = phy_get_drvdata(phy);
+
+	return mvebu_a3700_comphy_smc(COMPHY_SIP_POWER_OFF, lane->id, 0);
+}
+
+static const struct phy_ops mvebu_a3700_comphy_ops = {
+	.power_on	= mvebu_a3700_comphy_power_on,
+	.power_off	= mvebu_a3700_comphy_power_off,
+	.set_mode	= mvebu_a3700_comphy_set_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *mvebu_a3700_comphy_xlate(struct device *dev,
+					    struct of_phandle_args *args)
+{
+	struct mvebu_a3700_comphy_lane *lane;
+	struct phy *phy;
+
+	if (WARN_ON(args->args[0] >= MVEBU_A3700_COMPHY_PORTS))
+		return ERR_PTR(-EINVAL);
+
+	phy = of_phy_simple_xlate(dev, args);
+	if (IS_ERR(phy))
+		return phy;
+
+	lane = phy_get_drvdata(phy);
+	lane->port = args->args[0];
+	if (lane->port >= MVEBU_A3700_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);
+
+	return phy;
+}
+
+static int mvebu_a3700_comphy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *provider;
+	struct device_node *child;
+
+	for_each_available_child_of_node(pdev->dev.of_node, child) {
+		struct mvebu_a3700_comphy_lane *lane;
+		struct phy *phy;
+		int ret;
+		u32 lane_id;
+
+		ret = of_property_read_u32(child, "reg", &lane_id);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "missing 'reg' property (%d)\n",
+				ret);
+			continue;
+		}
+
+		if (lane_id >= MVEBU_A3700_COMPHY_LANES) {
+			dev_err(&pdev->dev, "invalid 'reg' property\n");
+			continue;
+		}
+
+		lane = devm_kzalloc(&pdev->dev, sizeof(*lane), GFP_KERNEL);
+		if (!lane)
+			return -ENOMEM;
+
+		phy = devm_phy_create(&pdev->dev, child,
+				      &mvebu_a3700_comphy_ops);
+		if (IS_ERR(phy))
+			return PTR_ERR(phy);
+
+		lane->dev = &pdev->dev;
+		lane->mode = PHY_MODE_INVALID;
+		lane->id = lane_id;
+		lane->port = -1;
+		phy_set_drvdata(phy, lane);
+	}
+
+	provider = devm_of_phy_provider_register(&pdev->dev,
+						 mvebu_a3700_comphy_xlate);
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id mvebu_a3700_comphy_of_match_table[] = {
+	{ .compatible = "marvell,comphy-a3700" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, mvebu_a3700_comphy_of_match_table);
+
+static struct platform_driver mvebu_a3700_comphy_driver = {
+	.probe	= mvebu_a3700_comphy_probe,
+	.driver	= {
+		.name = "mvebu-a3700-comphy",
+		.of_match_table = mvebu_a3700_comphy_of_match_table,
+	},
+};
+module_platform_driver(mvebu_a3700_comphy_driver);
+
+MODULE_AUTHOR("Miquèl Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Common PHY driver for A3700");
+MODULE_LICENSE("GPL v2");
-- 
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 5/8] dt-bindings: phy: mvebu-comphy: extend the file to describe a3700 bindings

[Miquel Raynal]

Current file describe COMPHY bindings for the IP available on the
CP110 of Armada 7k/8k. Bindings are very close (and serve the same
purpose) as the new Armada 3700 COMPHY driver so update this file to
describe both. Also add an example of how to use this second
compatible (same as for the ESPRESSObin).

While doing so, enhance a bit the file by adding upper case where
needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/phy/phy-mvebu-comphy.txt         | 65 +++++++++++++++----
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
index bfcf80341657..cf2cd86db267 100644
--- a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
@@ -1,16 +1,27 @@
-mvebu comphy driver
--------------------
+MVEBU comphy drivers
+--------------------
 
-A comphy controller can be found on Marvell Armada 7k/8k on the CP110. It
-provides a number of shared PHYs used by various interfaces (network, sata,
-usb, PCIe...).
+COMPHY controllers can be found on the following Marvell MVEBU SoCs:
+* Armada 7k/8k (on the CP110)
+* Armada 3700
+It provides a number of shared PHYs used by various interfaces (network, SATA,
+USB, PCIe...).
 
 Required properties:
 
-- compatible: should be "marvell,comphy-cp110"
-- reg: should contain the comphy register location and length.
-- marvell,system-controller: should contain a phandle to the
-                             system controller node.
+- compatible: should be one of:
+  * "marvell,comphy-cp110" for Armada 7k/8k
+  * "marvell,comphy-a3700" for Armada 3700
+- reg: should contain the COMPHY register(s) location(s) and length(s).
+  * 1 entry for Armada 7k/8k
+  * 4 entries for Armada 3700 along with the corresponding reg-names
+    properties, memory areas are:
+    * Generic COMPHY registers
+    * Lane 1 (PCIe/GbE)
+    * Lane 0 (USB3/GbE)
+    * Lane 2 (SATA/USB3)
+- marvell,system-controller: should contain a phandle to the system
+			     controller node (only for Armada 7k/8k)
 - #address-cells: should be 1.
 - #size-cells: should be 0.
 
@@ -18,11 +29,11 @@ A sub-node is required for each comphy lane provided by the comphy.
 
 Required properties (child nodes):
 
-- reg: comphy lane number.
-- #phy-cells : from the generic phy bindings, must be 1. Defines the
+- reg: COMPHY lane number.
+- #phy-cells : from the generic PHY bindings, must be 1. Defines the
                input port to use for a given comphy lane.
 
-Example:
+Examples:
 
 	cpm_comphy: phy@120000 {
 		compatible = "marvell,comphy-cp110";
@@ -41,3 +52,33 @@ Example:
 			#phy-cells = <1>;
 		};
 	};
+
+	comphy: phy@18300 {
+		compatible = "marvell,comphy-a3700";
+		reg = <0x18300 0x300>,
+		<0x1F000 0x400>,
+		<0x5C000 0x400>,
+		<0xe0178 0x8>;
+		reg-names = "comphy",
+		"lane1_pcie_gbe",
+		"lane0_usb3_gbe",
+		"lane2_sata_usb3";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+
+		comphy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+		};
+
+		comphy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <1>;
+		};
+
+		comphy2: phy@2 {
+			reg = <2>;
+			#phy-cells = <1>;
+		};
+	};
-- 
2.19.1

[PATCH v2 5/8] dt-bindings: phy: mvebu-comphy: extend the file to describe a3700 bindings

[Miquel Raynal]

Current file describe COMPHY bindings for the IP available on the
CP110 of Armada 7k/8k. Bindings are very close (and serve the same
purpose) as the new Armada 3700 COMPHY driver so update this file to
describe both. Also add an example of how to use this second
compatible (same as for the ESPRESSObin).

While doing so, enhance a bit the file by adding upper case where
needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/phy/phy-mvebu-comphy.txt         | 65 +++++++++++++++----
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
index bfcf80341657..cf2cd86db267 100644
--- a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
@@ -1,16 +1,27 @@
-mvebu comphy driver
--------------------
+MVEBU comphy drivers
+--------------------
 
-A comphy controller can be found on Marvell Armada 7k/8k on the CP110. It
-provides a number of shared PHYs used by various interfaces (network, sata,
-usb, PCIe...).
+COMPHY controllers can be found on the following Marvell MVEBU SoCs:
+* Armada 7k/8k (on the CP110)
+* Armada 3700
+It provides a number of shared PHYs used by various interfaces (network, SATA,
+USB, PCIe...).
 
 Required properties:
 
-- compatible: should be "marvell,comphy-cp110"
-- reg: should contain the comphy register location and length.
-- marvell,system-controller: should contain a phandle to the
-                             system controller node.
+- compatible: should be one of:
+  * "marvell,comphy-cp110" for Armada 7k/8k
+  * "marvell,comphy-a3700" for Armada 3700
+- reg: should contain the COMPHY register(s) location(s) and length(s).
+  * 1 entry for Armada 7k/8k
+  * 4 entries for Armada 3700 along with the corresponding reg-names
+    properties, memory areas are:
+    * Generic COMPHY registers
+    * Lane 1 (PCIe/GbE)
+    * Lane 0 (USB3/GbE)
+    * Lane 2 (SATA/USB3)
+- marvell,system-controller: should contain a phandle to the system
+			     controller node (only for Armada 7k/8k)
 - #address-cells: should be 1.
 - #size-cells: should be 0.
 
@@ -18,11 +29,11 @@ A sub-node is required for each comphy lane provided by the comphy.
 
 Required properties (child nodes):
 
-- reg: comphy lane number.
-- #phy-cells : from the generic phy bindings, must be 1. Defines the
+- reg: COMPHY lane number.
+- #phy-cells : from the generic PHY bindings, must be 1. Defines the
                input port to use for a given comphy lane.
 
-Example:
+Examples:
 
 	cpm_comphy: phy@120000 {
 		compatible = "marvell,comphy-cp110";
@@ -41,3 +52,33 @@ Example:
 			#phy-cells = <1>;
 		};
 	};
+
+	comphy: phy@18300 {
+		compatible = "marvell,comphy-a3700";
+		reg = <0x18300 0x300>,
+		<0x1F000 0x400>,
+		<0x5C000 0x400>,
+		<0xe0178 0x8>;
+		reg-names = "comphy",
+		"lane1_pcie_gbe",
+		"lane0_usb3_gbe",
+		"lane2_sata_usb3";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+
+		comphy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+		};
+
+		comphy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <1>;
+		};
+
+		comphy2: phy@2 {
+			reg = <2>;
+			#phy-cells = <1>;
+		};
+	};
-- 
2.19.1

[PATCH v2 5/8] dt-bindings: phy: mvebu-comphy: extend the file to describe a3700 bindings

[Miquel Raynal]

Current file describe COMPHY bindings for the IP available on the
CP110 of Armada 7k/8k. Bindings are very close (and serve the same
purpose) as the new Armada 3700 COMPHY driver so update this file to
describe both. Also add an example of how to use this second
compatible (same as for the ESPRESSObin).

While doing so, enhance a bit the file by adding upper case where
needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/phy/phy-mvebu-comphy.txt         | 65 +++++++++++++++----
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
index bfcf80341657..cf2cd86db267 100644
--- a/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
@@ -1,16 +1,27 @@
-mvebu comphy driver
--------------------
+MVEBU comphy drivers
+--------------------
 
-A comphy controller can be found on Marvell Armada 7k/8k on the CP110. It
-provides a number of shared PHYs used by various interfaces (network, sata,
-usb, PCIe...).
+COMPHY controllers can be found on the following Marvell MVEBU SoCs:
+* Armada 7k/8k (on the CP110)
+* Armada 3700
+It provides a number of shared PHYs used by various interfaces (network, SATA,
+USB, PCIe...).
 
 Required properties:
 
-- compatible: should be "marvell,comphy-cp110"
-- reg: should contain the comphy register location and length.
-- marvell,system-controller: should contain a phandle to the
-                             system controller node.
+- compatible: should be one of:
+  * "marvell,comphy-cp110" for Armada 7k/8k
+  * "marvell,comphy-a3700" for Armada 3700
+- reg: should contain the COMPHY register(s) location(s) and length(s).
+  * 1 entry for Armada 7k/8k
+  * 4 entries for Armada 3700 along with the corresponding reg-names
+    properties, memory areas are:
+    * Generic COMPHY registers
+    * Lane 1 (PCIe/GbE)
+    * Lane 0 (USB3/GbE)
+    * Lane 2 (SATA/USB3)
+- marvell,system-controller: should contain a phandle to the system
+			     controller node (only for Armada 7k/8k)
 - #address-cells: should be 1.
 - #size-cells: should be 0.
 
@@ -18,11 +29,11 @@ A sub-node is required for each comphy lane provided by the comphy.
 
 Required properties (child nodes):
 
-- reg: comphy lane number.
-- #phy-cells : from the generic phy bindings, must be 1. Defines the
+- reg: COMPHY lane number.
+- #phy-cells : from the generic PHY bindings, must be 1. Defines the
                input port to use for a given comphy lane.
 
-Example:
+Examples:
 
 	cpm_comphy: phy@120000 {
 		compatible = "marvell,comphy-cp110";
@@ -41,3 +52,33 @@ Example:
 			#phy-cells = <1>;
 		};
 	};
+
+	comphy: phy@18300 {
+		compatible = "marvell,comphy-a3700";
+		reg = <0x18300 0x300>,
+		<0x1F000 0x400>,
+		<0x5C000 0x400>,
+		<0xe0178 0x8>;
+		reg-names = "comphy",
+		"lane1_pcie_gbe",
+		"lane0_usb3_gbe",
+		"lane2_sata_usb3";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+
+		comphy0: phy@0 {
+			reg = <0>;
+			#phy-cells = <1>;
+		};
+
+		comphy1: phy@1 {
+			reg = <1>;
+			#phy-cells = <1>;
+		};
+
+		comphy2: phy@2 {
+			reg = <2>;
+			#phy-cells = <1>;
+		};
+	};
-- 
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 6/8] MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver

[Miquel Raynal]

Add myself as Armada 3700 COMPHY driver/bindings maintainer.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index f4855974f325..a1d6457cccb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8864,6 +8864,12 @@ F:	drivers/gpu/drm/armada/
 F:	include/uapi/drm/armada_drm.h
 F:	Documentation/devicetree/bindings/display/armada/
 
+MARVELL ARMADA 3700 COMPHY DRIVER
+M:	Miquel Raynal <miquel.raynal@bootlin.com>
+S:	Maintained
+F:	drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+F:	Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+
 MARVELL CRYPTO DRIVER
 M:	Boris Brezillon <boris.brezillon@bootlin.com>
 M:	Arnaud Ebalard <arno@natisbad.org>
-- 
2.19.1

[PATCH v2 6/8] MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver

[Miquel Raynal]

Add myself as Armada 3700 COMPHY driver/bindings maintainer.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index f4855974f325..a1d6457cccb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8864,6 +8864,12 @@ F:	drivers/gpu/drm/armada/
 F:	include/uapi/drm/armada_drm.h
 F:	Documentation/devicetree/bindings/display/armada/
 
+MARVELL ARMADA 3700 COMPHY DRIVER
+M:	Miquel Raynal <miquel.raynal@bootlin.com>
+S:	Maintained
+F:	drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+F:	Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+
 MARVELL CRYPTO DRIVER
 M:	Boris Brezillon <boris.brezillon@bootlin.com>
 M:	Arnaud Ebalard <arno@natisbad.org>
-- 
2.19.1

[PATCH v2 6/8] MAINTAINERS: phy: add entry for Armada 3700 COMPHY driver

[Miquel Raynal]

Add myself as Armada 3700 COMPHY driver/bindings maintainer.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index f4855974f325..a1d6457cccb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8864,6 +8864,12 @@ F:	drivers/gpu/drm/armada/
 F:	include/uapi/drm/armada_drm.h
 F:	Documentation/devicetree/bindings/display/armada/
 
+MARVELL ARMADA 3700 COMPHY DRIVER
+M:	Miquel Raynal <miquel.raynal@bootlin.com>
+S:	Maintained
+F:	drivers/phy/marvell/phy-mvebu-a3700-comphy.c
+F:	Documentation/devicetree/bindings/phy/phy-mvebu-comphy.txt
+
 MARVELL CRYPTO DRIVER
 M:	Boris Brezillon <boris.brezillon@bootlin.com>
 M:	Arnaud Ebalard <arno@natisbad.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 7/8] ARM64: dts: marvell: armada-37xx: fix SATA node scope

[Miquel Raynal]

Fix the SATA IP memory area which is only 0x178 bytes long (from
Marvell A3700 specification). Actually, starting from the offset
0xe0178, there is an area dedicated to the COMPHY driver.

Suggested-by: Grzegorz Jaszczyk <jaz@semihalf.com>
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 4472bcd8f9fb..e7405931f381 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -358,7 +358,7 @@
 
 			sata: sata@e0000 {
 				compatible = "marvell,armada-3700-ahci";
-				reg = <0xe0000 0x2000>;
+				reg = <0xe0000 0x178>;
 				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 			};
-- 
2.19.1

[PATCH v2 7/8] ARM64: dts: marvell: armada-37xx: fix SATA node scope

[Miquel Raynal]

Fix the SATA IP memory area which is only 0x178 bytes long (from
Marvell A3700 specification). Actually, starting from the offset
0xe0178, there is an area dedicated to the COMPHY driver.

Suggested-by: Grzegorz Jaszczyk <jaz@semihalf.com>
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 4472bcd8f9fb..e7405931f381 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -358,7 +358,7 @@
 
 			sata: sata@e0000 {
 				compatible = "marvell,armada-3700-ahci";
-				reg = <0xe0000 0x2000>;
+				reg = <0xe0000 0x178>;
 				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 			};
-- 
2.19.1

[PATCH v2 7/8] ARM64: dts: marvell: armada-37xx: fix SATA node scope

[Miquel Raynal]

Fix the SATA IP memory area which is only 0x178 bytes long (from
Marvell A3700 specification). Actually, starting from the offset
0xe0178, there is an area dedicated to the COMPHY driver.

Suggested-by: Grzegorz Jaszczyk <jaz@semihalf.com>
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 4472bcd8f9fb..e7405931f381 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -358,7 +358,7 @@
 
 			sata: sata@e0000 {
 				compatible = "marvell,armada-3700-ahci";
-				reg = <0xe0000 0x2000>;
+				reg = <0xe0000 0x178>;
 				interrupts = <GIC_SPI 27 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 8/8] ARM64: dts: marvell: armada-37xx: declare the COMPHY node

[Miquel Raynal]

Describe the A3700 COMPHY node. It has three PHYs that can be
configured as follow:
* PCIe or GbE
* USB3 or GbE
* SATA or USB3
Each of them has its own memory area.

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

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e7405931f381..036d6fd6c9ef 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -242,6 +242,35 @@
 				reg = <0x14000 0x60>;
 			};
 
+			comphy: phy@18300 {
+				compatible = "marvell,comphy-a3700";
+				reg = <0x18300 0x300>,
+				      <0x1F000 0x400>,
+				      <0x5C000 0x400>,
+				      <0xe0178 0x8>;
+				reg-names = "comphy",
+					    "lane1_pcie_gbe",
+					    "lane0_usb3_gbe",
+					    "lane2_sata_usb3";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+			};
+
 			pinctrl_sb: pinctrl@18800 {
 				compatible = "marvell,armada3710-sb-pinctrl",
 					     "syscon", "simple-mfd";
-- 
2.19.1

[PATCH v2 8/8] ARM64: dts: marvell: armada-37xx: declare the COMPHY node

[Miquel Raynal]

Describe the A3700 COMPHY node. It has three PHYs that can be
configured as follow:
* PCIe or GbE
* USB3 or GbE
* SATA or USB3
Each of them has its own memory area.

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

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e7405931f381..036d6fd6c9ef 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -242,6 +242,35 @@
 				reg = <0x14000 0x60>;
 			};
 
+			comphy: phy@18300 {
+				compatible = "marvell,comphy-a3700";
+				reg = <0x18300 0x300>,
+				      <0x1F000 0x400>,
+				      <0x5C000 0x400>,
+				      <0xe0178 0x8>;
+				reg-names = "comphy",
+					    "lane1_pcie_gbe",
+					    "lane0_usb3_gbe",
+					    "lane2_sata_usb3";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+			};
+
 			pinctrl_sb: pinctrl@18800 {
 				compatible = "marvell,armada3710-sb-pinctrl",
 					     "syscon", "simple-mfd";
-- 
2.19.1

[PATCH v2 8/8] ARM64: dts: marvell: armada-37xx: declare the COMPHY node

[Miquel Raynal]

Describe the A3700 COMPHY node. It has three PHYs that can be
configured as follow:
* PCIe or GbE
* USB3 or GbE
* SATA or USB3
Each of them has its own memory area.

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

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e7405931f381..036d6fd6c9ef 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -242,6 +242,35 @@
 				reg = <0x14000 0x60>;
 			};
 
+			comphy: phy@18300 {
+				compatible = "marvell,comphy-a3700";
+				reg = <0x18300 0x300>,
+				      <0x1F000 0x400>,
+				      <0x5C000 0x400>,
+				      <0xe0178 0x8>;
+				reg-names = "comphy",
+					    "lane1_pcie_gbe",
+					    "lane0_usb3_gbe",
+					    "lane2_sata_usb3";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				comphy0: phy@0 {
+					reg = <0>;
+					#phy-cells = <1>;
+				};
+
+				comphy1: phy@1 {
+					reg = <1>;
+					#phy-cells = <1>;
+				};
+
+				comphy2: phy@2 {
+					reg = <2>;
+					#phy-cells = <1>;
+				};
+			};
+
 			pinctrl_sb: pinctrl@18800 {
 				compatible = "marvell,armada3710-sb-pinctrl",
 					     "syscon", "simple-mfd";
-- 
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 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Russell King - ARM Linux]

On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> So far the PHY ->xlate() callback was checking if the port was
> "invalid" before continuing, meaning that the port has not been used
> yet. This check is not correct as there is no opposite call to
> ->xlate() once the PHY is released by the user and the port will
> remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> this driver is built as a module, inserted, removed and inserted
> again, the PHY will appear busy and the second probe will fail.
> 
> To fix this, just drop the faulty check and instead verify that the
> port number is valid (ie. in the possible range).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 31b9a1c18345..a40b876ff214 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
>  		return phy;
>  
>  	lane = phy_get_drvdata(phy);
> -	if (lane->port >= 0)
> -		return ERR_PTR(-EBUSY);
>  	lane->port = args->args[0];
> +	if (lane->port >= MVEBU_COMPHY_PORTS)
> +		return ERR_PTR(-EINVAL);

Shouldn't we validate args->args[0] before doing anything?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Russell King - ARM Linux]

On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> So far the PHY ->xlate() callback was checking if the port was
> "invalid" before continuing, meaning that the port has not been used
> yet. This check is not correct as there is no opposite call to
> ->xlate() once the PHY is released by the user and the port will
> remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> this driver is built as a module, inserted, removed and inserted
> again, the PHY will appear busy and the second probe will fail.
> 
> To fix this, just drop the faulty check and instead verify that the
> port number is valid (ie. in the possible range).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 31b9a1c18345..a40b876ff214 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
>  		return phy;
>  
>  	lane = phy_get_drvdata(phy);
> -	if (lane->port >= 0)
> -		return ERR_PTR(-EBUSY);
>  	lane->port = args->args[0];
> +	if (lane->port >= MVEBU_COMPHY_PORTS)
> +		return ERR_PTR(-EINVAL);

Shouldn't we validate args->args[0] before doing anything?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

Hi Russell,

Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
2018 19:00:31 +0000:

> On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> > So far the PHY ->xlate() callback was checking if the port was
> > "invalid" before continuing, meaning that the port has not been used
> > yet. This check is not correct as there is no opposite call to  
> > ->xlate() once the PHY is released by the user and the port will  
> > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > this driver is built as a module, inserted, removed and inserted
> > again, the PHY will appear busy and the second probe will fail.
> > 
> > To fix this, just drop the faulty check and instead verify that the
> > port number is valid (ie. in the possible range).
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > index 31b9a1c18345..a40b876ff214 100644
> > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> >  		return phy;
> >  
> >  	lane = phy_get_drvdata(phy);
> > -	if (lane->port >= 0)
> > -		return ERR_PTR(-EBUSY);
> >  	lane->port = args->args[0];
> > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > +		return ERR_PTR(-EINVAL);  
> 
> Shouldn't we validate args->args[0] before doing anything?
> 

I don't understand your point, there is a check on args->args[0] as
we check its value (through lane->port) right after. What do you
have in mind?


Thanks,
Miquèl

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

Hi Russell,

Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
2018 19:00:31 +0000:

> On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> > So far the PHY ->xlate() callback was checking if the port was
> > "invalid" before continuing, meaning that the port has not been used
> > yet. This check is not correct as there is no opposite call to  
> > ->xlate() once the PHY is released by the user and the port will  
> > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > this driver is built as a module, inserted, removed and inserted
> > again, the PHY will appear busy and the second probe will fail.
> > 
> > To fix this, just drop the faulty check and instead verify that the
> > port number is valid (ie. in the possible range).
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > index 31b9a1c18345..a40b876ff214 100644
> > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> >  		return phy;
> >  
> >  	lane = phy_get_drvdata(phy);
> > -	if (lane->port >= 0)
> > -		return ERR_PTR(-EBUSY);
> >  	lane->port = args->args[0];
> > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > +		return ERR_PTR(-EINVAL);  
> 
> Shouldn't we validate args->args[0] before doing anything?
> 

I don't understand your point, there is a check on args->args[0] as
we check its value (through lane->port) right after. What do you
have in mind?


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 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Russell King - ARM Linux]

On Sun, Dec 02, 2018 at 08:35:09PM +0100, Miquel Raynal wrote:
> Hi Russell,
> 
> Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
> 2018 19:00:31 +0000:
> 
> > On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> > > So far the PHY ->xlate() callback was checking if the port was
> > > "invalid" before continuing, meaning that the port has not been used
> > > yet. This check is not correct as there is no opposite call to  
> > > ->xlate() once the PHY is released by the user and the port will  
> > > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > > this driver is built as a module, inserted, removed and inserted
> > > again, the PHY will appear busy and the second probe will fail.
> > > 
> > > To fix this, just drop the faulty check and instead verify that the
> > > port number is valid (ie. in the possible range).
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > index 31b9a1c18345..a40b876ff214 100644
> > > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> > >  		return phy;
> > >  
> > >  	lane = phy_get_drvdata(phy);
> > > -	if (lane->port >= 0)
> > > -		return ERR_PTR(-EBUSY);
> > >  	lane->port = args->args[0];
> > > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > > +		return ERR_PTR(-EINVAL);  
> > 
> > Shouldn't we validate args->args[0] before doing anything?
> > 
> 
> I don't understand your point, there is a check on args->args[0] as
> we check its value (through lane->port) right after. What do you
> have in mind?

Right, there is already a check on args->args[0] for it being greater
than MVEBU_COMPHY_PORTS and returning an error (and in fact warning
if that is the case).  So in that case, what is the use of the above
additional test you are proposing to add?  The resulting code ends up
looking like this:

	if (WARN_ON(args->args[0] >= MVEBU_COMPHY_PORTS))
		return ERR_PTR(-EINVAL);
...
	lane->port = args->args[0];
+	if (lane->port >= MVEBU_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);  

which is just silly - the second test can never be evaluated as true,
and therefore is redundant.

In any case, my point was that in your patch, where you assign
lane->port and then validate the lane->port value, this is in
principle the wrong order - the order should always be: validate
first, then make use.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Russell King - ARM Linux]

On Sun, Dec 02, 2018 at 08:35:09PM +0100, Miquel Raynal wrote:
> Hi Russell,
> 
> Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
> 2018 19:00:31 +0000:
> 
> > On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:
> > > So far the PHY ->xlate() callback was checking if the port was
> > > "invalid" before continuing, meaning that the port has not been used
> > > yet. This check is not correct as there is no opposite call to  
> > > ->xlate() once the PHY is released by the user and the port will  
> > > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > > this driver is built as a module, inserted, removed and inserted
> > > again, the PHY will appear busy and the second probe will fail.
> > > 
> > > To fix this, just drop the faulty check and instead verify that the
> > > port number is valid (ie. in the possible range).
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > index 31b9a1c18345..a40b876ff214 100644
> > > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> > >  		return phy;
> > >  
> > >  	lane = phy_get_drvdata(phy);
> > > -	if (lane->port >= 0)
> > > -		return ERR_PTR(-EBUSY);
> > >  	lane->port = args->args[0];
> > > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > > +		return ERR_PTR(-EINVAL);  
> > 
> > Shouldn't we validate args->args[0] before doing anything?
> > 
> 
> I don't understand your point, there is a check on args->args[0] as
> we check its value (through lane->port) right after. What do you
> have in mind?

Right, there is already a check on args->args[0] for it being greater
than MVEBU_COMPHY_PORTS and returning an error (and in fact warning
if that is the case).  So in that case, what is the use of the above
additional test you are proposing to add?  The resulting code ends up
looking like this:

	if (WARN_ON(args->args[0] >= MVEBU_COMPHY_PORTS))
		return ERR_PTR(-EINVAL);
...
	lane->port = args->args[0];
+	if (lane->port >= MVEBU_COMPHY_PORTS)
+		return ERR_PTR(-EINVAL);  

which is just silly - the second test can never be evaluated as true,
and therefore is redundant.

In any case, my point was that in your patch, where you assign
lane->port and then validate the lane->port value, this is in
principle the wrong order - the order should always be: validate
first, then make use.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

Hi Russell,

Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Mon, 3 Dec
2018 00:36:23 +0000:

> On Sun, Dec 02, 2018 at 08:35:09PM +0100, Miquel Raynal wrote:
> > Hi Russell,
> > 
> > Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
> > 2018 19:00:31 +0000:
> >   
> > > On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:  
> > > > So far the PHY ->xlate() callback was checking if the port was
> > > > "invalid" before continuing, meaning that the port has not been used
> > > > yet. This check is not correct as there is no opposite call to    
> > > > ->xlate() once the PHY is released by the user and the port will    
> > > > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > > > this driver is built as a module, inserted, removed and inserted
> > > > again, the PHY will appear busy and the second probe will fail.
> > > > 
> > > > To fix this, just drop the faulty check and instead verify that the
> > > > port number is valid (ie. in the possible range).
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > index 31b9a1c18345..a40b876ff214 100644
> > > > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> > > >  		return phy;
> > > >  
> > > >  	lane = phy_get_drvdata(phy);
> > > > -	if (lane->port >= 0)
> > > > -		return ERR_PTR(-EBUSY);
> > > >  	lane->port = args->args[0];
> > > > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > > > +		return ERR_PTR(-EINVAL);    
> > > 
> > > Shouldn't we validate args->args[0] before doing anything?
> > >   
> > 
> > I don't understand your point, there is a check on args->args[0] as
> > we check its value (through lane->port) right after. What do you
> > have in mind?  
> 
> Right, there is already a check on args->args[0] for it being greater
> than MVEBU_COMPHY_PORTS and returning an error (and in fact warning
> if that is the case).  So in that case, what is the use of the above
> additional test you are proposing to add?  The resulting code ends up
> looking like this:
> 
> 	if (WARN_ON(args->args[0] >= MVEBU_COMPHY_PORTS))
> 		return ERR_PTR(-EINVAL);
> ...
> 	lane->port = args->args[0];
> +	if (lane->port >= MVEBU_COMPHY_PORTS)
> +		return ERR_PTR(-EINVAL);  
> 
> which is just silly - the second test can never be evaluated as true,
> and therefore is redundant.
> 
> In any case, my point was that in your patch, where you assign
> lane->port and then validate the lane->port value, this is in
> principle the wrong order - the order should always be: validate
> first, then make use.
> 

You are right, this test is redundant; I forgot about the first
check. I will just drop these additional two lines and just do:

                [...]
                lane->port = args->args[0];

                return 0;
        }

Thanks,
Miquèl

Re: [PATCH v2 2/8] phy: mvebu-cp110-comphy: fix port check in ->xlate()

[Miquel Raynal]

Hi Russell,

Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Mon, 3 Dec
2018 00:36:23 +0000:

> On Sun, Dec 02, 2018 at 08:35:09PM +0100, Miquel Raynal wrote:
> > Hi Russell,
> > 
> > Russell King - ARM Linux <linux@armlinux.org.uk> wrote on Fri, 30 Nov
> > 2018 19:00:31 +0000:
> >   
> > > On Fri, Nov 30, 2018 at 03:47:37PM +0100, Miquel Raynal wrote:  
> > > > So far the PHY ->xlate() callback was checking if the port was
> > > > "invalid" before continuing, meaning that the port has not been used
> > > > yet. This check is not correct as there is no opposite call to    
> > > > ->xlate() once the PHY is released by the user and the port will    
> > > > remain "valid" after the first phy_get()/phy_put() calls. Hence, if
> > > > this driver is built as a module, inserted, removed and inserted
> > > > again, the PHY will appear busy and the second probe will fail.
> > > > 
> > > > To fix this, just drop the faulty check and instead verify that the
> > > > port number is valid (ie. in the possible range).
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/phy/marvell/phy-mvebu-cp110-comphy.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > index 31b9a1c18345..a40b876ff214 100644
> > > > --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> > > > @@ -567,9 +567,9 @@ static struct phy *mvebu_comphy_xlate(struct device *dev,
> > > >  		return phy;
> > > >  
> > > >  	lane = phy_get_drvdata(phy);
> > > > -	if (lane->port >= 0)
> > > > -		return ERR_PTR(-EBUSY);
> > > >  	lane->port = args->args[0];
> > > > +	if (lane->port >= MVEBU_COMPHY_PORTS)
> > > > +		return ERR_PTR(-EINVAL);    
> > > 
> > > Shouldn't we validate args->args[0] before doing anything?
> > >   
> > 
> > I don't understand your point, there is a check on args->args[0] as
> > we check its value (through lane->port) right after. What do you
> > have in mind?  
> 
> Right, there is already a check on args->args[0] for it being greater
> than MVEBU_COMPHY_PORTS and returning an error (and in fact warning
> if that is the case).  So in that case, what is the use of the above
> additional test you are proposing to add?  The resulting code ends up
> looking like this:
> 
> 	if (WARN_ON(args->args[0] >= MVEBU_COMPHY_PORTS))
> 		return ERR_PTR(-EINVAL);
> ...
> 	lane->port = args->args[0];
> +	if (lane->port >= MVEBU_COMPHY_PORTS)
> +		return ERR_PTR(-EINVAL);  
> 
> which is just silly - the second test can never be evaluated as true,
> and therefore is redundant.
> 
> In any case, my point was that in your patch, where you assign
> lane->port and then validate the lane->port value, this is in
> principle the wrong order - the order should always be: validate
> first, then make use.
> 

You are right, this test is redundant; I forgot about the first
check. I will just drop these additional two lines and just do:

                [...]
                lane->port = args->args[0];

                return 0;
        }

Thanks,
Miquèl

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