mbox series

[0/3] Revert "usb: phy: add usb phy notify port status API"

Message ID 20231106110654.31090-1-johan+linaro@kernel.org (mailing list archive)
Headers show
Series Revert "usb: phy: add usb phy notify port status API" | expand

Message

Johan Hovold Nov. 6, 2023, 11:06 a.m. UTC 
The recently added Realtek PHY drivers depend on the new port status
notification mechanism which was built on the deprecated USB PHY
implementation and devicetree binding.

Specifically, using these PHYs would require describing the very same
PHY using both the generic "phy" property and the deprecated "usb-phy"
property which is clearly wrong.

We should not be building new functionality on top of the legacy USB PHY
implementation even if it is currently stuck in some kind of
transitional limbo.

Revert the new Realtek PHY drivers for now so that the port status
notification interface can be reverted and replaced before we dig
ourselves into an even deeper hole with this PHY mess.

Note that there are no upstream users of these PHYs and the drivers were
only included in 6.6 so there should still be time to undo this.

Preferably these should go in through Greg's tree for 6.7-rc1.

Johan


Johan Hovold (3):
  Revert "phy: realtek: usb: Add driver for the Realtek SoC USB 3.0 PHY"
  Revert "phy: realtek: usb: Add driver for the Realtek SoC USB 2.0 PHY"
  Revert "usb: phy: add usb phy notify port status API"

 drivers/phy/Kconfig                |    1 -
 drivers/phy/Makefile               |    1 -
 drivers/phy/realtek/Kconfig        |   32 -
 drivers/phy/realtek/Makefile       |    3 -
 drivers/phy/realtek/phy-rtk-usb2.c | 1325 ----------------------------
 drivers/phy/realtek/phy-rtk-usb3.c |  761 ----------------
 drivers/usb/core/hub.c             |   23 -
 include/linux/usb/phy.h            |   13 -
 8 files changed, 2159 deletions(-)
 delete mode 100644 drivers/phy/realtek/Kconfig
 delete mode 100644 drivers/phy/realtek/Makefile
 delete mode 100644 drivers/phy/realtek/phy-rtk-usb2.c
 delete mode 100644 drivers/phy/realtek/phy-rtk-usb3.c

Comments

Greg KH Nov. 6, 2023, 11:15 a.m. UTC | #1 
On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> The recently added Realtek PHY drivers depend on the new port status
> notification mechanism which was built on the deprecated USB PHY
> implementation and devicetree binding.
> 
> Specifically, using these PHYs would require describing the very same
> PHY using both the generic "phy" property and the deprecated "usb-phy"
> property which is clearly wrong.
> 
> We should not be building new functionality on top of the legacy USB PHY
> implementation even if it is currently stuck in some kind of
> transitional limbo.
> 
> Revert the new Realtek PHY drivers for now so that the port status
> notification interface can be reverted and replaced before we dig
> ourselves into an even deeper hole with this PHY mess.
> 
> Note that there are no upstream users of these PHYs and the drivers were
> only included in 6.6 so there should still be time to undo this.

No users of these phy drivers yet?  Why were they added?

> Preferably these should go in through Greg's tree for 6.7-rc1.

I'll be glad to take this if I can get an ack for it.

thanks,

greg k-h
Vinod Koul Nov. 6, 2023, 11:24 a.m. UTC | #2 
On 06-11-23, 12:15, Greg Kroah-Hartman wrote:
> On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> > The recently added Realtek PHY drivers depend on the new port status
> > notification mechanism which was built on the deprecated USB PHY
> > implementation and devicetree binding.
> > 
> > Specifically, using these PHYs would require describing the very same
> > PHY using both the generic "phy" property and the deprecated "usb-phy"
> > property which is clearly wrong.
> > 
> > We should not be building new functionality on top of the legacy USB PHY
> > implementation even if it is currently stuck in some kind of
> > transitional limbo.
> > 
> > Revert the new Realtek PHY drivers for now so that the port status
> > notification interface can be reverted and replaced before we dig
> > ourselves into an even deeper hole with this PHY mess.
> > 
> > Note that there are no upstream users of these PHYs and the drivers were
> > only included in 6.6 so there should still be time to undo this.
> 
> No users of these phy drivers yet?  Why were they added?

Not sure why, they didnt go thru phy ss!

> 
> > Preferably these should go in through Greg's tree for 6.7-rc1.
> 
> I'll be glad to take this if I can get an ack for it.

Pls do drop this:

