All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:30 ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:30 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Kishon Vijay Abraham I
  Cc: Florian Fainelli, Rob Herring, Mark Rutland, Ray Jui,
	Scott Branden, Jon Mason,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	Yendapally Reddy Dhananjaya Reddy,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
driver for USB 3.0 PHY on Northstar").

Instead of adding separated driver & duplicating code we should work on
improving existing (old) one. Thanks to work done by Broadcom we know
there is MDIO bus we weren't aware of & we know register names which
makes initialization more clear. This is very valuable info and we
should work on using it in existing driver afterwards.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 drivers/phy/Kconfig            |   8 --
 drivers/phy/Makefile           |   1 -
 drivers/phy/phy-bcm-nsp-usb3.c | 177 -----------------------------------------
 3 files changed, 186 deletions(-)
 delete mode 100644 drivers/phy/phy-bcm-nsp-usb3.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index bb5cf6f49b06..c49a914d6409 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -502,12 +502,4 @@ config PHY_MESON8B_USB2
 	  and GXBB SoCs.
 	  If unsure, say N.
 
-config PHY_NSP_USB3
-	tristate "Broadcom NorthStar plus USB3 PHY driver"
-	depends on OF && (ARCH_BCM_NSP || COMPILE_TEST)
-	select GENERIC_PHY
-	default ARCH_BCM_NSP
-	help
-	  Enable this to support the Broadcom Northstar plus USB3 PHY.
-	  If unsure, say N.
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9f008004f75d..0e4259473d28 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -61,4 +61,3 @@ obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-bcm-cygnus-pcie.o
 obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_PHY_NS2_PCIE)		+= phy-bcm-ns2-pcie.o
 obj-$(CONFIG_PHY_MESON8B_USB2)		+= phy-meson8b-usb2.o
-obj-$(CONFIG_PHY_NSP_USB3)		+= phy-bcm-nsp-usb3.o
diff --git a/drivers/phy/phy-bcm-nsp-usb3.c b/drivers/phy/phy-bcm-nsp-usb3.c
deleted file mode 100644
index 49024eaa5545..000000000000
--- a/drivers/phy/phy-bcm-nsp-usb3.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- * Copyright (C) 2016 Broadcom
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <linux/delay.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/mfd/syscon.h>
-#include <linux/mdio.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/phy/phy.h>
-#include <linux/regmap.h>
-
-#define NSP_USB3_RST_CTRL_OFFSET	0x3f8
-
-/* mdio reg access */
-#define NSP_USB3_PHY_BASE_ADDR_REG	0x1f
-
-#define NSP_USB3_PHY_PLL30_BLOCK	0x8000
-#define NSP_USB3_PLL_CONTROL		0x01
-#define NSP_USB3_PLLA_CONTROL0		0x0a
-#define NSP_USB3_PLLA_CONTROL1		0x0b
-
-#define NSP_USB3_PHY_TX_PMD_BLOCK	0x8040
-#define NSP_USB3_TX_PMD_CONTROL1	0x01
-
-#define NSP_USB3_PHY_PIPE_BLOCK		0x8060
-#define NSP_USB3_LFPS_CMP		0x02
-#define NSP_USB3_LFPS_DEGLITCH		0x03
-
-struct nsp_usb3_phy {
-	struct regmap *usb3_ctrl;
-	struct phy *phy;
-	struct mdio_device *mdiodev;
-};
-
-static int nsp_usb3_phy_init(struct phy *phy)
-{
-	struct nsp_usb3_phy *iphy = phy_get_drvdata(phy);
-	struct mii_bus *bus = iphy->mdiodev->bus;
-	int addr = iphy->mdiodev->addr;
-	u32 data;
-	int rc;
-
-	rc = regmap_read(iphy->usb3_ctrl, 0, &data);
-	if (rc)
-		return rc;
-	data |= 1;
-	rc = regmap_write(iphy->usb3_ctrl, 0, data);
-	if (rc)
-		return rc;
-
-	rc = regmap_write(iphy->usb3_ctrl, NSP_USB3_RST_CTRL_OFFSET, 1);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_PLL30_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLL_CONTROL, 0x1000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL0, 0x6400);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL1, 0xc000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL1, 0x8000);
-	if (rc)
-		return rc;
-
-	rc = regmap_write(iphy->usb3_ctrl, NSP_USB3_RST_CTRL_OFFSET, 0);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLL_CONTROL, 0x9000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_PIPE_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_LFPS_CMP, 0xf30d);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_LFPS_DEGLITCH, 0x6302);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_TX_PMD_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_TX_PMD_CONTROL1, 0x1003);
-
-	return rc;
-}
-
-static struct phy_ops nsp_usb3_phy_ops = {
-	.init	= nsp_usb3_phy_init,
-	.owner	= THIS_MODULE,
-};
-
-static int nsp_usb3_phy_probe(struct mdio_device *mdiodev)
-{
-	struct device *dev = &mdiodev->dev;
-	struct phy_provider *provider;
-	struct nsp_usb3_phy *iphy;
-
-	iphy = devm_kzalloc(dev, sizeof(*iphy), GFP_KERNEL);
-	if (!iphy)
-		return -ENOMEM;
-	iphy->mdiodev = mdiodev;
-
-	iphy->usb3_ctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
-						 "usb3-ctrl-syscon");
-	if (IS_ERR(iphy->usb3_ctrl))
-		return PTR_ERR(iphy->usb3_ctrl);
-
-	iphy->phy = devm_phy_create(dev, dev->of_node, &nsp_usb3_phy_ops);
-	if (IS_ERR(iphy->phy)) {
-		dev_err(dev, "failed to create PHY\n");
-		return PTR_ERR(iphy->phy);
-	}
-
-	phy_set_drvdata(iphy->phy, iphy);
-
-	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	if (IS_ERR(provider)) {
-		dev_err(dev, "could not register PHY provider\n");
-		return PTR_ERR(provider);
-	}
-
-	return 0;
-}
-
-static const struct of_device_id nsp_usb3_phy_of_match[] = {
-	{.compatible = "brcm,nsp-usb3-phy",},
-	{ /* sentinel */ }
-};
-
-static struct mdio_driver nsp_usb3_phy_driver = {
-	.mdiodrv = {
-		.driver = {
-			.name = "nsp-usb3-phy",
-			.of_match_table = nsp_usb3_phy_of_match,
-		},
-	},
-	.probe = nsp_usb3_phy_probe,
-};
-
-mdio_module_driver(nsp_usb3_phy_driver);
-
-MODULE_DESCRIPTION("Broadcom NSP USB3 PHY driver");
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Yendapally Reddy Dhananjaya Reddy <yendapally.reddy-dY08KVG/lbpWk0Htik3J/w@public.gmane.org");
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:30 ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:30 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
driver for USB 3.0 PHY on Northstar").

Instead of adding separated driver & duplicating code we should work on
improving existing (old) one. Thanks to work done by Broadcom we know
there is MDIO bus we weren't aware of & we know register names which
makes initialization more clear. This is very valuable info and we
should work on using it in existing driver afterwards.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
 drivers/phy/Kconfig            |   8 --
 drivers/phy/Makefile           |   1 -
 drivers/phy/phy-bcm-nsp-usb3.c | 177 -----------------------------------------
 3 files changed, 186 deletions(-)
 delete mode 100644 drivers/phy/phy-bcm-nsp-usb3.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index bb5cf6f49b06..c49a914d6409 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -502,12 +502,4 @@ config PHY_MESON8B_USB2
 	  and GXBB SoCs.
 	  If unsure, say N.
 
-config PHY_NSP_USB3
-	tristate "Broadcom NorthStar plus USB3 PHY driver"
-	depends on OF && (ARCH_BCM_NSP || COMPILE_TEST)
-	select GENERIC_PHY
-	default ARCH_BCM_NSP
-	help
-	  Enable this to support the Broadcom Northstar plus USB3 PHY.
-	  If unsure, say N.
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9f008004f75d..0e4259473d28 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -61,4 +61,3 @@ obj-$(CONFIG_PHY_CYGNUS_PCIE)		+= phy-bcm-cygnus-pcie.o
 obj-$(CONFIG_ARCH_TEGRA) += tegra/
 obj-$(CONFIG_PHY_NS2_PCIE)		+= phy-bcm-ns2-pcie.o
 obj-$(CONFIG_PHY_MESON8B_USB2)		+= phy-meson8b-usb2.o
