All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
@ 2016-03-07 18:40 Wolfram Sang
  2016-03-07 19:56 ` Geert Uytterhoeven
  2016-03-15  8:29 ` Linus Walleij
  0 siblings, 2 replies; 19+ messages in thread
From: Wolfram Sang @ 2016-03-07 18:40 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Wolfram Sang, Geert Uytterhoeven, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

If pinctrl_provide_dummies() is used unconditionally, then the dummy
state will be used even on DT platforms when the "init" state was
intentionally left out. Instead of "default", the dummy "init" state
will then be used during probe. Thus, when probing an I2C controller on
cold boot, communication triggered by bus notifiers broke because the
pins were not initialized.

Do it like OMAP2: use the dummy state only for non-DT platforms.

Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Geert recently pointed out the problem, that the IRQ2 fixup for the
DA9xxx PMICs failed on Lager on cold boot. I could verify this iff IIC3
was used and not I2C3. IIC3 is the default, however I mostly used I2C3
recently, because it has the slave capabilities.

The original pinctrl_provide_dummies() is there since the beginning of the
file, so in order to avoid regressions, the below solution looks plausible to
me. I do not have much experience with hardware older than Gen2, though, so
comments are much appreciated!


 drivers/pinctrl/sh-pfc/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 0c2d14c504aa1d..db53d7bbc16e18 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	pinctrl_provide_dummies();
+	/* Enable dummy states for those platforms without pinctrl support */
+	if (!of_have_populated_dt())
+		pinctrl_provide_dummies();
 
 	ret = sh_pfc_init_ranges(pfc);
 	if (ret < 0)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 18:40 [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms Wolfram Sang
@ 2016-03-07 19:56 ` Geert Uytterhoeven
  2016-03-07 20:02   ` Sergei Shtylyov
  2016-03-15  8:29 ` Linus Walleij
  1 sibling, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-07 19:56 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Laurent Pinchart, Kuninori Morimoto,
	Magnus Damm, Geert Uytterhoeven, linux-gpio, Sergei Shtylyov,
	Linus Walleij

Hi Wolfram,

On Mon, Mar 7, 2016 at 7:40 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> If pinctrl_provide_dummies() is used unconditionally, then the dummy
> state will be used even on DT platforms when the "init" state was
> intentionally left out. Instead of "default", the dummy "init" state
> will then be used during probe. Thus, when probing an I2C controller on
> cold boot, communication triggered by bus notifiers broke because the
> pins were not initialized.
>
> Do it like OMAP2: use the dummy state only for non-DT platforms.

Thanks!

Interestingly, Sergei submitted a similar patch a few days ago
"pinctrl: sh-pfc: core: only call pinctrl_provide_dummies"
(https://patchwork.ozlabs.org/patch/592245/)

>
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Geert recently pointed out the problem, that the IRQ2 fixup for the
> DA9xxx PMICs failed on Lager on cold boot. I could verify this iff IIC3
> was used and not I2C3. IIC3 is the default, however I mostly used I2C3
> recently, because it has the slave capabilities.
>
> The original pinctrl_provide_dummies() is there since the beginning of the
> file, so in order to avoid regressions, the below solution looks plausible to
> me. I do not have much experience with hardware older than Gen2, though, so
> comments are much appreciated!
>
>
>  drivers/pinctrl/sh-pfc/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 0c2d14c504aa1d..db53d7bbc16e18 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
>                         return ret;
>         }
>
> -       pinctrl_provide_dummies();
> +       /* Enable dummy states for those platforms without pinctrl support */
> +       if (!of_have_populated_dt())
> +               pinctrl_provide_dummies();
>
>         ret = sh_pfc_init_ranges(pfc);
>         if (ret < 0)
> --
> 2.7.0

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 19:56 ` Geert Uytterhoeven
@ 2016-03-07 20:02   ` Sergei Shtylyov
  2016-03-07 20:29     ` Geert Uytterhoeven
  0 siblings, 1 reply; 19+ messages in thread
From: Sergei Shtylyov @ 2016-03-07 20:02 UTC (permalink / raw)
  To: Geert Uytterhoeven, Wolfram Sang
  Cc: linux-renesas-soc, Laurent Pinchart, Kuninori Morimoto,
	Magnus Damm, Geert Uytterhoeven, linux-gpio, Linus Walleij

Hello.

On 03/07/2016 10:56 PM, Geert Uytterhoeven wrote:

>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> If pinctrl_provide_dummies() is used unconditionally, then the dummy
>> state will be used even on DT platforms when the "init" state was
>> intentionally left out. Instead of "default", the dummy "init" state
>> will then be used during probe. Thus, when probing an I2C controller on
>> cold boot, communication triggered by bus notifiers broke because the
>> pins were not initialized.
>>
>> Do it like OMAP2: use the dummy state only for non-DT platforms.
>
> Thanks!
>
> Interestingly, Sergei submitted a similar patch a few days ago
> "pinctrl: sh-pfc: core: only call pinctrl_provide_dummies"
> (https://patchwork.ozlabs.org/patch/592245/)

    I shot first! :-)

>> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> ---
>>
>> Geert recently pointed out the problem, that the IRQ2 fixup for the
>> DA9xxx PMICs failed on Lager on cold boot. I could verify this iff IIC3
>> was used and not I2C3. IIC3 is the default, however I mostly used I2C3
>> recently, because it has the slave capabilities.
>>
>> The original pinctrl_provide_dummies() is there since the beginning of the
>> file, so in order to avoid regressions, the below solution looks plausible to
>> me. I do not have much experience with hardware older than Gen2, though, so
>> comments are much appreciated!
>>
>>
>>   drivers/pinctrl/sh-pfc/core.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
>> index 0c2d14c504aa1d..db53d7bbc16e18 100644
>> --- a/drivers/pinctrl/sh-pfc/core.c
>> +++ b/drivers/pinctrl/sh-pfc/core.c
>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
>>                          return ret;
>>          }
>>
>> -       pinctrl_provide_dummies();
>> +       /* Enable dummy states for those platforms without pinctrl support */
>> +       if (!of_have_populated_dt())

    I'd considered this condition -- it won't fly on SH where CONFIG_OF=n, the 