Acked-by: Vinod Koul <vkoul@kernel.org>
Johan Hovold Nov. 6, 2023, 11:25 a.m. UTC | #3 
On Mon, Nov 06, 2023 at 12:15:59PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> > The recently added Realtek PHY drivers depend on the new port status
> > notification mechanism which was built on the deprecated USB PHY
> > implementation and devicetree binding.
> > 
> > Specifically, using these PHYs would require describing the very same
> > PHY using both the generic "phy" property and the deprecated "usb-phy"
> > property which is clearly wrong.
> > 
> > We should not be building new functionality on top of the legacy USB PHY
> > implementation even if it is currently stuck in some kind of
> > transitional limbo.
> > 
> > Revert the new Realtek PHY drivers for now so that the port status
> > notification interface can be reverted and replaced before we dig
> > ourselves into an even deeper hole with this PHY mess.
> > 
> > Note that there are no upstream users of these PHYs and the drivers were
> > only included in 6.6 so there should still be time to undo this.
> 
> No users of these phy drivers yet?  Why were they added?

The devicetree bindings were also merged in 6.6 (and are left in place),
but there are no devicetrees that actually use these new bindings in
mainline yet.

> > Preferably these should go in through Greg's tree for 6.7-rc1.
> 
> I'll be glad to take this if I can get an ack for it.

They went in through your tree, but note that you may now get a conflict
with 

	7e909370a5cd ("phy: realtek: Replace of_device.h with explicit includes")

in the phy tree.

Johan
Greg KH Nov. 6, 2023, 1:48 p.m. UTC | #4 
On Mon, Nov 06, 2023 at 04:54:45PM +0530, Vinod Koul wrote:
> On 06-11-23, 12:15, Greg Kroah-Hartman wrote:
> > On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> > > The recently added Realtek PHY drivers depend on the new port status
> > > notification mechanism which was built on the deprecated USB PHY
> > > implementation and devicetree binding.
> > > 
> > > Specifically, using these PHYs would require describing the very same
> > > PHY using both the generic "phy" property and the deprecated "usb-phy"
> > > property which is clearly wrong.
> > > 
> > > We should not be building new functionality on top of the legacy USB PHY
> > > implementation even if it is currently stuck in some kind of
> > > transitional limbo.
> > > 
> > > Revert the new Realtek PHY drivers for now so that the port status
> > > notification interface can be reverted and replaced before we dig
> > > ourselves into an even deeper hole with this PHY mess.
> > > 
> > > Note that there are no upstream users of these PHYs and the drivers were
> > > only included in 6.6 so there should still be time to undo this.
> > 
> > No users of these phy drivers yet?  Why were they added?
> 
> Not sure why, they didnt go thru phy ss!
> 
> > 
> > > Preferably these should go in through Greg's tree for 6.7-rc1.
> > 
> > I'll be glad to take this if I can get an ack for it.
> 
> Pls do drop this:
> 
> Acked-by: Vinod Koul <vkoul@kernel.org>

Thanks, now applied.

greg k-h
Greg KH Nov. 6, 2023, 1:48 p.m. UTC | #5 
On Mon, Nov 06, 2023 at 12:25:34PM +0100, Johan Hovold wrote:
> On Mon, Nov 06, 2023 at 12:15:59PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> > > The recently added Realtek PHY drivers depend on the new port status
> > > notification mechanism which was built on the deprecated USB PHY
> > > implementation and devicetree binding.
> > > 
> > > Specifically, using these PHYs would require describing the very same
> > > PHY using both the generic "phy" property and the deprecated "usb-phy"
> > > property which is clearly wrong.
> > > 
> > > We should not be building new functionality on top of the legacy USB PHY
> > > implementation even if it is currently stuck in some kind of
> > > transitional limbo.
> > > 
> > > Revert the new Realtek PHY drivers for now so that the port status
> > > notification interface can be reverted and replaced before we dig
> > > ourselves into an even deeper hole with this PHY mess.
> > > 
> > > Note that there are no upstream users of these PHYs and the drivers were
> > > only included in 6.6 so there should still be time to undo this.
> > 
> > No users of these phy drivers yet?  Why were they added?
> 
> The devicetree bindings were also merged in 6.6 (and are left in place),
> but there are no devicetrees that actually use these new bindings in
> mainline yet.
> 
> > > Preferably these should go in through Greg's tree for 6.7-rc1.
> > 
> > I'll be glad to take this if I can get an ack for it.
> 
> They went in through your tree, but note that you may now get a conflict
> with 
> 
> 	7e909370a5cd ("phy: realtek: Replace of_device.h with explicit includes")
> 
> in the phy tree.

I fixed it up by hand, should be good now, thanks.

