* [PATCH 1/2] pinctrl: imx: work around select input quirk
@ 2013-08-01 4:22 Shawn Guo
2013-08-01 4:22 ` [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID Shawn Guo
2013-08-01 6:51 ` [PATCH 1/2] pinctrl: imx: work around select input quirk Peter Chen
0 siblings, 2 replies; 12+ messages in thread
From: Shawn Guo @ 2013-08-01 4:22 UTC (permalink / raw)
To: linux-arm-kernel
The select input for some pin may not be implemented using the regular
select input register but the general purpose register. A real example
is that imx6q designers found the select input for USB OTG ID pin is
missing at the very late stage, and can not add a new select input
register but have to use a general purpose register bit to implement it.
The patch adds a workaround for such select input quirk by interpreting
the input_val cell of pin function ID in a different way, so that all
the info that needed for setting up select input bits in general purpose
register could be decoded from there.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/pinctrl/pinctrl-imx.c | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
index 57a4eb0..6ebe2e9 100644
--- a/drivers/pinctrl/pinctrl-imx.c
+++ b/drivers/pinctrl/pinctrl-imx.c
@@ -241,7 +241,30 @@ static int imx_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
/* some pins also need select input setting, set it if found */
if (input_reg[i]) {
- writel(input_val[i], ipctl->base + input_reg[i]);
+ u32 val = input_val[i];
+ /*
+ * If the select input value begins with 0xff, the value
+ * will be interpreted as below.
+ * 31 23 15 7 0
+ * | 0xff | shift | width | select |
+ * It's used to work around the problem that the select
+ * input for some pin is not implemented in the select
+ * input register but in some general purpose register.
+ * We encode the select input value, width and shift of
+ * the bit field into input_val cell of pin function ID
+ * in device tree, and then decode them here for setting
+ * up the select input bits in general purpose register.
+ */
+ if (val >> 24 == 0xff) {
+ u8 select = val & 0xff;
+ u8 width = (val >> 8) & 0xff;
+ u8 shift = (val >> 16) & 0xff;
+ u32 mask = ((1 << width) - 1) << shift;
+ val = readl(ipctl->base + input_reg[i]);
+ val &= ~mask;
+ val |= select << shift;
+ }
+ writel(val, ipctl->base + input_reg[i]);
dev_dbg(ipctl->dev,
"==>select_input: offset 0x%x val 0x%x\n",
input_reg[i], input_val[i]);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID
2013-08-01 4:22 [PATCH 1/2] pinctrl: imx: work around select input quirk Shawn Guo
@ 2013-08-01 4:22 ` Shawn Guo
2013-08-07 18:34 ` Linus Walleij
2013-08-01 6:51 ` [PATCH 1/2] pinctrl: imx: work around select input quirk Peter Chen
1 sibling, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2013-08-01 4:22 UTC (permalink / raw)
To: linux-arm-kernel
For some reason, the select input of pin function USB_OTG_ID is not
implemented via a regular select input register but using the bit
USB_OTG_ID_ SEL (shift 13) of IOMUXC_GPR1 register (offset 0x4).
As per the workaround for such quirk implemented in pinctrl driver,
we need to compose the input_val cell as below.
31 23 15 7 0
| 0xff | shift | width | select |
Thus, we have 0xff0d0100 for MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID and
0xff0d0101 for MX6QDL_PAD_GPIO_1__USB_OTG_ID in input_val cell.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
arch/arm/boot/dts/imx6q-pinfunc.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-pinfunc.h b/arch/arm/boot/dts/imx6q-pinfunc.h
index c0e38a4..44507a3 100644
--- a/arch/arm/boot/dts/imx6q-pinfunc.h
+++ b/arch/arm/boot/dts/imx6q-pinfunc.h
@@ -536,7 +536,7 @@
#define MX6QDL_PAD_ENET_REF_CLK__ESAI_RX_FS 0x1d4 0x4e8 0x85c 0x2 0x0
#define MX6QDL_PAD_ENET_REF_CLK__GPIO1_IO23 0x1d4 0x4e8 0x000 0x5 0x0
#define MX6QDL_PAD_ENET_REF_CLK__SPDIF_SR_CLK 0x1d4 0x4e8 0x000 0x6 0x0
-#define MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x1d8 0x4ec 0x000 0x0 0x0
+#define MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID 0x1d8 0x4ec 0x004 0x0 0xff0d0100
#define MX6QDL_PAD_ENET_RX_ER__ENET_RX_ER 0x1d8 0x4ec 0x000 0x1 0x0
#define MX6QDL_PAD_ENET_RX_ER__ESAI_RX_HF_CLK 0x1d8 0x4ec 0x864 0x2 0x0
#define MX6QDL_PAD_ENET_RX_ER__SPDIF_IN 0x1d8 0x4ec 0x914 0x3 0x1
@@ -654,7 +654,7 @@
#define MX6QDL_PAD_GPIO_1__ESAI_RX_CLK 0x224 0x5f4 0x86c 0x0 0x1
#define MX6QDL_PAD_GPIO_1__WDOG2_B 0x224 0x5f4 0x000 0x1 0x0
#define MX6QDL_PAD_GPIO_1__KEY_ROW5 0x224 0x5f4 0x8f4 0x2 0x0
-#define MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x224 0x5f4 0x000 0x3 0x0
+#define MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x224 0x5f4 0x004 0x3 0xff0d0101
#define MX6QDL_PAD_GPIO_1__PWM2_OUT 0x224 0x5f4 0x000 0x4 0x0
#define MX6QDL_PAD_GPIO_1__GPIO1_IO01 0x224 0x5f4 0x000 0x5 0x0
#define MX6QDL_PAD_GPIO_1__SD1_CD_B 0x224 0x5f4 0x000 0x6 0x0
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-01 4:22 [PATCH 1/2] pinctrl: imx: work around select input quirk Shawn Guo
2013-08-01 4:22 ` [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID Shawn Guo
@ 2013-08-01 6:51 ` Peter Chen
2013-08-01 7:08 ` Shawn Guo
1 sibling, 1 reply; 12+ messages in thread
From: Peter Chen @ 2013-08-01 6:51 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 01, 2013 at 12:22:03PM +0800, Shawn Guo wrote:
> The select input for some pin may not be implemented using the regular
> select input register but the general purpose register. A real example
> is that imx6q designers found the select input for USB OTG ID pin is
> missing at the very late stage, and can not add a new select input
> register but have to use a general purpose register bit to implement it.
>
> The patch adds a workaround for such select input quirk by interpreting
> the input_val cell of pin function ID in a different way, so that all
> the info that needed for setting up select input bits in general purpose
> register could be decoded from there.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/pinctrl/pinctrl-imx.c | 25 ++++++++++++++++++++++++-
> 1 files changed, 24 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-imx.c b/drivers/pinctrl/pinctrl-imx.c
> index 57a4eb0..6ebe2e9 100644
> --- a/drivers/pinctrl/pinctrl-imx.c
> +++ b/drivers/pinctrl/pinctrl-imx.c
> @@ -241,7 +241,30 @@ static int imx_pmx_enable(struct pinctrl_dev *pctldev, unsigned selector,
>
> /* some pins also need select input setting, set it if found */
> if (input_reg[i]) {
> - writel(input_val[i], ipctl->base + input_reg[i]);
> + u32 val = input_val[i];
> + /*
> + * If the select input value begins with 0xff, the value
> + * will be interpreted as below.
> + * 31 23 15 7 0
> + * | 0xff | shift | width | select |
> + * It's used to work around the problem that the select
> + * input for some pin is not implemented in the select
> + * input register but in some general purpose register.
> + * We encode the select input value, width and shift of
> + * the bit field into input_val cell of pin function ID
> + * in device tree, and then decode them here for setting
> + * up the select input bits in general purpose register.
> + */
> + if (val >> 24 == 0xff) {
> + u8 select = val & 0xff;
> + u8 width = (val >> 8) & 0xff;
> + u8 shift = (val >> 16) & 0xff;
> + u32 mask = ((1 << width) - 1) << shift;
> + val = readl(ipctl->base + input_reg[i]);
> + val &= ~mask;
> + val |= select << shift;
> + }
> + writel(val, ipctl->base + input_reg[i]);
> dev_dbg(ipctl->dev,
> "==>select_input: offset 0x%x val 0x%x\n",
> input_reg[i], input_val[i]);
> --
> 1.7.1
>
Shawn, input_reg[i] may be 0 if the fixed one at first register of IOMUXC
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-01 6:51 ` [PATCH 1/2] pinctrl: imx: work around select input quirk Peter Chen
@ 2013-08-01 7:08 ` Shawn Guo
2013-08-01 8:32 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2013-08-01 7:08 UTC (permalink / raw)
To: linux-arm-kernel
On 1 August 2013 14:51, Peter Chen <peter.chen@freescale.com> wrote:
> Shawn, input_reg[i] may be 0 if the fixed one at first register of IOMUXC
Ah, yes. Now select input register at offset 0 becomes possible. I
will try to fix it.
Regardless of this, does the series work for you?
Shawn
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-01 7:08 ` Shawn Guo
@ 2013-08-01 8:32 ` Peter Chen
2013-08-04 12:54 ` Shawn Guo
0 siblings, 1 reply; 12+ messages in thread
From: Peter Chen @ 2013-08-01 8:32 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 01, 2013 at 03:08:06PM +0800, Shawn Guo wrote:
> On 1 August 2013 14:51, Peter Chen <peter.chen@freescale.com> wrote:
> > Shawn, input_reg[i] may be 0 if the fixed one at first register of IOMUXC
>
> Ah, yes. Now select input register at offset 0 becomes possible. I
> will try to fix it.
>
> Regardless of this, does the series work for you?
>
Yes, it works, you can add : Tested-by: Peter Chen <peter.chen@freescale.com>
I changed the second patch manually, have you changed the pin prefix
at arch/arm/boot/dts/imx6q-pinfunc.h? At 3.10, it is MX6Q_XXX.
Besides, please add comments for u16 *input_reg at struct imx_pin_group.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-01 8:32 ` Peter Chen
@ 2013-08-04 12:54 ` Shawn Guo
2013-08-05 1:14 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2013-08-04 12:54 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 01, 2013 at 04:32:23PM +0800, Peter Chen wrote:
> On Thu, Aug 01, 2013 at 03:08:06PM +0800, Shawn Guo wrote:
> > On 1 August 2013 14:51, Peter Chen <peter.chen@freescale.com> wrote:
> > > Shawn, input_reg[i] may be 0 if the fixed one at first register of IOMUXC
> >
> > Ah, yes. Now select input register at offset 0 becomes possible. I
> > will try to fix it.
> >
> > Regardless of this, does the series work for you?
> >
> Yes, it works, you can add : Tested-by: Peter Chen <peter.chen@freescale.com>
Thanks.
>
> I changed the second patch manually, have you changed the pin prefix
> at arch/arm/boot/dts/imx6q-pinfunc.h? At 3.10, it is MX6Q_XXX.
Yes, we changed the prefix to simplify the DTS files for imx6q and
imx6dl.
>
> Besides, please add comments for u16 *input_reg at struct imx_pin_group.
I'm not fond of documenting a workaround for a random quirky select
input as a feature all over the files where input_reg is documented.
It should be good enough to have it well documented at where the quirk
is handled.
Shawn
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-04 12:54 ` Shawn Guo
@ 2013-08-05 1:14 ` Peter Chen
2013-08-05 3:26 ` Shawn Guo
0 siblings, 1 reply; 12+ messages in thread
From: Peter Chen @ 2013-08-05 1:14 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Aug 04, 2013 at 08:54:54PM +0800, Shawn Guo wrote:
> Yes, we changed the prefix to simplify the DTS files for imx6q and
> imx6dl.
>
> >
> > Besides, please add comments for u16 *input_reg at struct imx_pin_group.
>
> I'm not fond of documenting a workaround for a random quirky select
> input as a feature all over the files where input_reg is documented.
> It should be good enough to have it well documented at where the quirk
> is handled.
If the user finds "odd value" at xxx-pinfunc.h, how he knows what
it stands for? At least, It should be documented where the user
can find its meaning.
Best regards,
Peter
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-05 3:26 ` Shawn Guo
@ 2013-08-05 1:35 ` Peter Chen
0 siblings, 0 replies; 12+ messages in thread
From: Peter Chen @ 2013-08-05 1:35 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 05, 2013 at 11:26:14AM +0800, Shawn Guo wrote:
> On Mon, Aug 05, 2013 at 09:14:50AM +0800, Peter Chen wrote:
> > On Sun, Aug 04, 2013 at 08:54:54PM +0800, Shawn Guo wrote:
> > > Yes, we changed the prefix to simplify the DTS files for imx6q and
> > > imx6dl.
> > >
> > > >
> > > > Besides, please add comments for u16 *input_reg at struct imx_pin_group.
> > >
> > > I'm not fond of documenting a workaround for a random quirky select
> > > input as a feature all over the files where input_reg is documented.
> > > It should be good enough to have it well documented at where the quirk
> > > is handled.
> >
> > If the user finds "odd value" at xxx-pinfunc.h, how he knows what
> > it stands for? At least, It should be documented where the user
> > can find its meaning.
>
> We have it well documented in pinctrl-imx.c, function imx_pmx_enable()
> where the "odd value" is handled.
>
> But I would try to look at the git log of xxx-pinfunc.h at the first
> place to see where and how the "odd value" comes.
>
Yes, it is also OK.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pinctrl: imx: work around select input quirk
2013-08-05 1:14 ` Peter Chen
@ 2013-08-05 3:26 ` Shawn Guo
2013-08-05 1:35 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2013-08-05 3:26 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Aug 05, 2013 at 09:14:50AM +0800, Peter Chen wrote:
> On Sun, Aug 04, 2013 at 08:54:54PM +0800, Shawn Guo wrote:
> > Yes, we changed the prefix to simplify the DTS files for imx6q and
> > imx6dl.
> >
> > >
> > > Besides, please add comments for u16 *input_reg at struct imx_pin_group.
> >
> > I'm not fond of documenting a workaround for a random quirky select
> > input as a feature all over the files where input_reg is documented.
> > It should be good enough to have it well documented at where the quirk
> > is handled.
>
> If the user finds "odd value" at xxx-pinfunc.h, how he knows what
> it stands for? At least, It should be documented where the user
> can find its meaning.
We have it well documented in pinctrl-imx.c, function imx_pmx_enable()
where the "odd value" is handled.
But I would try to look at the git log of xxx-pinfunc.h at the first
place to see where and how the "odd value" comes.
Shawn
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID
2013-08-01 4:22 ` [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID Shawn Guo
@ 2013-08-07 18:34 ` Linus Walleij
2013-08-08 1:27 ` Shawn Guo
0 siblings, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2013-08-07 18:34 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 1, 2013 at 6:22 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> For some reason, the select input of pin function USB_OTG_ID is not
> implemented via a regular select input register but using the bit
> USB_OTG_ID_ SEL (shift 13) of IOMUXC_GPR1 register (offset 0x4).
>
> As per the workaround for such quirk implemented in pinctrl driver,
> we need to compose the input_val cell as below.
>
> 31 23 15 7 0
> | 0xff | shift | width | select |
>
> Thus, we have 0xff0d0100 for MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID and
> 0xff0d0101 for MX6QDL_PAD_GPIO_1__USB_OTG_ID in input_val cell.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
This does not apply on top of my "devel" branch in the pinctrl tree.
Could you respin this on top of devel and resend?
Or do I need some other patch(es) for this to apply?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID
2013-08-07 18:34 ` Linus Walleij
@ 2013-08-08 1:27 ` Shawn Guo
2013-08-13 6:18 ` Linus Walleij
0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2013-08-08 1:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Aug 07, 2013 at 08:34:05PM +0200, Linus Walleij wrote:
> On Thu, Aug 1, 2013 at 6:22 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> > For some reason, the select input of pin function USB_OTG_ID is not
> > implemented via a regular select input register but using the bit
> > USB_OTG_ID_ SEL (shift 13) of IOMUXC_GPR1 register (offset 0x4).
> >
> > As per the workaround for such quirk implemented in pinctrl driver,
> > we need to compose the input_val cell as below.
> >
> > 31 23 15 7 0
> > | 0xff | shift | width | select |
> >
> > Thus, we have 0xff0d0100 for MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID and
> > 0xff0d0101 for MX6QDL_PAD_GPIO_1__USB_OTG_ID in input_val cell.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>
> This does not apply on top of my "devel" branch in the pinctrl tree.
>
> Could you respin this on top of devel and resend?
>
> Or do I need some other patch(es) for this to apply?
Sorry for not making this clear. Due to the changes happened on IMX
tree, we will have to queue this one for 3.13 merge window through IMX
tree, after driver change hits mainline. In short, I will take care of
the merge of this patch.
Shawn
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID
2013-08-08 1:27 ` Shawn Guo
@ 2013-08-13 6:18 ` Linus Walleij
0 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2013-08-13 6:18 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Aug 8, 2013 at 3:27 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Wed, Aug 07, 2013 at 08:34:05PM +0200, Linus Walleij wrote:
>> On Thu, Aug 1, 2013 at 6:22 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>
>> > For some reason, the select input of pin function USB_OTG_ID is not
>> > implemented via a regular select input register but using the bit
>> > USB_OTG_ID_ SEL (shift 13) of IOMUXC_GPR1 register (offset 0x4).
>> >
>> > As per the workaround for such quirk implemented in pinctrl driver,
>> > we need to compose the input_val cell as below.
>> >
>> > 31 23 15 7 0
>> > | 0xff | shift | width | select |
>> >
>> > Thus, we have 0xff0d0100 for MX6QDL_PAD_ENET_RX_ER__USB_OTG_ID and
>> > 0xff0d0101 for MX6QDL_PAD_GPIO_1__USB_OTG_ID in input_val cell.
>> >
>> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>
>> This does not apply on top of my "devel" branch in the pinctrl tree.
>>
>> Could you respin this on top of devel and resend?
>>
>> Or do I need some other patch(es) for this to apply?
>
> Sorry for not making this clear. Due to the changes happened on IMX
> tree, we will have to queue this one for 3.13 merge window through IMX
> tree, after driver change hits mainline. In short, I will take care of
> the merge of this patch.
OK Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-08-13 6:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01 4:22 [PATCH 1/2] pinctrl: imx: work around select input quirk Shawn Guo
2013-08-01 4:22 ` [PATCH 2/2] ARM: dts: imx6q: add quirky select input for USB_OTG_ID Shawn Guo
2013-08-07 18:34 ` Linus Walleij
2013-08-08 1:27 ` Shawn Guo
2013-08-13 6:18 ` Linus Walleij
2013-08-01 6:51 ` [PATCH 1/2] pinctrl: imx: work around select input quirk Peter Chen
2013-08-01 7:08 ` Shawn Guo
2013-08-01 8:32 ` Peter Chen
2013-08-04 12:54 ` Shawn Guo
2013-08-05 1:14 ` Peter Chen
2013-08-05 3:26 ` Shawn Guo
2013-08-05 1:35 ` Peter Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).