| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150120152337.GA8988@saruman>
Date: Tue, 20 Jan 2015 09:23:37 -0600
From: Felipe Balbi <balbi@...com>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
CC: Kishon Vijay Abraham I <kishon@...com>,
Baolu Lu <baolu.lu@...ux.intel.com>,
<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Felipe Balbi <balbi@...com>
Subject: Re: [PATCH 2/3] usb: dwc3: add ULPI interface support
Hi,
On Tue, Jan 20, 2015 at 11:18:21AM +0200, Heikki Krogerus wrote:
> Registers ULPI interface with the ULPI bus if HSPHY type is
> ULPI.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Cc: Felipe Balbi <balbi@...com>
you're doing quite a bit in a single patch...
> ---
> drivers/usb/dwc3/Kconfig | 7 ++++
> drivers/usb/dwc3/Makefile | 4 ++
> drivers/usb/dwc3/core.c | 9 +++-
> drivers/usb/dwc3/core.h | 22 ++++++++++
> drivers/usb/dwc3/ulpi.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 143 insertions(+), 1 deletion(-)
> create mode 100644 drivers/usb/dwc3/ulpi.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 58b5b2c..6d0c5e6 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -11,6 +11,13 @@ config USB_DWC3
>
> if USB_DWC3
>
> +config USB_DWC3_ULPI
> + bool "Provide ULPI PHY Interface"
> + depends on ULPI_PHY=y || ULPI_PHY=USB_DWC3
> + help
> + Select this if you have ULPI type PHY attached to your DWC3
> + controller.
> +
> choice
> bool "DWC3 Mode Selection"
> default USB_DWC3_DUAL_ROLE if (USB && USB_GADGET)
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index bb34fbc..2fc44e0 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -16,6 +16,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
> dwc3-y += gadget.o ep0.o
> endif
>
> +ifneq ($(CONFIG_USB_DWC3_ULPI),)
> + dwc3-y += ulpi.o
> +endif
> +
> ifneq ($(CONFIG_DEBUG_FS),)
> dwc3-y += debugfs.o
> endif
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25ddc39..5219bc7 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -876,12 +876,17 @@ static int dwc3_probe(struct platform_device *pdev)
> dwc->hird_threshold = hird_threshold
> | (dwc->is_utmi_l1_suspend << 4);
>
> + platform_set_drvdata(pdev, dwc);
> +
> + ret = dwc3_ulpi_init(dwc);
> + if (ret)
> + return ret;
> +
> ret = dwc3_core_get_phy(dwc);
> if (ret)
> return ret;
>
> spin_lock_init(&dwc->lock);
> - platform_set_drvdata(pdev, dwc);
why do you need to move this ? Looks like this should be a cleanup and
split into a single patch.
it also appears that you need another patch moving dwc3_cache_hwparams()
before all of these other calls, so you can use it from
dwc3_ulpi_init().
> @@ -965,6 +970,7 @@ err1:
>
> err0:
> dwc3_free_event_buffers(dwc);
> + dwc3_ulpi_exit(dwc);
>
> return ret;
> }
> @@ -984,6 +990,7 @@ static int dwc3_remove(struct platform_device *pdev)
> phy_power_off(dwc->usb3_generic_phy);
>
> dwc3_core_exit(dwc);
> + dwc3_ulpi_exit(dwc);
>
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 0842aa8..f6881a6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -32,6 +32,7 @@
> #include <linux/usb/otg.h>
>
> #include <linux/phy/phy.h>
> +#include <linux/phy/ulpi/interface.h>
>
> #define DWC3_MSG_MAX 500
>
> @@ -174,6 +175,14 @@
> #define DWC3_GUSB2PHYCFG_PHYSOFTRST (1 << 31)
> #define DWC3_GUSB2PHYCFG_SUSPHY (1 << 6)
>
> +/* Global USB2 PHY Vendor Control Register */
> +#define DWC3_GUSB2PHYACC_NEWREGREQ (1 << 25)
> +#define DWC3_GUSB2PHYACC_BUSY (1 << 23)
> +#define DWC3_GUSB2PHYACC_WRITE (1 << 22)
> +#define DWC3_GUSB2PHYACC_ADDR(n) (n << 16)
> +#define DWC3_GUSB2PHYACC_EXTEND_ADDR(n) (n << 8)
> +#define DWC3_GUSB2PHYACC_DATA(n) (n & 0xff)
separate patch
> @@ -590,6 +599,7 @@ struct dwc3_hwparams {
> #define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
>
> /* HWPARAMS3 */
> +#define DWC3_ULPI_HSPHY (1 << 3)
you also need a patch which defines this bit of HWPARAMS3. This is also
the wrong definition. Which core revision do you have ? I can see that
bit 3 is part of a 2 bit field called:
DWC_USB3_HSPHY_INTERFACE
moreover, there are systems which have both ULPI and UTMI enabled and
you can't really know which one the PHY is using.
This needs a bit more thought.
> #define DWC3_NUM_IN_EPS_MASK (0x1f << 18)
> #define DWC3_NUM_EPS_MASK (0x3f << 12)
> #define DWC3_NUM_EPS(p) (((p)->hwparams3 & \
> @@ -739,6 +749,8 @@ struct dwc3 {
> struct phy *usb2_generic_phy;
> struct phy *usb3_generic_phy;
>
> + struct ulpi *ulpi;
> +
> void __iomem *regs;
> size_t regs_size;
>
> @@ -1035,4 +1047,14 @@ static inline int dwc3_gadget_resume(struct dwc3 *dwc)
> }
> #endif /* !IS_ENABLED(CONFIG_USB_DWC3_HOST) */
>
> +#if IS_ENABLED(CONFIG_USB_DWC3_ULPI)
> +int dwc3_ulpi_init(struct dwc3 *dwc);
> +void dwc3_ulpi_exit(struct dwc3 *dwc);
> +#else
> +static inline int dwc3_ulpi_init(struct dwc3 *dwc)
> +{ return 0; }
> +static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{ }
> +#endif
> +
> #endif /* __DRIVERS_USB_DWC3_CORE_H */
> diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
> new file mode 100644
> index 0000000..ee3ebbe
> --- /dev/null
> +++ b/drivers/usb/dwc3/ulpi.c
> @@ -0,0 +1,102 @@
> +/**
> + * ulpi.c - DesignWare USB3 Controller's ULPI PHY interface
> + *
> + * Copyright (C) 2015 Intel Corporation
> + *
> + * Author: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/phy/ulpi/regs.h>
> +
> +#include "core.h"
> +#include "io.h"
> +
> +#define DWC3_ULPI_ADDR(a) \
> + ((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
> + DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
> + DWC3_GUSB2PHYACC_EXTEND_ADDR(a) : DWC3_GUSB2PHYACC_ADDR(a))
> +
> +static int dwc3_ulpi_busyloop(struct dwc3 *dwc)
> +{
> + unsigned count = 1000;
> + u32 reg;
> +
> + while (count--) {
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> + if (!(reg & DWC3_GUSB2PHYACC_BUSY))
> + return 0;
this could use a cpu_relax();
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static int dwc3_ulpi_read(struct ulpi_ops *ops, u8 addr)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> + u32 reg;
> + int ret;
> +
> + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> +
> + ret = dwc3_ulpi_busyloop(dwc);
> + if (ret)
> + return ret;
> +
> + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYACC(0));
> +
> + return DWC3_GUSB2PHYACC_DATA(reg);
> +}
> +
> +static int dwc3_ulpi_write(struct ulpi_ops *ops, u8 addr, u8 val)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(ops->dev);
> + u32 reg;
> + int ret;
> +
> + reg = DWC3_GUSB2PHYACC_NEWREGREQ | DWC3_ULPI_ADDR(addr);
> + reg |= DWC3_GUSB2PHYACC_WRITE | val;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYACC(0), reg);
> +
> + ret = dwc3_ulpi_busyloop(dwc);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static struct ulpi_ops dwc3_ulpi = {
> + .read = dwc3_ulpi_read,
> + .write = dwc3_ulpi_write,
> +};
> +
> +int dwc3_ulpi_init(struct dwc3 *dwc)
> +{
> + int ret;
> +
> + /* First check if there is ULPI PHY */
> + ret = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
> + if (!(ret & DWC3_ULPI_HSPHY))
> + return 0;
should use the cached structure.
> + /* Register the interface */
> + dwc->ulpi = ulpi_register_interface(dwc->dev, &dwc3_ulpi);
> + if (IS_ERR(dwc->ulpi)) {
> + dev_err(dwc->dev, "failed to register ULPI interface");
> + return PTR_ERR(dwc->ulpi);
> + }
> +
> + return 0;
> +}
> +
> +void dwc3_ulpi_exit(struct dwc3 *dwc)
> +{
> + if (dwc->ulpi) {
> + ulpi_unregister_interface(dwc->ulpi);
> + dwc->ulpi = NULL;
> + }
> +}
> --
> 2.1.4
>
--
balbi
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists