Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"

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

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 <[email protected]>

---
 .../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

Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt deleted +0 -40

deleted file mode 100644
index e68ae5dec9c9..000000000000
@@ -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

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

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 <[email protected]>

---
 .../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

Documentation/devicetree/bindings/phy/brcm,nsp-usb3-phy.txt deleted +0 -40

deleted file mode 100644
index e68ae5dec9c9..000000000000
@@ -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

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

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 <[email protected]>

---
 .../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

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

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.

Quoted 7 lines · Rafa? Mi?ecki

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

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").

Quoted 5 lines

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

Quoted 8 lines · Rafa? Mi?ecki

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

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").

Quoted 6 lines

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")?

Quoted 14 lines · Florian Fainelli

Quoted 12 lines · Rafa? Mi?ecki

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

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.

Quoted 2 lines

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.

Quoted 19 lines · Rafa? Mi?ecki

Quoted 18 lines · Florian Fainelli

Quoted 16 lines · Rafa? Mi?ecki

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

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.

Quoted 8 lines

Quoted 3 lines

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!

Quoted 30 lines · Florian Fainelli

Quoted 17 lines · Rafa? Mi?ecki

Quoted 16 lines · Florian Fainelli

Quoted 14 lines · Rafa? Mi?ecki

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

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 34 lines · Rafał Miłecki

Quoted 33 lines · Florian Fainelli

Quoted 17 lines · Rafał Miłecki

Quoted 16 lines · Florian Fainelli

Quoted 14 lines · Rafał Miłecki

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

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 7 lines

Quoted 5 lines

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.

Quoted 9 lines

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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 2 lines

I vote for focusing on existing driver improvements instead of looking

for

Quoted 4 lines

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

Quoted 89 lines · Florian Fainelli

Quoted 60 lines · Rafa? Mi?ecki

Quoted 33 lines · Florian Fainelli

Quoted 19 lines · Rafa? Mi?ecki

Quoted 18 lines · Florian Fainelli

Quoted 16 lines · Rafa? Mi?ecki

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

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 8 lines

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).

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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 <[email protected]>

Quoted 98 lines · Jon Mason

Quoted 95 lines · Florian Fainelli

Quoted 60 lines · Rafa? Mi?ecki

Quoted 33 lines · Florian Fainelli

Quoted 19 lines · Rafa? Mi?ecki

Quoted 18 lines · Florian Fainelli

Quoted 16 lines · Rafa? Mi?ecki

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

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 8 lines

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).

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

Per the discussion with Rafal, this is acceptable

Acked-by Jon Mason <[email protected]>

Hi Kishon, what's the status of this?

Hi Rafal,

Quoted 99 lines · Rafa? Mi?ecki

Quoted 98 lines · Jon Mason

Quoted 95 lines · Florian Fainelli

Quoted 60 lines · Rafa? Mi?ecki

Quoted 33 lines · Florian Fainelli

Quoted 19 lines · Rafa? Mi?ecki

Quoted 18 lines · Florian Fainelli

Quoted 16 lines · Rafa? Mi?ecki

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

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 8 lines

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).

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 5 lines

Quoted 3 lines

Per the discussion with Rafal, this is acceptable

Acked-by Jon Mason <[email protected]>

Hi Kishon, what's the status of this?

Will be merging this in a day or so.

Thanks
Kishon

Quoted 104 lines · Kishon Vijay Abraham I

Hi Rafal,

Quoted 101 lines · Rafa? Mi?ecki

Quoted 100 lines · Jon Mason

Quoted 89 lines · Florian Fainelli

Quoted 60 lines · Rafa? Mi?ecki

Quoted 33 lines · Florian Fainelli

Quoted 19 lines · Rafa? Mi?ecki

Quoted 18 lines · Florian Fainelli

Quoted 16 lines · Rafa? Mi?ecki

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

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 8 lines

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).

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Acked-by Jon Mason <[email protected]>

Hi Kishon, what's the status of this?

Will be merging this in a day or so.

merged, thanks.

Quoted 50 lines · Rafa? Mi?ecki

Quoted 30 lines · Florian Fainelli

Quoted 17 lines · Rafa? Mi?ecki

Quoted 16 lines · Florian Fainelli

Quoted 14 lines · Rafa? Mi?ecki

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

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 6 lines

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