-obj-$(CONFIG_PHY_NSP_USB3)		+= phy-bcm-nsp-usb3.o
diff --git a/drivers/phy/phy-bcm-nsp-usb3.c b/drivers/phy/phy-bcm-nsp-usb3.c
deleted file mode 100644
index 49024eaa5545..000000000000
--- a/drivers/phy/phy-bcm-nsp-usb3.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- * Copyright (C) 2016 Broadcom
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation version 2.
- *
- * This program is distributed "as is" WITHOUT ANY WARRANTY of any
- * kind, whether express or implied; without even the implied warranty
- * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
-
-#include <linux/delay.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/mfd/syscon.h>
-#include <linux/mdio.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/phy/phy.h>
-#include <linux/regmap.h>
-
-#define NSP_USB3_RST_CTRL_OFFSET	0x3f8
-
-/* mdio reg access */
-#define NSP_USB3_PHY_BASE_ADDR_REG	0x1f
-
-#define NSP_USB3_PHY_PLL30_BLOCK	0x8000
-#define NSP_USB3_PLL_CONTROL		0x01
-#define NSP_USB3_PLLA_CONTROL0		0x0a
-#define NSP_USB3_PLLA_CONTROL1		0x0b
-
-#define NSP_USB3_PHY_TX_PMD_BLOCK	0x8040
-#define NSP_USB3_TX_PMD_CONTROL1	0x01
-
-#define NSP_USB3_PHY_PIPE_BLOCK		0x8060
-#define NSP_USB3_LFPS_CMP		0x02
-#define NSP_USB3_LFPS_DEGLITCH		0x03
-
-struct nsp_usb3_phy {
-	struct regmap *usb3_ctrl;
-	struct phy *phy;
-	struct mdio_device *mdiodev;
-};
-
-static int nsp_usb3_phy_init(struct phy *phy)
-{
-	struct nsp_usb3_phy *iphy = phy_get_drvdata(phy);
-	struct mii_bus *bus = iphy->mdiodev->bus;
-	int addr = iphy->mdiodev->addr;
-	u32 data;
-	int rc;
-
-	rc = regmap_read(iphy->usb3_ctrl, 0, &data);
-	if (rc)
-		return rc;
-	data |= 1;
-	rc = regmap_write(iphy->usb3_ctrl, 0, data);
-	if (rc)
-		return rc;
-
-	rc = regmap_write(iphy->usb3_ctrl, NSP_USB3_RST_CTRL_OFFSET, 1);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_PLL30_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLL_CONTROL, 0x1000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL0, 0x6400);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL1, 0xc000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLLA_CONTROL1, 0x8000);
-	if (rc)
-		return rc;
-
-	rc = regmap_write(iphy->usb3_ctrl, NSP_USB3_RST_CTRL_OFFSET, 0);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PLL_CONTROL, 0x9000);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_PIPE_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_LFPS_CMP, 0xf30d);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_LFPS_DEGLITCH, 0x6302);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_PHY_BASE_ADDR_REG,
-			   NSP_USB3_PHY_TX_PMD_BLOCK);
-	if (rc)
-		return rc;
-
-	rc = mdiobus_write(bus, addr, NSP_USB3_TX_PMD_CONTROL1, 0x1003);
-
-	return rc;
-}
-
-static struct phy_ops nsp_usb3_phy_ops = {
-	.init	= nsp_usb3_phy_init,
-	.owner	= THIS_MODULE,
-};
-
-static int nsp_usb3_phy_probe(struct mdio_device *mdiodev)
-{
-	struct device *dev = &mdiodev->dev;
-	struct phy_provider *provider;
-	struct nsp_usb3_phy *iphy;
-
-	iphy = devm_kzalloc(dev, sizeof(*iphy), GFP_KERNEL);
-	if (!iphy)
-		return -ENOMEM;
-	iphy->mdiodev = mdiodev;
-
-	iphy->usb3_ctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
-						 "usb3-ctrl-syscon");
-	if (IS_ERR(iphy->usb3_ctrl))
-		return PTR_ERR(iphy->usb3_ctrl);
-
-	iphy->phy = devm_phy_create(dev, dev->of_node, &nsp_usb3_phy_ops);
-	if (IS_ERR(iphy->phy)) {
-		dev_err(dev, "failed to create PHY\n");
-		return PTR_ERR(iphy->phy);
-	}
-
-	phy_set_drvdata(iphy->phy, iphy);
-
-	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
-	if (IS_ERR(provider)) {
-		dev_err(dev, "could not register PHY provider\n");
-		return PTR_ERR(provider);
-	}
-
-	return 0;
-}
-
-static const struct of_device_id nsp_usb3_phy_of_match[] = {
-	{.compatible = "brcm,nsp-usb3-phy",},
-	{ /* sentinel */ }
-};
-
-static struct mdio_driver nsp_usb3_phy_driver = {
-	.mdiodrv = {
-		.driver = {
-			.name = "nsp-usb3-phy",
-			.of_match_table = nsp_usb3_phy_of_match,
-		},
-	},
-	.probe = nsp_usb3_phy_probe,
-};
-
-mdio_module_driver(nsp_usb3_phy_driver);
-
-MODULE_DESCRIPTION("Broadcom NSP USB3 PHY driver");
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Yendapally Reddy Dhananjaya Reddy <yendapally.reddy@broadcom.com");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
  2017-02-08 23:30 ` Rafał Miłecki
@ 2017-02-08 23:30     ` Rafał Miłecki
  -1 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:30 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Kishon Vijay Abraham I
  Cc: Florian Fainelli, Rob Herring, Mark Rutland, Ray Jui,
	Scott Branden, Jon Mason,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	Yendapally Reddy Dhananjaya Reddy,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki

From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
Please note we revert this commit before it reached stable release.

If new compatible string is needed it should be added to the existing
bcm-ns-usb3-phy.txt which already describes this PHY.

Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
---
 .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
 1 file changed, 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
deleted file mode 100644
index e68ae5dec9c9..000000000000
--- a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-Broadcom USB3 phy binding for northstar plus SoC
-The USB3 phy is internal to the SoC and is accessed using mdio interface.
-
-Required mdio bus properties:
-- reg: Should be 0x0 for SoC internal USB3 phy
-- #address-cells: must be 1
-- #size-cells: must be 0
-
-Required USB3 PHY properties:
-- compatible: should be "brcm,nsp-usb3-phy"
-- reg: USB3 Phy address on SoC internal MDIO bus and it should be 0x10.
-- usb3-ctrl-syscon: handler of syscon node defining physical address
-  of usb3 control register.
-- #phy-cells: must be 0
-
-Required usb3 control properties:
-- compatible: should be "brcm,nsp-usb3-ctrl"
-- reg: offset and length of the control registers
-
-Example:
-
-	mdio@0 {
-		reg = <0x0>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		usb3_phy: usb-phy@10 {
-			compatible = "brcm,nsp-usb3-phy";
-			reg = <0x10>;
-			usb3-ctrl-syscon = <&usb3_ctrl>;
-			#phy-cells = <0>;
-			status = "disabled";
-		};
-	};
-
-	usb3_ctrl: syscon@104408 {
-		compatible = "brcm,nsp-usb3-ctrl", "syscon";
-		reg = <0x104408 0x3fc>;
-	};
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
@ 2017-02-08 23:30     ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:30 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
Please note we revert this commit before it reached stable release.

If new compatible string is needed it should be added to the existing
bcm-ns-usb3-phy.txt which already describes this PHY.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
 .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
 1 file changed, 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
deleted file mode 100644
index e68ae5dec9c9..000000000000
--- a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
+++ /dev/null
@@ -1,39 +0,0 @@
-Broadcom USB3 phy binding for northstar plus SoC
-The USB3 phy is internal to the SoC and is accessed using mdio interface.
-
-Required mdio bus properties:
-- reg: Should be 0x0 for SoC internal USB3 phy
-- #address-cells: must be 1
-- #size-cells: must be 0
-
-Required USB3 PHY properties:
-- compatible: should be "brcm,nsp-usb3-phy"
-- reg: USB3 Phy address on SoC internal MDIO bus and it should be 0x10.
-- usb3-ctrl-syscon: handler of syscon node defining physical address
-  of usb3 control register.
-- #phy-cells: must be 0
-
-Required usb3 control properties:
-- compatible: should be "brcm,nsp-usb3-ctrl"
-- reg: offset and length of the control registers
-
-Example:
-
-	mdio at 0 {
-		reg = <0x0>;
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		usb3_phy: usb-phy at 10 {
-			compatible = "brcm,nsp-usb3-phy";
-			reg = <0x10>;
-			usb3-ctrl-syscon = <&usb3_ctrl>;
-			#phy-cells = <0>;
-			status = "disabled";
-		};
-	};
-
-	usb3_ctrl: syscon at 104408 {
-		compatible = "brcm,nsp-usb3-ctrl", "syscon";
-		reg = <0x104408 0x3fc>;
-	};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-08 23:30 ` Rafał Miłecki
@ 2017-02-08 23:32   ` Florian Fainelli
  -1 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-08 23:32 UTC (permalink / raw)
  To: Rafał Miłecki, linux-arm-kernel, Kishon Vijay Abraham I
  Cc: Mark Rutland, devicetree, Scott Branden, Jon Mason, Ray Jui,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	bcm-kernel-feedback-list, Rafał Miłecki

On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
> driver for USB 3.0 PHY on Northstar").
> 
> Instead of adding separated driver & duplicating code we should work on
> improving existing (old) one. Thanks to work done by Broadcom we know
> there is MDIO bus we weren't aware of & we know register names which
> makes initialization more clear. This is very valuable info and we
> should work on using it in existing driver afterwards.

Should not we first extend the old driver to support NSP and then revert
d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
-- 
Florian

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:32   ` Florian Fainelli
  0 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-08 23:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
> driver for USB 3.0 PHY on Northstar").
> 
> Instead of adding separated driver & duplicating code we should work on
> improving existing (old) one. Thanks to work done by Broadcom we know
> there is MDIO bus we weren't aware of & we know register names which
> makes initialization more clear. This is very valuable info and we
> should work on using it in existing driver afterwards.

Should not we first extend the old driver to support NSP and then revert
d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
-- 
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-08 23:32   ` Florian Fainelli
@ 2017-02-08 23:36       ` Jon Mason
  -1 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-08 23:36 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, linux-arm-kernel,
	Kishon Vijay Abraham I, Rob Herring, Mark Rutland, Ray Jui,
	Scott Branden, Jon Mason, BCM Kernel Feedback,
	Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rafał Miłecki

On Wed, Feb 8, 2017 at 6:32 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>
>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>> driver for USB 3.0 PHY on Northstar").
>>
>> Instead of adding separated driver & duplicating code we should work on
>> improving existing (old) one. Thanks to work done by Broadcom we know
>> there is MDIO bus we weren't aware of & we know register names which
>> makes initialization more clear. This is very valuable info and we
>> should work on using it in existing driver afterwards.
>
> Should not we first extend the old driver to support NSP and then revert
> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?

I am currently in the process of doing this, but I'm guessing that I
am 1-2 weeks away from having it completed.  It might not hit the
merge window.

Thanks,
Jon

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:36       ` Jon Mason
  0 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-08 23:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 8, 2017 at 6:32 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>> driver for USB 3.0 PHY on Northstar").
>>
>> Instead of adding separated driver & duplicating code we should work on
>> improving existing (old) one. Thanks to work done by Broadcom we know
>> there is MDIO bus we weren't aware of & we know register names which
>> makes initialization more clear. This is very valuable info and we
>> should work on using it in existing driver afterwards.
>
> Should not we first extend the old driver to support NSP and then revert
> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?

I am currently in the process of doing this, but I'm guessing that I
am 1-2 weeks away from having it completed.  It might not hit the
merge window.

Thanks,
Jon

> --
> Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-08 23:32   ` Florian Fainelli
@ 2017-02-08 23:39     ` Rafał Miłecki
  -1 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:39 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, devicetree, Scott Branden, Jon Mason, Ray Jui,
	Rafał Miłecki, Kishon Vijay Abraham I,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	bcm-kernel-feedback-list, linux-arm-kernel

On 2017-02-09 00:32, Florian Fainelli wrote:
> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>> 
>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> Broadcom NSP SoC") as we already have driver for this PHY (shared by 
>> NS
>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>> driver for USB 3.0 PHY on Northstar").
>> 
>> Instead of adding separated driver & duplicating code we should work 
>> on
>> improving existing (old) one. Thanks to work done by Broadcom we know
>> there is MDIO bus we weren't aware of & we know register names which
>> makes initialization more clear. This is very valuable info and we
>> should work on using it in existing driver afterwards.
> 
> Should not we first extend the old driver to support NSP and then 
> revert
> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?

