[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 8a1094b..4381c5a6 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -21,6 +21,10 @@
 #define MX25_USB_PHY_CTRL_OFFSET	0x08
 #define MX25_BM_EXTERNAL_VBUS_DIVIDER	BIT(23)
 
+#define MX27_H1_PM_BIT			BIT(8)
+#define MX27_H2_PM_BIT			BIT(16)
+#define MX27_OTG_PM_BIT			BIT(24)
+
 #define MX53_USB_OTG_PHY_CTRL_0_OFFSET	0x08
 #define MX53_USB_UH2_CTRL_OFFSET	0x14
 #define MX53_USB_UH3_CTRL_OFFSET	0x18
@@ -68,6 +72,36 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
 	return 0;
 }
 
+static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
+{
+	unsigned long flags;
+	u32 val;
+
+	switch (data->index) {
+	case 0:
+		val = MX27_OTG_PM_BIT;
+		break;
+	case 1:
+		val = MX27_H1_PM_BIT;
+		break;
+	case 2:
+		val = MX27_H2_PM_BIT;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	spin_lock_irqsave(&usbmisc->lock, flags);
+	if (data->disable_oc)
+		val = readl(usbmisc->base) | val;
+	else
+		val = readl(usbmisc->base) & ~val;
+	writel(val, usbmisc->base);
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+	return 0;
+}
+
 static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
 {
 	void __iomem *reg = NULL;
@@ -128,6 +162,10 @@ static const struct usbmisc_ops imx25_usbmisc_ops = {
 	.post = usbmisc_imx25_post,
 };
 
+static const struct usbmisc_ops imx27_usbmisc_ops = {
+	.init = usbmisc_imx27_init,
+};
+
 static const struct usbmisc_ops imx53_usbmisc_ops = {
 	.init = usbmisc_imx53_init,
 };
@@ -162,6 +200,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
 		.data = &imx25_usbmisc_ops,
 	},
 	{
+		.compatible = "fsl,imx27-usbmisc",
+		.data = &imx27_usbmisc_ops,
+	},
+	{
 		.compatible = "fsl,imx53-usbmisc",
 		.data = &imx53_usbmisc_ops,
 	},
-- 
1.8.1.5

[PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Alexander Shiyan]

This adds i.MX51 as the next user of the usbmisc driver.
Since the functional is similar i.MX53, we just rename the
definitions and add an alias for the new CPU.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 4381c5a6..f348ebb 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -25,12 +25,12 @@
 #define MX27_H2_PM_BIT			BIT(16)
 #define MX27_OTG_PM_BIT			BIT(24)
 
-#define MX53_USB_OTG_PHY_CTRL_0_OFFSET	0x08
-#define MX53_USB_UH2_CTRL_OFFSET	0x14
-#define MX53_USB_UH3_CTRL_OFFSET	0x18
-#define MX53_BM_OVER_CUR_DIS_H1		BIT(5)
-#define MX53_BM_OVER_CUR_DIS_OTG	BIT(8)
-#define MX53_BM_OVER_CUR_DIS_UHx	BIT(30)
+#define MX5_USB_OTG_PHY_CTRL_0_OFFSET	0x08
+#define MX5_USB_UH2_CTRL_OFFSET		0x14
+#define MX5_USB_UH3_CTRL_OFFSET		0x18
+#define MX5_BM_OVER_CUR_DIS_H1		BIT(5)
+#define MX5_BM_OVER_CUR_DIS_OTG		BIT(8)
+#define MX5_BM_OVER_CUR_DIS_UHx		BIT(30)
 
 #define MX6_BM_OVER_CUR_DIS		BIT(7)
 
@@ -102,7 +102,7 @@ static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
 	return 0;
 }
 