kernel just won't build IIUC...

>> +               pinctrl_provide_dummies();
>>
>>          ret = sh_pfc_init_ranges(pfc);
>>          if (ret < 0)
>> --
>> 2.7.0
>
> Gr{oetje,eeting}s,
>
>                          Geert

MBR, Sergei


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 20:02   ` Sergei Shtylyov
@ 2016-03-07 20:29     ` Geert Uytterhoeven
  2016-03-07 20:34       ` Sergei Shtylyov
  0 siblings, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-07 20:29 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij

On Mon, Mar 7, 2016 at 9:02 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>>> --- a/drivers/pinctrl/sh-pfc/core.c
>>> +++ b/drivers/pinctrl/sh-pfc/core.c
>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
>>>                          return ret;
>>>          }
>>>
>>> -       pinctrl_provide_dummies();
>>> +       /* Enable dummy states for those platforms without pinctrl
>>> support */
>>> +       if (!of_have_populated_dt())
>
>    I'd considered this condition -- it won't fly on SH where CONFIG_OF=n,
> the kernel just won't build IIUC...

I haven't tried to compile it yet, but <linux/of.h> does provide a dummy that
returns false.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 20:29     ` Geert Uytterhoeven
@ 2016-03-07 20:34       ` Sergei Shtylyov
  2016-03-07 21:00         ` Geert Uytterhoeven
  0 siblings, 1 reply; 19+ messages in thread
From: Sergei Shtylyov @ 2016-03-07 20:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij

On 03/07/2016 11:29 PM, Geert Uytterhoeven wrote:

