[U-Boot] [PATCH] drivers/usb : Introduce APIs for fsl usb registers

[U-Boot] [PATCH] drivers/usb : Introduce APIs for fsl usb registers

[Nikhil Badola]

Introduce new APIs to write fsl usb registers that have w1c bits.
Existing API framework do not take care of w1c bits

Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 drivers/usb/host/ehci-fsl.c | 17 ++++++++++++-----
 include/usb/ehci-fsl.h      |  6 ++++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 6cb4d98..3be69c4 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -104,15 +104,22 @@ int ehci_hcd_init(int index, enum usb_init_type init,
 
 	if (!strncmp(phy_type, "utmi", 4)) {
 #if defined(CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY)
-		setbits_be32(&ehci->control, PHY_CLK_SEL_UTMI);
-		setbits_be32(&ehci->control, UTMI_PHY_EN);
+		fsl_usb_setbits(&ehci->control, PHY_CLK_SEL_UTMI,
+				CONTROL_REGISTER_W1C_MASK);
+		fsl_usb_setbits(&ehci->control, UTMI_PHY_EN,
+				CONTROL_REGISTER_W1C_MASK);
 		udelay(1000); /* delay required for PHY Clk to appear */
 #endif
 		out_le32(&(*hcor)->or_portsc[0], PORT_PTS_UTMI);
-		setbits_be32(&ehci->control, USB_EN);
+		fsl_usb_setbits(&ehci->control, USB_EN,
+				CONTROL_REGISTER_W1C_MASK);
 	} else {
-		setbits_be32(&ehci->control, PHY_CLK_SEL_ULPI);
-		clrsetbits_be32(&ehci->control, UTMI_PHY_EN, USB_EN);
+		fsl_usb_setbits(&ehci->control, PHY_CLK_SEL_ULPI,
+				CONTROL_REGISTER_W1C_MASK);
+		fsl_usb_clrbits(&ehci->control, UTMI_PHY_EN,
+				CONTROL_REGISTER_W1C_MASK);
+		fsl_usb_setbits(&ehci->control, USB_EN,
+				CONTROL_REGISTER_W1C_MASK);
 		udelay(1000); /* delay required for PHY Clk to appear */
 		if (!usb_phy_clk_valid(ehci))
 			return -EINVAL;
diff --git a/include/usb/ehci-fsl.h b/include/usb/ehci-fsl.h
index c9ee1d5..fb3d6b4 100644
--- a/include/usb/ehci-fsl.h
+++ b/include/usb/ehci-fsl.h
@@ -11,6 +11,12 @@
 
 #include <asm/processor.h>
 
+#define CONTROL_REGISTER_W1C_MASK       0x00020000  /* W1C: PHY_CLK_VALID */
+#define fsl_usb_setbits(_addr, _v, _mask) out_be32((_addr), \
+			((in_be32(_addr) & ~_mask) | _v))
+#define fsl_usb_clrbits(_addr, _v, _mask) out_be32((_addr), \
+			((in_be32(_addr) & ~_mask) & ~_v))
+
 /* Global offsets */
 #define FSL_SKIP_PCI		0x100
 
-- 
1.7.11.7

[U-Boot] [PATCH] drivers/usb : Introduce APIs for fsl usb registers

[Wolfgang Denk]

Dear Nikhil Badola,

In message <1399468595-953-1-git-send-email-nikhil.badola@freescale.com> you wrote:
> Introduce new APIs to write fsl usb registers that have w1c bits.
> Existing API framework do not take care of w1c bits

Why exactly do we need a new API for that?  All you'r doing is adding
just more bits to the mask, right?

> -		setbits_be32(&ehci->control, PHY_CLK_SEL_UTMI);
> -		setbits_be32(&ehci->control, UTMI_PHY_EN);
> +		fsl_usb_setbits(&ehci->control, PHY_CLK_SEL_UTMI,
> +				CONTROL_REGISTER_W1C_MASK);
> +		fsl_usb_setbits(&ehci->control, UTMI_PHY_EN,
> +				CONTROL_REGISTER_W1C_MASK);
...

> +#define CONTROL_REGISTER_W1C_MASK       0x00020000  /* W1C: PHY_CLK_VALID */
> +#define fsl_usb_setbits(_addr, _v, _mask) out_be32((_addr), \
> +			((in_be32(_addr) & ~_mask) | _v))
> +#define fsl_usb_clrbits(_addr, _v, _mask) out_be32((_addr), \
> +			((in_be32(_addr) & ~_mask) & ~_v))
> +

I cannot understand in which way

	fsl_usb_setbits(&ehci->control, PHY_CLK_SEL_UTMI, CONTROL_REGISTER_W1C_MASK);

would be different from

	setbits_be32(&ehci->control, PHY_CLK_SEL_UTMI | CONTROL_REGISTER_W1C_MASK);

?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I am not now, nor have I ever been, a member of the demigodic party.
                                                   -- Dennis Ritchie

[U-Boot] [PATCH] drivers/usb : Introduce APIs for fsl usb registers

[Wolfgang Denk]

Dear "nikhil.badola at freescale.com",

In message <0adb0aa7c7c6434fb762e1d076b24dae@BLUPR03MB133.namprd03.prod.outlook.com> you wrote:
> >
> > Why exactly do we need a new API for that?  All you'r doing is adding
> > just more bits to the mask, right?
> 
> I am adding mask for a register with w1c(write 1 to clear) bits set. So while 
> setting/resetting any other bit in this register, these bits(w1c bits) should not 
> get set so as to prevent them from clearing. For this purpose a new API is needed.
> This API can be used for any fsl registers that have any number of w1c bit(s).
> All one needs is to define mask for that register with w1c bits set.

You do not need a new API.  We have clrsetbits_*(), which can do
exactly what you need.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"To IBM, 'open' means there is a modicum  of  interoperability  among
some of their equipment."                            - Harv Masterson