-static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
+static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
 {
 	void __iomem *reg = NULL;
 	unsigned long flags;
@@ -115,20 +115,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
 		spin_lock_irqsave(&usbmisc->lock, flags);
 		switch (data->index) {
 		case 0:
-			reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
-			val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
+			reg = usbmisc->base + MX5_USB_OTG_PHY_CTRL_0_OFFSET;
+			val = readl(reg) | MX5_BM_OVER_CUR_DIS_OTG;
 			break;
 		case 1:
-			reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
-			val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
+			reg = usbmisc->base + MX5_USB_OTG_PHY_CTRL_0_OFFSET;
+			val = readl(reg) | MX5_BM_OVER_CUR_DIS_H1;
 			break;
 		case 2:
-			reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
-			val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
+			reg = usbmisc->base + MX5_USB_UH2_CTRL_OFFSET;
+			val = readl(reg) | MX5_BM_OVER_CUR_DIS_UHx;
 			break;
 		case 3:
-			reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
-			val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
+			reg = usbmisc->base + MX5_USB_UH3_CTRL_OFFSET;
+			val = readl(reg) | MX5_BM_OVER_CUR_DIS_UHx;
 			break;
 		}
 		if (reg && val)
@@ -166,8 +166,8 @@ static const struct usbmisc_ops imx27_usbmisc_ops = {
 	.init = usbmisc_imx27_init,
 };
 
-static const struct usbmisc_ops imx53_usbmisc_ops = {
-	.init = usbmisc_imx53_init,
+static const struct usbmisc_ops imx5_usbmisc_ops = {
+	.init = usbmisc_imx5_init,
 };
 
 static const struct usbmisc_ops imx6q_usbmisc_ops = {
@@ -204,8 +204,12 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
 		.data = &imx27_usbmisc_ops,
 	},
 	{
+		.compatible = "fsl,imx51-usbmisc",
+		.data = &imx5_usbmisc_ops,
+	},
+	{
 		.compatible = "fsl,imx53-usbmisc",
-		.data = &imx53_usbmisc_ops,
+		.data = &imx5_usbmisc_ops,
 	},
 	{
 		.compatible = "fsl,imx6q-usbmisc",
-- 
1.8.1.5

[PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Peter Chen]

On Sun, Nov 10, 2013 at 3:18 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
> This adds i.MX51 as the next user of the usbmisc driver.
> Since the functional is similar i.MX53, we just rename the
> definitions and add an alias for the new CPU.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 4381c5a6..f348ebb 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -25,12 +25,12 @@
>  #define MX27_H2_PM_BIT                 BIT(16)
>  #define MX27_OTG_PM_BIT                        BIT(24)
>
> -#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08
> -#define MX53_USB_UH2_CTRL_OFFSET       0x14
> -#define MX53_USB_UH3_CTRL_OFFSET       0x18
> -#define MX53_BM_OVER_CUR_DIS_H1                BIT(5)
> -#define MX53_BM_OVER_CUR_DIS_OTG       BIT(8)
> -#define MX53_BM_OVER_CUR_DIS_UHx       BIT(30)
> +#define MX5_USB_OTG_PHY_CTRL_0_OFFSET  0x08
> +#define MX5_USB_UH2_CTRL_OFFSET                0x14
> +#define MX5_USB_UH3_CTRL_OFFSET                0x18
> +#define MX5_BM_OVER_CUR_DIS_H1         BIT(5)
> +#define MX5_BM_OVER_CUR_DIS_OTG                BIT(8)
> +#define MX5_BM_OVER_CUR_DIS_UHx                BIT(30)
>
>  #define MX6_BM_OVER_CUR_DIS            BIT(7)
>
> @@ -102,7 +102,7 @@ static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
>         return 0;
>  }
>
> -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
>  {
>         void __iomem *reg = NULL;
>         unsigned long flags;
> @@ -115,20 +115,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>                 spin_lock_irqsave(&usbmisc->lock, flags);
>                 switch (data->index) {
>                 case 0:
> -                       reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
> -                       val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG;
> +                       reg = usbmisc->base + MX5_USB_OTG_PHY_CTRL_0_OFFSET;
> +                       val = readl(reg) | MX5_BM_OVER_CUR_DIS_OTG;
>                         break;
>                 case 1:
> -                       reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET;
> -                       val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1;
> +                       reg = usbmisc->base + MX5_USB_OTG_PHY_CTRL_0_OFFSET;
> +                       val = readl(reg) | MX5_BM_OVER_CUR_DIS_H1;
>                         break;
>                 case 2:
> -                       reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
> -                       val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> +                       reg = usbmisc->base + MX5_USB_UH2_CTRL_OFFSET;
> +                       val = readl(reg) | MX5_BM_OVER_CUR_DIS_UHx;
>                         break;
>                 case 3:
> -                       reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
> -                       val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
> +                       reg = usbmisc->base + MX5_USB_UH3_CTRL_OFFSET;
> +                       val = readl(reg) | MX5_BM_OVER_CUR_DIS_UHx;
>                         break;
>                 }
>                 if (reg && val)
> @@ -166,8 +166,8 @@ static const struct usbmisc_ops imx27_usbmisc_ops = {
>         .init = usbmisc_imx27_init,
>  };
>
> -static const struct usbmisc_ops imx53_usbmisc_ops = {
> -       .init = usbmisc_imx53_init,
> +static const struct usbmisc_ops imx5_usbmisc_ops = {
> +       .init = usbmisc_imx5_init,
>  };
>
>  static const struct usbmisc_ops imx6q_usbmisc_ops = {
> @@ -204,8 +204,12 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
>                 .data = &imx27_usbmisc_ops,
>         },
>         {
> +               .compatible = "fsl,imx51-usbmisc",
> +               .data = &imx5_usbmisc_ops,
> +       },
> +       {
>                 .compatible = "fsl,imx53-usbmisc",
> -               .data = &imx53_usbmisc_ops,
> +               .data = &imx5_usbmisc_ops,
>         },
>         {
>                 .compatible = "fsl,imx6q-usbmisc",
> --

Acked-by: Peter Chen <peter.chen@freescale.com>

-- 
BR,
Peter Chen

[PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Matt Sealey]

I'd prefer this was kept separated out a little bit, as follows:

On Sun, Nov 10, 2013 at 1:18 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> This adds i.MX51 as the next user of the usbmisc driver.
> Since the functional is similar i.MX53, we just rename the
> definitions and add an alias for the new CPU.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
>
>
> -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)

Can we keep usbmisc_imx53_init named this?

There is more to do here on i.MX5 for cases where the bootloader did
not already try to set up USB (or did it badly) such as the
transceiver reference clock rate and also setting the USB sysclk clock
source (internal DPLL or from external transceiver in the external
ULPI case) and so on, which is related and needs doing on both, but is
different on i.MX51 than i.MX53. This is information sort of best
passed in the PHY node that goes along with this, but it's set within
the usbmisc block of the chips so the usbmisc driver will have a
responsibility to go see if it's an external PHY that is feeding it's
clock back into the USB block in this way.

I am not sure we (Peter etc.) discussed how best to do this, the code
to pull the correct information out always seems kind of misplaced no
matter where it goes, but the responsibility for tweaking those
registers is most certainly this driver.

Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
is different when doing the above, and dependent on a board-specific
option for the input clock to the transceiver. We could reduce a
little churn, later, when usbmisc_imx could be given related usbphy
information and actually do the right thing. I have a patch kinda
sitting in the wings to do this.. and two *real* pieces of consumer
hardware that need it, and some other kicking, to make USB work in the
never-touched-before-Linux case.

> -static const struct usbmisc_ops imx53_usbmisc_ops = {
> -       .init = usbmisc_imx53_init,
> +static const struct usbmisc_ops imx5_usbmisc_ops = {
> +       .init = usbmisc_imx5_init,
>  };

And keep imx53_usbmisc_ops named this?

>  static const struct usbmisc_ops imx6q_usbmisc_ops = {
> @@ -204,8 +204,12 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
>                 .data = &imx27_usbmisc_ops,
>         },
>         {
> +               .compatible = "fsl,imx51-usbmisc",
> +               .data = &imx5_usbmisc_ops,

And then just use &imx53_usbmisc_ops?

This gives us some breathing room later to actually do the right thing
without additionally performing renames all over the place to make
imx5 -> imx53 (again)/imx51 (new).

Thanks,
Matt Sealey <neko@bakuhatsu.net>

[PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Peter Chen]

 
> >
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++---------
> --------
> >  1 file changed, 22 insertions(+), 18 deletions(-)
> >
> >
> > -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> > +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
> 
> Can we keep usbmisc_imx53_init named this?
> 
> There is more to do here on i.MX5 for cases where the bootloader did
> not already try to set up USB (or did it badly) such as the
> transceiver reference clock rate and also setting the USB sysclk clock
> source (internal DPLL or from external transceiver in the external
> ULPI case) and so on, which is related and needs doing on both, but is
> different on i.MX51 than i.MX53. This is information sort of best
> passed in the PHY node that goes along with this, but it's set within
> the usbmisc block of the chips so the usbmisc driver will have a
> responsibility to go see if it's an external PHY that is feeding it's
> clock back into the USB block in this way.
> 
> I am not sure we (Peter etc.) discussed how best to do this, the code
> to pull the correct information out always seems kind of misplaced no
> matter where it goes, but the responsibility for tweaking those
> registers is most certainly this driver.
> 
> Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
> is different when doing the above, and dependent on a board-specific
> option for the input clock to the transceiver. We could reduce a
> little churn, later, when usbmisc_imx could be given related usbphy
> information and actually do the right thing. I have a patch kinda
> sitting in the wings to do this.. and two *real* pieces of consumer
> hardware that need it, and some other kicking, to make USB work in the
> never-touched-before-Linux case.
> 
> > -static const struct usbmisc_ops imx53_usbmisc_ops = {
> > -       .init = usbmisc_imx53_init,
> > +static const struct usbmisc_ops imx5_usbmisc_ops = {
> > +       .init = usbmisc_imx5_init,
> >  };
> 
> And keep imx53_usbmisc_ops named this?
> 
> >  static const struct usbmisc_ops imx6q_usbmisc_ops = {
> > @@ -204,8 +204,12 @@ static const struct of_device_id
> usbmisc_imx_dt_ids[] = {
> >                 .data = &imx27_usbmisc_ops,
> >         },
> >         {
> > +               .compatible = "fsl,imx51-usbmisc",
> > +               .data = &imx5_usbmisc_ops,
> 
> And then just use &imx53_usbmisc_ops?
> 
> This gives us some breathing room later to actually do the right thing
> without additionally performing renames all over the place to make
> imx5 -> imx53 (again)/imx51 (new).
 
Hi Alexander,

You may take matt's suggestion, it can reduce the code change now and in future.
We can only add device_id for imx51 in this patch, split imx53 and imx51's ops
when their differences are added in future.

Peter

Re: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Alexander Shiyan]

Hello.

> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > > ---
> > >  drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++---------
> > --------
> > >  1 file changed, 22 insertions(+), 18 deletions(-)
> > >
> > >
> > > -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> > > +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
> > 
> > Can we keep usbmisc_imx53_init named this?
> > 
> > There is more to do here on i.MX5 for cases where the bootloader did
> > not already try to set up USB (or did it badly) such as the
> > transceiver reference clock rate and also setting the USB sysclk clock
> > source (internal DPLL or from external transceiver in the external
> > ULPI case) and so on, which is related and needs doing on both, but is
> > different on i.MX51 than i.MX53. This is information sort of best
> > passed in the PHY node that goes along with this, but it's set within
> > the usbmisc block of the chips so the usbmisc driver will have a
> > responsibility to go see if it's an external PHY that is feeding it's
> > clock back into the USB block in this way.
> > 
> > I am not sure we (Peter etc.) discussed how best to do this, the code
> > to pull the correct information out always seems kind of misplaced no
> > matter where it goes, but the responsibility for tweaking those
> > registers is most certainly this driver.
> > 
> > Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
> > is different when doing the above, and dependent on a board-specific
> > option for the input clock to the transceiver. We could reduce a
> > little churn, later, when usbmisc_imx could be given related usbphy
> > information and actually do the right thing. I have a patch kinda
> > sitting in the wings to do this.. and two *real* pieces of consumer
> > hardware that need it, and some other kicking, to make USB work in the
> > never-touched-before-Linux case.
> > 
> > > -static const struct usbmisc_ops imx53_usbmisc_ops = {
> > > -       .init = usbmisc_imx53_init,
> > > +static const struct usbmisc_ops imx5_usbmisc_ops = {
> > > +       .init = usbmisc_imx5_init,
> > >  };
> > 
> > And keep imx53_usbmisc_ops named this?
> > 
> > >  static const struct usbmisc_ops imx6q_usbmisc_ops = {
> > > @@ -204,8 +204,12 @@ static const struct of_device_id
> > usbmisc_imx_dt_ids[] = {
> > >                 .data = &imx27_usbmisc_ops,
> > >         },
> > >         {
> > > +               .compatible = "fsl,imx51-usbmisc",
> > > +               .data = &imx5_usbmisc_ops,
> > 
> > And then just use &imx53_usbmisc_ops?
> > 
> > This gives us some breathing room later to actually do the right thing
> > without additionally performing renames all over the place to make
> > imx5 -> imx53 (again)/imx51 (new).
> 
> You may take matt's suggestion, it can reduce the code change now and in future.
> We can only add device_id for imx51 in this patch, split imx53 and imx51's ops
> when their differences are added in future.

OK. Names of registers and bits also keep as is?

---

[PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU

[Peter Chen]

 
> > > This gives us some breathing room later to actually do the right
> thing
> > > without additionally performing renames all over the place to make
> > > imx5 -> imx53 (again)/imx51 (new).
> >
> > You may take matt's suggestion, it can reduce the code change now and
> in future.
> > We can only add device_id for imx51 in this patch, split imx53 and
> imx51's ops
> > when their differences are added in future.
> 
> OK. Names of registers and bits also keep as is?
> 

Yes.

Peter

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Chris Ruehl]

Hi Alexander,

On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>
> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> ---
>   drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index 8a1094b..4381c5a6 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -21,6 +21,10 @@
>   #define MX25_USB_PHY_CTRL_OFFSET	0x08
>   #define MX25_BM_EXTERNAL_VBUS_DIVIDER	BIT(23)
>
> +#define MX27_H1_PM_BIT			BIT(8)
> +#define MX27_H2_PM_BIT			BIT(16)
> +#define MX27_OTG_PM_BIT			BIT(24)
> +
>   #define MX53_USB_OTG_PHY_CTRL_0_OFFSET	0x08
>   #define MX53_USB_UH2_CTRL_OFFSET	0x14
>   #define MX53_USB_UH3_CTRL_OFFSET	0x18
> @@ -68,6 +72,36 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
>   	return 0;
>   }
>
> +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
> +{
> +	unsigned long flags;
> +	u32 val;
> +
> +	switch (data->index) {
> +	case 0:
> +		val = MX27_OTG_PM_BIT;
> +		break;
> +	case 1:
> +		val = MX27_H1_PM_BIT;
> +		break;
> +	case 2:
> +		val = MX27_H2_PM_BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	};
> +

 From my understanding this can not work, the usbmisc->base not point into the
usb control register (USB_CTRL). Reference manual 30.5.1.1 says
BASE + 0x600
you must add the offset to the readl instruction.

> +	spin_lock_irqsave(&usbmisc->lock, flags);
> +	if (data->disable_oc)
> +		val = readl(usbmisc->base) | val;

else part not needed, the registers bits are set to 0 (reset)
the function is called on start-up once only, right?!

> +	else
> +		val = readl(usbmisc->base)&  ~val;
> +	writel(val, usbmisc->base);
> +	spin_unlock_irqrestore(&usbmisc->lock, flags);
> +
> +	return 0;
> +}
> +
>   static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>   {
>   	void __iomem *reg = NULL;
> @@ -128,6 +162,10 @@ static const struct usbmisc_ops imx25_usbmisc_ops = {
>   	.post = usbmisc_imx25_post,
>   };
>
> +static const struct usbmisc_ops imx27_usbmisc_ops = {
> +	.init = usbmisc_imx27_init,
> +};
> +
>   static const struct usbmisc_ops imx53_usbmisc_ops = {
>   	.init = usbmisc_imx53_init,
>   };
> @@ -162,6 +200,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
>   		.data =&imx25_usbmisc_ops,
>   	},
>   	{
> +		.compatible = "fsl,imx27-usbmisc",
> +		.data =&imx27_usbmisc_ops,
> +	},
> +	{
>   		.compatible = "fsl,imx53-usbmisc",
>   		.data =&imx53_usbmisc_ops,
>   	},



Regards
Chris

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

> Hi Alexander,
> 
> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> > This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> >
> > Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> > ---
> >   drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
...
> > +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
> > +{
> > +	unsigned long flags;
> > +	u32 val;
> > +
> > +	switch (data->index) {
> > +	case 0:
> > +		val = MX27_OTG_PM_BIT;
> > +		break;
> > +	case 1:
> > +		val = MX27_H1_PM_BIT;
> > +		break;
> > +	case 2:
> > +		val = MX27_H2_PM_BIT;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	};
> > +
> 
>  From my understanding this can not work, the usbmisc->base not point into the
> usb control register (USB_CTRL). Reference manual 30.5.1.1 says
> BASE + 0x600
> you must add the offset to the readl instruction.

Why not work?
usbotg: usb at 10024000
usbh1: usb at 10024200
usbh2: usb at 10024400
usbmisc: usbmisc at 10024600
So, offset to USB_CTRL should already be defined by DTS.


> > +	spin_lock_irqsave(&usbmisc->lock, flags);
> > +	if (data->disable_oc)
> > +		val = readl(usbmisc->base) | val;
> 
> else part not needed, the registers bits are set to 0 (reset)
> the function is called on start-up once only, right?!
> 
> > +	else
> > +		val = readl(usbmisc->base)&  ~val;
> > +	writel(val, usbmisc->base);
> > +	spin_unlock_irqrestore(&usbmisc->lock, flags);
> > +
> > +	return 0;
> > +}

Bit can be set/cleared wrongly by the bootloader, it is not a big
overhead to set it in proper state.

...

---

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Chris Ruehl]

On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:
>> Hi Alexander,
>>
>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>>>
>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
>>> ---
>>>    drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 42 insertions(+)
>>>
>>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> ...
>>> +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
>>> +{
>>> +	unsigned long flags;
>>> +	u32 val;
>>> +
>>> +	switch (data->index) {
>>> +	case 0:
>>> +		val = MX27_OTG_PM_BIT;
>>> +		break;
>>> +	case 1:
>>> +		val = MX27_H1_PM_BIT;
>>> +		break;
>>> +	case 2:
>>> +		val = MX27_H2_PM_BIT;
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	};
>>> +
>>
>>    From my understanding this can not work, the usbmisc->base not point into the
>> usb control register (USB_CTRL). Reference manual 30.5.1.1 says
>> BASE + 0x600
>> you must add the offset to the readl instruction.
>
> Why not work?
> usbotg: usb at 10024000
> usbh1: usb at 10024200
> usbh2: usb at 10024400
> usbmisc: usbmisc at 10024600
> So, offset to USB_CTRL should already be defined by DTS.

in the usbmisc_imx_probe() the base pointer is loaded from

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data->base = devm_ioremap_resource(&pdev->dev, res);

(and I did not see any of_ operations)

usbmisc = data;

  base is set to 0x10024000

when I look around all other functions init functions did a offset calculation.

>
>
>>> +	spin_lock_irqsave(&usbmisc->lock, flags);
>>> +	if (data->disable_oc)
>>> +		val = readl(usbmisc->base) | val;
>>
>> else part not needed, the registers bits are set to 0 (reset)
>> the function is called on start-up once only, right?!
>>
>>> +	else
>>> +		val = readl(usbmisc->base)&   ~val;
>>> +	writel(val, usbmisc->base);
>>> +	spin_unlock_irqrestore(&usbmisc->lock, flags);
>>> +
>>> +	return 0;
>>> +}
>
> Bit can be set/cleared wrongly by the bootloader, it is not a big
> overhead to set it in proper state.
>
ohh, yes that true!

> ...
>

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

> On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:
> >> Hi Alexander,
> >>
> >> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> >>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> >>>
> >>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> >>> ---
> >>>    drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> >>>    1 file changed, 42 insertions(+)
> >>>
> >>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> > ...
> >>> +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
> >>> +{
> >>> +	unsigned long flags;
> >>> +	u32 val;
> >>> +
> >>> +	switch (data->index) {
> >>> +	case 0:
> >>> +		val = MX27_OTG_PM_BIT;
> >>> +		break;
> >>> +	case 1:
> >>> +		val = MX27_H1_PM_BIT;
> >>> +		break;
> >>> +	case 2:
> >>> +		val = MX27_H2_PM_BIT;
> >>> +		break;
> >>> +	default:
> >>> +		return -EINVAL;
> >>> +	};
> >>> +
> >>
> >>    From my understanding this can not work, the usbmisc->base not point into the
> >> usb control register (USB_CTRL). Reference manual 30.5.1.1 says
> >> BASE + 0x600
> >> you must add the offset to the readl instruction.
> >
> > Why not work?
> > usbotg: usb at 10024000
> > usbh1: usb at 10024200
> > usbh2: usb at 10024400
> > usbmisc: usbmisc at 10024600
> > So, offset to USB_CTRL should already be defined by DTS.
> 
> in the usbmisc_imx_probe() the base pointer is loaded from
> 
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> data->base = devm_ioremap_resource(&pdev->dev, res);
> 
> (and I did not see any of_ operations)

Yes, and this is an address of usbmisc node, not otg.

> usbmisc = data;
> 
>   base is set to 0x10024000
> 
> when I look around all other functions init functions did a offset calculation.

Can you point me on this?
For example, for i.MX5 CPUs we calculate only offset to PHY_CTRL_X register
relative to basic offset 0x800, which is already defined in DTS.

...

---

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Chris Ruehl]

On Monday, November 11, 2013 01:47 PM, Alexander Shiyan wrote:
>> On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:
>>>> Hi Alexander,
>>>>
>>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
>>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>>>>>
>>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
>>>>> ---
>>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>>>>>     1 file changed, 42 insertions(+)
>>>>>
>>>>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
>>> ...
>>>>> +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
>>>>> +{
>>>>> +	unsigned long flags;
>>>>> +	u32 val;
>>>>> +
>>>>> +	switch (data->index) {
>>>>> +	case 0:
>>>>> +		val = MX27_OTG_PM_BIT;
>>>>> +		break;
>>>>> +	case 1:
>>>>> +		val = MX27_H1_PM_BIT;
>>>>> +		break;
>>>>> +	case 2:
>>>>> +		val = MX27_H2_PM_BIT;
>>>>> +		break;
>>>>> +	default:
>>>>> +		return -EINVAL;
>>>>> +	};
>>>>> +
>>>>
>>>>      From my understanding this can not work, the usbmisc->base not point into the
>>>> usb control register (USB_CTRL). Reference manual 30.5.1.1 says
>>>> BASE + 0x600
>>>> you must add the offset to the readl instruction.
>>>
>>> Why not work?
>>> usbotg: usb at 10024000
>>> usbh1: usb at 10024200
>>> usbh2: usb at 10024400
>>> usbmisc: usbmisc at 10024600
>>> So, offset to USB_CTRL should already be defined by DTS.
>>
>> in the usbmisc_imx_probe() the base pointer is loaded from
>>
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> data->base = devm_ioremap_resource(&pdev->dev, res);
>>
>> (and I did not see any of_ operations)
>
> Yes, and this is an address of usbmisc node, not otg.

Sorry, you are right. I misunderstood because I didn't see any of sample DTS 
files define usbmisc yet.

At this point might be good to patch the imx27.dtsi with the usb defines.

--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -30,6 +30,9 @@
                 spi0 = &cspi1;
                 spi1 = &cspi2;
                 spi2 = &cspi3;
+               usb0 = &usbotg;
+               usb1 = &usbh1;
+               usb2 = &usbh2;
         };

@@ -404,6 +419,44 @@
                                 iram = <&iram>;
                         };

+                       usbotg: usb at 10024000 {
+                               compatible = "fsl,imx27-usb";
+                               reg = <0x10024000 0x200>;
+                               interrupts = <56>;
+                               clocks = <&clks 75>, <&clks 62>;
+                               clock-names = "ipg", "ahb";
+                               dr_mode = "host";
+                               phy_type = "ulpi";
+                               status = "disabled";
+                       };
+
+                       usbh1: usb at 10024200 {
+                               compatible = "fsl,imx27-usb";
+                               reg = <0x10024200 0x200>;
+                               interrupts = <54>;
+                               clocks = <&clks 75>, <&clks 62>;
+                               clock-names = "ipg", "ahb";
+                               dr_mode = "host";
+                               phy_type = "serial";
+                               status = "disabled";
+                       };
+
+                       usbh2: usb at 10024400 {
+                               compatible = "fsl,imx27-usb";
+                               reg = <0x10024400 0x200>;
+                               interrupts = <55>;
+                               clocks = <&clks 75>, <&clks 62>;
+                               clock-names = "ipg", "ahb";
+                               dr_mode = "host";
+                               phy_type = "ulpi";
+                               status = "disabled";
+                       };
+
+                       usbmisc: usbmisc at 10024600 {
+                               compatible = "fsl,imx27-usb";
+                               reg = <0x10024600 0x4>;
+                       };
+

Regards
Chris
>
>> usbmisc = data;
>>
>>    base is set to 0x10024000
>>
>> when I look around all other functions init functions did a offset calculation.
>
> Can you point me on this?
> For example, for i.MX5 CPUs we calculate only offset to PHY_CTRL_X register
> relative to basic offset 0x800, which is already defined in DTS.
>
> ...
>

r

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

Hello.

> >>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> >>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> >>>>>
> >>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> >>>>> ---
> >>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> >>>>>     1 file changed, 42 insertions(+)
...
> At this point might be good to patch the imx27.dtsi with the usb defines.
...

I have a working configuration for i.MX27 USB, but I prefer to make a few more
tests before the addition of definitions in DTS. This will be a next step.
Thanks.

---

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
> Hello.
> 
> > >>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> > >>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> > >>>>>
> > >>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> > >>>>> ---
> > >>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> > >>>>>     1 file changed, 42 insertions(+)
> ...
> > At this point might be good to patch the imx27.dtsi with the usb defines.
> ...
> 
> I have a working configuration for i.MX27 USB, but I prefer to make a few more
> tests before the addition of definitions in DTS. This will be a next step.
> Thanks.
Any news here?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
> > Hello.
> > 
> > > >>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> > > >>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> > > >>>>>
> > > >>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> > > >>>>> ---
> > > >>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> > > >>>>>     1 file changed, 42 insertions(+)
> > ...
> > > At this point might be good to patch the imx27.dtsi with the usb defines.
> > ...
> > 
> > I have a working configuration for i.MX27 USB, but I prefer to make a few more
> > tests before the addition of definitions in DTS. This will be a next step.
> > Thanks.
> Any news here?

Not ready yet.

---

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
> ???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
> > > Hello.
> > > 
> > > > >>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> > > > >>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> > > > >>>>>
> > > > >>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> > > > >>>>> ---
> > > > >>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> > > > >>>>>     1 file changed, 42 insertions(+)
> > > ...
> > > > At this point might be good to patch the imx27.dtsi with the usb defines.
> > > ...
> > > 
> > > I have a working configuration for i.MX27 USB, but I prefer to make a few more
> > > tests before the addition of definitions in DTS. This will be a next step.
> > > Thanks.
> > Any news here?
> 
> Not ready yet.
Are you still working at it? Would you mind sharing more details, like
your current tree/patch stack and what works/doesn't work for you?

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Alexander Shiyan]

???????????, 13 ?????? 2014, 22:31 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
> > ???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > > On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
> > > > Hello.
> > > > 
> > > > > >>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> > > > > >>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> > > > > >>>>>
> > > > > >>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> > > > > >>>>> ---
> > > > > >>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> > > > > >>>>>     1 file changed, 42 insertions(+)
> > > > ...
> > > > > At this point might be good to patch the imx27.dtsi with the usb defines.
> > > > ...
> > > > 
> > > > I have a working configuration for i.MX27 USB, but I prefer to make a few more
> > > > tests before the addition of definitions in DTS. This will be a next step.
> > > > Thanks.
> > > Any news here?
> > 
> > Not ready yet.
> Are you still working at it? Would you mind sharing more details, like
> your current tree/patch stack and what works/doesn't work for you?


Now my work on this is suspended, but will continue later.
I'll send you a personal letter with DT configuration.
Ports (both Host & OTG) are detected by kernel, but works Host only.
OTG not works nor as Host, nor as Device...

---

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Chris Ruehl]

On Tuesday, January 14, 2014 11:30 AM, Alexander Shiyan wrote:
> ???????????, 13 ?????? 2014, 22:31 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
>>> ???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>>>> On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
>>>>> Hello.
>>>>>
>>>>>>>>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
>>>>>>>>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
>>>>>>>>>>> ---
>>>>>>>>>>>      drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>      1 file changed, 42 insertions(+)
>>>>> ...
>>>>>> At this point might be good to patch the imx27.dtsi with the usb defines.
>>>>> ...
>>>>>
>>>>> I have a working configuration for i.MX27 USB, but I prefer to make a few more
>>>>> tests before the addition of definitions in DTS. This will be a next step.
>>>>> Thanks.
>>>> Any news here?
>>> Not ready yet.
>> Are you still working at it? Would you mind sharing more details, like
>> your current tree/patch stack and what works/doesn't work for you?
>
> Now my work on this is suspended, but will continue later.
> I'll send you a personal letter with DT configuration.
> Ports (both Host & OTG) are detected by kernel, but works Host only.
> OTG not works nor as Host, nor as Device...
>
> ---
hi,
I'm still working on my patches for imx27 and ULPI integration.  But 
have more urgent things in the queue.
I have a running version but it cant make it in the kernel. Rework on 
platform device code needed.
Just wait until the patches already accepted appears in the linux-next 
before commit new patches.

Chris

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

Hello Chris,

On Tue, Jan 14, 2014 at 11:53:47AM +0800, Chris Ruehl wrote:
> On Tuesday, January 14, 2014 11:30 AM, Alexander Shiyan wrote:
> >???????????, 13 ?????? 2014, 22:31 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> >>On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
> >>>???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> >>>>On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
> >>>>>Hello.
> >>>>>
> >>>>>>>>>>On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> >>>>>>>>>>>This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> >>>>>>>>>>>
> >>>>>>>>>>>Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> >>>>>>>>>>>---
> >>>>>>>>>>>     drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> >>>>>>>>>>>     1 file changed, 42 insertions(+)
> >>>>>...
> >>>>>>At this point might be good to patch the imx27.dtsi with the usb defines.
> >>>>>...
> >>>>>
> >>>>>I have a working configuration for i.MX27 USB, but I prefer to make a few more
> >>>>>tests before the addition of definitions in DTS. This will be a next step.
> >>>>>Thanks.
> >>>>Any news here?
> >>>Not ready yet.
> >>Are you still working at it? Would you mind sharing more details, like
> >>your current tree/patch stack and what works/doesn't work for you?
> >
> >Now my work on this is suspended, but will continue later.
> >I'll send you a personal letter with DT configuration.
> >Ports (both Host & OTG) are detected by kernel, but works Host only.
> >OTG not works nor as Host, nor as Device...
> >
> >---
> hi,
> I'm still working on my patches for imx27 and ULPI integration.  But
> have more urgent things in the queue.

Starting from here I'm not sure I understand you.
> I have a running version but it cant make it in the kernel. Rework
> on platform device code needed.
> Just wait until the patches already accepted appears in the
> linux-next before commit new patches.

That means: USB works for you? What does "it cant make it in the kernel"
mean? Problems to upstream it?
Do you wait or should I?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Chris Ruehl]

On Tuesday, January 14, 2014 09:49 PM, Uwe Kleine-K?nig wrote:
> Hello Chris,
>
> On Tue, Jan 14, 2014 at 11:53:47AM +0800, Chris Ruehl wrote:
>> On Tuesday, January 14, 2014 11:30 AM, Alexander Shiyan wrote:
>>> ???????????, 13 ?????? 2014, 22:31 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>>>> On Sat, Jan 11, 2014 at 06:01:48PM +0400, Alexander Shiyan wrote:
>>>>> ???????, 11 ?????? 2014, 13:55 +01:00 ?? Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>>>>>> On Mon, Nov 11, 2013 at 11:09:16AM +0400, Alexander Shiyan wrote:
>>>>>>> Hello.
>>>>>>>
>>>>>>>>>>>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
>>>>>>>>>>>>> This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>      drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>      1 file changed, 42 insertions(+)
>>>>>>> ...
>>>>>>>> At this point might be good to patch the imx27.dtsi with the usb defines.
>>>>>>> ...
>>>>>>>
>>>>>>> I have a working configuration for i.MX27 USB, but I prefer to make a few more
>>>>>>> tests before the addition of definitions in DTS. This will be a next step.
>>>>>>> Thanks.
>>>>>> Any news here?
>>>>> Not ready yet.
>>>> Are you still working at it? Would you mind sharing more details, like
>>>> your current tree/patch stack and what works/doesn't work for you?
>>>
>>> Now my work on this is suspended, but will continue later.
>>> I'll send you a personal letter with DT configuration.
>>> Ports (both Host & OTG) are detected by kernel, but works Host only.
>>> OTG not works nor as Host, nor as Device...
>>>
>>> ---
>> hi,
>> I'm still working on my patches for imx27 and ULPI integration.  But
>> have more urgent things in the queue.
>
> Starting from here I'm not sure I understand you.
>> I have a running version but it cant make it in the kernel. Rework
>> on platform device code needed.
I have a customized board running OTG/host and USB2/host (USB1 not connected)
Both ports are connected to a ISP1504 ULPI
a) I need a rs-gpio to reset in addition to the cs-gpio the ISP1504 (done)
b) Implement UPLI viewport (IORESOURCE_MEM) and logic to set the external power 
supply.
The code (b) was rejected and needs rework.


>> Just wait until the patches already accepted appears in the
>> linux-next before commit new patches.
>
> That means: USB works for you? What does "it cant make it in the kernel"
> mean? Problems to upstream it?
> Do you wait or should I?
I will fix the problem with (b) and resend my patches.


>
> Best regards
> Uwe
>

-- 
GTSYS Limited RFID Technology
A01 24/F Gold King Industrial Bld
35-41 Tai Lin Pai Road, Kwai Chung, Hong Kong
Fax (852) 8167 4060 - Tel (852) 3598 9488

Disclaimer: http://www.gtsys.com.hk/email/classified.html

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

Hello Chris,

On Wed, Jan 15, 2014 at 09:52:20AM +0800, Chris Ruehl wrote:
> I have a customized board running OTG/host and USB2/host (USB1 not connected)
> Both ports are connected to a ISP1504 ULPI
Similar for me here, my board has usb2 only with an ISP1504.
> a) I need a rs-gpio to reset in addition to the cs-gpio the ISP1504 (done)
I see there are several new chipidea patches in next-20140116 compared
to next-20140110 what I am using. Is there explicit support for a
cs-gpios property? If so, it's not obvious.

> b) Implement UPLI viewport (IORESOURCE_MEM) and logic to set the
> external power supply.
> The code (b) was rejected and needs rework.
The patch I'm currently using is appended below. It doesn't work yet,
but I think that's because CS isn't set yet. In my machine's dts I'm
using:

&usbphy2 {
        reset-gpios = <&gpio3 20 GPIO_ACTIVE_LOW>;
};

&usbh2 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_usbh2>;
        status = "okay";
};

