* [PATCH 1/3] pinctrl: single: fix DT bindings documentation
@ 2013-11-28 11:29 Tomi Valkeinen
2013-11-28 11:29 ` [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux Tomi Valkeinen
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tomi Valkeinen @ 2013-11-28 11:29 UTC (permalink / raw)
To: Tony Lindgren, linux-kernel; +Cc: Peter Ujfalusi, Tomi Valkeinen
Remove extra comma in pinctrl-single documentation.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
index 7069a0b84e3a..bc0dfdfdb148 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -98,7 +98,7 @@ below for more information.
In case when one register changes more than one pin's mux the
pinctrl-single,bits need to be used which takes three parameters:
- pinctrl-single,bits = <0xdc 0x18, 0xff>;
+ pinctrl-single,bits = <0xdc 0x18 0xff>;
Where 0xdc is the offset from the pinctrl register base address for the
device pinctrl register, 0x18 is the desired value, and 0xff is the sub mask to
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux
2013-11-28 11:29 [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tomi Valkeinen
@ 2013-11-28 11:29 ` Tomi Valkeinen
2013-11-28 12:00 ` Peter Ujfalusi
2013-11-28 11:29 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
2013-11-30 17:05 ` [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tony Lindgren
2 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2013-11-28 11:29 UTC (permalink / raw)
To: Tony Lindgren, linux-kernel; +Cc: Peter Ujfalusi, Tomi Valkeinen
pcs_enable() uses vals->mask instead of pcs->fmask when bits_per_mux is
enabled. However, pcs_disable() always uses pcs->fmask.
Fix pcs_disable() to use vals->mask with bits_per_mux.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/pinctrl/pinctrl-single.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 829b98c5c66f..174f4c50cd77 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -525,12 +525,18 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
for (i = 0; i < func->nvals; i++) {
struct pcs_func_vals *vals;
unsigned long flags;
- unsigned val;
+ unsigned val, mask;
vals = &func->vals[i];
raw_spin_lock_irqsave(&pcs->lock, flags);
val = pcs->read(vals->reg);
- val &= ~pcs->fmask;
+
+ if (pcs->bits_per_mux)
+ mask = vals->mask;
+ else
+ mask = pcs->fmask;
+
+ val &= ~mask;
val |= pcs->foff << pcs->fshift;
pcs->write(val, vals->reg);
raw_spin_unlock_irqrestore(&pcs->lock, flags);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask
2013-11-28 11:29 [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tomi Valkeinen
2013-11-28 11:29 ` [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux Tomi Valkeinen
@ 2013-11-28 11:29 ` Tomi Valkeinen
2013-11-30 17:21 ` Tony Lindgren
2013-11-30 17:05 ` [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tony Lindgren
2 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2013-11-28 11:29 UTC (permalink / raw)
To: Tony Lindgren, linux-kernel; +Cc: Peter Ujfalusi, Tomi Valkeinen
If the masks in DT data are not quite right,
pcs_parse_bits_in_pinctrl_entry() can end up in an infinite loop,
trashing memory at the same time.
Add a check to verify that each loop actually removes bits from the
'mask', so that the loop can eventually end.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
drivers/pinctrl/pinctrl-single.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 174f4c50cd77..de6459628b4f 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1318,6 +1318,14 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
mask_pos = ((pcs->fmask) << (bit_pos - 1));
val_pos = val & mask_pos;
submask = mask & mask_pos;
+
+ if ((mask & mask_pos) == 0) {
+ dev_err(pcs->dev,
+ "Invalid mask for %s at 0x%x\n",
+ np->name, offset);
+ break;
+ }
+
mask &= ~mask_pos;
if (submask != mask_pos) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux
2013-11-28 11:29 ` [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux Tomi Valkeinen
@ 2013-11-28 12:00 ` Peter Ujfalusi
2013-11-30 17:07 ` Tony Lindgren
0 siblings, 1 reply; 10+ messages in thread
From: Peter Ujfalusi @ 2013-11-28 12:00 UTC (permalink / raw)
To: Tomi Valkeinen, Tony Lindgren, linux-kernel
On 11/28/2013 01:29 PM, Tomi Valkeinen wrote:
> pcs_enable() uses vals->mask instead of pcs->fmask when bits_per_mux is
> enabled. However, pcs_disable() always uses pcs->fmask.
>
> Fix pcs_disable() to use vals->mask with bits_per_mux.
I wonder how did I missed this?
Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/pinctrl/pinctrl-single.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 829b98c5c66f..174f4c50cd77 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -525,12 +525,18 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
> for (i = 0; i < func->nvals; i++) {
> struct pcs_func_vals *vals;
> unsigned long flags;
> - unsigned val;
> + unsigned val, mask;
>
> vals = &func->vals[i];
> raw_spin_lock_irqsave(&pcs->lock, flags);
> val = pcs->read(vals->reg);
> - val &= ~pcs->fmask;
> +
> + if (pcs->bits_per_mux)
> + mask = vals->mask;
> + else
> + mask = pcs->fmask;
> +
> + val &= ~mask;
> val |= pcs->foff << pcs->fshift;
> pcs->write(val, vals->reg);
> raw_spin_unlock_irqrestore(&pcs->lock, flags);
>
--
Péter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] pinctrl: single: fix DT bindings documentation
2013-11-28 11:29 [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tomi Valkeinen
2013-11-28 11:29 ` [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux Tomi Valkeinen
2013-11-28 11:29 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
@ 2013-11-30 17:05 ` Tony Lindgren
2 siblings, 0 replies; 10+ messages in thread
From: Tony Lindgren @ 2013-11-30 17:05 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-kernel, Peter Ujfalusi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [131128 03:30]:
> Remove extra comma in pinctrl-single documentation.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
> ---
> Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
> index 7069a0b84e3a..bc0dfdfdb148 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
> @@ -98,7 +98,7 @@ below for more information.
> In case when one register changes more than one pin's mux the
> pinctrl-single,bits need to be used which takes three parameters:
>
> - pinctrl-single,bits = <0xdc 0x18, 0xff>;
> + pinctrl-single,bits = <0xdc 0x18 0xff>;
>
> Where 0xdc is the offset from the pinctrl register base address for the
> device pinctrl register, 0x18 is the desired value, and 0xff is the sub mask to
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux
2013-11-28 12:00 ` Peter Ujfalusi
@ 2013-11-30 17:07 ` Tony Lindgren
2013-11-30 17:24 ` Tony Lindgren
0 siblings, 1 reply; 10+ messages in thread
From: Tony Lindgren @ 2013-11-30 17:07 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: Tomi Valkeinen, linux-kernel
* Peter Ujfalusi <peter.ujfalusi@ti.com> [131128 04:01]:
> On 11/28/2013 01:29 PM, Tomi Valkeinen wrote:
> > pcs_enable() uses vals->mask instead of pcs->fmask when bits_per_mux is
> > enabled. However, pcs_disable() always uses pcs->fmask.
> >
> > Fix pcs_disable() to use vals->mask with bits_per_mux.
>
> I wonder how did I missed this?
Probably because the disable can be a nop for some hardware.
> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
> > drivers/pinctrl/pinctrl-single.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> > index 829b98c5c66f..174f4c50cd77 100644
> > --- a/drivers/pinctrl/pinctrl-single.c
> > +++ b/drivers/pinctrl/pinctrl-single.c
> > @@ -525,12 +525,18 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
> > for (i = 0; i < func->nvals; i++) {
> > struct pcs_func_vals *vals;
> > unsigned long flags;
> > - unsigned val;
> > + unsigned val, mask;
> >
> > vals = &func->vals[i];
> > raw_spin_lock_irqsave(&pcs->lock, flags);
> > val = pcs->read(vals->reg);
> > - val &= ~pcs->fmask;
> > +
> > + if (pcs->bits_per_mux)
> > + mask = vals->mask;
> > + else
> > + mask = pcs->fmask;
> > +
> > + val &= ~mask;
> > val |= pcs->foff << pcs->fshift;
> > pcs->write(val, vals->reg);
> > raw_spin_unlock_irqrestore(&pcs->lock, flags);
> >
>
>
> --
> Péter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask
2013-11-28 11:29 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
@ 2013-11-30 17:21 ` Tony Lindgren
0 siblings, 0 replies; 10+ messages in thread
From: Tony Lindgren @ 2013-11-30 17:21 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-kernel, Peter Ujfalusi
* Tomi Valkeinen <tomi.valkeinen@ti.com> [131128 03:30]:
> If the masks in DT data are not quite right,
> pcs_parse_bits_in_pinctrl_entry() can end up in an infinite loop,
> trashing memory at the same time.
>
> Add a check to verify that each loop actually removes bits from the
> 'mask', so that the loop can eventually end.
Linus W, might be worth updating the change log with the regression
causing commit. Something like commit 4e7e8017a80e1 (pinctrl: pinctrl-single:
enhance to configure multiple pins of different modules) improved
support for pinctrl-single,bits option, but also caused a regression
in parsing badly configured mask data.
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/pinctrl/pinctrl-single.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 174f4c50cd77..de6459628b4f 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -1318,6 +1318,14 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
> mask_pos = ((pcs->fmask) << (bit_pos - 1));
> val_pos = val & mask_pos;
> submask = mask & mask_pos;
> +
> + if ((mask & mask_pos) == 0) {
> + dev_err(pcs->dev,
> + "Invalid mask for %s at 0x%x\n",
> + np->name, offset);
> + break;
> + }
> +
> mask &= ~mask_pos;
>
> if (submask != mask_pos) {
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux
2013-11-30 17:07 ` Tony Lindgren
@ 2013-11-30 17:24 ` Tony Lindgren
0 siblings, 0 replies; 10+ messages in thread
From: Tony Lindgren @ 2013-11-30 17:24 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: Tomi Valkeinen, linux-kernel
* Tony Lindgren <tony@atomide.com> [131130 09:08]:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [131128 04:01]:
> > On 11/28/2013 01:29 PM, Tomi Valkeinen wrote:
> > > pcs_enable() uses vals->mask instead of pcs->fmask when bits_per_mux is
> > > enabled. However, pcs_disable() always uses pcs->fmask.
> > >
> > > Fix pcs_disable() to use vals->mask with bits_per_mux.
> >
> > I wonder how did I missed this?
>
> Probably because the disable can be a nop for some hardware.
>
> > Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Linus W, might be worth also mentioning the regression causing commit
here. Commit 9e605cb68a21d5 (pinctrl: pinctrl-single: Add
pinctrl-single,bits type of mux) added support for multiple muxes
in a single register, but forgot to modify pcs_disable() function the
same way as pcs_enable() was modified.
> Acked-by: Tony Lindgren <tony@atomide.com>
>
>
> > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > > ---
> > > drivers/pinctrl/pinctrl-single.c | 10 ++++++++--
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> > > index 829b98c5c66f..174f4c50cd77 100644
> > > --- a/drivers/pinctrl/pinctrl-single.c
> > > +++ b/drivers/pinctrl/pinctrl-single.c
> > > @@ -525,12 +525,18 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
> > > for (i = 0; i < func->nvals; i++) {
> > > struct pcs_func_vals *vals;
> > > unsigned long flags;
> > > - unsigned val;
> > > + unsigned val, mask;
> > >
> > > vals = &func->vals[i];
> > > raw_spin_lock_irqsave(&pcs->lock, flags);
> > > val = pcs->read(vals->reg);
> > > - val &= ~pcs->fmask;
> > > +
> > > + if (pcs->bits_per_mux)
> > > + mask = vals->mask;
> > > + else
> > > + mask = pcs->fmask;
> > > +
> > > + val &= ~mask;
> > > val |= pcs->foff << pcs->fshift;
> > > pcs->write(val, vals->reg);
> > > raw_spin_unlock_irqrestore(&pcs->lock, flags);
> > >
> >
> >
> > --
> > Péter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask
2014-01-09 12:50 Tomi Valkeinen
@ 2014-01-09 12:50 ` Tomi Valkeinen
2014-01-15 7:32 ` Linus Walleij
0 siblings, 1 reply; 10+ messages in thread
From: Tomi Valkeinen @ 2014-01-09 12:50 UTC (permalink / raw)
To: linux-kernel, Linus Walleij; +Cc: Tomi Valkeinen
commit 4e7e8017a80e1 (pinctrl: pinctrl-single:
enhance to configure multiple pins of different modules) improved
support for pinctrl-single,bits option, but also caused a regression
in parsing badly configured mask data.
If the masks in DT data are not quite right,
pcs_parse_bits_in_pinctrl_entry() can end up in an infinite loop,
trashing memory at the same time.
Add a check to verify that each loop actually removes bits from the
'mask', so that the loop can eventually end.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
drivers/pinctrl/pinctrl-single.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 174f4c50cd77..de6459628b4f 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1318,6 +1318,14 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
mask_pos = ((pcs->fmask) << (bit_pos - 1));
val_pos = val & mask_pos;
submask = mask & mask_pos;
+
+ if ((mask & mask_pos) == 0) {
+ dev_err(pcs->dev,
+ "Invalid mask for %s at 0x%x\n",
+ np->name, offset);
+ break;
+ }
+
mask &= ~mask_pos;
if (submask != mask_pos) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask
2014-01-09 12:50 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
@ 2014-01-15 7:32 ` Linus Walleij
0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2014-01-15 7:32 UTC (permalink / raw)
To: Tomi Valkeinen, Haojian Zhuang; +Cc: linux-kernel@vger.kernel.org
On Thu, Jan 9, 2014 at 1:50 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> commit 4e7e8017a80e1 (pinctrl: pinctrl-single:
> enhance to configure multiple pins of different modules) improved
> support for pinctrl-single,bits option, but also caused a regression
> in parsing badly configured mask data.
>
> If the masks in DT data are not quite right,
> pcs_parse_bits_in_pinctrl_entry() can end up in an infinite loop,
> trashing memory at the same time.
>
> Add a check to verify that each loop actually removes bits from the
> 'mask', so that the loop can eventually end.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-01-15 7:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-28 11:29 [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tomi Valkeinen
2013-11-28 11:29 ` [PATCH 2/3] pinctrl: single: fix pcs_disable with bits_per_mux Tomi Valkeinen
2013-11-28 12:00 ` Peter Ujfalusi
2013-11-30 17:07 ` Tony Lindgren
2013-11-30 17:24 ` Tony Lindgren
2013-11-28 11:29 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
2013-11-30 17:21 ` Tony Lindgren
2013-11-30 17:05 ` [PATCH 1/3] pinctrl: single: fix DT bindings documentation Tony Lindgren
-- strict thread matches above, loose matches on Subject: below --
2014-01-09 12:50 Tomi Valkeinen
2014-01-09 12:50 ` [PATCH 3/3] pinctrl: single: fix infinite loop caused by bad mask Tomi Valkeinen
2014-01-15 7:32 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox