usb: phy: mv_u3d: Remove usb phy driver for mv_u3d

PATCH In review Paul Bolle 2 participants 6 messages Apr 15, 2014 – Apr 17, 2014 USB

Share & tools

Paul Bolle

author

2014-04-15 11:36 UTC #0 lore raw

PATCH 1/1 usb: phy: mv_u3d: Remove usb phy driver for mv_u3d

The usb phy driver for mv_u3d got added in v3.7 through commit a67e76ac904c ("usb: phy: mv_u3d: Add usb phy driver for mv_u3d"). It then depended on USB_MV_U3D. And that symbol depended
on CPU_MMP3 at that time. But CPU_MMP3 has never been part of the tree. This means that this drive was unbuildable when it was added.

In commit 60630c2eabd4 ("usb: gadget: mv_u3d: drop ARCH dependency") MV_U3D_PHY was made depended directly on CPU_MMP3. That kept it unbuildable, of course.

Remove this driver. It can be re-added once its dependencies are part of the tree.

Signed-off-by Paul Bolle <[email protected]>

---
Tested with git grep.

This was triggered by Richard's "[PATCH 01/28] Remove CPU_MMP3" of two months ago, which I acked to eagerly. See
https://lkml.org/lkml/2014/2/11/714 for the details.

 drivers/usb/phy/Kconfig          |   8 -
 drivers/usb/phy/Makefile         |   1 -
 drivers/usb/phy/phy-mv-u3d-usb.c | 338 ---------------------------------------
 drivers/usb/phy/phy-mv-u3d-usb.h | 105 ------------
 4 files changed, 452 deletions(-)
 delete mode 100644 drivers/usb/phy/phy-mv-u3d-usb.c
 delete mode 100644 drivers/usb/phy/phy-mv-u3d-usb.h

drivers/usb/phy/Kconfig +0 -8

index 416e0c8cf6ff..33dd6a6c320a 100644
@@ -59,14 +59,6 @@ config KEYSTONE_USB_PHY
 	  interface to interact with USB 2.0 and USB 3.0 PHY that is part
 	  of the Keystone SOC.
 
-config MV_U3D_PHY
-	bool "Marvell USB 3.0 PHY controller Driver"
-	depends on CPU_MMP3
-	select USB_PHY
-	help
-	  Enable this to support Marvell USB 3.0 phy controller for Marvell
-	  SoC.
-
 config NOP_USB_XCEIV
 	tristate "NOP USB Transceiver Driver"
 	select USB_PHY

drivers/usb/phy/Makefile +0 -1

index f8fa719a31b9..a2d05690d925 100644
@@ -10,7 +10,6 @@ obj-$(CONFIG_USB_OTG_FSM)		+= phy-fsm-usb.o
 obj-$(CONFIG_AB8500_USB)		+= phy-ab8500-usb.o
 obj-$(CONFIG_FSL_USB2_OTG)		+= phy-fsl-usb.o
 obj-$(CONFIG_ISP1301_OMAP)		+= phy-isp1301-omap.o
-obj-$(CONFIG_MV_U3D_PHY)		+= phy-mv-u3d-usb.o
 obj-$(CONFIG_NOP_USB_XCEIV)		+= phy-generic.o
 obj-$(CONFIG_TAHVO_USB)			+= phy-tahvo.o
 obj-$(CONFIG_AM335X_CONTROL_USB)	+= phy-am335x-control.o

drivers/usb/phy/phy-mv-u3d-usb.c deleted +0 ~~-338~~

338 lines not on this page · raw message

drivers/usb/phy/phy-mv-u3d-usb.h deleted +0 ~~-105~~

105 lines not on this page · raw message

Signature · 1 line

1.9.0

Felipe Balbi

2014-04-15 17:25 UTC #1 lore raw

Quoted 13 lines · Paul Bolle

The usb phy driver for mv_u3d got added in v3.7 through commit a67e76ac904c ("usb: phy: mv_u3d: Add usb phy driver for mv_u3d"). It then depended on USB_MV_U3D. And that symbol depended
on CPU_MMP3 at that time. But CPU_MMP3 has never been part of the tree. This means that this drive was unbuildable when it was added.
In commit 60630c2eabd4 ("usb: gadget: mv_u3d: drop ARCH dependency") MV_U3D_PHY was made depended directly on CPU_MMP3. That kept it unbuildable, of course.
Remove this driver. It can be re-added once its dependencies are part of the tree.