Does this look ok?

I'm out of time currently for this project, but I will search the
mailing list for your patches. If you Cc: on new attempts for b) I will
try to test it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

 arch/arm/boot/dts/imx27-pingrp.h | 14 +++++++++
 arch/arm/boot/dts/imx27.dtsi     | 67 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/arch/arm/boot/dts/imx27-pingrp.h b/arch/arm/boot/dts/imx27-pingrp.h
index 57ca02f89dff..c4d698fddbc9 100644
--- a/arch/arm/boot/dts/imx27-pingrp.h
+++ b/arch/arm/boot/dts/imx27-pingrp.h
@@ -148,4 +148,18 @@
 	MX27_PAD_UART3_CTS__UART3_CTS 0x0 \
 	MX27_PAD_UART3_RTS__UART3_RTS 0x0
 
+#define MX27_USBH2_PINGRP1 \
+	MX27_PAD_USBH2_CLK__USBH2_CLK 0x0 \
+	MX27_PAD_USBH2_DIR__USBH2_DIR 0x0 \
+	MX27_PAD_USBH2_DATA7__USBH2_DATA7 0x0 \
+	MX27_PAD_USBH2_NXT__USBH2_NXT 0x0 \
+	MX27_PAD_USBH2_STP__USBH2_STP 0x0 \
+	MX27_PAD_CSPI2_SS2__USBH2_DATA4 0x0 \
+	MX27_PAD_CSPI2_SS1__USBH2_DATA3 0x0 \
+	MX27_PAD_CSPI2_SS0__USBH2_DATA6 0x0 \
+	MX27_PAD_CSPI2_SCLK__USBH2_DATA0 0x0 \
+	MX27_PAD_CSPI2_MISO__USBH2_DATA2 0x0 \
+	MX27_PAD_CSPI2_MOSI__USBH2_DATA1 0x0 \
+	MX27_PAD_CSPI1_SS2__USBH2_DATA5 0x0
+
 #endif /* __DTS_IMX27_PINGRP_H */
diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index 7e98966b1834..391769fb291e 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -33,6 +33,9 @@
 		spi0 = &cspi1;
 		spi1 = &cspi2;
 		spi2 = &cspi3;
+		usb0 = &usbotg;
+		usb1 = &usbh1;
+		usb2 = &usbh2;
 	};
 
 	aitc: aitc-interrupt-controller at e0000000 {
@@ -70,6 +73,33 @@
 		};
 	};
 
+	usbphy {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usbphy0: usbphy at 0 {
+			compatible = "usb-nop-xceiv";
+			reg = <0>;
+			clocks = <&clks 75>;
+			clock-names = "main_clk";
+		};
+
+		usbphy1: usbphy at 1 {
+			compatible = "usb-nop-xceiv";
+			reg = <1>;
+			clocks = <&clks 75>;
+			clock-names = "main_clk";
+		};
+
+		usbphy2: usbphy at 2 {
+			compatible = "usb-nop-xceiv";
+			reg = <2>;
+			clocks = <&clks 75>;
+			clock-names = "main_clk";
+		};
+	};
+
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
@@ -439,6 +469,43 @@
 				iram = <&iram>;
 			};
 
+			usbotg: usb at 10024000 {
+				compatible = "fsl,imx27-usb";
+				reg = <0x10024000 0x200>;
+				interrupts = <56>;
+				clocks = <&clks 15>;
+				fsl,usbmisc = <&usbmisc 0>;
+				fsl,usbphy = <&usbphy0>;
+				status = "disabled";
+			};
+
+			usbh1: usb at 10024200 {
+				compatible = "fsl,imx27-usb";
+				reg = <0x10024200 0x200>;
+				interrupts = <54>;
+				clocks = <&clks 15>;
+				fsl,usbmisc = <&usbmisc 1>;
+				fsl,usbphy = <&usbphy1>;
+				status = "disabled";
+			};
+
+			usbh2: usb at 10024400 {
+				compatible = "fsl,imx27-usb";
+				reg = <0x10024400 0x200>;
+				interrupts = <55>;
+				clocks = <&clks 15>;
+				fsl,usbmisc = <&usbmisc 2>;
+				fsl,usbphy = <&usbphy2>;
+				status = "disabled";
+			};
+
+			usbmisc: usbmisc at 10024600 {
+				#index-cells = <1>;
+				compatible = "fsl,imx27-usbmisc";
+				reg = <0x10024600 0x200>;
+				clocks = <&clks 62>;
+			};
+
 			sahara2: sahara at 10025000 {
 				compatible = "fsl,imx27-sahara";
 				reg = <0x10025000 0x1000>;
-- 
1.8.5.2

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

Hello Alexander,

On Tue, Jan 14, 2014 at 07:30:46AM +0400, Alexander Shiyan wrote:
> I'll send you a personal letter with DT configuration.
That would be great. You didn't send it yet, did you?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

Hello Alexander,

On Tue, Jan 14, 2014 at 02:47:39PM +0100, Uwe Kleine-K?nig wrote:
> On Tue, Jan 14, 2014 at 07:30:46AM +0400, Alexander Shiyan wrote:
> > I'll send you a personal letter with DT configuration.
> That would be great. You didn't send it yet, did you?
I take that back, just found it in my inbox. Don't know why I needed a
reply on this mail to find it. Sorry.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Uwe Kleine-König]

