Re: [PATCH 3/4] usb: host: Add EHCI driver for NVIDIA Tegra SoCs

[Matthieu CASTET <matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>]

Hi,

Benoit Goby a écrit :
> Signed-off-by: Benoit Goby <benoit-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
_clk);
> +}
> +
> +static int tegra_ehci_hub_control(
> +       struct usb_hcd  *hcd,
> +       u16             typeReq,
> +       u16             wValue,
> +       u16             wIndex,
> +       char            *buf,
> +       u16             wLength
> +)
> +{
> +       struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> +       struct tegra_ehci_hcd *tegra = dev_get_drvdata(hcd->self.controller);
> +       u32 __iomem     *status_reg;
> +       u32             temp;
> +       unsigned long   flags;
> +       int             retval = 0;
> +
> +       status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
> +
> +       spin_lock_irqsave(&ehci->lock, flags);
> +
> +       /*
> +        * In ehci_hub_control() for USB_PORT_FEAT_ENABLE clears the other bits
> +        * that are write on clear, by writing back the register read value, so
> +        * USB_PORT_FEAT_ENABLE is handled by masking the set on clear bits
> +        */
> +       if (typeReq == ClearPortFeature && wValue == USB_PORT_FEAT_ENABLE) {
> +               temp = ehci_readl(ehci, status_reg) & ~PORT_RWC_BITS;
> +               ehci_writel(ehci, temp & ~PORT_PE, status_reg);
> +               goto done;
> +       }
> +
> +       else if (typeReq == GetPortStatus) {
> +               temp = ehci_readl(ehci, status_reg);
> +               if (tegra->port_resuming && !(temp & PORT_SUSPEND)) {
> +                       /* resume completed */
> +                       tegra->port_resuming = 0;
> +                       tegra_usb_phy_postresume(tegra->phy);
> +               }
> +       }
> +
> +       else if (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_SUSPEND) {
> +               temp = ehci_readl(ehci, status_reg);
> +               if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) {
> +                       retval = -EPIPE;
> +                       goto done;
> +               }
> +
> +               /* After above check the port must be connected.
> +                * Set appropriate bit thus could put phy into low power
> +                * mode if we have hostpc feature
> +                */
> +               temp &= ~PORT_WKCONN_E;
> +               temp |= PORT_WKDISC_E | PORT_WKOC_E;
> +               ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
> +               if (handshake(ehci, status_reg, PORT_SUSPEND,
> +                                               PORT_SUSPEND, 5000))
> +                       pr_err("%s: timeout waiting for SUSPEND\n", __func__);
> +               set_bit((wIndex & 0xff) - 1, &ehci->suspended_ports);
> +               goto done;
> +       }
> +
> +       /*
> +        * Tegra host controller will time the resume operation to clear the bit
> +        * when the port control state switches to HS or FS Idle. This behavior
> +        * is different from EHCI where the host controller driver is required
> +        * to set this bit to a zero after the resume duration is timed in the
> +        * driver.
> +        */
> +       else if (typeReq == ClearPortFeature &&
> +                                       wValue == USB_PORT_FEAT_SUSPEND) {
> +               temp = ehci_readl(ehci, status_reg);
> +               if ((temp & PORT_RESET) || !(temp & PORT_PE)) {
> +                       retval = -EPIPE;
> +                       goto done;
> +               }
> +
> +               if (!(temp & PORT_SUSPEND))
> +                       goto done;
> +
> +               tegra_usb_phy_preresume(tegra->phy);
> +
> +               ehci->reset_done[wIndex-1] = jiffies + msecs_to_jiffies(25);
> +
> +               temp &= ~(PORT_RWC_BITS | PORT_WAKE_BITS);
> +               /* start resume signalling */
> +               ehci_writel(ehci, temp | PORT_RESUME, status_reg);
> +
> +               spin_unlock_irqrestore(&ehci->lock, flags);
> +               msleep(20);
> +               spin_lock_irqsave(&ehci->lock, flags);
> +
> +               /* polling PORT_RESUME until the controller clear this bit */
> +               if (handshake(ehci, status_reg, PORT_RESUME, 0, 2000))
> +                       pr_err("%s: timeout waiting for RESUME\n", __func__);
> +
> +               /* polling PORT_SUSPEND until the controller clear this bit */
> +               if (handshake(ehci, status_reg, PORT_SUSPEND, 0, 2000))
> +                       pr_err("%s: timeout waiting for SUSPEND\n", __func__);
> +
> +               ehci->reset_done[wIndex-1] = 0;
> +
> +               tegra->port_resuming = 1;
> +               goto done;
> +       }
> +
> +       spin_unlock_irqrestore(&ehci->lock, flags);
> +
> +       /* Handle the hub control events here */
> +       return ehci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
> +done:
> +       spin_unlock_irqrestore(&ehci->lock, flags);
> +       return retval;
> +}
> +
I don't know enough usb layer, but can't you done something similar than 
in msm otg : let's the otg layer handle the power management ?

http://www.spinics.net/lists/linux-usb/msg41247.html


> +static int tegra_ehci_reset(struct usb_hcd *hcd)
> +{
> +       unsigned long temp;
> +       int usec = 250*1000; /* see ehci_reset */
> +
> +       temp = readl(hcd->regs + TEGRA_USB_USBCMD_REG_OFFSET);
> +       temp |= TEGRA_USB_USBCMD_RESET;
> +       writel(temp, hcd->regs + TEGRA_USB_USBCMD_REG_OFFSET);
> +
> +       do {
> +               temp = readl(hcd->regs + TEGRA_USB_USBCMD_REG_OFFSET);
> +               if (!(temp & TEGRA_USB_USBCMD_RESET))
> +                       break;
> +               udelay(1);
> +               usec--;
> +       } while (usec);
> +
> +       if (!usec)
> +               return -ETIMEDOUT;
> +
> +       /* Set to Host mode by setting bit 0-1 of USB device mode register */
> +       temp = readl(hcd->regs + TEGRA_USB_USBMODE_REG_OFFSET);
> +       writel((temp | TEGRA_USB_USBMODE_HOST),
> +                       (hcd->regs + TEGRA_USB_USBMODE_REG_OFFSET));
> +
> +       return 0;
> +}
Why can't you use tdi_reset that already this for you ?



Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html