greg k-h
Stanley Chang[昌育德] Nov. 7, 2023, 6:44 a.m. UTC | #6 
Hi Johan and Vinod,

I modified the Realtek phy to solve this issue and only use the generic PHY.
And submitted these patches today as follows
https://lore.kernel.org/linux-usb/20231107063518.27824-1-stanley_chang@realtek.com/
https://lore.kernel.org/linux-usb/20231107063518.27824-2-stanley_chang@realtek.com/
https://lore.kernel.org/linux-usb/20231107063518.27824-3-stanley_chang@realtek.com/
https://lore.kernel.org/linux-usb/20231107063518.27824-4-stanley_chang@realtek.com/

I don't think this patch is needed to revert a08799cf17c2 ("usb:phy: New usb phy notification port status API").

Thanks,
Stanley

> On 06-11-23, 12:15, Greg Kroah-Hartman wrote:
> > On Mon, Nov 06, 2023 at 12:06:51PM +0100, Johan Hovold wrote:
> > > The recently added Realtek PHY drivers depend on the new port status
> > > notification mechanism which was built on the deprecated USB PHY
> > > implementation and devicetree binding.
> > >
> > > Specifically, using these PHYs would require describing the very
> > > same PHY using both the generic "phy" property and the deprecated
> "usb-phy"
> > > property which is clearly wrong.
> > >
> > > We should not be building new functionality on top of the legacy USB
> > > PHY implementation even if it is currently stuck in some kind of
> > > transitional limbo.
> > >
> > > Revert the new Realtek PHY drivers for now so that the port status
> > > notification interface can be reverted and replaced before we dig
> > > ourselves into an even deeper hole with this PHY mess.
> > >
> > > Note that there are no upstream users of these PHYs and the drivers
> > > were only included in 6.6 so there should still be time to undo this.
> >
> > No users of these phy drivers yet?  Why were they added?
> 
> Not sure why, they didnt go thru phy ss!
> 
> >
> > > Preferably these should go in through Greg's tree for 6.7-rc1.
> >
> > I'll be glad to take this if I can get an ack for it.
> 
> Pls do drop this:
> 
> Acked-by: Vinod Koul <vkoul@kernel.org>
> 
> --
> ~Vinod
Greg KH Nov. 7, 2023, 7:02 a.m. UTC | #7 
On Tue, Nov 07, 2023 at 06:44:26AM +0000, Stanley Chang[昌育德] wrote:
> Hi Johan and Vinod,
> 
> I modified the Realtek phy to solve this issue and only use the generic PHY.
> And submitted these patches today as follows
> https://lore.kernel.org/linux-usb/20231107063518.27824-1-stanley_chang@realtek.com/
> https://lore.kernel.org/linux-usb/20231107063518.27824-2-stanley_chang@realtek.com/
> https://lore.kernel.org/linux-usb/20231107063518.27824-3-stanley_chang@realtek.com/
> https://lore.kernel.org/linux-usb/20231107063518.27824-4-stanley_chang@realtek.com/
> 
> I don't think this patch is needed to revert a08799cf17c2 ("usb:phy: New usb phy notification port status API").

I had already applied those reverts yesterday, but forgot to push them
out (sorry about that, now fixed.)  Let's start over here and you can
rebase your new series on the 6.7-rc1.

thanks,

greg k-h
Stanley Chang[昌育德] Nov. 7, 2023, 7:54 a.m. UTC | #8 
Hi Greg,

> On Tue, Nov 07, 2023 at 06:44:26AM +0000, Stanley Chang[昌育德] wrote:
> > Hi Johan and Vinod,
> >
> > I modified the Realtek phy to solve this issue and only use the generic PHY.
> > And submitted these patches today as follows
> > https://lore.kernel.org/linux-usb/20231107063518.27824-1-stanley_chang
> > @realtek.com/
> > https://lore.kernel.org/linux-usb/20231107063518.27824-2-stanley_chang
> > @realtek.com/
> > https://lore.kernel.org/linux-usb/20231107063518.27824-3-stanley_chang
> > @realtek.com/
> > https://lore.kernel.org/linux-usb/20231107063518.27824-4-stanley_chang
> > @realtek.com/
> >
> > I don't think this patch is needed to revert a08799cf17c2 ("usb:phy: New usb
> phy notification port status API").
> 
> I had already applied those reverts yesterday, but forgot to push them out
> (sorry about that, now fixed.)  Let's start over here and you can rebase your
> new series on the 6.7-rc1.

Okay, I will resubmit later.

Thanks,
Stanley

> thanks,
> 
> greg k-h