Sounds like a weird / dirty development method to me: adding duplicated 
code
first then working on cleaning it. Unless you mean drivers/staging/.

We also should never drop support for supported DT bindings, so I really 
think
it's the cleanest to revert it now.

I think Jon was also OK with this solution.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:39     ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-08 23:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 2017-02-09 00:32, Florian Fainelli wrote:
> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>> 
>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> Broadcom NSP SoC") as we already have driver for this PHY (shared by 
>> NS
>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>> driver for USB 3.0 PHY on Northstar").
>> 
>> Instead of adding separated driver & duplicating code we should work 
>> on
>> improving existing (old) one. Thanks to work done by Broadcom we know
>> there is MDIO bus we weren't aware of & we know register names which
>> makes initialization more clear. This is very valuable info and we
>> should work on using it in existing driver afterwards.
> 
> Should not we first extend the old driver to support NSP and then 
> revert
> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?

Sounds like a weird / dirty development method to me: adding duplicated 
code
first then working on cleaning it. Unless you mean drivers/staging/.

We also should never drop support for supported DT bindings, so I really 
think
it's the cleanest to revert it now.

I think Jon was also OK with this solution.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-08 23:39     ` Rafał Miłecki
@ 2017-02-08 23:44       ` Florian Fainelli
  -1 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-08 23:44 UTC (permalink / raw)
  To: Rafał Miłecki, Florian Fainelli
  Cc: Mark Rutland, devicetree, Scott Branden, Jon Mason, Ray Jui,
	Rafał Miłecki, Kishon Vijay Abraham I,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	bcm-kernel-feedback-list, linux-arm-kernel

On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
> On 2017-02-09 00:32, Florian Fainelli wrote:
>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>> driver for USB 3.0 PHY on Northstar").
>>>
>>> Instead of adding separated driver & duplicating code we should work on
>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>> there is MDIO bus we weren't aware of & we know register names which
>>> makes initialization more clear. This is very valuable info and we
>>> should work on using it in existing driver afterwards.
>>
>> Should not we first extend the old driver to support NSP and then revert
>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
> 
> Sounds like a weird / dirty development method to me: adding duplicated
> code
> first then working on cleaning it. Unless you mean drivers/staging/.

There was clearly a mistake in submitting this NSP USB PHY driver, and
it should have been a patch against the existing NS USB PHY driver, but
it was not, okay fair enough.

It's one thing to address that in the future, and it's another thing to
flat out revert the driver just because you don't like the duplication.

I don't like that either, and we can discuss on how to improve things
(like have the maintainer review that too), but duplication is a lesser
evil than not having the hardware supported at all, and even more so,
purposely reverting in the name of removing that duplication, that's
intentionally breaking working hardware!

> 
> We also should never drop support for supported DT bindings, so I really
> think
> it's the cleanest to revert it now.

The binding document seems fine, and does not preclude adding NSP
support to the existing PHY driver. Binding ain't driver code, the
binding could stay and is certainly a more accurate description of the
HW on NSP.

> 
> I think Jon was also OK with this solution.

-- 
Florian

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-08 23:44       ` Florian Fainelli
  0 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-08 23:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
> On 2017-02-09 00:32, Florian Fainelli wrote:
>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>
>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>> Broadcom NSP SoC") as we already have driver for this PHY (shared by NS
>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>> driver for USB 3.0 PHY on Northstar").
>>>
>>> Instead of adding separated driver & duplicating code we should work on
>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>> there is MDIO bus we weren't aware of & we know register names which
>>> makes initialization more clear. This is very valuable info and we
>>> should work on using it in existing driver afterwards.
>>
>> Should not we first extend the old driver to support NSP and then revert
>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
> 
> Sounds like a weird / dirty development method to me: adding duplicated
> code
> first then working on cleaning it. Unless you mean drivers/staging/.

There was clearly a mistake in submitting this NSP USB PHY driver, and
it should have been a patch against the existing NS USB PHY driver, but
it was not, okay fair enough.

It's one thing to address that in the future, and it's another thing to
flat out revert the driver just because you don't like the duplication.

I don't like that either, and we can discuss on how to improve things
(like have the maintainer review that too), but duplication is a lesser
evil than not having the hardware supported at all, and even more so,
purposely reverting in the name of removing that duplication, that's
intentionally breaking working hardware!

> 
> We also should never drop support for supported DT bindings, so I really
> think
> it's the cleanest to revert it now.

The binding document seems fine, and does not preclude adding NSP
support to the existing PHY driver. Binding ain't driver code, the
binding could stay and is certainly a more accurate description of the
HW on NSP.

> 
> I think Jon was also OK with this solution.

-- 
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-08 23:44       ` Florian Fainelli
@ 2017-02-09  7:21         ` Rafał Miłecki
  -1 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-09  7:21 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Mark Rutland, devicetree, Scott Branden, Jon Mason, Ray Jui,
	Rafał Miłecki, Kishon Vijay Abraham I,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	bcm-kernel-feedback-list, linux-arm-kernel

On 2017-02-09 00:44, Florian Fainelli wrote:
> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>> 
>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared by 
>>>> NS
>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: 
>>>> new
>>>> driver for USB 3.0 PHY on Northstar").
>>>> 
>>>> Instead of adding separated driver & duplicating code we should work 
>>>> on
>>>> improving existing (old) one. Thanks to work done by Broadcom we 
>>>> know
>>>> there is MDIO bus we weren't aware of & we know register names which
>>>> makes initialization more clear. This is very valuable info and we
>>>> should work on using it in existing driver afterwards.
>>> 
>>> Should not we first extend the old driver to support NSP and then 
>>> revert
>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>> 
>> Sounds like a weird / dirty development method to me: adding 
>> duplicated
>> code
>> first then working on cleaning it. Unless you mean drivers/staging/.
> 
> There was clearly a mistake in submitting this NSP USB PHY driver, and
> it should have been a patch against the existing NS USB PHY driver, but
> it was not, okay fair enough.
> 
> It's one thing to address that in the future, and it's another thing to
> flat out revert the driver just because you don't like the duplication.
> 
> I don't like that either, and we can discuss on how to improve things
> (like have the maintainer review that too), but duplication is a lesser
> evil than not having the hardware supported at all, and even more so,
> purposely reverting in the name of removing that duplication, that's
> intentionally breaking working hardware!

Hardware support is not excuse and I don't think it ever was in the 
Linux.

We don't accept badly designed drivers just because they provide new hw 
support.
We have various standards (for quality, style, design, code) at kernel 
and we
stick to them unless it's drivers/staging/. As you said this driver 
shouldn't be
pushed in the first place.

Dropping hardware support in kernel happens. Sometimes it's about 
ancient
devices, sometimes about code quality (some forgotten staging drivers 
used to be
dropped AFAIK).

Additionally you're talking about support that was *just* added and 
isn't used
by anyone in the wild world yet.

This hardware was missing upstream support for 4 years so 2 extra months 
won't
really hurt anyone.

I really don't see excusee or need for keeping this driver.

If you want to (and you feel it's well designed), we can keep
brcm,nsp-usb3-phy.txt

I vote for focusing on existing driver improvements instead of looking 
for
excuses for keeping driver that shouldn't be added in the first place.
Jon seems to be already working on this, I'm willing to help him, I'm 
sure we
can get you a proper support for the next merge window.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-09  7:21         ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-02-09  7:21 UTC (permalink / raw)
  To: linux-arm-kernel

On 2017-02-09 00:44, Florian Fainelli wrote:
> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>> 
>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared by 
>>>> NS
>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: 
>>>> new
>>>> driver for USB 3.0 PHY on Northstar").
>>>> 
>>>> Instead of adding separated driver & duplicating code we should work 
>>>> on
>>>> improving existing (old) one. Thanks to work done by Broadcom we 
>>>> know
>>>> there is MDIO bus we weren't aware of & we know register names which
>>>> makes initialization more clear. This is very valuable info and we
>>>> should work on using it in existing driver afterwards.
>>> 
>>> Should not we first extend the old driver to support NSP and then 
>>> revert
>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>> 
>> Sounds like a weird / dirty development method to me: adding 
>> duplicated
>> code
>> first then working on cleaning it. Unless you mean drivers/staging/.
> 
> There was clearly a mistake in submitting this NSP USB PHY driver, and
> it should have been a patch against the existing NS USB PHY driver, but
> it was not, okay fair enough.
> 
> It's one thing to address that in the future, and it's another thing to
> flat out revert the driver just because you don't like the duplication.
> 
> I don't like that either, and we can discuss on how to improve things
> (like have the maintainer review that too), but duplication is a lesser
> evil than not having the hardware supported at all, and even more so,
> purposely reverting in the name of removing that duplication, that's
> intentionally breaking working hardware!

Hardware support is not excuse and I don't think it ever was in the 
Linux.

We don't accept badly designed drivers just because they provide new hw 
support.
We have various standards (for quality, style, design, code) at kernel 
and we
stick to them unless it's drivers/staging/. As you said this driver 
shouldn't be
pushed in the first place.

Dropping hardware support in kernel happens. Sometimes it's about 
ancient
devices, sometimes about code quality (some forgotten staging drivers 
used to be
dropped AFAIK).

Additionally you're talking about support that was *just* added and 
isn't used
by anyone in the wild world yet.

This hardware was missing upstream support for 4 years so 2 extra months 
won't
really hurt anyone.

I really don't see excusee or need for keeping this driver.

If you want to (and you feel it's well designed), we can keep
brcm,nsp-usb3-phy.txt

I vote for focusing on existing driver improvements instead of looking 
for
excuses for keeping driver that shouldn't be added in the first place.
Jon seems to be already working on this, I'm willing to help him, I'm 
sure we
can get you a proper support for the next merge window.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-09  7:21         ` Rafał Miłecki
@ 2017-02-09 19:18           ` Florian Fainelli
  -1 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-09 19:18 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Mark Rutland, devicetree, Scott Branden, Jon Mason, Ray Jui,
	Rafał Miłecki, Kishon Vijay Abraham I,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	bcm-kernel-feedback-list, linux-arm-kernel

On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
> On 2017-02-09 00:44, Florian Fainelli wrote:
>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>
>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>> by NS
>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>
>>>>> Instead of adding separated driver & duplicating code we should
>>>>> work on
>>>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>> makes initialization more clear. This is very valuable info and we
>>>>> should work on using it in existing driver afterwards.
>>>>
>>>> Should not we first extend the old driver to support NSP and then
>>>> revert
>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>
>>> Sounds like a weird / dirty development method to me: adding duplicated
>>> code
>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>
>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>> it should have been a patch against the existing NS USB PHY driver, but
>> it was not, okay fair enough.
>>
>> It's one thing to address that in the future, and it's another thing to
>> flat out revert the driver just because you don't like the duplication.
>>
>> I don't like that either, and we can discuss on how to improve things
>> (like have the maintainer review that too), but duplication is a lesser
>> evil than not having the hardware supported at all, and even more so,
>> purposely reverting in the name of removing that duplication, that's
>> intentionally breaking working hardware!
> 
> Hardware support is not excuse and I don't think it ever was in the Linux.
> 
> We don't accept badly designed drivers just because they provide new hw
> support.
> We have various standards (for quality, style, design, code) at kernel
> and we
> stick to them unless it's drivers/staging/. As you said this driver
> shouldn't be
> pushed in the first place.
> 
> Dropping hardware support in kernel happens. Sometimes it's about ancient
> devices, sometimes about code quality (some forgotten staging drivers
> used to be
> dropped AFAIK).
> 
> Additionally you're talking about support that was *just* added and
> isn't used
> by anyone in the wild world yet.

Except people working on it at Broadcom, but fair enough.

> 
> This hardware was missing upstream support for 4 years so 2 extra months
> won't
> really hurt anyone.
> 
> I really don't see excusee or need for keeping this driver.
> 
> If you want to (and you feel it's well designed), we can keep
> brcm,nsp-usb3-phy.txt

No it's fine, let's drop it all and replace it with whatever you and Jon
come up with next.

> 
> I vote for focusing on existing driver improvements instead of looking for
> excuses for keeping driver that shouldn't be added in the first place.
> Jon seems to be already working on this, I'm willing to help him, I'm
> sure we
> can get you a proper support for the next merge window.

Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
for Northstar Plus") from devicetree/next so you and Jon can figure out
what is the best thing to move forward and we minimize the amount of
incompatible DT stuff to be sorted out later on. So as far as I am
concerned, there are no board/SoC DTS changes to be patched later on, we
could re-apply this patch as-is, or we could have to define a new binding.
-- 
Florian

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-09 19:18           ` Florian Fainelli
  0 siblings, 0 replies; 32+ messages in thread
