On Thursday, December 10, 2015 at 10:41:41 PM, Mateusz Kulikowski wrote:
This driver is able to reconfigure OTG controller into HOST mode. Board can add board-specific initialization as board_prepare_usb(). It requires USB_ULPI_VIEWPORT enabled in board configuration.
Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com
Hi,
minor nits below.
[...]
diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c new file mode 100644 index 0000000..d17a29a --- /dev/null +++ b/drivers/usb/host/ehci-msm.c @@ -0,0 +1,198 @@ +/*
- Qualcomm EHCI driver
- (C) Copyright 2015 Mateusz Kulikowski mateusz.kulikowski@gmail.com
- Based on Linux driver
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/gpio.h> +#include <asm-generic/errno.h> +#include <linux/compat.h> +#include <dm.h> +#include <fdtdec.h> +#include <libfdt.h> +#include <asm/io.h> +#include <usb.h> +#include <usb/ulpi.h> +#include "ehci.h"
+#ifndef CONFIG_USB_ULPI_VIEWPORT +#error Please enable CONFIG_USB_ULPI_VIEWPORT +#endif
The driver should select this in Kconfig instead of this check, right ?
+#define MSM_USB_ULPI_OFFSET 0x170 /* ULPI viewport (PHY) */ +#define MSM_USB_EHCI_OFFSET 0x100 /* Start of EHCI registers */
+/* PHY viewport regs */ +#define ULPI_MISC_A_READ 0x96 +#define ULPI_MISC_A_SET 0x97 +#define ULPI_MISC_A_CLEAR 0x98 +#define ULPI_MISC_A_VBUSVLDEXTSEL (1 << 1) +#define ULPI_MISC_A_VBUSVLDEXT (1 << 0)
+/* qcom specific registers (OTG) */ +#define USB_GENCONFIG_2 0x00A0 +#define GEN2_SESS_VLD_CTRL_EN (1 << 7)
+#define USB_USBCMD (0x0140)
Please drop the parenthesis.
btw. this register layout looks very similar to struct usb_ehci in include/usb/ehci-fsl.h , can the header be made more universal to cover your driver as well ? Then these macros here won't be needed.
+#define SESS_VLD_CTRL (1 << 25) +#define USBCMD_RESET 2 +#define USBCMD_ATTACH 1
+/* USB2_HSIC_USB_OTG_HS_BASE_USB_OTG_HS_PORTSC */ +#define USB_PORTSC 0x0184 +#define USB_SBUSCFG 0x0090 +#define USB_AHB_MODE 0x0098
+#define USB_USBMODE 0x01A8 +#define USBMODE_DEVICE 2 +#define USBMODE_HOST 3
+struct msm_ehci_priv {
- struct ehci_ctrl ctrl; /* Needed by EHCI */
- phys_addr_t base;
- phys_addr_t ehci_base;
- u32 ulpi_base;
- u32 ulpi_port;
+};
+int __weak board_prepare_usb(enum usb_init_type type) +{
- return 0;
+}
+static void setup_usb_phy(struct msm_ehci_priv *priv) +{
- struct ulpi_viewport ulpi_vp = {.port_num = priv->ulpi_port,
.viewport_addr = priv->ulpi_base};- /* Select and enable external configuration with USB PHY */
- ulpi_write(&ulpi_vp, (u8 *)ULPI_MISC_A_SET,
ULPI_MISC_A_VBUSVLDEXTSEL | ULPI_MISC_A_VBUSVLDEXT);+}
+static void reset_usb_phy(struct msm_ehci_priv *priv) +{
- struct ulpi_viewport ulpi_vp = {.port_num = priv->ulpi_port,
.viewport_addr = priv->ulpi_base};- /* Disable VBUS mimicing in the controller. */
- ulpi_write(&ulpi_vp, (u8 *)ULPI_MISC_A_CLEAR,
This should be a pointer to a field in struct ulpi_regs, so the (u8 *) cast does not seem right. See for example ehci-zynq.c
ULPI_MISC_A_VBUSVLDEXTSEL | ULPI_MISC_A_VBUSVLDEXT);+}
[...]
+static int ehci_usb_remove(struct udevice *dev) +{
- struct msm_ehci_priv *p = dev_get_priv(dev);
- phys_addr_t reg = p->base + USB_USBCMD;
- int ret;
- ret = ehci_deregister(dev);
- if (ret)
return ret;- /* Stop controller. */
- writel(readl(reg) & ~USBCMD_ATTACH, reg);
This should use clrbits_le32() instead.
- reset_usb_phy(p);
- ret = board_prepare_usb(USB_INIT_DEVICE); /* Board specific hook */
- if (ret < 0)
return ret;- /* Reset controller */
- writel(0x00080002, reg); /* reset usb */
- mdelay(20);
- /* Wait for completion */
- while (readl(reg) & 2)
;
Look at wait_for_bit() implementations in the U-Boot tree and avoid the unbounded waiting here please.
- return 0;
+}
+static int ehci_usb_ofdata_to_platdata(struct udevice *dev) +{
- struct msm_ehci_priv *priv = dev_get_priv(dev);
- priv->base = dev_get_addr(dev);
- priv->ehci_base = priv->base + MSM_USB_EHCI_OFFSET;
- priv->ulpi_base = priv->base + MSM_USB_ULPI_OFFSET;
- priv->ulpi_port = 0;
- return 0;
+}
+static const struct udevice_id ehci_usb_ids[] = {
- { .compatible = "qcom,ehci-host", },
- { }
+};
+U_BOOT_DRIVER(usb_ehci) = {
- .name = "ehci_msm",
- .id = UCLASS_USB,
- .of_match = ehci_usb_ids,
- .ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
- .probe = ehci_usb_probe,
- .remove = ehci_usb_remove,
- .ops = &ehci_usb_ops,
- .priv_auto_alloc_size = sizeof(struct msm_ehci_priv),
- .flags = DM_FLAG_ALLOC_PRIV_DMA,
+};