Quoted 7 lines

Signed-off-by Paul Bolle <[email protected]>
---
Tested with git grep.
This was triggered by Richard's "[PATCH 01/28] Remove CPU_MMP3" of two months ago, which I acked to eagerly. See
https://lkml.org/lkml/2014/2/11/714 for the details.

so this means that drivers/usb/gadget/mv_u3d_core.c isn't used either ? Instead of deleting this and introducing a new driver, why don't you just help fix what's already in-tree ?

Signature · 1 line

balbi

Paul Bolle

author

2014-04-15 18:08 UTC #2 lore raw

Quoted 1 line · Felipe Balbi

so this means that drivers/usb/gadget/mv_u3d_core.c isn't used either ?

Why should it? There's no dependency on CPU_MMP3 for USB_MV_U3D anymore, is there?

Quoted 2 lines

Instead of deleting this and introducing a new driver, why don't you just help fix what's already in-tree ?

Were any of the reasons I gave for removing this driver incorrect? Has it actually ever been possible to build it?

Paul Bolle

Felipe Balbi

2014-04-16 04:37 UTC #3 lore raw

Quoted 5 lines · Paul Bolle

Quoted 1 line · Felipe Balbi
so this means that drivers/usb/gadget/mv_u3d_core.c isn't used either ?
Why should it? There's no dependency on CPU_MMP3 for USB_MV_U3D anymore, is there?

no, but the UDC needs its PHY driver.

Quoted 5 lines

Quoted 2 lines
Instead of deleting this and introducing a new driver, why don't you just help fix what's already in-tree ?
Were any of the reasons I gave for removing this driver incorrect? Has it actually ever been possible to build it?

I don't know, let me check:

$ make drivers/usb/phy/phy-mv-u3d-usb.o
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
make[1]: `include/generated/mach-types.h' is up to date.
  CALL    scripts/checksyscalls.sh
  CC      drivers/usb/phy/phy-mv-u3d-usb.o

yup, builds just fine. Even if the ARCH support isn't in place, this driver is *not* breaking anything, it's not preventing anyone from getting work done and it might be helping Marvell decrease the amount of changes they keep out of tree.

I don't see any problems this driver in tree as long as there are people working on it and I see the latest commit was 10 days ago, it wouldn't be fair to Marvell to delete their driver if they're still finding ways to make it useful one way or another.

cheers

Signature · 1 line

balbi

Paul Bolle

author

2014-04-17 07:50 UTC #4 lore raw

Quoted 7 lines · Felipe Balbi

$ make drivers/usb/phy/phy-mv-u3d-usb.o
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
make[1]: `include/generated/mach-types.h' is up to date.
  CALL    scripts/checksyscalls.sh
  CC      drivers/usb/phy/phy-mv-u3d-usb.o

