RE: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
-Original Message- From: Quadros, Roger Sent: Monday, December 09, 2013 10:09 PM To: Kwok, WingMan; linux-usb@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org; Shilimkar, Santosh; Balbi, Felipe; Greg Kroah-Hartman Subject: Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver On 12/10/2013 03:47 AM, WingMan Kwok wrote: Add Keystone platform USB PHY driver support. Current main purpose of this driver is to enable the PHY reference clock gate on the Keystone SoC. Otherwise it is a nop PHY. Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Felipe Balbi ba...@ti.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: WingMan Kwok w-kw...@ti.com --- drivers/usb/phy/Kconfig| 10 +++ drivers/usb/phy/Makefile |1 + drivers/usb/phy/phy-keystone.c | 142 3 files changed, 153 insertions(+) create mode 100644 drivers/usb/phy/phy-keystone.c diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 08e2f39..c6792f43 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -40,6 +40,16 @@ config ISP1301_OMAP This driver can also be built as a module. If so, the module will be called isp1301_omap. +config KEYSTONE_USB_PHY + tristate Keystone USB PHY Driver + depends on ARCH_KEYSTONE + select USB_PHY NOP_USB_XCEIV selects USB_PHY so not necessary. Yes I have fixed it and updated patch which I'll post. config AM335X_PHY_USB needs to be fixed as well. Regards WingMan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
On Thu, Dec 12, 2013 at 11:20:54AM -0600, Kwok, WingMan wrote: -Original Message- From: Shilimkar, Santosh Sent: Tuesday, December 10, 2013 10:15 AM To: Balbi, Felipe Cc: Kwok, WingMan; linux-usb@vger.kernel.org; linux-arm- ker...@lists.infradead.org; Greg Kroah-Hartman Subject: Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver On Monday 09 December 2013 09:54 PM, Balbi, Felipe wrote: Hi, On Mon, Dec 09, 2013 at 05:17:04PM -0500, WingMan Kwok wrote: +ret = usb_add_phy_dev(k_phy-usb_phy_gen.phy); +if (ret) +return ret; +k_phy-usb_phy_gen.phy.init = keystone_usbphy_init; +k_phy-usb_phy_gen.phy.shutdown = keystone_usbphy_shutdown; this *must* be initialized before adding the PHY to the subsystem. So these two lines must be moved before usb_add_phy_dev(). Make sense. Probably its good idea to repost the $subject patch with above as well as other delay related comment. Thanks. I have updated my patch accordingly. Please note that the same issue exists on drivers/usb/phy/phy-am335x.c also. If you want, I can send a fix for that. Will do, thanks for notifying me :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
On Monday 09 December 2013 09:54 PM, Balbi, Felipe wrote: Hi, On Mon, Dec 09, 2013 at 05:17:04PM -0500, WingMan Kwok wrote: +ret = usb_add_phy_dev(k_phy-usb_phy_gen.phy); +if (ret) +return ret; +k_phy-usb_phy_gen.phy.init = keystone_usbphy_init; +k_phy-usb_phy_gen.phy.shutdown = keystone_usbphy_shutdown; this *must* be initialized before adding the PHY to the subsystem. So these two lines must be moved before usb_add_phy_dev(). Make sense. Probably its good idea to repost the $subject patch with above as well as other delay related comment. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
On Monday 09 December 2013 11:47 PM, Felipe Balbi wrote:
Hi again,
On Mon, Dec 09, 2013 at 05:17:04PM -0500, WingMan Kwok wrote:
+static int keystone_usbphy_init(struct usb_phy *phy)
+{
+struct keystone_usbphy *k_phy = dev_get_drvdata(phy-dev);
+u32 val;
+
+val = keystone_usbphy_readl(k_phy-phy_ctrl, USB_PHY_CTL_CLOCK);
+keystone_usbphy_writel(k_phy-phy_ctrl, USB_PHY_CTL_CLOCK,
+val | PHY_REF_SSP_EN);
you need to enable this device's clock to access its registers right ?
Nope.. This clock is always running for CFG block where the phy control
is residing.
+udelay(20);
why the magic 20 usecs ? Where does that come from ? Empirically found
or is there a documentation reference ? At least add a comment there.
Above probably isn't needed either but good to check why was this added.
In refreshed patch, this can be either removed or a comment can be
added accordingly.
Thanks for spotting that.
Regards,
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
Hi, On Mon, Dec 09, 2013 at 05:17:04PM -0500, WingMan Kwok wrote: + ret = usb_add_phy_dev(k_phy-usb_phy_gen.phy); + if (ret) + return ret; + k_phy-usb_phy_gen.phy.init = keystone_usbphy_init; + k_phy-usb_phy_gen.phy.shutdown = keystone_usbphy_shutdown; this *must* be initialized before adding the PHY to the subsystem. So these two lines must be moved before usb_add_phy_dev(). -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
On 12/10/2013 03:47 AM, WingMan Kwok wrote: Add Keystone platform USB PHY driver support. Current main purpose of this driver is to enable the PHY reference clock gate on the Keystone SoC. Otherwise it is a nop PHY. Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Felipe Balbi ba...@ti.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: WingMan Kwok w-kw...@ti.com --- drivers/usb/phy/Kconfig| 10 +++ drivers/usb/phy/Makefile |1 + drivers/usb/phy/phy-keystone.c | 142 3 files changed, 153 insertions(+) create mode 100644 drivers/usb/phy/phy-keystone.c diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 08e2f39..c6792f43 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -40,6 +40,16 @@ config ISP1301_OMAP This driver can also be built as a module. If so, the module will be called isp1301_omap. +config KEYSTONE_USB_PHY + tristate Keystone USB PHY Driver + depends on ARCH_KEYSTONE + select USB_PHY NOP_USB_XCEIV selects USB_PHY so not necessary. + select NOP_USB_XCEIV + help + Enable this to support Keystone USB phy. This driver provides + 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 diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 022c1da..311b47b 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_USB_RCAR_PHY) += phy-rcar-usb.o obj-$(CONFIG_USB_RCAR_GEN2_PHY) += phy-rcar-gen2-usb.o obj-$(CONFIG_USB_ULPI) += phy-ulpi.o obj-$(CONFIG_USB_ULPI_VIEWPORT) += phy-ulpi-viewport.o +obj-$(CONFIG_KEYSTONE_USB_PHY) += phy-keystone.o cheers, -roger -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] usb: phy: Add keystone usb phy driver
Hi again,
On Mon, Dec 09, 2013 at 05:17:04PM -0500, WingMan Kwok wrote:
+static int keystone_usbphy_init(struct usb_phy *phy)
+{
+ struct keystone_usbphy *k_phy = dev_get_drvdata(phy-dev);
+ u32 val;
+
+ val = keystone_usbphy_readl(k_phy-phy_ctrl, USB_PHY_CTL_CLOCK);
+ keystone_usbphy_writel(k_phy-phy_ctrl, USB_PHY_CTL_CLOCK,
+ val | PHY_REF_SSP_EN);
you need to enable this device's clock to access its registers right ?
+ udelay(20);
why the magic 20 usecs ? Where does that come from ? Empirically found
or is there a documentation reference ? At least add a comment there.
--
balbi
signature.asc
Description: Digital signature
