RE: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
Signed-off-by: Alexander Shiyan shc_w...@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
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
Hello.
Signed-off-by: Alexander Shiyan shc_w...@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?
---
RE: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
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
N�r��yb�X��ǧv�^�){.n�+{��^n�r���z���h��� �G���h�(�階�ݢj���m��z�ޖ���f���h���~�m�
Re: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
On Sun, Nov 10, 2013 at 3:18 PM, Alexander Shiyan shc_w...@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_w...@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_BITBIT(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_H1BIT(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_OFFSET0x14
+#define MX5_USB_UH3_CTRL_OFFSET0x18
+#define MX5_BM_OVER_CUR_DIS_H1 BIT(5)
+#define MX5_BM_OVER_CUR_DIS_OTGBIT(8)
+#define MX5_BM_OVER_CUR_DIS_UHxBIT(30)
#define MX6_BM_OVER_CUR_DISBIT(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.c...@freescale.com
--
BR,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: chipidea: usbmisc: Add support for i.MX51 CPU
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_w...@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_w...@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 n...@bakuhatsu.net
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