(On x86_64 this manual make command triggers the error Greg reported in
https://lkml.org/lkml/2014/2/11/693 .)

Quoted 9 lines

yup, builds just fine. Even if the ARCH support isn't in place, this driver is *not* breaking anything, it's not preventing anyone from getting work done and it might be helping Marvell decrease the amount of changes they keep out of tree.
I don't see any problems this driver in tree as long as there are people working on it and I see the latest commit was 10 days ago, it wouldn't be fair to Marvell to delete their driver if they're still finding ways to make it useful one way or another.

Wouldn't it then be better if this driver is at least hooked into the build system? Like, say, this:
config MV_U3D_PHY
bool "Marvell USB 3.0 PHY controller Driver"
# XXX should depend on CPU_MMP3 when support for MMP3 lands depends on ARM && COMPILE_TEST
select USB_PHY
help
Enable this to support Marvell USB 3.0 phy controller for Marvell SoC.

(Or perhaps only ARCH_MMP && COMPILE_TEST.) That should give it build coverage by the automated tests people appear to run, while work is done on adding MMP3 support.

Thanks,

Paul Bolle

Felipe Balbi

2014-04-17 16:31 UTC #5 lore raw

Quoted 33 lines · Paul Bolle

Quoted 7 lines · Felipe Balbi
$ make drivers/usb/phy/phy-mv-u3d-usb.o
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
make[1]: `include/generated/mach-types.h' is up to date.
  CALL    scripts/checksyscalls.sh
  CC      drivers/usb/phy/phy-mv-u3d-usb.o
(On x86_64 this manual make command triggers the error Greg reported in
https://lkml.org/lkml/2014/2/11/693 .)
Quoted 9 lines
yup, builds just fine. Even if the ARCH support isn't in place, this driver is *not* breaking anything, it's not preventing anyone from getting work done and it might be helping Marvell decrease the amount of changes they keep out of tree.
I don't see any problems this driver in tree as long as there are people working on it and I see the latest commit was 10 days ago, it wouldn't be fair to Marvell to delete their driver if they're still finding ways to make it useful one way or another.
Wouldn't it then be better if this driver is at least hooked into the build system? Like, say, this:
config MV_U3D_PHY
bool "Marvell USB 3.0 PHY controller Driver"
# XXX should depend on CPU_MMP3 when support for MMP3 lands depends on ARM && COMPILE_TEST
select USB_PHY
help
Enable this to support Marvell USB 3.0 phy controller for Marvell SoC.

Quoted 3 lines

(Or perhaps only ARCH_MMP && COMPILE_TEST.) That should give it build coverage by the automated tests people appear to run, while work is done on adding MMP3 support.

I guess we should make it depend on BROKEN instead, or we just apply the patch below:

8<---------------------------------------------------------------------

From 60e524d7db4ed04624850fcb84a844c609a960e3 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <[email protected]>
Date: Thu, 17 Apr 2014 11:24:27 -0500
Subject: [PATCH] usb: phy: mv-u3d: switch over to writel/readl

by removing the _relaxed suffix, we can build
this driver in other architectures.

Signed-off-by Felipe Balbi <[email protected]>

---
 drivers/usb/phy/phy-mv-u3d-usb.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

drivers/usb/phy/phy-mv-u3d-usb.c +10 ~~-10~~

index d317903..d342175 100644
@@ -39,8 +39,8 @@ static u32 mv_u3d_phy_read(void __iomem *base, u32 reg)
 	addr = base;
 	data = base + 0x4;
 
-	writel_relaxed(reg, addr);
-	return readl_relaxed(data);
+	writel(reg, addr);
+	return readl(data);
 }
 
 static void mv_u3d_phy_set(void __iomem *base, u32 reg, u32 value)

@@ -51,10 +51,10 @@ static void mv_u3d_phy_set(void __iomem *base, u32 reg, u32 value)
 	addr = base;
 	data = base + 0x4;
 
-	writel_relaxed(reg, addr);
-	tmp = readl_relaxed(data);
+	writel(reg, addr);
+	tmp = readl(data);
 	tmp |= value;
-	writel_relaxed(tmp, data);
+	writel(tmp, data);
 }
 
 static void mv_u3d_phy_clear(void __iomem *base, u32 reg, u32 value)

@@ -65,10 +65,10 @@ static void mv_u3d_phy_clear(void __iomem *base, u32 reg, u32 value)
 	addr = base;
 	data = base + 0x4;
 
-	writel_relaxed(reg, addr);
-	tmp = readl_relaxed(data);
+	writel(reg, addr);
+	tmp = readl(data);
 	tmp &= ~value;
-	writel_relaxed(tmp, data);
+	writel(tmp, data);
 }
 
 static void mv_u3d_phy_write(void __iomem *base, u32 reg, u32 value)

@@ -78,8 +78,8 @@ static void mv_u3d_phy_write(void __iomem *base, u32 reg, u32 value)
 	addr = base;
 	data = base + 0x4;
 
-	writel_relaxed(reg, addr);
-	writel_relaxed(value, data);
+	writel(reg, addr);
+	writel(value, data);
 }
 
 static void mv_u3d_phy_shutdown(struct usb_phy *phy)

Signature · 5 lines

1.9.2.459.g68773ac


-- 
balbi