2017-06-08 20:04:42

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 0/5] phy: bcm-ns-usb3: add MDIO driver

From: Rafał Miłecki <[email protected]>

As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support
for Broadcom NSP SoC"") this module should be modified to use MDIO bus as
this is how PHY is really attached.

This should allow reusing this driver on NSP and any other platform with
MDIO bus and this particular PHY.

V2: Rebase and update 4/5 and 5/5.

Rafał Miłecki (5):
phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg
phy: bcm-ns-usb3: use pointer for PHY writing function
phy: bcm-ns-usb3: enable MDIO in the platform specific code
dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO
phy: bcm-ns-usb3: add MDIO driver using proper bus layer

.../devicetree/bindings/phy/bcm-ns-usb3-phy.txt | 27 ++-
drivers/phy/broadcom/Kconfig | 1 +
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 230 +++++++++++++++------
3 files changed, 186 insertions(+), 72 deletions(-)

--
2.11.0


2017-06-08 20:04:47

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 2/5] phy: bcm-ns-usb3: use pointer for PHY writing function

From: Rafał Miłecki <[email protected]>

Our current writing function accesses PHY directly bypassing MDIO layer.

The aim is to extend this module to also behave as MDIO driver. This
will require using different writing function which can be handled
cleanly by having an extra pointer like this.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 98 +++++++++++++++++++---------------
1 file changed, 56 insertions(+), 42 deletions(-)

diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb3.c b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
index 5e89326886dc..3d0fe5728029 100644
--- a/drivers/phy/broadcom/phy-bcm-ns-usb3.c
+++ b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
@@ -53,6 +53,8 @@ struct bcm_ns_usb3 {
void __iomem *dmp;
void __iomem *ccb_mii;
struct phy *phy;
+
+ int (*phy_write)(struct bcm_ns_usb3 *usb3, u16 reg, u16 value);
};

static const struct of_device_id bcm_ns_usb3_id_table[] = {
@@ -68,51 +70,10 @@ static const struct of_device_id bcm_ns_usb3_id_table[] = {
};
MODULE_DEVICE_TABLE(of, bcm_ns_usb3_id_table);

-static int bcm_ns_usb3_wait_reg(struct bcm_ns_usb3 *usb3, void __iomem *addr,
- u32 mask, u32 value, unsigned long timeout)
-{
- unsigned long deadline = jiffies + timeout;
- u32 val;
-
- do {
- val = readl(addr);
- if ((val & mask) == value)
- return 0;
- cpu_relax();
- udelay(10);
- } while (!time_after_eq(jiffies, deadline));
-
- dev_err(usb3->dev, "Timeout waiting for register %p\n", addr);
-
- return -EBUSY;
-}
-
-static inline int bcm_ns_usb3_mii_mng_wait_idle(struct bcm_ns_usb3 *usb3)
-{
- return bcm_ns_usb3_wait_reg(usb3, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL,
- 0x0100, 0x0000,
- usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US));
-}
-
static int bcm_ns_usb3_mdio_phy_write(struct bcm_ns_usb3 *usb3, u16 reg,
u16 value)
{
- u32 tmp = 0;
- int err;
-
- err = bcm_ns_usb3_mii_mng_wait_idle(usb3);
- if (err < 0) {
- dev_err(usb3->dev, "Couldn't write 0x%08x value\n", value);
- return err;
- }
-
- /* TODO: Use a proper MDIO bus layer */
- tmp |= 0x58020000; /* Magic value for MDIO PHY write */
- tmp |= reg << 18;
- tmp |= value;
- writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA);
-
- return bcm_ns_usb3_mii_mng_wait_idle(usb3);
+ return usb3->phy_write(usb3, reg, value);
}

static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3)
@@ -233,6 +194,57 @@ static const struct phy_ops ops = {
.owner = THIS_MODULE,
};