From: Florian Fainelli @ 2017-02-09 19:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
> On 2017-02-09 00:44, Florian Fainelli wrote:
>> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>
>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>> by NS
>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>
>>>>> Instead of adding separated driver & duplicating code we should
>>>>> work on
>>>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>> makes initialization more clear. This is very valuable info and we
>>>>> should work on using it in existing driver afterwards.
>>>>
>>>> Should not we first extend the old driver to support NSP and then
>>>> revert
>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>
>>> Sounds like a weird / dirty development method to me: adding duplicated
>>> code
>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>
>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>> it should have been a patch against the existing NS USB PHY driver, but
>> it was not, okay fair enough.
>>
>> It's one thing to address that in the future, and it's another thing to
>> flat out revert the driver just because you don't like the duplication.
>>
>> I don't like that either, and we can discuss on how to improve things
>> (like have the maintainer review that too), but duplication is a lesser
>> evil than not having the hardware supported at all, and even more so,
>> purposely reverting in the name of removing that duplication, that's
>> intentionally breaking working hardware!
> 
> Hardware support is not excuse and I don't think it ever was in the Linux.
> 
> We don't accept badly designed drivers just because they provide new hw
> support.
> We have various standards (for quality, style, design, code) at kernel
> and we
> stick to them unless it's drivers/staging/. As you said this driver
> shouldn't be
> pushed in the first place.
> 
> Dropping hardware support in kernel happens. Sometimes it's about ancient
> devices, sometimes about code quality (some forgotten staging drivers
> used to be
> dropped AFAIK).
> 
> Additionally you're talking about support that was *just* added and
> isn't used
> by anyone in the wild world yet.

Except people working on it at Broadcom, but fair enough.

> 
> This hardware was missing upstream support for 4 years so 2 extra months
> won't
> really hurt anyone.
> 
> I really don't see excusee or need for keeping this driver.
> 
> If you want to (and you feel it's well designed), we can keep
> brcm,nsp-usb3-phy.txt

No it's fine, let's drop it all and replace it with whatever you and Jon
come up with next.

> 
> I vote for focusing on existing driver improvements instead of looking for
> excuses for keeping driver that shouldn't be added in the first place.
> Jon seems to be already working on this, I'm willing to help him, I'm
> sure we
> can get you a proper support for the next merge window.

Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
for Northstar Plus") from devicetree/next so you and Jon can figure out
what is the best thing to move forward and we minimize the amount of
incompatible DT stuff to be sorted out later on. So as far as I am
concerned, there are no board/SoC DTS changes to be patched later on, we
could re-apply this patch as-is, or we could have to define a new binding.
-- 
Florian

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
  2017-02-08 23:30     ` Rafał Miłecki
  (?)
@ 2017-02-10  0:27     ` Jon Mason
  -1 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:27 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-arm-kernel, Kishon Vijay Abraham I, Florian Fainelli,
	Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rafał Miłecki

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

On Wed, Feb 8, 2017 at 6:30 PM, Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
> for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
> Please note we revert this commit before it reached stable release.
>
> If new compatible string is needed it should be added to the existing
> bcm-ns-usb3-phy.txt which already describes this PHY.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason@broadcom.com>


> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39
> ----------------------
>  1 file changed, 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-
> phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> deleted file mode 100644
> index e68ae5dec9c9..000000000000
> --- a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -Broadcom USB3 phy binding for northstar plus SoC
> -The USB3 phy is internal to the SoC and is accessed using mdio interface.
> -
> -Required mdio bus properties:
> -- reg: Should be 0x0 for SoC internal USB3 phy
> -- #address-cells: must be 1
> -- #size-cells: must be 0
> -
> -Required USB3 PHY properties:
> -- compatible: should be "brcm,nsp-usb3-phy"
> -- reg: USB3 Phy address on SoC internal MDIO bus and it should be 0x10.
> -- usb3-ctrl-syscon: handler of syscon node defining physical address
> -  of usb3 control register.
> -- #phy-cells: must be 0
> -
> -Required usb3 control properties:
> -- compatible: should be "brcm,nsp-usb3-ctrl"
> -- reg: offset and length of the control registers
> -
> -Example:
> -
> -       mdio@0 {
> -               reg = <0x0>;
> -               #address-cells = <1>;
> -               #size-cells = <0>;
> -
> -               usb3_phy: usb-phy@10 {
> -                       compatible = "brcm,nsp-usb3-phy";
> -                       reg = <0x10>;
> -                       usb3-ctrl-syscon = <&usb3_ctrl>;
> -                       #phy-cells = <0>;
> -                       status = "disabled";
> -               };
> -       };
> -
> -       usb3_ctrl: syscon@104408 {
> -               compatible = "brcm,nsp-usb3-ctrl", "syscon";
> -               reg = <0x104408 0x3fc>;
> -       };
> --
> 2.11.0
>
>

[-- Attachment #2: Type: text/html, Size: 3692 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-09 19:18           ` Florian Fainelli
  (?)
@ 2017-02-10  0:27           ` Jon Mason
       [not found]             ` <CAC3K-4o+ARP=_hg2XQszQh6v+ySpubyKRspCF38SLUUx4mXQDA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  -1 siblings, 1 reply; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:27 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Rafał Miłecki,
	linux-arm-kernel, Kishon Vijay Abraham I, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]

On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
wrote:

> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
> > On 2017-02-09 00:44, Florian Fainelli wrote:
> >> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
> >>> On 2017-02-09 00:32, Florian Fainelli wrote:
> >>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
> >>>>> From: Rafał Miłecki <rafal@milecki.pl>
> >>>>>
> >>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
> >>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
> >>>>> by NS
> >>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
> >>>>> driver for USB 3.0 PHY on Northstar").
> >>>>>
> >>>>> Instead of adding separated driver & duplicating code we should
> >>>>> work on
> >>>>> improving existing (old) one. Thanks to work done by Broadcom we know
> >>>>> there is MDIO bus we weren't aware of & we know register names which
> >>>>> makes initialization more clear. This is very valuable info and we
> >>>>> should work on using it in existing driver afterwards.
> >>>>
> >>>> Should not we first extend the old driver to support NSP and then
> >>>> revert
> >>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
> >>>
> >>> Sounds like a weird / dirty development method to me: adding duplicated
> >>> code
> >>> first then working on cleaning it. Unless you mean drivers/staging/.
> >>
> >> There was clearly a mistake in submitting this NSP USB PHY driver, and
> >> it should have been a patch against the existing NS USB PHY driver, but
> >> it was not, okay fair enough.
> >>
> >> It's one thing to address that in the future, and it's another thing to
> >> flat out revert the driver just because you don't like the duplication.
> >>
> >> I don't like that either, and we can discuss on how to improve things
> >> (like have the maintainer review that too), but duplication is a lesser
> >> evil than not having the hardware supported at all, and even more so,
> >> purposely reverting in the name of removing that duplication, that's
> >> intentionally breaking working hardware!
> >
> > Hardware support is not excuse and I don't think it ever was in the
> Linux.
> >
> > We don't accept badly designed drivers just because they provide new hw
> > support.
> > We have various standards (for quality, style, design, code) at kernel
> > and we
> > stick to them unless it's drivers/staging/. As you said this driver
> > shouldn't be
> > pushed in the first place.
> >
> > Dropping hardware support in kernel happens. Sometimes it's about ancient
> > devices, sometimes about code quality (some forgotten staging drivers
> > used to be
> > dropped AFAIK).
> >
> > Additionally you're talking about support that was *just* added and
> > isn't used
> > by anyone in the wild world yet.
>
> Except people working on it at Broadcom, but fair enough.
>
> >
> > This hardware was missing upstream support for 4 years so 2 extra months
> > won't
> > really hurt anyone.
> >
> > I really don't see excusee or need for keeping this driver.
> >
> > If you want to (and you feel it's well designed), we can keep
> > brcm,nsp-usb3-phy.txt
>
> No it's fine, let's drop it all and replace it with whatever you and Jon
> come up with next.
>
> >
> > I vote for focusing on existing driver improvements instead of looking
> for
> > excuses for keeping driver that shouldn't be added in the first place.
> > Jon seems to be already working on this, I'm willing to help him, I'm
> > sure we
> > can get you a proper support for the next merge window.
>
> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
> for Northstar Plus") from devicetree/next so you and Jon can figure out
> what is the best thing to move forward and we minimize the amount of
> incompatible DT stuff to be sorted out later on. So as far as I am
> concerned, there are no board/SoC DTS changes to be patched later on, we
> could re-apply this patch as-is, or we could have to define a new binding.
> --
> Florian
>


Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason@broadcom.com>

[-- Attachment #2: Type: text/html, Size: 5861 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-09 19:18           ` Florian Fainelli
@ 2017-02-10  0:29               ` Jon Mason
  -1 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:29 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rafał Miłecki, Rafał Miłecki,
	linux-arm-kernel, Kishon Vijay Abraham I, Rob Herring,
	Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>
>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>> by NS
>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>
>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>> work on
>>>>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>> should work on using it in existing driver afterwards.
>>>>>
>>>>> Should not we first extend the old driver to support NSP and then
>>>>> revert
>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>
>>>> Sounds like a weird / dirty development method to me: adding duplicated
>>>> code
>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>
>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>> it should have been a patch against the existing NS USB PHY driver, but
>>> it was not, okay fair enough.
>>>
>>> It's one thing to address that in the future, and it's another thing to
>>> flat out revert the driver just because you don't like the duplication.
>>>
>>> I don't like that either, and we can discuss on how to improve things
>>> (like have the maintainer review that too), but duplication is a lesser
>>> evil than not having the hardware supported at all, and even more so,
>>> purposely reverting in the name of removing that duplication, that's
>>> intentionally breaking working hardware!
>>
>> Hardware support is not excuse and I don't think it ever was in the Linux.
>>
>> We don't accept badly designed drivers just because they provide new hw
>> support.
>> We have various standards (for quality, style, design, code) at kernel
>> and we
>> stick to them unless it's drivers/staging/. As you said this driver
>> shouldn't be
>> pushed in the first place.
>>
>> Dropping hardware support in kernel happens. Sometimes it's about ancient
>> devices, sometimes about code quality (some forgotten staging drivers
>> used to be
>> dropped AFAIK).
>>
>> Additionally you're talking about support that was *just* added and
>> isn't used
>> by anyone in the wild world yet.
>
> Except people working on it at Broadcom, but fair enough.
>
>>
>> This hardware was missing upstream support for 4 years so 2 extra months
>> won't
>> really hurt anyone.
>>
>> I really don't see excusee or need for keeping this driver.
>>
>> If you want to (and you feel it's well designed), we can keep
>> brcm,nsp-usb3-phy.txt
>
> No it's fine, let's drop it all and replace it with whatever you and Jon
> come up with next.
>
>>
>> I vote for focusing on existing driver improvements instead of looking for
>> excuses for keeping driver that shouldn't be added in the first place.
>> Jon seems to be already working on this, I'm willing to help him, I'm
>> sure we
>> can get you a proper support for the next merge window.
>
> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
> for Northstar Plus") from devicetree/next so you and Jon can figure out
> what is the best thing to move forward and we minimize the amount of
> incompatible DT stuff to be sorted out later on. So as far as I am
> concerned, there are no board/SoC DTS changes to be patched later on, we
> could re-apply this patch as-is, or we could have to define a new binding.
> --
> Florian

(previous email got caught in HTML filters)

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-02-10  0:29               ` Jon Mason
  0 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>>
>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>> by NS
>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3: new
>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>
>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>> work on
>>>>>> improving existing (old) one. Thanks to work done by Broadcom we know
>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>> should work on using it in existing driver afterwards.
>>>>>
>>>>> Should not we first extend the old driver to support NSP and then
>>>>> revert
>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>
>>>> Sounds like a weird / dirty development method to me: adding duplicated
>>>> code
>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>
>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>> it should have been a patch against the existing NS USB PHY driver, but
>>> it was not, okay fair enough.
>>>
>>> It's one thing to address that in the future, and it's another thing to
>>> flat out revert the driver just because you don't like the duplication.
>>>
>>> I don't like that either, and we can discuss on how to improve things
>>> (like have the maintainer review that too), but duplication is a lesser
>>> evil than not having the hardware supported at all, and even more so,
>>> purposely reverting in the name of removing that duplication, that's
>>> intentionally breaking working hardware!
>>
>> Hardware support is not excuse and I don't think it ever was in the Linux.
>>
>> We don't accept badly designed drivers just because they provide new hw
>> support.
>> We have various standards (for quality, style, design, code) at kernel
>> and we
>> stick to them unless it's drivers/staging/. As you said this driver
>> shouldn't be
>> pushed in the first place.
>>
>> Dropping hardware support in kernel happens. Sometimes it's about ancient
>> devices, sometimes about code quality (some forgotten staging drivers
>> used to be
>> dropped AFAIK).
>>
>> Additionally you're talking about support that was *just* added and
>> isn't used
>> by anyone in the wild world yet.
>
> Except people working on it at Broadcom, but fair enough.
>
>>
>> This hardware was missing upstream support for 4 years so 2 extra months
>> won't
>> really hurt anyone.
>>
>> I really don't see excusee or need for keeping this driver.
>>
>> If you want to (and you feel it's well designed), we can keep
>> brcm,nsp-usb3-phy.txt
>
> No it's fine, let's drop it all and replace it with whatever you and Jon
> come up with next.
>
>>
>> I vote for focusing on existing driver improvements instead of looking for
>> excuses for keeping driver that shouldn't be added in the first place.
>> Jon seems to be already working on this, I'm willing to help him, I'm
>> sure we
>> can get you a proper support for the next merge window.
>
> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
> for Northstar Plus") from devicetree/next so you and Jon can figure out
> what is the best thing to move forward and we minimize the amount of
> incompatible DT stuff to be sorted out later on. So as far as I am
> concerned, there are no board/SoC DTS changes to be patched later on, we
> could re-apply this patch as-is, or we could have to define a new binding.
> --
> Florian

(previous email got caught in HTML filters)

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason@broadcom.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
  2017-02-08 23:30     ` Rafał Miłecki
@ 2017-02-10  0:30         ` Jon Mason
  -1 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:30 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-arm-kernel, Kishon Vijay Abraham I, Florian Fainelli,
	Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rafał Miłecki

On Wed, Feb 8, 2017 at 6:30 PM, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
> for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
> Please note we revert this commit before it reached stable release.
>
> If new compatible string is needed it should be added to the existing
> bcm-ns-usb3-phy.txt which already describes this PHY.
>
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>

(previous email got caught in HTML filters)

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
>  1 file changed, 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> deleted file mode 100644
> index e68ae5dec9c9..000000000000
> --- a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -Broadcom USB3 phy binding for northstar plus SoC
> -The USB3 phy is internal to the SoC and is accessed using mdio interface.
> -
> -Required mdio bus properties:
> -- reg: Should be 0x0 for SoC internal USB3 phy
> -- #address-cells: must be 1
> -- #size-cells: must be 0
> -
> -Required USB3 PHY properties:
> -- compatible: should be "brcm,nsp-usb3-phy"
> -- reg: USB3 Phy address on SoC internal MDIO bus and it should be 0x10.
> -- usb3-ctrl-syscon: handler of syscon node defining physical address
> -  of usb3 control register.
> -- #phy-cells: must be 0
> -
> -Required usb3 control properties:
> -- compatible: should be "brcm,nsp-usb3-ctrl"
> -- reg: offset and length of the control registers
> -
> -Example:
> -
> -       mdio@0 {
> -               reg = <0x0>;
> -               #address-cells = <1>;
> -               #size-cells = <0>;
> -
> -               usb3_phy: usb-phy@10 {
> -                       compatible = "brcm,nsp-usb3-phy";
> -                       reg = <0x10>;
> -                       usb3-ctrl-syscon = <&usb3_ctrl>;
> -                       #phy-cells = <0>;
> -                       status = "disabled";
> -               };
> -       };
> -
> -       usb3_ctrl: syscon@104408 {
> -               compatible = "brcm,nsp-usb3-ctrl", "syscon";
> -               reg = <0x104408 0x3fc>;
> -       };
> --
> 2.11.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
@ 2017-02-10  0:30         ` Jon Mason
  0 siblings, 0 replies; 32+ messages in thread
From: Jon Mason @ 2017-02-10  0:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 8, 2017 at 6:30 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
>
> This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
> for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
> Please note we revert this commit before it reached stable release.
>
> If new compatible string is needed it should be added to the existing
> bcm-ns-usb3-phy.txt which already describes this PHY.
>
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>

(previous email got caught in HTML filters)

Per the discussion with Rafal, this is acceptable

Acked-by: Jon Mason <jon.mason@broadcom.com>

> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
>  1 file changed, 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt b/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> deleted file mode 100644
> index e68ae5dec9c9..000000000000
> --- a/Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -Broadcom USB3 phy binding for northstar plus SoC
> -The USB3 phy is internal to the SoC and is accessed using mdio interface.
> -
> -Required mdio bus properties:
> -- reg: Should be 0x0 for SoC internal USB3 phy
> -- #address-cells: must be 1
> -- #size-cells: must be 0
> -
> -Required USB3 PHY properties:
> -- compatible: should be "brcm,nsp-usb3-phy"
> -- reg: USB3 Phy address on SoC internal MDIO bus and it should be 0x10.
> -- usb3-ctrl-syscon: handler of syscon node defining physical address
> -  of usb3 control register.
> -- #phy-cells: must be 0
> -
> -Required usb3 control properties:
> -- compatible: should be "brcm,nsp-usb3-ctrl"
> -- reg: offset and length of the control registers
> -
> -Example:
> -
> -       mdio at 0 {
> -               reg = <0x0>;
> -               #address-cells = <1>;
> -               #size-cells = <0>;
> -
> -               usb3_phy: usb-phy at 10 {
> -                       compatible = "brcm,nsp-usb3-phy";
> -                       reg = <0x10>;
> -                       usb3-ctrl-syscon = <&usb3_ctrl>;
> -                       #phy-cells = <0>;
> -                       status = "disabled";
> -               };
> -       };
> -
> -       usb3_ctrl: syscon at 104408 {
> -               compatible = "brcm,nsp-usb3-ctrl", "syscon";
> -               reg = <0x104408 0x3fc>;
> -       };
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
  2017-02-08 23:30     ` Rafał Miłecki
@ 2017-02-15 23:04         ` Rob Herring
  -1 siblings, 0 replies; 32+ messages in thread
From: Rob Herring @ 2017-02-15 23:04 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Kishon Vijay Abraham I, Florian Fainelli, Mark Rutland, Ray Jui,
	Scott Branden, Jon Mason,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
	Yendapally Reddy Dhananjaya Reddy,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rafał Miłecki

On Thu, Feb 09, 2017 at 12:30:23AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> 
> This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
> for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
> Please note we revert this commit before it reached stable release.
> 
> If new compatible string is needed it should be added to the existing
> bcm-ns-usb3-phy.txt which already describes this PHY.
> 
> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
>  1 file changed, 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY"
@ 2017-02-15 23:04         ` Rob Herring
  0 siblings, 0 replies; 32+ messages in thread
From: Rob Herring @ 2017-02-15 23:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 09, 2017 at 12:30:23AM +0100, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> This reverts commit c8ca631f9480 ("dt-bindings: phy: Add documentation
> for NSP USB3 PHY") to match reverting commit adding the new PHY driver.
> Please note we revert this commit before it reached stable release.
> 
> If new compatible string is needed it should be added to the existing
> bcm-ns-usb3-phy.txt which already describes this PHY.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/phy/brcm,nsp-usb3-phy.txt  | 39 ----------------------
>  1 file changed, 39 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt

Acked-by: Rob Herring <robh@kernel.org> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-02-10  0:27           ` Jon Mason
@ 2017-03-08 12:13                 ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-03-08 12:13 UTC (permalink / raw)
  To: Jon Mason, Kishon Vijay Abraham I
  Cc: Florian Fainelli, Rafał Miłecki, linux-arm-kernel,
	Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On 10 February 2017 at 01:27, Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> wrote:
>>
>> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>> > On 2017-02-09 00:44, Florian Fainelli wrote:
>> >> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>> >>> On 2017-02-09 00:32, Florian Fainelli wrote:
>> >>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>> >>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>> >>>>>
>> >>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> >>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>> >>>>> by NS
>> >>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>> >>>>> new
>> >>>>> driver for USB 3.0 PHY on Northstar").
>> >>>>>
>> >>>>> Instead of adding separated driver & duplicating code we should
>> >>>>> work on
>> >>>>> improving existing (old) one. Thanks to work done by Broadcom we
>> >>>>> know
>> >>>>> there is MDIO bus we weren't aware of & we know register names which
>> >>>>> makes initialization more clear. This is very valuable info and we
>> >>>>> should work on using it in existing driver afterwards.
>> >>>>
>> >>>> Should not we first extend the old driver to support NSP and then
>> >>>> revert
>> >>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>> >>>
>> >>> Sounds like a weird / dirty development method to me: adding
>> >>> duplicated
>> >>> code
>> >>> first then working on cleaning it. Unless you mean drivers/staging/.
>> >>
>> >> There was clearly a mistake in submitting this NSP USB PHY driver, and
>> >> it should have been a patch against the existing NS USB PHY driver, but
>> >> it was not, okay fair enough.
>> >>
>> >> It's one thing to address that in the future, and it's another thing to
>> >> flat out revert the driver just because you don't like the duplication.
>> >>
>> >> I don't like that either, and we can discuss on how to improve things
>> >> (like have the maintainer review that too), but duplication is a lesser
>> >> evil than not having the hardware supported at all, and even more so,
>> >> purposely reverting in the name of removing that duplication, that's
>> >> intentionally breaking working hardware!
>> >
>> > Hardware support is not excuse and I don't think it ever was in the
>> > Linux.
>> >
>> > We don't accept badly designed drivers just because they provide new hw
>> > support.
>> > We have various standards (for quality, style, design, code) at kernel
>> > and we
>> > stick to them unless it's drivers/staging/. As you said this driver
>> > shouldn't be
>> > pushed in the first place.
>> >
>> > Dropping hardware support in kernel happens. Sometimes it's about
>> > ancient
>> > devices, sometimes about code quality (some forgotten staging drivers
>> > used to be
>> > dropped AFAIK).
>> >
>> > Additionally you're talking about support that was *just* added and
>> > isn't used
>> > by anyone in the wild world yet.
>>
>> Except people working on it at Broadcom, but fair enough.
>>
>> >
>> > This hardware was missing upstream support for 4 years so 2 extra months
>> > won't
>> > really hurt anyone.
>> >
>> > I really don't see excusee or need for keeping this driver.
>> >
>> > If you want to (and you feel it's well designed), we can keep
>> > brcm,nsp-usb3-phy.txt
>>
>> No it's fine, let's drop it all and replace it with whatever you and Jon
>> come up with next.
>>
>> >
>> > I vote for focusing on existing driver improvements instead of looking
>> > for
>> > excuses for keeping driver that shouldn't be added in the first place.
>> > Jon seems to be already working on this, I'm willing to help him, I'm
>> > sure we
>> > can get you a proper support for the next merge window.
>>
>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>> what is the best thing to move forward and we minimize the amount of
>> incompatible DT stuff to be sorted out later on. So as far as I am
>> concerned, there are no board/SoC DTS changes to be patched later on, we
>> could re-apply this patch as-is, or we could have to define a new binding.
>
> Per the discussion with Rafal, this is acceptable
>
> Acked-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Hi Kishon, what's the status of this?

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-03-08 12:13                 ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-03-08 12:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
> wrote:
>>
>> On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
>> > On 2017-02-09 00:44, Florian Fainelli wrote:
>> >> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>> >>> On 2017-02-09 00:32, Florian Fainelli wrote:
>> >>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>> >>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>> >>>>>
>> >>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>> >>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>> >>>>> by NS
>> >>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>> >>>>> new
>> >>>>> driver for USB 3.0 PHY on Northstar").
>> >>>>>
>> >>>>> Instead of adding separated driver & duplicating code we should
>> >>>>> work on
>> >>>>> improving existing (old) one. Thanks to work done by Broadcom we
>> >>>>> know
>> >>>>> there is MDIO bus we weren't aware of & we know register names which
>> >>>>> makes initialization more clear. This is very valuable info and we
>> >>>>> should work on using it in existing driver afterwards.
>> >>>>
>> >>>> Should not we first extend the old driver to support NSP and then
>> >>>> revert
>> >>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>> >>>
>> >>> Sounds like a weird / dirty development method to me: adding
>> >>> duplicated
>> >>> code
>> >>> first then working on cleaning it. Unless you mean drivers/staging/.
>> >>
>> >> There was clearly a mistake in submitting this NSP USB PHY driver, and
>> >> it should have been a patch against the existing NS USB PHY driver, but
>> >> it was not, okay fair enough.
>> >>
>> >> It's one thing to address that in the future, and it's another thing to
>> >> flat out revert the driver just because you don't like the duplication.
>> >>
>> >> I don't like that either, and we can discuss on how to improve things
>> >> (like have the maintainer review that too), but duplication is a lesser
>> >> evil than not having the hardware supported at all, and even more so,
>> >> purposely reverting in the name of removing that duplication, that's
>> >> intentionally breaking working hardware!
>> >
>> > Hardware support is not excuse and I don't think it ever was in the
>> > Linux.
>> >
>> > We don't accept badly designed drivers just because they provide new hw
>> > support.
>> > We have various standards (for quality, style, design, code) at kernel
>> > and we
>> > stick to them unless it's drivers/staging/. As you said this driver
>> > shouldn't be
>> > pushed in the first place.
>> >
>> > Dropping hardware support in kernel happens. Sometimes it's about
>> > ancient
>> > devices, sometimes about code quality (some forgotten staging drivers
>> > used to be
>> > dropped AFAIK).
>> >
>> > Additionally you're talking about support that was *just* added and
>> > isn't used
>> > by anyone in the wild world yet.
>>
>> Except people working on it at Broadcom, but fair enough.
>>
>> >
>> > This hardware was missing upstream support for 4 years so 2 extra months
>> > won't
>> > really hurt anyone.
>> >
>> > I really don't see excusee or need for keeping this driver.
>> >
>> > If you want to (and you feel it's well designed), we can keep
>> > brcm,nsp-usb3-phy.txt
>>
>> No it's fine, let's drop it all and replace it with whatever you and Jon
>> come up with next.
>>
>> >
>> > I vote for focusing on existing driver improvements instead of looking
>> > for
>> > excuses for keeping driver that shouldn't be added in the first place.
>> > Jon seems to be already working on this, I'm willing to help him, I'm
>> > sure we
>> > can get you a proper support for the next merge window.
>>
>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>> what is the best thing to move forward and we minimize the amount of
>> incompatible DT stuff to be sorted out later on. So as far as I am
>> concerned, there are no board/SoC DTS changes to be patched later on, we
>> could re-apply this patch as-is, or we could have to define a new binding.
>
> Per the discussion with Rafal, this is acceptable
>
> Acked-by: Jon Mason <jon.mason@broadcom.com>

Hi Kishon, what's the status of this?

-- 
Rafa?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-03-08 12:13                 ` Rafał Miłecki
@ 2017-03-08 12:26                   ` Kishon Vijay Abraham I
  -1 siblings, 0 replies; 32+ messages in thread
From: Kishon Vijay Abraham I @ 2017-03-08 12:26 UTC (permalink / raw)
  To: Rafał Miłecki, Jon Mason
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Scott Branden, Jon Mason, Ray Jui,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	BCM Kernel Feedback, Rafał Miłecki, linux-arm-kernel

Hi Rafal,

On Wednesday 08 March 2017 05:43 PM, Rafał Miłecki wrote:
> On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
>> wrote:
>>>
>>> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>>>>
>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>> by NS
>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>> new
>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>
>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>> work on
>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>> know
>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>
>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>> revert
>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>
>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>> duplicated
>>>>>> code
>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>
>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>> it was not, okay fair enough.
>>>>>
>>>>> It's one thing to address that in the future, and it's another thing to
>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>
>>>>> I don't like that either, and we can discuss on how to improve things
>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>> evil than not having the hardware supported at all, and even more so,
>>>>> purposely reverting in the name of removing that duplication, that's
>>>>> intentionally breaking working hardware!
>>>>
>>>> Hardware support is not excuse and I don't think it ever was in the
>>>> Linux.
>>>>
>>>> We don't accept badly designed drivers just because they provide new hw
>>>> support.
>>>> We have various standards (for quality, style, design, code) at kernel
>>>> and we
>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>> shouldn't be
>>>> pushed in the first place.
>>>>
>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>> ancient
>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>> used to be
>>>> dropped AFAIK).
>>>>
>>>> Additionally you're talking about support that was *just* added and
>>>> isn't used
>>>> by anyone in the wild world yet.
>>>
>>> Except people working on it at Broadcom, but fair enough.
>>>
>>>>
>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>> won't
>>>> really hurt anyone.
>>>>
>>>> I really don't see excusee or need for keeping this driver.
>>>>
>>>> If you want to (and you feel it's well designed), we can keep
>>>> brcm,nsp-usb3-phy.txt
>>>
>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>> come up with next.
>>>
>>>>
>>>> I vote for focusing on existing driver improvements instead of looking
>>>> for
>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>> sure we
>>>> can get you a proper support for the next merge window.
>>>
>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>> what is the best thing to move forward and we minimize the amount of
>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>> could re-apply this patch as-is, or we could have to define a new binding.
>>
>> Per the discussion with Rafal, this is acceptable
>>
>> Acked-by: Jon Mason <jon.mason@broadcom.com>
> 
> Hi Kishon, what's the status of this?

Will be merging this in a day or so.

Thanks
Kishon

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-03-08 12:26                   ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 32+ messages in thread
From: Kishon Vijay Abraham I @ 2017-03-08 12:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rafal,

On Wednesday 08 March 2017 05:43 PM, Rafa? Mi?ecki wrote:
> On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
>> wrote:
>>>
>>> On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>>>>
>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>> by NS
>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>> new
>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>
>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>> work on
>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>> know
>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>
>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>> revert
>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>
>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>> duplicated
>>>>>> code
>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>
>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>> it was not, okay fair enough.
>>>>>
>>>>> It's one thing to address that in the future, and it's another thing to
>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>
>>>>> I don't like that either, and we can discuss on how to improve things
>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>> evil than not having the hardware supported at all, and even more so,
>>>>> purposely reverting in the name of removing that duplication, that's
>>>>> intentionally breaking working hardware!
>>>>
>>>> Hardware support is not excuse and I don't think it ever was in the
>>>> Linux.
>>>>
>>>> We don't accept badly designed drivers just because they provide new hw
>>>> support.
>>>> We have various standards (for quality, style, design, code) at kernel
>>>> and we
>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>> shouldn't be
>>>> pushed in the first place.
>>>>
>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>> ancient
>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>> used to be
>>>> dropped AFAIK).
>>>>
>>>> Additionally you're talking about support that was *just* added and
>>>> isn't used
>>>> by anyone in the wild world yet.
>>>
>>> Except people working on it at Broadcom, but fair enough.
>>>
>>>>
>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>> won't
>>>> really hurt anyone.
>>>>
>>>> I really don't see excusee or need for keeping this driver.
>>>>
>>>> If you want to (and you feel it's well designed), we can keep
>>>> brcm,nsp-usb3-phy.txt
>>>
>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>> come up with next.
>>>
>>>>
>>>> I vote for focusing on existing driver improvements instead of looking
>>>> for
>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>> sure we
>>>> can get you a proper support for the next merge window.
>>>
>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>> what is the best thing to move forward and we minimize the amount of
>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>> could re-apply this patch as-is, or we could have to define a new binding.
>>
>> Per the discussion with Rafal, this is acceptable
>>
>> Acked-by: Jon Mason <jon.mason@broadcom.com>
> 
> Hi Kishon, what's the status of this?

Will be merging this in a day or so.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-03-08 12:26                   ` Kishon Vijay Abraham I
@ 2017-03-09  8:11                       ` Kishon Vijay Abraham I
  -1 siblings, 0 replies; 32+ messages in thread
From: Kishon Vijay Abraham I @ 2017-03-09  8:11 UTC (permalink / raw)
  To: Rafał Miłecki, Jon Mason
  Cc: Florian Fainelli, Rafał Miłecki, linux-arm-kernel,
	Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Yendapally Reddy Dhananjaya Reddy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS



On Wednesday 08 March 2017 05:56 PM, Kishon Vijay Abraham I wrote:
> Hi Rafal,
> 
> On Wednesday 08 March 2017 05:43 PM, Rafał Miłecki wrote:
>> On 10 February 2017 at 01:27, Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> wrote:
>>>>
>>>> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>>>>
>>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>>> by NS
>>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>>> new
>>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>>
>>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>>> work on
>>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>>> know
>>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>>
>>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>>> revert
>>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>>
>>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>>> duplicated
>>>>>>> code
>>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>>
>>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>>> it was not, okay fair enough.
>>>>>>
>>>>>> It's one thing to address that in the future, and it's another thing to
>>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>>
>>>>>> I don't like that either, and we can discuss on how to improve things
>>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>>> evil than not having the hardware supported at all, and even more so,
>>>>>> purposely reverting in the name of removing that duplication, that's
>>>>>> intentionally breaking working hardware!
>>>>>
>>>>> Hardware support is not excuse and I don't think it ever was in the
>>>>> Linux.
>>>>>
>>>>> We don't accept badly designed drivers just because they provide new hw
>>>>> support.
>>>>> We have various standards (for quality, style, design, code) at kernel
>>>>> and we
>>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>>> shouldn't be
>>>>> pushed in the first place.
>>>>>
>>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>>> ancient
>>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>>> used to be
>>>>> dropped AFAIK).
>>>>>
>>>>> Additionally you're talking about support that was *just* added and
>>>>> isn't used
>>>>> by anyone in the wild world yet.
>>>>
>>>> Except people working on it at Broadcom, but fair enough.
>>>>
>>>>>
>>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>>> won't
>>>>> really hurt anyone.
>>>>>
>>>>> I really don't see excusee or need for keeping this driver.
>>>>>
>>>>> If you want to (and you feel it's well designed), we can keep
>>>>> brcm,nsp-usb3-phy.txt
>>>>
>>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>>> come up with next.
>>>>
>>>>>
>>>>> I vote for focusing on existing driver improvements instead of looking
>>>>> for
>>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>>> sure we
>>>>> can get you a proper support for the next merge window.
>>>>
>>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>>> what is the best thing to move forward and we minimize the amount of
>>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>>> could re-apply this patch as-is, or we could have to define a new binding.
>>>
>>> Per the discussion with Rafal, this is acceptable
>>>
>>> Acked-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>
>> Hi Kishon, what's the status of this?
> 
> Will be merging this in a day or so.

merged, thanks.

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-03-09  8:11                       ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 32+ messages in thread
From: Kishon Vijay Abraham I @ 2017-03-09  8:11 UTC (permalink / raw)
  To: linux-arm-kernel



On Wednesday 08 March 2017 05:56 PM, Kishon Vijay Abraham I wrote:
> Hi Rafal,
> 
> On Wednesday 08 March 2017 05:43 PM, Rafa? Mi?ecki wrote:
>> On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
>>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
>>> wrote:
>>>>
>>>> On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
>>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>>> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>>>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>>>>>
>>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>>> by NS
>>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>>> new
>>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>>
>>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>>> work on
>>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>>> know
>>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>>
>>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>>> revert
>>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>>
>>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>>> duplicated
>>>>>>> code
>>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>>
>>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>>> it was not, okay fair enough.
>>>>>>
>>>>>> It's one thing to address that in the future, and it's another thing to
>>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>>
>>>>>> I don't like that either, and we can discuss on how to improve things
>>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>>> evil than not having the hardware supported at all, and even more so,
>>>>>> purposely reverting in the name of removing that duplication, that's
>>>>>> intentionally breaking working hardware!
>>>>>
>>>>> Hardware support is not excuse and I don't think it ever was in the
>>>>> Linux.
>>>>>
>>>>> We don't accept badly designed drivers just because they provide new hw
>>>>> support.
>>>>> We have various standards (for quality, style, design, code) at kernel
>>>>> and we
>>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>>> shouldn't be
>>>>> pushed in the first place.
>>>>>
>>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>>> ancient
>>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>>> used to be
>>>>> dropped AFAIK).
>>>>>
>>>>> Additionally you're talking about support that was *just* added and
>>>>> isn't used
>>>>> by anyone in the wild world yet.
>>>>
>>>> Except people working on it at Broadcom, but fair enough.
>>>>
>>>>>
>>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>>> won't
>>>>> really hurt anyone.
>>>>>
>>>>> I really don't see excusee or need for keeping this driver.
>>>>>
>>>>> If you want to (and you feel it's well designed), we can keep
>>>>> brcm,nsp-usb3-phy.txt
>>>>
>>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>>> come up with next.
>>>>
>>>>>
>>>>> I vote for focusing on existing driver improvements instead of looking
>>>>> for
>>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>>> sure we
>>>>> can get you a proper support for the next merge window.
>>>>
>>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>>> what is the best thing to move forward and we minimize the amount of
>>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>>> could re-apply this patch as-is, or we could have to define a new binding.
>>>
>>> Per the discussion with Rafal, this is acceptable
>>>
>>> Acked-by: Jon Mason <jon.mason@broadcom.com>
>>
>> Hi Kishon, what's the status of this?
> 
> Will be merging this in a day or so.

merged, thanks.

-Kishon
> 
> Thanks
> Kishon
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
  2017-03-09  8:11                       ` Kishon Vijay Abraham I
@ 2017-03-09  9:17                         ` Rafał Miłecki
  -1 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-03-09  9:17 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Florian Fainelli, Scott Branden, Jon Mason, Ray Jui, Jon Mason,
	Yendapally Reddy Dhananjaya Reddy, Rob Herring,
	BCM Kernel Feedback, Rafał Miłecki, linux-arm-kernel

On 9 March 2017 at 09:11, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
>
> On Wednesday 08 March 2017 05:56 PM, Kishon Vijay Abraham I wrote:
>> Hi Rafal,
>>
>> On Wednesday 08 March 2017 05:43 PM, Rafał Miłecki wrote:
>>> On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
>>>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
>>>> wrote:
>>>>>
>>>>> On 02/08/2017 11:21 PM, Rafał Miłecki wrote:
>>>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>>>> On 02/08/2017 03:39 PM, Rafał Miłecki wrote:
>>>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>>>> On 02/08/2017 03:30 PM, Rafał Miłecki wrote:
>>>>>>>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>>>>>>>
>>>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>>>> by NS
>>>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>>>> new
>>>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>>>
>>>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>>>> work on
>>>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>>>> know
>>>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>>>
>>>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>>>> revert
>>>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>>>
>>>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>>>> duplicated
>>>>>>>> code
>>>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>>>
>>>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>>>> it was not, okay fair enough.
>>>>>>>
>>>>>>> It's one thing to address that in the future, and it's another thing to
>>>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>>>
>>>>>>> I don't like that either, and we can discuss on how to improve things
>>>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>>>> evil than not having the hardware supported at all, and even more so,
>>>>>>> purposely reverting in the name of removing that duplication, that's
>>>>>>> intentionally breaking working hardware!
>>>>>>
>>>>>> Hardware support is not excuse and I don't think it ever was in the
>>>>>> Linux.
>>>>>>
>>>>>> We don't accept badly designed drivers just because they provide new hw
>>>>>> support.
>>>>>> We have various standards (for quality, style, design, code) at kernel
>>>>>> and we
>>>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>>>> shouldn't be
>>>>>> pushed in the first place.
>>>>>>
>>>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>>>> ancient
>>>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>>>> used to be
>>>>>> dropped AFAIK).
>>>>>>
>>>>>> Additionally you're talking about support that was *just* added and
>>>>>> isn't used
>>>>>> by anyone in the wild world yet.
>>>>>
>>>>> Except people working on it at Broadcom, but fair enough.
>>>>>
>>>>>>
>>>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>>>> won't
>>>>>> really hurt anyone.
>>>>>>
>>>>>> I really don't see excusee or need for keeping this driver.
>>>>>>
>>>>>> If you want to (and you feel it's well designed), we can keep
>>>>>> brcm,nsp-usb3-phy.txt
>>>>>
>>>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>>>> come up with next.
>>>>>
>>>>>>
>>>>>> I vote for focusing on existing driver improvements instead of looking
>>>>>> for
>>>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>>>> sure we
>>>>>> can get you a proper support for the next merge window.
>>>>>
>>>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>>>> what is the best thing to move forward and we minimize the amount of
>>>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>>>> could re-apply this patch as-is, or we could have to define a new binding.
>>>>
>>>> Per the discussion with Rafal, this is acceptable
>>>>
>>>> Acked-by: Jon Mason <jon.mason@broadcom.com>
>>>
>>> Hi Kishon, what's the status of this?
>>
>> Will be merging this in a day or so.
>
> merged, thanks.

Thank you!

-- 
Rafał

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"
@ 2017-03-09  9:17                         ` Rafał Miłecki
  0 siblings, 0 replies; 32+ messages in thread
From: Rafał Miłecki @ 2017-03-09  9:17 UTC (permalink / raw)
  To: linux-arm-kernel

On 9 March 2017 at 09:11, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
>
> On Wednesday 08 March 2017 05:56 PM, Kishon Vijay Abraham I wrote:
>> Hi Rafal,
>>
>> On Wednesday 08 March 2017 05:43 PM, Rafa? Mi?ecki wrote:
>>> On 10 February 2017 at 01:27, Jon Mason <jon.mason@broadcom.com> wrote:
>>>> On Thu, Feb 9, 2017 at 2:18 PM, Florian Fainelli <f.fainelli@gmail.com>
>>>> wrote:
>>>>>
>>>>> On 02/08/2017 11:21 PM, Rafa? Mi?ecki wrote:
>>>>>> On 2017-02-09 00:44, Florian Fainelli wrote:
>>>>>>> On 02/08/2017 03:39 PM, Rafa? Mi?ecki wrote:
>>>>>>>> On 2017-02-09 00:32, Florian Fainelli wrote:
>>>>>>>>> On 02/08/2017 03:30 PM, Rafa? Mi?ecki wrote:
>>>>>>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>>>>>>
>>>>>>>>>> This reverts commit d7bc1a7d41bf ("phy: Add USB3 PHY support for
>>>>>>>>>> Broadcom NSP SoC") as we already have driver for this PHY (shared
>>>>>>>>>> by NS
>>>>>>>>>> and NSP). It was added in commit e5666281d9ea ("phy: bcm-ns-usb3:
>>>>>>>>>> new
>>>>>>>>>> driver for USB 3.0 PHY on Northstar").
>>>>>>>>>>
>>>>>>>>>> Instead of adding separated driver & duplicating code we should
>>>>>>>>>> work on
>>>>>>>>>> improving existing (old) one. Thanks to work done by Broadcom we
>>>>>>>>>> know
>>>>>>>>>> there is MDIO bus we weren't aware of & we know register names which
>>>>>>>>>> makes initialization more clear. This is very valuable info and we
>>>>>>>>>> should work on using it in existing driver afterwards.
>>>>>>>>>
>>>>>>>>> Should not we first extend the old driver to support NSP and then
>>>>>>>>> revert
>>>>>>>>> d7bc1a7d41bf ("phy: Add USB3 PHY support for Broadcom NSP SoC")?
>>>>>>>>
>>>>>>>> Sounds like a weird / dirty development method to me: adding
>>>>>>>> duplicated
>>>>>>>> code
>>>>>>>> first then working on cleaning it. Unless you mean drivers/staging/.
>>>>>>>
>>>>>>> There was clearly a mistake in submitting this NSP USB PHY driver, and
>>>>>>> it should have been a patch against the existing NS USB PHY driver, but
>>>>>>> it was not, okay fair enough.
>>>>>>>
>>>>>>> It's one thing to address that in the future, and it's another thing to
>>>>>>> flat out revert the driver just because you don't like the duplication.
>>>>>>>
>>>>>>> I don't like that either, and we can discuss on how to improve things
>>>>>>> (like have the maintainer review that too), but duplication is a lesser
>>>>>>> evil than not having the hardware supported at all, and even more so,
>>>>>>> purposely reverting in the name of removing that duplication, that's
>>>>>>> intentionally breaking working hardware!
>>>>>>
>>>>>> Hardware support is not excuse and I don't think it ever was in the
>>>>>> Linux.
>>>>>>
>>>>>> We don't accept badly designed drivers just because they provide new hw
>>>>>> support.
>>>>>> We have various standards (for quality, style, design, code) at kernel
>>>>>> and we
>>>>>> stick to them unless it's drivers/staging/. As you said this driver
>>>>>> shouldn't be
>>>>>> pushed in the first place.
>>>>>>
>>>>>> Dropping hardware support in kernel happens. Sometimes it's about
>>>>>> ancient
>>>>>> devices, sometimes about code quality (some forgotten staging drivers
>>>>>> used to be
>>>>>> dropped AFAIK).
>>>>>>
>>>>>> Additionally you're talking about support that was *just* added and
>>>>>> isn't used
>>>>>> by anyone in the wild world yet.
>>>>>
>>>>> Except people working on it at Broadcom, but fair enough.
>>>>>
>>>>>>
>>>>>> This hardware was missing upstream support for 4 years so 2 extra months
>>>>>> won't
>>>>>> really hurt anyone.
>>>>>>
>>>>>> I really don't see excusee or need for keeping this driver.
>>>>>>
>>>>>> If you want to (and you feel it's well designed), we can keep
>>>>>> brcm,nsp-usb3-phy.txt
>>>>>
>>>>> No it's fine, let's drop it all and replace it with whatever you and Jon
>>>>> come up with next.
>>>>>
>>>>>>
>>>>>> I vote for focusing on existing driver improvements instead of looking
>>>>>> for
>>>>>> excuses for keeping driver that shouldn't be added in the first place.
>>>>>> Jon seems to be already working on this, I'm willing to help him, I'm
>>>>>> sure we
>>>>>> can get you a proper support for the next merge window.
>>>>>
>>>>> Fair enough, I dropped Dhanajay's changes ("ARM: dts: NSP: Add USB nodes
>>>>> for Northstar Plus") from devicetree/next so you and Jon can figure out
>>>>> what is the best thing to move forward and we minimize the amount of
>>>>> incompatible DT stuff to be sorted out later on. So as far as I am
>>>>> concerned, there are no board/SoC DTS changes to be patched later on, we
>>>>> could re-apply this patch as-is, or we could have to define a new binding.
>>>>
>>>> Per the discussion with Rafal, this is acceptable
>>>>
>>>> Acked-by: Jon Mason <jon.mason@broadcom.com>
>>>
>>> Hi Kishon, what's the status of this?
>>
>> Will be merging this in a day or so.
>
> merged, thanks.

Thank you!

-- 
Rafa?

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2017-03-09  9:17 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-08 23:30 [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC" Rafał Miłecki
2017-02-08 23:30 ` Rafał Miłecki
     [not found] ` <20170208233023.31922-1-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-08 23:30   ` [PATCH for-4.11 2/2] Revert "dt-bindings: phy: Add documentation for NSP USB3 PHY" Rafał Miłecki
2017-02-08 23:30     ` Rafał Miłecki
2017-02-10  0:27     ` Jon Mason
     [not found]     ` <20170208233023.31922-2-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-10  0:30       ` Jon Mason
2017-02-10  0:30         ` Jon Mason
2017-02-15 23:04       ` Rob Herring
2017-02-15 23:04         ` Rob Herring
2017-02-08 23:32 ` [PATCH for-4.11 1/2] Revert "phy: Add USB3 PHY support for Broadcom NSP SoC" Florian Fainelli
2017-02-08 23:32   ` Florian Fainelli
     [not found]   ` <82b47a03-c41b-378b-2d7c-f263eff514a9-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-08 23:36     ` Jon Mason
2017-02-08 23:36       ` Jon Mason
2017-02-08 23:39   ` Rafał Miłecki
2017-02-08 23:39     ` Rafał Miłecki
2017-02-08 23:44     ` Florian Fainelli
2017-02-08 23:44       ` Florian Fainelli
2017-02-09  7:21       ` Rafał Miłecki
2017-02-09  7:21         ` Rafał Miłecki
2017-02-09 19:18         ` Florian Fainelli
2017-02-09 19:18           ` Florian Fainelli
2017-02-10  0:27           ` Jon Mason
     [not found]             ` <CAC3K-4o+ARP=_hg2XQszQh6v+ySpubyKRspCF38SLUUx4mXQDA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-08 12:13               ` Rafał Miłecki
2017-03-08 12:13                 ` Rafał Miłecki
2017-03-08 12:26                 ` Kishon Vijay Abraham I
2017-03-08 12:26                   ` Kishon Vijay Abraham I
     [not found]                   ` <58BFF88F.8080008-l0cyMroinI0@public.gmane.org>
2017-03-09  8:11                     ` Kishon Vijay Abraham I
2017-03-09  8:11                       ` Kishon Vijay Abraham I
2017-03-09  9:17                       ` Rafał Miłecki
2017-03-09  9:17                         ` Rafał Miłecki
     [not found]           ` <b2bdc1a7-4e5c-a13e-0cda-c90759d7308c-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-10  0:29             ` Jon Mason
2017-02-10  0:29               ` Jon Mason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.