Hello,

On Mon, Nov 11, 2013 at 02:10:10PM +0800, Chris Ruehl wrote:
> On Monday, November 11, 2013 01:47 PM, Alexander Shiyan wrote:
> >>On Monday, November 11, 2013 12:45 PM, Alexander Shiyan wrote:
> >>>>Hi Alexander,
> >>>>
> >>>>On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
> >>>>>This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
> >>>>>
> >>>>>Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
> >>>>>---
> >>>>>    drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
> >>>>>    1 file changed, 42 insertions(+)
> >>>>>
> >>>>>diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> >>>...
> >>>>>+static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
> >>>>>+{
> >>>>>+	unsigned long flags;
> >>>>>+	u32 val;
> >>>>>+
> >>>>>+	switch (data->index) {
> >>>>>+	case 0:
> >>>>>+		val = MX27_OTG_PM_BIT;
> >>>>>+		break;
> >>>>>+	case 1:
> >>>>>+		val = MX27_H1_PM_BIT;
> >>>>>+		break;
> >>>>>+	case 2:
> >>>>>+		val = MX27_H2_PM_BIT;
> >>>>>+		break;
> >>>>>+	default:
> >>>>>+		return -EINVAL;
> >>>>>+	};
> >>>>>+
> >>>>
> >>>>     From my understanding this can not work, the usbmisc->base not point into the
> >>>>usb control register (USB_CTRL). Reference manual 30.5.1.1 says
> >>>>BASE + 0x600
> >>>>you must add the offset to the readl instruction.
> >>>
> >>>Why not work?
> >>>usbotg: usb at 10024000
> >>>usbh1: usb at 10024200
> >>>usbh2: usb at 10024400
> >>>usbmisc: usbmisc at 10024600
> >>>So, offset to USB_CTRL should already be defined by DTS.
> >>
> >>in the usbmisc_imx_probe() the base pointer is loaded from
> >>
> >>res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>data->base = devm_ioremap_resource(&pdev->dev, res);
> >>
> >>(and I did not see any of_ operations)
> >
> >Yes, and this is an address of usbmisc node, not otg.
> 
> Sorry, you are right. I misunderstood because I didn't see any of
> sample DTS files define usbmisc yet.
> 
> At this point might be good to patch the imx27.dtsi with the usb defines.
> 
> --- a/arch/arm/boot/dts/imx27.dtsi
> +++ b/arch/arm/boot/dts/imx27.dtsi
> @@ -30,6 +30,9 @@
>                 spi0 = &cspi1;
>                 spi1 = &cspi2;
>                 spi2 = &cspi3;
> +               usb0 = &usbotg;
> +               usb1 = &usbh1;
> +               usb2 = &usbh2;
>         };
> 
> @@ -404,6 +419,44 @@
>                                 iram = <&iram>;
>                         };
> 
> +                       usbotg: usb at 10024000 {
> +                               compatible = "fsl,imx27-usb";
> +                               reg = <0x10024000 0x200>;
> +                               interrupts = <56>;
> +                               clocks = <&clks 75>, <&clks 62>;
> +                               clock-names = "ipg", "ahb";
> +                               dr_mode = "host";
> +                               phy_type = "ulpi";
> +                               status = "disabled";
> +                       };
> +
> +                       usbh1: usb at 10024200 {
> +                               compatible = "fsl,imx27-usb";
> +                               reg = <0x10024200 0x200>;
> +                               interrupts = <54>;
> +                               clocks = <&clks 75>, <&clks 62>;
> +                               clock-names = "ipg", "ahb";
> +                               dr_mode = "host";
> +                               phy_type = "serial";
> +                               status = "disabled";
> +                       };
> +
> +                       usbh2: usb at 10024400 {
> +                               compatible = "fsl,imx27-usb";
> +                               reg = <0x10024400 0x200>;
> +                               interrupts = <55>;
> +                               clocks = <&clks 75>, <&clks 62>;
> +                               clock-names = "ipg", "ahb";
> +                               dr_mode = "host";
> +                               phy_type = "ulpi";
> +                               status = "disabled";
> +                       };
> +
> +                       usbmisc: usbmisc at 10024600 {
> +                               compatible = "fsl,imx27-usb";
> +                               reg = <0x10024600 0x4>;
> +                       };
I didn't try to get this up yet, but I wonder if the last compatible
here should better be fsl,imx27-usbmisc?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/2] usb: chipidea: usbmisc: Add support for i.MX27/i.MX31 CPUs