+/**************************************************
+ * Platform driver code
+ **************************************************/
+
+static int bcm_ns_usb3_wait_reg(struct bcm_ns_usb3 *usb3, void __iomem *addr,
+ u32 mask, u32 value, unsigned long timeout)
+{
+ unsigned long deadline = jiffies + timeout;
+ u32 val;
+
+ do {
+ val = readl(addr);
+ if ((val & mask) == value)
+ return 0;
+ cpu_relax();
+ udelay(10);
+ } while (!time_after_eq(jiffies, deadline));
+
+ dev_err(usb3->dev, "Timeout waiting for register %p\n", addr);
+
+ return -EBUSY;
+}
+
+static inline int bcm_ns_usb3_mii_mng_wait_idle(struct bcm_ns_usb3 *usb3)
+{
+ return bcm_ns_usb3_wait_reg(usb3, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL,
+ 0x0100, 0x0000,
+ usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US));
+}
+
+static int bcm_ns_usb3_platform_phy_write(struct bcm_ns_usb3 *usb3, u16 reg,
+ u16 value)
+{
+ u32 tmp = 0;
+ int err;
+
+ err = bcm_ns_usb3_mii_mng_wait_idle(usb3);
+ if (err < 0) {
+ dev_err(usb3->dev, "Couldn't write 0x%08x value\n", value);
+ return err;
+ }
+
+ /* TODO: Use a proper MDIO bus layer */
+ tmp |= 0x58020000; /* Magic value for MDIO PHY write */
+ tmp |= reg << 18;
+ tmp |= value;
+ writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA);
+
+ return bcm_ns_usb3_mii_mng_wait_idle(usb3);
+}
+
static int bcm_ns_usb3_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -266,6 +278,8 @@ static int bcm_ns_usb3_probe(struct platform_device *pdev)
return PTR_ERR(usb3->ccb_mii);
}

+ usb3->phy_write = bcm_ns_usb3_platform_phy_write;
+
usb3->phy = devm_phy_create(dev, NULL, &ops);
if (IS_ERR(usb3->phy)) {
dev_err(dev, "Failed to create PHY\n");
--
2.11.0

2017-06-08 20:05:01

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 4/5] dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO

From: Rafał Miłecki <[email protected]>

Thanks to work done by Broadcom explaining their USB 3.0 PHY details we
know it's attached to the MDIO bus. Use this knowledge to update the
binding: make it a subnode to the MDIO bus and rework way of specifying
required registers. This will describe hardware more precisely and will
allow to support (describe) more devices attached to the MDIO.

While compatibility strings remain the same there isn't a direct
conflict (compatibility breakage) for the binding. Originally it wasn't
supposed to be used for MDIO subnode so this change should be safe
unless some operating system was probing MDIO subnodes as standalone
devices.

Signed-off-by: Rafał Miłecki <[email protected]>
---
V2: Extend commit message to better describe reason and includ info on
(non-)breakage.
---
.../devicetree/bindings/phy/bcm-ns-usb3-phy.txt | 27 +++++++++++++++-------
1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt
index 09aeba94538d..32f057260351 100644
--- a/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt
+++ b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt
@@ -3,9 +3,10 @@ Driver for Broadcom Northstar USB 3.0 PHY
Required properties:

- compatible: one of: "brcm,ns-ax-usb3-phy", "brcm,ns-bx-usb3-phy".
-- reg: register mappings for DMP (Device Management Plugin) and ChipCommon B
- MMI.
-- reg-names: "dmp" and "ccb-mii"
+- reg: address of MDIO bus device
+- usb3-dmp-syscon: phandle to syscon with DMP (Device Management Plugin)
+ registers
+- #phy-cells: must be 0

Initialization of USB 3.0 PHY depends on Northstar version. There are currently
three known series: Ax, Bx and Cx.
@@ -15,9 +16,19 @@ Known B1: BCM4707 rev 6
Known C0: BCM47094 rev 0