>>>> --- a/drivers/pinctrl/sh-pfc/core.c
>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
>>>>                           return ret;
>>>>           }
>>>>
>>>> -       pinctrl_provide_dummies();
>>>> +       /* Enable dummy states for those platforms without pinctrl
>>>> support */
>>>> +       if (!of_have_populated_dt())
>>
>>     I'd considered this condition -- it won't fly on SH where CONFIG_OF=n,
>> the kernel just won't build IIUC...
>
> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy that
> returns false.

    Oops, indeed. I missed it. :-(
    This approach is better then, won't have to fix again whenever SH gets DT 
support.

> Gr{oetje,eeting}s,
>
>                          Geert

MBR, Sergei


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 20:34       ` Sergei Shtylyov
@ 2016-03-07 21:00         ` Geert Uytterhoeven
  2016-03-07 21:19           ` Sergei Shtylyov
                             ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-07 21:00 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij, Shawn Guo, Sascha Hauer

CC Shawn, Sascha for imx1

On Mon, Mar 7, 2016 at 9:34 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 03/07/2016 11:29 PM, Geert Uytterhoeven wrote:
>
>>>>> --- a/drivers/pinctrl/sh-pfc/core.c
>>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
>>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device
>>>>> *pdev)
>>>>>                           return ret;
>>>>>           }
>>>>>
>>>>> -       pinctrl_provide_dummies();
>>>>> +       /* Enable dummy states for those platforms without pinctrl
>>>>> support */
>>>>> +       if (!of_have_populated_dt())
>>>
>>>
>>>     I'd considered this condition -- it won't fly on SH where
>>> CONFIG_OF=n,
>>> the kernel just won't build IIUC...
>>
>>
>> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy
>> that
>> returns false.
>
>
>    Oops, indeed. I missed it. :-(
>    This approach is better then, won't have to fix again whenever SH gets DT
> support.

Perhaps the of_have_populated_dt() check should be moved inside
pinctrl_provide_dummies()?

Besides omap2, which has its own check, the only other user is imx1.
Does imx1 (still) needs this on DT-based platforms?

(Context for Shawn and Sascha:
 http://article.gmane.org/gmane.linux.kernel.renesas-soc/1639)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 21:00         ` Geert Uytterhoeven
@ 2016-03-07 21:19           ` Sergei Shtylyov
  2016-03-08  8:15             ` Geert Uytterhoeven
  2016-03-07 22:21           ` Wolfram Sang
                             ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Sergei Shtylyov @ 2016-03-07 21:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij, Shawn Guo, Sascha Hauer

On 03/08/2016 12:00 AM, Geert Uytterhoeven wrote:

>>>>>> --- a/drivers/pinctrl/sh-pfc/core.c
>>>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
>>>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device
>>>>>> *pdev)
>>>>>>                            return ret;
>>>>>>            }
>>>>>>
>>>>>> -       pinctrl_provide_dummies();
>>>>>> +       /* Enable dummy states for those platforms without pinctrl
>>>>>> support */
>>>>>> +       if (!of_have_populated_dt())
>>>>
>>>>
>>>>      I'd considered this condition -- it won't fly on SH where
>>>> CONFIG_OF=n,
>>>> the kernel just won't build IIUC...
>>>
>>>
>>> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy
>>> that
>>> returns false.
>>
>>
>>     Oops, indeed. I missed it. :-(
>>     This approach is better then, won't have to fix again whenever SH gets DT
>> support.

> Perhaps the of_have_populated_dt() check should be moved inside
> pinctrl_provide_dummies()?

    That's probably a good idea...

> Besides omap2, which has its own check, the only other user is imx1.

    i.MX2x/3x as well, no?

[...]

> Gr{oetje,eeting}s,
>
>                          Geert

MBR, Sergei

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 21:00         ` Geert Uytterhoeven
  2016-03-07 21:19           ` Sergei Shtylyov
@ 2016-03-07 22:21           ` Wolfram Sang
  2016-03-09 10:30             ` Linus Walleij
  2016-03-09  9:58           ` Wolfram Sang
  2016-03-29  5:48           ` Shawn Guo
  3 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2016-03-07 22:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sergei Shtylyov, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij, Shawn Guo, Sascha Hauer

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]


> Perhaps the of_have_populated_dt() check should be moved inside
> pinctrl_provide_dummies()?

Hmm, the kernel-doc for pinctrl_provide_dummies() says "Usually this
function is called by platforms without pinctrl driver support...".
Is "without pintctrl" == "no DT"? Linus?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 21:19           ` Sergei Shtylyov
@ 2016-03-08  8:15             ` Geert Uytterhoeven
  0 siblings, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-08  8:15 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij, Shawn Guo, Sascha Hauer

Hi Sergei,

On Mon, Mar 7, 2016 at 10:19 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 03/08/2016 12:00 AM, Geert Uytterhoeven wrote:
>>>>>>> --- a/drivers/pinctrl/sh-pfc/core.c
>>>>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
>>>>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device
>>>>>>> *pdev)
>>>>>>>                            return ret;
>>>>>>>            }
>>>>>>>
>>>>>>> -       pinctrl_provide_dummies();
>>>>>>> +       /* Enable dummy states for those platforms without pinctrl
>>>>>>> support */
>>>>>>> +       if (!of_have_populated_dt())
>>>>>
>>>>>
>>>>>
>>>>>      I'd considered this condition -- it won't fly on SH where
>>>>> CONFIG_OF=n,
>>>>> the kernel just won't build IIUC...
>>>>
>>>> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy
>>>> that
>>>> returns false.
>>>
>>>     Oops, indeed. I missed it. :-(
>>>     This approach is better then, won't have to fix again whenever SH
>>> gets DT
>>> support.
>
>> Perhaps the of_have_populated_dt() check should be moved inside
>> pinctrl_provide_dummies()?
>
>    That's probably a good idea...
>
>> Besides omap2, which has its own check, the only other user is imx1.
>
>    i.MX2x/3x as well, no?

Yeah sorry, several i.MX. I accidentally typed a trailing "1".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 21:00         ` Geert Uytterhoeven
  2016-03-07 21:19           ` Sergei Shtylyov
  2016-03-07 22:21           ` Wolfram Sang
@ 2016-03-09  9:58           ` Wolfram Sang
  2016-03-09 10:32             ` Linus Walleij
  2016-03-29  5:48           ` Shawn Guo
  3 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2016-03-09  9:58 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sergei Shtylyov, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Linus Walleij, Shawn Guo, Sascha Hauer

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Mon, Mar 07, 2016 at 10:00:37PM +0100, Geert Uytterhoeven wrote:
> CC Shawn, Sascha for imx1
> 
> On Mon, Mar 7, 2016 at 9:34 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > On 03/07/2016 11:29 PM, Geert Uytterhoeven wrote:
> >
> >>>>> --- a/drivers/pinctrl/sh-pfc/core.c
> >>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
> >>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device
> >>>>> *pdev)
> >>>>>                           return ret;
> >>>>>           }
> >>>>>
> >>>>> -       pinctrl_provide_dummies();
> >>>>> +       /* Enable dummy states for those platforms without pinctrl
> >>>>> support */
> >>>>> +       if (!of_have_populated_dt())
> >>>
> >>>
> >>>     I'd considered this condition -- it won't fly on SH where
> >>> CONFIG_OF=n,
> >>> the kernel just won't build IIUC...
> >>
> >>
> >> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy
> >> that
> >> returns false.
> >
> >
> >    Oops, indeed. I missed it. :-(
> >    This approach is better then, won't have to fix again whenever SH gets DT
> > support.
> 
> Perhaps the of_have_populated_dt() check should be moved inside
> pinctrl_provide_dummies()?

I suggest we pick up this patch now even with stable tag (the irq storm
fixup doesn't work correctly because of this) and consider the
refactoring as a second step?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 22:21           ` Wolfram Sang
@ 2016-03-09 10:30             ` Linus Walleij
  0 siblings, 0 replies; 19+ messages in thread
From: Linus Walleij @ 2016-03-09 10:30 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Sergei Shtylyov, linux-renesas-soc,
	Laurent Pinchart, Kuninori Morimoto, Magnus Damm,
	Geert Uytterhoeven, linux-gpio, Shawn Guo, Sascha Hauer

On Tue, Mar 8, 2016 at 5:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> Perhaps the of_have_populated_dt() check should be moved inside
>> pinctrl_provide_dummies()?
>
> Hmm, the kernel-doc for pinctrl_provide_dummies() says "Usually this
> function is called by platforms without pinctrl driver support...".
> Is "without pintctrl" == "no DT"? Linus?

The primary example is a platform with one pin controller and
two identical GPIO controllers, one of the GPIO controllers
is using a pin control backend.

Since the GPIO driver has to call pinctrl_request_gpio() etc,
it needs something to hold on to if there is no backing pin
controller for one of them.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-09  9:58           ` Wolfram Sang
@ 2016-03-09 10:32             ` Linus Walleij
  2016-03-09 12:51               ` Sergei Shtylyov
  2016-03-09 13:17               ` Geert Uytterhoeven
  0 siblings, 2 replies; 19+ messages in thread
From: Linus Walleij @ 2016-03-09 10:32 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Geert Uytterhoeven, Sergei Shtylyov, linux-renesas-soc,
	Laurent Pinchart, Kuninori Morimoto, Magnus Damm,
	Geert Uytterhoeven, linux-gpio, Shawn Guo, Sascha Hauer

On Wed, Mar 9, 2016 at 4:58 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Mon, Mar 07, 2016 at 10:00:37PM +0100, Geert Uytterhoeven wrote:

>> >    This approach is better then, won't have to fix again whenever SH gets DT
>> > support.
>>
>> Perhaps the of_have_populated_dt() check should be moved inside
>> pinctrl_provide_dummies()?
>
> I suggest we pick up this patch now even with stable tag (the irq storm
> fixup doesn't work correctly because of this) and consider the
> refactoring as a second step?

Sergei, Geert, do you ACK this?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-09 10:32             ` Linus Walleij
@ 2016-03-09 12:51               ` Sergei Shtylyov
  2016-03-09 15:37                 ` Wolfram Sang
  2016-03-09 13:17               ` Geert Uytterhoeven
  1 sibling, 1 reply; 19+ messages in thread
From: Sergei Shtylyov @ 2016-03-09 12:51 UTC (permalink / raw)
  To: Linus Walleij, Wolfram Sang
  Cc: Geert Uytterhoeven, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio,
	Shawn Guo, Sascha Hauer

Hello.

On 3/9/2016 1:32 PM, Linus Walleij wrote:

>>>>     This approach is better then, won't have to fix again whenever SH gets DT
>>>> support.
>>>
>>> Perhaps the of_have_populated_dt() check should be moved inside
>>> pinctrl_provide_dummies()?
>>
>> I suggest we pick up this patch now even with stable tag (the irq storm
>> fixup doesn't work correctly because of this) and consider the
>> refactoring as a second step?
>
> Sergei, Geert, do you ACK this?

    Since this patch fixes the real issue in mainline (which I wasn't aware of 
-- my patch only describes an issue with the 'ravb' driver the patches for 
using which in gen2 were never sent upstream):

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

    I'd only like that the "Fixes:" and "Cc:" tags were added to it.

> Yours,
> Linus Walleij

MBR, Sergei


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-09 10:32             ` Linus Walleij
  2016-03-09 12:51               ` Sergei Shtylyov
@ 2016-03-09 13:17               ` Geert Uytterhoeven
  1 sibling, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-09 13:17 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Wolfram Sang, Sergei Shtylyov, linux-renesas-soc,
	Laurent Pinchart, Kuninori Morimoto, Magnus Damm,
	Geert Uytterhoeven, linux-gpio, Shawn Guo, Sascha Hauer

Hi Linus,

On Wed, Mar 9, 2016 at 11:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Mar 9, 2016 at 4:58 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> On Mon, Mar 07, 2016 at 10:00:37PM +0100, Geert Uytterhoeven wrote:
>
>>> >    This approach is better then, won't have to fix again whenever SH gets DT
>>> > support.
>>>
>>> Perhaps the of_have_populated_dt() check should be moved inside
>>> pinctrl_provide_dummies()?
>>
>> I suggest we pick up this patch now even with stable tag (the irq storm
>> fixup doesn't work correctly because of this) and consider the
>> refactoring as a second step?
>
> Sergei, Geert, do you ACK this?

I gave this a try on all my boards, plus the remote r8a7790/lager that
exhibited the problem.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Please pick it up. Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-09 12:51               ` Sergei Shtylyov
@ 2016-03-09 15:37                 ` Wolfram Sang
  0 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2016-03-09 15:37 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Linus Walleij, Geert Uytterhoeven, linux-renesas-soc,
	Laurent Pinchart, Kuninori Morimoto, Magnus Damm,
	Geert Uytterhoeven, linux-gpio, Shawn Guo, Sascha Hauer

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

>    I'd only like that the "Fixes:" and "Cc:" tags were added to it.

To make it easier for you, Linus:

Fixes: ef0eebc05130 ("drivers/pinctrl: Add the concept of an "init" state")
CC: stable@vger.kernel.org


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 18:40 [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms Wolfram Sang
  2016-03-07 19:56 ` Geert Uytterhoeven
@ 2016-03-15  8:29 ` Linus Walleij
  2016-03-15  9:08   ` Geert Uytterhoeven
  2016-03-15 12:31   ` Sergei Shtylyov
  1 sibling, 2 replies; 19+ messages in thread
From: Linus Walleij @ 2016-03-15  8:29 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Geert Uytterhoeven, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio

On Mon, Mar 7, 2016 at 7:40 PM, Wolfram Sang <wsa@the-dreams.de> wrote:

> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> If pinctrl_provide_dummies() is used unconditionally, then the dummy
> state will be used even on DT platforms when the "init" state was
> intentionally left out. Instead of "default", the dummy "init" state
> will then be used during probe. Thus, when probing an I2C controller on
> cold boot, communication triggered by bus notifiers broke because the
> pins were not initialized.
>
> Do it like OMAP2: use the dummy state only for non-DT platforms.
>
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Patch applied for fixes with all the ACKs etc.

This unfortiunately coincide with the merge window so was slow
to pick it up, but it will be in the first round of fixes to Torvalds,
possibly at -rc1 possibly earlier.

> -       pinctrl_provide_dummies();
> +       /* Enable dummy states for those platforms without pinctrl support */
> +       if (!of_have_populated_dt())
> +               pinctrl_provide_dummies();

So remind we: what Renesas platforms are still not using DT?
arch/sh?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-15  8:29 ` Linus Walleij
@ 2016-03-15  9:08   ` Geert Uytterhoeven
  2016-03-15 12:31   ` Sergei Shtylyov
  1 sibling, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2016-03-15  9:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Wolfram Sang, linux-renesas-soc, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio

Hi Linus,

On Tue, Mar 15, 2016 at 9:29 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Mar 7, 2016 at 7:40 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> If pinctrl_provide_dummies() is used unconditionally, then the dummy
>> state will be used even on DT platforms when the "init" state was
>> intentionally left out. Instead of "default", the dummy "init" state
>> will then be used during probe. Thus, when probing an I2C controller on
>> cold boot, communication triggered by bus notifiers broke because the
>> pins were not initialized.
>>
>> Do it like OMAP2: use the dummy state only for non-DT platforms.
>>
>> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Patch applied for fixes with all the ACKs etc.
>
> This unfortiunately coincide with the merge window so was slow
> to pick it up, but it will be in the first round of fixes to Torvalds,
> possibly at -rc1 possibly earlier.

Thanks, I understand.

>> -       pinctrl_provide_dummies();
>> +       /* Enable dummy states for those platforms without pinctrl support */
>> +       if (!of_have_populated_dt())
>> +               pinctrl_provide_dummies();
>
> So remind we: what Renesas platforms are still not using DT?
> arch/sh?

Yep, the mighty SuperH. Old H8 and new ARM are DT-only.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-15  8:29 ` Linus Walleij
  2016-03-15  9:08   ` Geert Uytterhoeven
@ 2016-03-15 12:31   ` Sergei Shtylyov
  1 sibling, 0 replies; 19+ messages in thread
From: Sergei Shtylyov @ 2016-03-15 12:31 UTC (permalink / raw)
  To: Linus Walleij, Wolfram Sang
  Cc: linux-renesas-soc, Geert Uytterhoeven, Laurent Pinchart,
	Kuninori Morimoto, Magnus Damm, Geert Uytterhoeven, linux-gpio

On 3/15/2016 11:29 AM, Linus Walleij wrote:

>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> If pinctrl_provide_dummies() is used unconditionally, then the dummy
>> state will be used even on DT platforms when the "init" state was
>> intentionally left out. Instead of "default", the dummy "init" state
>> will then be used during probe. Thus, when probing an I2C controller on
>> cold boot, communication triggered by bus notifiers broke because the
>> pins were not initialized.
>>
>> Do it like OMAP2: use the dummy state only for non-DT platforms.
>>
>> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Patch applied for fixes with all the ACKs etc.
>
> This unfortiunately coincide with the merge window so was slow
> to pick it up, but it will be in the first round of fixes to Torvalds,
> possibly at -rc1 possibly earlier.

    TIA!

>> -       pinctrl_provide_dummies();
>> +       /* Enable dummy states for those platforms without pinctrl support */
>> +       if (!of_have_populated_dt())
>> +               pinctrl_provide_dummies();
>
> So remind we: what Renesas platforms are still not using DT?
> arch/sh?

    Yes, only those are left.

> Yours,
> Linus Walleij

MBR, Sergei

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms
  2016-03-07 21:00         ` Geert Uytterhoeven
                             ` (2 preceding siblings ...)
  2016-03-09  9:58           ` Wolfram Sang
@ 2016-03-29  5:48           ` Shawn Guo
  3 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2016-03-29  5:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sergei Shtylyov, Wolfram Sang, linux-renesas-soc,
	Laurent Pinchart, Kuninori Morimoto, Magnus Damm,
	Geert Uytterhoeven, linux-gpio, Linus Walleij, Sascha Hauer

On Mon, Mar 07, 2016 at 10:00:37PM +0100, Geert Uytterhoeven wrote:
> CC Shawn, Sascha for imx1

Sorry for the late response.

> 
> On Mon, Mar 7, 2016 at 9:34 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > On 03/07/2016 11:29 PM, Geert Uytterhoeven wrote:
> >
> >>>>> --- a/drivers/pinctrl/sh-pfc/core.c
> >>>>> +++ b/drivers/pinctrl/sh-pfc/core.c
> >>>>> @@ -545,7 +545,9 @@ static int sh_pfc_probe(struct platform_device
> >>>>> *pdev)
> >>>>>                           return ret;
> >>>>>           }
> >>>>>
> >>>>> -       pinctrl_provide_dummies();
> >>>>> +       /* Enable dummy states for those platforms without pinctrl
> >>>>> support */
> >>>>> +       if (!of_have_populated_dt())
> >>>
> >>>
> >>>     I'd considered this condition -- it won't fly on SH where
> >>> CONFIG_OF=n,
> >>> the kernel just won't build IIUC...
> >>
> >>
> >> I haven't tried to compile it yet, but <linux/of.h> does provide a dummy
> >> that
> >> returns false.
> >
> >
> >    Oops, indeed. I missed it. :-(
> >    This approach is better then, won't have to fix again whenever SH gets DT
> > support.
> 
> Perhaps the of_have_populated_dt() check should be moved inside
> pinctrl_provide_dummies()?
> 
> Besides omap2, which has its own check, the only other user is imx1.
> Does imx1 (still) needs this on DT-based platforms?
> 
> (Context for Shawn and Sascha:
>  http://article.gmane.org/gmane.linux.kernel.renesas-soc/1639)

pinctrl_provide_dummies() is only used on a few legacy i.MX platforms
for non-DT boot, so it should be safe to move the check inside the
function from i.MX view.

Shawn

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-03-29  5:48 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-07 18:40 [RFC] pinctrl: sh-pfc: only use dummy states for non-DT platforms Wolfram Sang
2016-03-07 19:56 ` Geert Uytterhoeven
2016-03-07 20:02   ` Sergei Shtylyov
2016-03-07 20:29     ` Geert Uytterhoeven
2016-03-07 20:34       ` Sergei Shtylyov
2016-03-07 21:00         ` Geert Uytterhoeven
2016-03-07 21:19           ` Sergei Shtylyov
2016-03-08  8:15             ` Geert Uytterhoeven
2016-03-07 22:21           ` Wolfram Sang
2016-03-09 10:30             ` Linus Walleij
2016-03-09  9:58           ` Wolfram Sang
2016-03-09 10:32             ` Linus Walleij
2016-03-09 12:51               ` Sergei Shtylyov
2016-03-09 15:37                 ` Wolfram Sang
2016-03-09 13:17               ` Geert Uytterhoeven
2016-03-29  5:48           ` Shawn Guo
2016-03-15  8:29 ` Linus Walleij
2016-03-15  9:08   ` Geert Uytterhoeven
2016-03-15 12:31   ` Sergei Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.