[Peter Chen]

On Mon, Nov 11, 2013 at 12:45 PM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> Hi Alexander,
>>
>> On Sunday, November 10, 2013 03:18 PM, Alexander Shiyan wrote:
>> > This adds i.MX27 and i.MX31 as the next user of the usbmisc driver.
>> >
>> > Signed-off-by: Alexander Shiyan<shc_work@mail.ru>
>> > ---
>> >   drivers/usb/chipidea/usbmisc_imx.c | 42 ++++++++++++++++++++++++++++++++++++++
>> >   1 file changed, 42 insertions(+)
>> >
>> > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> ...
>> > +static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
>> > +{
>> > +   unsigned long flags;
>> > +   u32 val;
>> > +
>> > +   switch (data->index) {
>> > +   case 0:
>> > +           val = MX27_OTG_PM_BIT;
>> > +           break;
>> > +   case 1:
>> > +           val = MX27_H1_PM_BIT;
>> > +           break;
>> > +   case 2:
>> > +           val = MX27_H2_PM_BIT;
>> > +           break;
>> > +   default:
>> > +           return -EINVAL;
>> > +   };
>> > +
>>
>>  From my understanding this can not work, the usbmisc->base not point into the
>> usb control register (USB_CTRL). Reference manual 30.5.1.1 says
>> BASE + 0x600
>> you must add the offset to the readl instruction.
>
> Why not work?
> usbotg: usb at 10024000
> usbh1: usb at 10024200
> usbh2: usb at 10024400
> usbmisc: usbmisc at 10024600
> So, offset to USB_CTRL should already be defined by DTS.
>
>
>> > +   spin_lock_irqsave(&usbmisc->lock, flags);
>> > +   if (data->disable_oc)
>> > +           val = readl(usbmisc->base) | val;
>>
>> else part not needed, the registers bits are set to 0 (reset)
>> the function is called on start-up once only, right?!
>>
>> > +   else
>> > +           val = readl(usbmisc->base)&  ~val;
>> > +   writel(val, usbmisc->base);
>> > +   spin_unlock_irqrestore(&usbmisc->lock, flags);
>> > +
>> > +   return 0;
>> > +}
>
> Bit can be set/cleared wrongly by the bootloader, it is not a big
> overhead to set it in proper state.
>

I think the "else" is needed, unless the over-current is not connected
or the user wants to disable it, we need to enable over-current by
default.

Acked-by: Peter Chen <peter.chen@freescale.com>

> ...
>
> ---
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
BR,
Peter Chen