Example:
- usb3-phy {
- compatible = "brcm,ns-ax-usb3-phy";
- reg = <0x18105000 0x1000>, <0x18003000 0x1000>;
- reg-names = "dmp", "ccb-mii";
- #phy-cells = <0>;
+ mdio: mdio@0 {
+ reg = <0x0>;
+ #size-cells = <1>;
+ #address-cells = <0>;
+
+ usb3-phy@10 {
+ compatible = "brcm,ns-ax-usb3-phy";
+ reg = <0x10>;
+ usb3-dmp-syscon = <&usb3_dmp>;
+ #phy-cells = <0>;
+ };
+ };
+
+ usb3_dmp: syscon@18105000 {
+ reg = <0x18105000 0x1000>;
};
--
2.11.0

2017-06-08 20:04:55

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 3/5] phy: bcm-ns-usb3: enable MDIO in the platform specific code

From: Rafał Miłecki <[email protected]>

When we finally start using MDIO layer then bus initialization will be
handled in a separated driver. It means our code handling this has to be
used for the platform driver only.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb3.c b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
index 3d0fe5728029..2c9a0d5f43d8 100644
--- a/drivers/phy/broadcom/phy-bcm-ns-usb3.c
+++ b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
@@ -80,12 +80,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3)
{
int err;

- /* Enable MDIO. Setting MDCDIV as 26 */
- writel(0x0000009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL);
-
- /* Wait for MDIO? */
- udelay(2);
-
/* USB3 PLL Block */
err = bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PHY_BASE_ADDR_REG,
BCM_NS_USB3_PHY_PLL30_BLOCK);
@@ -134,12 +128,6 @@ static int bcm_ns_usb3_phy_init_ns_ax(struct bcm_ns_usb3 *usb3)
{
int err;

- /* Enable MDIO. Setting MDCDIV as 26 */
- writel(0x0000009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL);
-
- /* Wait for MDIO? */
- udelay(2);
-
/* PLL30 block */
err = bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PHY_BASE_ADDR_REG,
BCM_NS_USB3_PHY_PLL30_BLOCK);
@@ -278,6 +266,12 @@ static int bcm_ns_usb3_probe(struct platform_device *pdev)
return PTR_ERR(usb3->ccb_mii);
}

+ /* Enable MDIO. Setting MDCDIV as 26 */
+ writel(0x0000009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL);
+
+ /* Wait for MDIO? */
+ udelay(2);
+
usb3->phy_write = bcm_ns_usb3_platform_phy_write;

usb3->phy = devm_phy_create(dev, NULL, &ops);
--
2.11.0

2017-06-08 20:04:52

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 1/5] phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg

From: Rafał Miłecki <[email protected]>

Move MDIO specific code to the writing helper function. This makes init
code a bit more generic and doesn't require it to track what happens
after every write.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb3.c b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
index 22b5e7047fa6..5e89326886dc 100644
--- a/drivers/phy/broadcom/phy-bcm-ns-usb3.c
+++ b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
@@ -112,7 +112,7 @@ static int bcm_ns_usb3_mdio_phy_write(struct bcm_ns_usb3 *usb3, u16 reg,
tmp |= value;
writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA);

- return 0;
+ return bcm_ns_usb3_mii_mng_wait_idle(usb3);
}

static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3)
@@ -143,9 +143,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3)
/* Deaaserting PLL Reset */
bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PLLA_CONTROL1, 0x8000);

- /* Waiting MII Mgt interface idle */
- bcm_ns_usb3_mii_mng_wait_idle(usb3);
-
/* Deasserting USB3 system reset */
writel(0, usb3->dmp + BCMA_RESET_CTL);

@@ -169,9 +166,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3)
/* Enabling SSC */
bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_TX_PMD_CONTROL1, 0x1003);

- /* Waiting MII Mgt interface idle */
- bcm_ns_usb3_mii_mng_wait_idle(usb3);
-
return 0;
}

@@ -205,9 +199,6 @@ static int bcm_ns_usb3_phy_init_ns_ax(struct bcm_ns_usb3 *usb3)

bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_TX_PMD_CONTROL1, 0x1003);

