Re: [PATCH v2 4/4] usb: Add APIs to access host registers from Tegra PHY

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3767 bytes --]

Hi,

On Thu, Jan 17, 2013 at 01:58:12PM +0530, Venu Byravarasu wrote:
> As Tegra PHY driver need to access one of the Host registers,
> added few APIs to ehci tegra driver.
> 
> Signed-off-by: Venu Byravarasu <vbyravarasu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Stephen is this another of those patches you're gonna take care of
yourself ?

> ---
> delta from v1:
> Taken care of RWC bits, while accessing PORTSC register.
> 
>  drivers/usb/host/ehci-tegra.c     |   70 ++++++++++++++++++++++++++++++++++++-
>  drivers/usb/phy/tegra_usb_phy.c   |   51 +++++++--------------------
>  include/linux/usb/tegra_usb_phy.h |    6 +++
>  3 files changed, 88 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
> index 55a9cde..6bbf66a 100644
> --- a/drivers/usb/host/ehci-tegra.c
> +++ b/drivers/usb/host/ehci-tegra.c
> @@ -2,7 +2,7 @@
>   * EHCI-compliant USB host controller driver for NVIDIA Tegra SoCs
>   *
>   * Copyright (C) 2010 Google, Inc.
> - * Copyright (C) 2009 NVIDIA Corporation
> + * Copyright (C) 2009 - 2013 NVIDIA Corporation
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms of the GNU General Public License as published by the
> @@ -33,6 +33,16 @@
>  #define TEGRA_USB2_BASE			0xC5004000
>  #define TEGRA_USB3_BASE			0xC5008000
>  
> +/* PORTSC registers */
> +#define USB_PORTSC1			0x184
> +#define USB_PORTSC1_PTS(x)	(((x) & 0x3) << 30)
> +#define USB_PORTSC1_PHCD	(1 << 23)
> +#define USB_PORTSC1_WKOC	(1 << 22)
> +#define USB_PORTSC1_WKDS	(1 << 21)
> +#define USB_PORTSC1_WKCN	(1 << 20)
> +#define USB_PORTSC1_PEC		(1 << 3)
> +#define USB_PORTSC1_CSC		(1 << 1)

please prepend this with TEGRA_ just as other defines.

> +
>  #define TEGRA_USB_DMA_ALIGN 32
>  
>  struct tegra_ehci_hcd {
> @@ -605,6 +615,53 @@ static const struct dev_pm_ops tegra_ehci_pm_ops = {
>  
>  #endif
>  
> +/* Bits of PORTSC1, which will get cleared by writing 1 into them */
> +#define TEGRA_PORTSC_RWC_BITS (USB_PORTSC1_CSC | USB_PORTSC1_PEC)
> +
> +void tegra_ehci_set_wakeon_events(struct usb_phy *x, bool enable)
> +{
> +	unsigned long val;
> +	struct usb_hcd *hcd = bus_to_hcd(x->otg->host);
> +	void __iomem *base = hcd->regs;
> +	u32 wake = USB_PORTSC1_WKOC | USB_PORTSC1_WKDS | USB_PORTSC1_WKCN;
> +
> +	val = readl(base + USB_PORTSC1) & ~TEGRA_PORTSC_RWC_BITS;
> +	if (enable)
> +		val |= wake;
> +	else
> +		val &= ~wake;
> +	writel(val, base + USB_PORTSC1);
> +}
> +EXPORT_SYMBOL_GPL(tegra_ehci_set_wakeon_events);
> +
> +void tegra_ehci_set_pts(struct usb_phy *x, u8 pts_val)
> +{
> +	unsigned long val;
> +	struct usb_hcd *hcd = bus_to_hcd(x->otg->host);
> +	void __iomem *base = hcd->regs;
> +
> +	val = readl(base + USB_PORTSC1) & ~TEGRA_PORTSC_RWC_BITS;
> +	val &= ~USB_PORTSC1_PTS(3);
> +	val |= USB_PORTSC1_PTS(pts_val & 3);
> +	writel(val, base + USB_PORTSC1);
> +}
> +EXPORT_SYMBOL_GPL(tegra_ehci_set_pts);
> +
> +void tegra_ehci_set_phcd(struct usb_phy *x, bool enable)
> +{
> +	unsigned long val;
> +	struct usb_hcd *hcd = bus_to_hcd(x->otg->host);
> +	void __iomem *base = hcd->regs;
> +
> +	val = readl(base + USB_PORTSC1) & ~TEGRA_PORTSC_RWC_BITS;
> +	if (enable)
> +		val |= USB_PORTSC1_PHCD;
> +	else
> +		val &= ~USB_PORTSC1_PHCD;
> +	writel(val, base + USB_PORTSC1);
> +}
> +EXPORT_SYMBOL_GPL(tegra_ehci_set_phcd);

NAK to these three functions, you need to use whatever the PHY API
provides you, if it misses something, let's see how we can add those as
generic calls.

In fact, I wonder why do you want PHY driver to access Host address
space. Why don't you let your host driver handle the above ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]