- /* Waiting MII Mgt interface idle */
- bcm_ns_usb3_mii_mng_wait_idle(usb3);
-
/* Deasserting USB3 system reset */
writel(0, usb3->dmp + BCMA_RESET_CTL);

--
2.11.0

2017-06-08 20:05:31

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH V2 5/5] phy: bcm-ns-usb3: add MDIO driver using proper bus layer

From: Rafał Miłecki <[email protected]>

As USB 3.0 PHY is attached to the MDIO bus this module should provide a
MDIO driver and use a proper bus layer. This is a proper (cleaner)
solution which doesn't require code to know this specific MDIO bus
details. It also allows reusing the driver with other MDIO buses.

For now keep platform device support in place. We may consider dropping
it once MDIO bindings gets used "everywhere".

Signed-off-by: Rafał Miłecki <[email protected]>
---
V2: Select MDIO_DEVICE which was introduced in 4.12.
Add info about possible removal of platform driver in the future.
---
drivers/phy/broadcom/Kconfig | 1 +
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 105 ++++++++++++++++++++++++++++++++-
2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
index d2d99023ec50..517fe0f3e639 100644
--- a/drivers/phy/broadcom/Kconfig
+++ b/drivers/phy/broadcom/Kconfig
@@ -31,6 +31,7 @@ config PHY_BCM_NS_USB3
depends on ARCH_BCM_IPROC || COMPILE_TEST
depends on HAS_IOMEM && OF
select GENERIC_PHY
+ select MDIO_DEVICE
help
Enable this to support Broadcom USB 3.0 PHY connected to the USB
controller on Northstar family.
diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb3.c b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
index 2c9a0d5f43d8..a53ae128eadf 100644
--- a/drivers/phy/broadcom/phy-bcm-ns-usb3.c
+++ b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
@@ -16,7 +16,9 @@
#include <linux/bcma/bcma.h>
#include <linux/delay.h>
#include <linux/err.h>
+#include <linux/mdio.h>
#include <linux/module.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/phy/phy.h>
@@ -52,6 +54,7 @@ struct bcm_ns_usb3 {
enum bcm_ns_family family;
void __iomem *dmp;
void __iomem *ccb_mii;
+ struct mdio_device *mdiodev;
struct phy *phy;

int (*phy_write)(struct bcm_ns_usb3 *usb3, u16 reg, u16 value);
@@ -183,6 +186,77 @@ static const struct phy_ops ops = {
};

/**************************************************
+ * MDIO driver code
+ **************************************************/
+
+static int bcm_ns_usb3_mdiodev_phy_write(struct bcm_ns_usb3 *usb3, u16 reg,
+ u16 value)
+{
+ struct mdio_device *mdiodev = usb3->mdiodev;
+
+ return mdiobus_write(mdiodev->bus, mdiodev->addr, reg, value);
+}
+
+static int bcm_ns_usb3_mdio_probe(struct mdio_device *mdiodev)
+{
+ struct device *dev = &mdiodev->dev;
+ const struct of_device_id *of_id;
+ struct phy_provider *phy_provider;
+ struct device_node *syscon_np;
+ struct bcm_ns_usb3 *usb3;
+ struct resource res;
+ int err;
+
+ usb3 = devm_kzalloc(dev, sizeof(*usb3), GFP_KERNEL);
+ if (!usb3)
+ return -ENOMEM;
+
+ usb3->dev = dev;
+ usb3->mdiodev = mdiodev;
+
+ of_id = of_match_device(bcm_ns_usb3_id_table, dev);
+ if (!of_id)
+ return -EINVAL;
+ usb3->family = (enum bcm_ns_family)of_id->data;
+
+ syscon_np = of_parse_phandle(dev->of_node, "usb3-dmp-syscon", 0);
+ err = of_address_to_resource(syscon_np, 0, &res);
+ of_node_put(syscon_np);
+ if (err)
+ return err;
+
+ usb3->dmp = devm_ioremap_resource(dev, &res);
+ if (IS_ERR(usb3->dmp)) {
+ dev_err(dev, "Failed to map DMP regs\n");
+ return PTR_ERR(usb3->dmp);
+ }
+
+ usb3->phy_write = bcm_ns_usb3_mdiodev_phy_write;
+
+ usb3->phy = devm_phy_create(dev, NULL, &ops);
+ if (IS_ERR(usb3->phy)) {
+ dev_err(dev, "Failed to create PHY\n");
+ return PTR_ERR(usb3->phy);
+ }
+
+ phy_set_drvdata(usb3->phy, usb3);
+
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static struct mdio_driver bcm_ns_usb3_mdio_driver = {
+ .mdiodrv = {
+ .driver = {
+ .name = "bcm_ns_mdio_usb3",
+ .of_match_table = bcm_ns_usb3_id_table,
+ },
+ },
+ .probe = bcm_ns_usb3_mdio_probe,
+};
+
+/**************************************************
* Platform driver code
**************************************************/

@@ -297,6 +371,35 @@ static struct platform_driver bcm_ns_usb3_driver = {
.of_match_table = bcm_ns_usb3_id_table,
},
};
-module_platform_driver(bcm_ns_usb3_driver);
+
+static int __init bcm_ns_usb3_module_init(void)
+{
+ int err;
+
+ /*
+ * For backward compatibility we register as MDIO and platform driver.
+ * After getting MDIO binding commonly used (e.g. switching all DT files
+ * to use it) we should deprecate the old binding and eventually drop
+ * support for it.
+ */
+
+ err = mdio_driver_register(&bcm_ns_usb3_mdio_driver);
+ if (err)
+ return err;
+
+ err = platform_driver_register(&bcm_ns_usb3_driver);
+ if (err)
+ mdio_driver_unregister(&bcm_ns_usb3_mdio_driver);
+
+ return err;
+}
+module_init(bcm_ns_usb3_module_init);
+
+static void __exit bcm_ns_usb3_module_exit(void)
+{
+ platform_driver_unregister(&bcm_ns_usb3_driver);
+ mdio_driver_unregister(&bcm_ns_usb3_mdio_driver);
+}
+module_exit(bcm_ns_usb3_module_exit)

MODULE_LICENSE("GPL v2");
--
2.11.0

2017-06-14 15:26:35

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH V2 4/5] dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO

On Thu, Jun 08, 2017 at 10:04:27PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> Thanks to work done by Broadcom explaining their USB 3.0 PHY details we
> know it's attached to the MDIO bus. Use this knowledge to update the
> binding: make it a subnode to the MDIO bus and rework way of specifying
> required registers. This will describe hardware more precisely and will
> allow to support (describe) more devices attached to the MDIO.
>
> While compatibility strings remain the same there isn't a direct
> conflict (compatibility breakage) for the binding. Originally it wasn't
> supposed to be used for MDIO subnode so this change should be safe
> unless some operating system was probing MDIO subnodes as standalone
> devices.
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> V2: Extend commit message to better describe reason and includ info on
> (non-)breakage.
> ---
> .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt | 27 +++++++++++++++-------
> 1 file changed, 19 insertions(+), 8 deletions(-)

Acked-by: Rob Herring <[email protected]>

2017-06-16 06:37:54

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V2 0/5] phy: bcm-ns-usb3: add MDIO driver



On Friday 09 June 2017 01:34 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support
> for Broadcom NSP SoC"") this module should be modified to use MDIO bus as
> this is how PHY is really attached.
>
> This should allow reusing this driver on NSP and any other platform with
> MDIO bus and this particular PHY.

can you run checkpatch and fix all warnings in this series?

Thanks
Kishon
>
> V2: Rebase and update 4/5 and 5/5.
>
> Rafał Miłecki (5):
> phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg
> phy: bcm-ns-usb3: use pointer for PHY writing function
> phy: bcm-ns-usb3: enable MDIO in the platform specific code
> dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO
> phy: bcm-ns-usb3: add MDIO driver using proper bus layer
>
> .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt | 27 ++-
> drivers/phy/broadcom/Kconfig | 1 +
> drivers/phy/broadcom/phy-bcm-ns-usb3.c | 230 +++++++++++++++------
> 3 files changed, 186 insertions(+), 72 deletions(-)
>

2017-06-16 07:20:23

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH V2 0/5] phy: bcm-ns-usb3: add MDIO driver

On 06/16/2017 08:36 AM, Kishon Vijay Abraham I wrote:
> On Friday 09 June 2017 01:34 AM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <[email protected]>
>>
>> As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support
>> for Broadcom NSP SoC"") this module should be modified to use MDIO bus as
>> this is how PHY is really attached.
>>
>> This should allow reusing this driver on NSP and any other platform with
>> MDIO bus and this particular PHY.
>
> can you run checkpatch and fix all warnings in this series?

I always check my patches before sending. For this set the only warning I
get is:

WARNING: line over 80 characters
#117: FILE: drivers/phy/broadcom/phy-bcm-ns-usb3.c:224:
+ usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US));

I've problem finding a nice way of fixing this.
1) I can't break the line between arguments as there is only 1 argument
2) I shouldn't use small indention as it would misalign this line
3) Using tmp var for BCM_NS_USB3_MII_MNG_TIMEOUT_US sounds a bit pointless

According to the coding-style.rst having 80+ chars lines is acceptable if it
"significantly increases readability and does not hide information". Maybe
we can just live with this single line like that? Isn't this a bit bike
shedding in this case?

2017-06-16 07:52:51

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V2 0/5] phy: bcm-ns-usb3: add MDIO driver

Hi,

On Friday 16 June 2017 12:44 PM, Rafał Miłecki wrote:
> On 06/16/2017 08:36 AM, Kishon Vijay Abraham I wrote:
>> On Friday 09 June 2017 01:34 AM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support
>>> for Broadcom NSP SoC"") this module should be modified to use MDIO bus as
>>> this is how PHY is really attached.
>>>
>>> This should allow reusing this driver on NSP and any other platform with
>>> MDIO bus and this particular PHY.
>>
>> can you run checkpatch and fix all warnings in this series?
>
> I always check my patches before sending. For this set the only warning I
> get is:
>
> WARNING: line over 80 characters
> #117: FILE: drivers/phy/broadcom/phy-bcm-ns-usb3.c:224:
> +
> usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US));
>
> I've problem finding a nice way of fixing this.
> 1) I can't break the line between arguments as there is only 1 argument
> 2) I shouldn't use small indention as it would misalign this line
> 3) Using tmp var for BCM_NS_USB3_MII_MNG_TIMEOUT_US sounds a bit pointless
>
> According to the coding-style.rst having 80+ chars lines is acceptable if it
> "significantly increases readability and does not hide information". Maybe
> we can just live with this single line like that? Isn't this a bit bike
> shedding in this case?

Sure, just wanted to make sure we didn't overlook checkpatch warnings.

Thanks
Kishon

2017-06-16 07:54:33

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V2 0/5] phy: bcm-ns-usb3: add MDIO driver



On Friday 09 June 2017 01:34 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support
> for Broadcom NSP SoC"") this module should be modified to use MDIO bus as
> this is how PHY is really attached.
>
> This should allow reusing this driver on NSP and any other platform with
> MDIO bus and this particular PHY.
>
> V2: Rebase and update 4/5 and 5/5.

merged, thanks!

-Kishon
>
> Rafał Miłecki (5):
> phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg
> phy: bcm-ns-usb3: use pointer for PHY writing function
> phy: bcm-ns-usb3: enable MDIO in the platform specific code
> dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO
> phy: bcm-ns-usb3: add MDIO driver using proper bus layer
>
> .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt | 27 ++-
> drivers/phy/broadcom/Kconfig | 1 +
> drivers/phy/broadcom/phy-bcm-ns-usb3.c | 230 +++++++++++++++------
> 3 files changed, 186 insertions(+), 72 deletions(-)
>