All of lore.kernel.org
 help / color / mirror / Atom feed
* gpio-pxa initcall level change and machine init breakage
@ 2013-04-20 15:26 Mike Dunn
  2013-04-21  6:02 ` Haojian Zhuang
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Dunn @ 2013-04-20 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

After a few months of neglect, I rebased my palm treo 680 kernel with the latest
from Linus' official tree, and now all the calls to gpio lib functions
(gpio_request(), etc) made from the init_machine() method in struct machine_desc
fail.  I tracked this back to commit 6c7e660a27da7494c670bfba21cfeba30457656c
dated Jan 23 2013

    gpio: pxa: set initcall level to module init

    gpio & pinctrl driver are used together. The pinctrl driver is already
    launched before gpio driver in Makefile. So set gpio driver to module
    init level. Otherwise, the sequence will be inverted.

    Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

At the time, Haojian addressed a question about this patch regarding module
dependencies, advising use of the deferred probe infrastructure.  But unless I'm
mistaken, mine is an initcall level problem, not a dependency issue among driver
probes.  Now that gpio-pxa initialization is at the device level (initcall6),
calls to gpiolib are no longer possible from init_machine(), which remains at
the initcall3 level.

I feel like I must be missing something, because a lot of pxa boards call
gpiolib from init machine(), so my breakage should not be an isolated case.

If I'm not missing anything and the patch is necessary, it looks like one
solution would be to move all the initializations that use gpiolib to the
init_late method in struct machine_desc, which runs as initcall7.

Grateful for any advice!

Thanks,
Mike

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-20 15:26 gpio-pxa initcall level change and machine init breakage Mike Dunn
@ 2013-04-21  6:02 ` Haojian Zhuang
  2013-04-21 22:23   ` Mike Dunn
  2013-04-24 16:07   ` Mike Dunn
  0 siblings, 2 replies; 13+ messages in thread
From: Haojian Zhuang @ 2013-04-21  6:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 20 April 2013 23:26, Mike Dunn <mikedunn@newsguy.com> wrote:
> Hi,
>
> After a few months of neglect, I rebased my palm treo 680 kernel with the latest
> from Linus' official tree, and now all the calls to gpio lib functions
> (gpio_request(), etc) made from the init_machine() method in struct machine_desc
> fail.  I tracked this back to commit 6c7e660a27da7494c670bfba21cfeba30457656c
> dated Jan 23 2013
>
>     gpio: pxa: set initcall level to module init
>
>     gpio & pinctrl driver are used together. The pinctrl driver is already
>     launched before gpio driver in Makefile. So set gpio driver to module
>     init level. Otherwise, the sequence will be inverted.
>
>     Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> At the time, Haojian addressed a question about this patch regarding module
> dependencies, advising use of the deferred probe infrastructure.  But unless I'm
> mistaken, mine is an initcall level problem, not a dependency issue among driver
> probes.  Now that gpio-pxa initialization is at the device level (initcall6),
> calls to gpiolib are no longer possible from init_machine(), which remains at
> the initcall3 level.
>
> I feel like I must be missing something, because a lot of pxa boards call
> gpiolib from init machine(), so my breakage should not be an isolated case.
>
> If I'm not missing anything and the patch is necessary, it looks like one
> solution would be to move all the initializations that use gpiolib to the
> init_late method in struct machine_desc, which runs as initcall7.
>
> Grateful for any advice!
>
> Thanks,
> Mike

Actually gpio request should be handled in the driver, not machine platform
driver. For example, lcd init may request gpio. pxafb.c provides the callback
to handle it. We need to move code of requesting gpio into the callback.

Since this change breaks current platform, we have to revert it. We still need
a code cleanup in arch-pxa directory.

Linus,
Could you help to revert this commit? Or do I need to send a revert commit
to you?

Regards
Haojian

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-21  6:02 ` Haojian Zhuang
@ 2013-04-21 22:23   ` Mike Dunn
  2013-04-22  0:58     ` Haojian Zhuang
  2013-04-24 16:07   ` Mike Dunn
  1 sibling, 1 reply; 13+ messages in thread
From: Mike Dunn @ 2013-04-21 22:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/20/2013 11:02 PM, Haojian Zhuang wrote:
> On 20 April 2013 23:26, Mike Dunn <mikedunn@newsguy.com> wrote:
>> Hi,
>>
>> After a few months of neglect, I rebased my palm treo 680 kernel with the latest
>> from Linus' official tree, and now all the calls to gpio lib functions
>> (gpio_request(), etc) made from the init_machine() method in struct machine_desc
>> fail.  I tracked this back to commit 6c7e660a27da7494c670bfba21cfeba30457656c
>> dated Jan 23 2013
>>
>>     gpio: pxa: set initcall level to module init
>>
>>     gpio & pinctrl driver are used together. The pinctrl driver is already
>>     launched before gpio driver in Makefile. So set gpio driver to module
>>     init level. Otherwise, the sequence will be inverted.
>>
>>     Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> At the time, Haojian addressed a question about this patch regarding module
>> dependencies, advising use of the deferred probe infrastructure.  But unless I'm
>> mistaken, mine is an initcall level problem, not a dependency issue among driver
>> probes.  Now that gpio-pxa initialization is at the device level (initcall6),
>> calls to gpiolib are no longer possible from init_machine(), which remains at
>> the initcall3 level.
>>
>> I feel like I must be missing something, because a lot of pxa boards call
>> gpiolib from init machine(), so my breakage should not be an isolated case.
>>
>> If I'm not missing anything and the patch is necessary, it looks like one
>> solution would be to move all the initializations that use gpiolib to the
>> init_late method in struct machine_desc, which runs as initcall7.
>>
>> Grateful for any advice!
>>
>> Thanks,
>> Mike
> 
> Actually gpio request should be handled in the driver, not machine platform
> driver. For example, lcd init may request gpio. pxafb.c provides the callback
> to handle it. We need to move code of requesting gpio into the callback.


Yes, I see.  Thanks for the example.


> 
> Since this change breaks current platform, we have to revert it. We still need
> a code cleanup in arch-pxa directory.


I knew some cleanup was needed to the palm stuff, but didn't realize the extent.
 Should not be too difficult; I hope to have patches soon.  But there are other
pxa boards that grab various gpios in init_machine() and may not be as
straightforward.


> 
> Linus,
> Could you help to revert this commit? Or do I need to send a revert commit
> to you?


Will the big guy see this?  Only posted to linux-arm-kernel ML.

Thanks Haojian,
Mike

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-21 22:23   ` Mike Dunn
@ 2013-04-22  0:58     ` Haojian Zhuang
  2013-04-23  7:26       ` Linus Walleij
  0 siblings, 1 reply; 13+ messages in thread
From: Haojian Zhuang @ 2013-04-22  0:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 22 April 2013 06:23, Mike Dunn <mikedunn@newsguy.com> wrote:
> On 04/20/2013 11:02 PM, Haojian Zhuang wrote:
>> On 20 April 2013 23:26, Mike Dunn <mikedunn@newsguy.com> wrote:
>>> Hi,
>>>
>>> After a few months of neglect, I rebased my palm treo 680 kernel with the latest
>>> from Linus' official tree, and now all the calls to gpio lib functions
>>> (gpio_request(), etc) made from the init_machine() method in struct machine_desc
>>> fail.  I tracked this back to commit 6c7e660a27da7494c670bfba21cfeba30457656c
>>> dated Jan 23 2013
>>>
>>>     gpio: pxa: set initcall level to module init
>>>
>>>     gpio & pinctrl driver are used together. The pinctrl driver is already
>>>     launched before gpio driver in Makefile. So set gpio driver to module
>>>     init level. Otherwise, the sequence will be inverted.
>>>
>>>     Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>>     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> At the time, Haojian addressed a question about this patch regarding module
>>> dependencies, advising use of the deferred probe infrastructure.  But unless I'm
>>> mistaken, mine is an initcall level problem, not a dependency issue among driver
>>> probes.  Now that gpio-pxa initialization is at the device level (initcall6),
>>> calls to gpiolib are no longer possible from init_machine(), which remains at
>>> the initcall3 level.
>>>
>>> I feel like I must be missing something, because a lot of pxa boards call
>>> gpiolib from init machine(), so my breakage should not be an isolated case.
>>>
>>> If I'm not missing anything and the patch is necessary, it looks like one
>>> solution would be to move all the initializations that use gpiolib to the
>>> init_late method in struct machine_desc, which runs as initcall7.
>>>
>>> Grateful for any advice!
>>>
>>> Thanks,
>>> Mike
>>
>> Actually gpio request should be handled in the driver, not machine platform
>> driver. For example, lcd init may request gpio. pxafb.c provides the callback
>> to handle it. We need to move code of requesting gpio into the callback.
>
>
> Yes, I see.  Thanks for the example.
>
>
>>
>> Since this change breaks current platform, we have to revert it. We still need
>> a code cleanup in arch-pxa directory.
>
>
> I knew some cleanup was needed to the palm stuff, but didn't realize the extent.
>  Should not be too difficult; I hope to have patches soon.  But there are other
> pxa boards that grab various gpios in init_machine() and may not be as
> straightforward.
>
>
>>
>> Linus,
>> Could you help to revert this commit? Or do I need to send a revert commit
>> to you?
>
>
> Will the big guy see this?  Only posted to linux-arm-kernel ML.
>
> Thanks Haojian,
> Mike

Sorry, I forgot to loop him in.

Linus,
Should I send the revert commit to you? Or will you revert it directly?

Regards
Haojian

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-22  0:58     ` Haojian Zhuang
@ 2013-04-23  7:26       ` Linus Walleij
  2013-04-23  7:49         ` Haojian Zhuang
  2013-04-23 19:42         ` Mike Dunn
  0 siblings, 2 replies; 13+ messages in thread
From: Linus Walleij @ 2013-04-23  7:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 22, 2013 at 2:58 AM, Haojian Zhuang
<haojian.zhuang@linaro.org> wrote:
> On 22 April 2013 06:23, Mike Dunn <mikedunn@newsguy.com> wrote:
>> Will the big guy see this?  Only posted to linux-arm-kernel ML.
>>
>> Thanks Haojian,
>> Mike

I guess Haojian is referring to me, not Torvalds ...

> Sorry, I forgot to loop him in.
>
> Linus,
> Should I send the revert commit to you? Or will you revert it directly?

How hard is it to fix the real problem?

I'm looking at arch/arm/mach-pxa/palmtreo.c, would
something like this solve the problem:

>From 5acf99a576f563d48ca5d25b9e5a1bcdd09331eb Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij@linaro.org>
Date: Tue, 23 Apr 2013 09:24:43 +0200
Subject: [PATCH] ARM: pxa: set up Treo GPIOs using device initcall

This augments the Treo setup to grab LCD GPIOs at device_initcall()
level instead of at init_machine() time, and propagates any
returned errors so that deferred probe will work.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-pxa/palmtreo.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-pxa/palmtreo.c b/arch/arm/mach-pxa/palmtreo.c
index d82a50b..669b609 100644
--- a/arch/arm/mach-pxa/palmtreo.c
+++ b/arch/arm/mach-pxa/palmtreo.c
@@ -455,16 +455,20 @@ static void __init palmphone_common_init(void)
 }

 #ifdef CONFIG_MACH_TREO680
-void __init treo680_gpio_init(void)
+static int __init treo680_gpio_init(void)
 {
     unsigned int gpio;
+    int ret;

     /* drive all three lcd gpios high initially */
     const unsigned long lcd_flags = GPIOF_INIT_HIGH | GPIOF_DIR_OUT;

     /*
      * LCD GPIO initialization...
+     * only run this on the Treo680
      */
+    if (!machine_is_treo680())
+        return 0;

     /*
      * This is likely the power to the lcd.  Toggling it low/high appears to
@@ -474,8 +478,9 @@ void __init treo680_gpio_init(void)
      * treo680 configures it as gpio.
      */
     gpio = GPIO_NR_TREO680_LCD_POWER;
-    if (gpio_request_one(gpio, lcd_flags, "LCD power") < 0)
-        goto fail;
+    ret = gpio_request_one(gpio, lcd_flags, "LCD power");
+    if (ret < 0)
+        goto fail_lcd_power;

     /*
      * These two are called "enables", for lack of a better understanding.
@@ -486,28 +491,33 @@ void __init treo680_gpio_init(void)
      * revisited.
      */
     gpio = GPIO_NR_TREO680_LCD_EN;
-    if (gpio_request_one(gpio, lcd_flags, "LCD enable") < 0)
-        goto fail;
+    ret = gpio_request_one(gpio, lcd_flags, "LCD enable");
+    if (ret < 0)
+        goto fail_lcd_en;
     gpio = GPIO_NR_TREO680_LCD_EN_N;
-    if (gpio_request_one(gpio, lcd_flags, "LCD enable_n") < 0)
-        goto fail;
+    ret = gpio_request_one(gpio, lcd_flags, "LCD enable_n");
+    if (ret < 0)
+        goto fail_lcd_en_n;

     /* driving this low turns LCD on */
     gpio_set_value(GPIO_NR_TREO680_LCD_EN_N, 0);

-    return;
- fail:
-    pr_err("gpio %d initialization failed\n", gpio);
-    gpio_free(GPIO_NR_TREO680_LCD_POWER);
+    return 0;
+
+fail_lcd_en_n:
     gpio_free(GPIO_NR_TREO680_LCD_EN);
-    gpio_free(GPIO_NR_TREO680_LCD_EN_N);
+fail_lcd_en:
+    gpio_free(GPIO_NR_TREO680_LCD_POWER);
+fail_lcd_power:
+    pr_err("gpio %d initialization failed\n", gpio);
+    return ret;
 }
+device_initcall(treo680_gpio_init);

 static void __init treo680_init(void)
 {
     pxa2xx_mfp_config(ARRAY_AND_SIZE(treo680_pin_config));
     palmphone_common_init();
-    treo680_gpio_init();
     palm27x_mmc_init(GPIO_NR_TREO_SD_DETECT_N, GPIO_NR_TREO680_SD_READONLY,
             GPIO_NR_TREO680_SD_POWER, 0);
     treo680_docg4_flash_init();
--
1.7.11.3

(Tell me if gmail screws up the whitespace and I'll send it with
git-send-email ...)

Yours,
Linus Walleij

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-23  7:26       ` Linus Walleij
@ 2013-04-23  7:49         ` Haojian Zhuang
  2013-04-23 19:42         ` Mike Dunn
  1 sibling, 0 replies; 13+ messages in thread
From: Haojian Zhuang @ 2013-04-23  7:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 23 April 2013 15:26, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Apr 22, 2013 at 2:58 AM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>> On 22 April 2013 06:23, Mike Dunn <mikedunn@newsguy.com> wrote:
>>> Will the big guy see this?  Only posted to linux-arm-kernel ML.
>>>
>>> Thanks Haojian,
>>> Mike
>
> I guess Haojian is referring to me, not Torvalds ...
>
>> Sorry, I forgot to loop him in.
>>
>> Linus,
>> Should I send the revert commit to you? Or will you revert it directly?
>
> How hard is it to fix the real problem?
>
> I'm looking at arch/arm/mach-pxa/palmtreo.c, would
> something like this solve the problem:
>
Your fix could resolve the issue on palmtreo. After grep
gpio_request() in arch-pxa,
I found that there're a lot of files using gpio_request() in .init_machine().

Regards
Haojian

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-23  7:26       ` Linus Walleij
  2013-04-23  7:49         ` Haojian Zhuang
@ 2013-04-23 19:42         ` Mike Dunn
  2013-04-24 19:37           ` Linus Walleij
  1 sibling, 1 reply; 13+ messages in thread
From: Mike Dunn @ 2013-04-23 19:42 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/23/2013 12:26 AM, Linus Walleij wrote:
> On Mon, Apr 22, 2013 at 2:58 AM, Haojian Zhuang
> <haojian.zhuang@linaro.org> wrote:
>> On 22 April 2013 06:23, Mike Dunn <mikedunn@newsguy.com> wrote:
>>> Will the big guy see this?  Only posted to linux-arm-kernel ML.
>>>
>>> Thanks Haojian,
>>> Mike
> 
> I guess Haojian is referring to me, not Torvalds ...


Oops, sorry Linus.  Note that the other Linus has the patch in his official tree.

[...]


> 
> How hard is it to fix the real problem?
> 
> I'm looking at arch/arm/mach-pxa/palmtreo.c, would
> something like this solve the problem:
> 
>>From 5acf99a576f563d48ca5d25b9e5a1bcdd09331eb Mon Sep 17 00:00:00 2001
> From: Linus Walleij <linus.walleij@linaro.org>
> Date: Tue, 23 Apr 2013 09:24:43 +0200
> Subject: [PATCH] ARM: pxa: set up Treo GPIOs using device initcall
> 
> This augments the Treo setup to grab LCD GPIOs at device_initcall()
> level instead of at init_machine() time, and propagates any
> returned errors so that deferred probe will work.


Thanks for the patch.  I am not having initial success, but will troubleshoot
tomorrow morning.

On a more general note... I'm surprised that gpiolib can not run earlier, for
something so low-level.  The kernel documentation on gpio talks about running
early.  I'm not involved in nor knowledgeable of the pinctrl/gpio work so I
won't question the need.  I'll probably be able to educate myself some more
tomorrow.

Thanks again,
Mike

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-21  6:02 ` Haojian Zhuang
  2013-04-21 22:23   ` Mike Dunn
@ 2013-04-24 16:07   ` Mike Dunn
  1 sibling, 0 replies; 13+ messages in thread
From: Mike Dunn @ 2013-04-24 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

Linus,

The patch you offered yesterday does not fix things on palmtreo, because there
are calls to gpio_request() in another file (palm27x.c) that are made from
init_machine().  I'd be grateful for any insight into how to resolve this.
Please see below...


On 04/20/2013 11:02 PM, Haojian Zhuang wrote:
> On 20 April 2013 23:26, Mike Dunn <mikedunn@newsguy.com> wrote:
>> Hi,
>>
>> After a few months of neglect, I rebased my palm treo 680 kernel with the latest
>> from Linus' official tree, and now all the calls to gpio lib functions
>> (gpio_request(), etc) made from the init_machine() method in struct machine_desc
>> fail.  I tracked this back to commit 6c7e660a27da7494c670bfba21cfeba30457656c
>> dated Jan 23 2013
>>
>>     gpio: pxa: set initcall level to module init
>>
>>     gpio & pinctrl driver are used together. The pinctrl driver is already
>>     launched before gpio driver in Makefile. So set gpio driver to module
>>     init level. Otherwise, the sequence will be inverted.
>>
>>     Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> At the time, Haojian addressed a question about this patch regarding module
>> dependencies, advising use of the deferred probe infrastructure.  But unless I'm
>> mistaken, mine is an initcall level problem, not a dependency issue among driver
>> probes.  Now that gpio-pxa initialization is at the device level (initcall6),
>> calls to gpiolib are no longer possible from init_machine(), which remains at
>> the initcall3 level.
>>
>> I feel like I must be missing something, because a lot of pxa boards call
>> gpiolib from init machine(), so my breakage should not be an isolated case.
>>
>> If I'm not missing anything and the patch is necessary, it looks like one
>> solution would be to move all the initializations that use gpiolib to the
>> init_late method in struct machine_desc, which runs as initcall7.
>>
>> Grateful for any advice!
>>
>> Thanks,
>> Mike
> 
> Actually gpio request should be handled in the driver, not machine platform
> driver. For example, lcd init may request gpio. pxafb.c provides the callback
> to handle it. We need to move code of requesting gpio into the callback.


It appears that some pxa27x drivers will need to be reworked to accomplish this.
 For example, in the case of the pxa27x lcd driver (pxafb), there are callbacks
for turning the lcd and its backlight on and off, but it seems that the gpios
should have already been requested when these run.  The board-specific gpios
*could* be passed to the pxafb driver via platform_data, and the driver could
request them in its probe function, but currently this is not done.

Moving gpiolib calls to init_late() in struct machine_desc, which runs as
initcall7, may also be a solution.


> 
> Since this change breaks current platform, we have to revert it. We still need
> a code cleanup in arch-pxa directory.


Indeed, if I'm not mistaken, at least these boards are now broken:
  - tosa
  - baloon3
  - gumstix
  - hx4700
  - magician
  - mioa701
  - palmtreo/palmcentro
  - palmld
  - palmtc
  - palmz72


> 
> Linus,
> Could you help to revert this commit? Or do I need to send a revert commit
> to you?
> 

Thanks,
Mike

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-23 19:42         ` Mike Dunn
@ 2013-04-24 19:37           ` Linus Walleij
  2013-04-25 19:36             ` Robert Jarzmik
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2013-04-24 19:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 23, 2013 at 9:42 PM, Mike Dunn <mikedunn@newsguy.com> wrote:

> On a more general note... I'm surprised that gpiolib can not run earlier, for
> something so low-level.

Well I think the patch we're discussing is making it run less early, and it
can actually run as arch_initcall() or similar if you wish (it's up to the
driver).

However that is beside the point, Haojian is still doing the right thing
trying to push this to driver init (==module_init) level. The reason is
that basically all hardware drivers should be initialized at this level
because the other init levels are basically for other things and driver
deps are just shoehorned into them.

Instead of relying on things to be set up early one shall rely on
deferred probe.

Alas, it is not an easy change, and not expected to happen quickly
or painlessly.

Yours,
Linus Walleij

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-24 19:37           ` Linus Walleij
@ 2013-04-25 19:36             ` Robert Jarzmik
  2013-04-25 21:22               ` Linus Walleij
  2013-04-26 12:38               ` Mike Dunn
  0 siblings, 2 replies; 13+ messages in thread
From: Robert Jarzmik @ 2013-04-25 19:36 UTC (permalink / raw)
  To: linux-arm-kernel

Linus Walleij <linus.walleij@linaro.org> writes:

> On Tue, Apr 23, 2013 at 9:42 PM, Mike Dunn <mikedunn@newsguy.com> wrote:
>
>> On a more general note... I'm surprised that gpiolib can not run earlier, for
>> something so low-level.
>
> Well I think the patch we're discussing is making it run less early, and it
> can actually run as arch_initcall() or similar if you wish (it's up to the
> driver).
>
> However that is beside the point, Haojian is still doing the right thing
> trying to push this to driver init (==module_init) level. The reason is
> that basically all hardware drivers should be initialized at this level
> because the other init levels are basically for other things and driver
> deps are just shoehorned into them.
>
> Instead of relying on things to be set up early one shall rely on
> deferred probe.
Hi Linus,

Even if that is the long term goal, and I agree with that goal, let me quote
Documentation/gpio.txt : "However, for spinlock-safe GPIOs it's OK to use them
before tasking is enabled, as part of early board setup.".

When gpio abstraction was written, it was assumed GPIO usage was usable in early
platform setup code. As this was granted, we (board maintainers) coded
accordingly.

Now this patch breaks boards. I disagree in having my board code broken without
notice. What I would find less intrusive would be to :
 (a) revert the patch
 (b) inform board maintainers that they must fix their board code (for example
 give them 1 window)
 (c) put back the patch. Board maintainers who did not amend their board
 code cannot say anymore they didn't know it. Board maintainers will also have
 time to rise objections if they think they cannot change their board code
 (which I do not believe is possible, but who knows ...)

Ah and change the Documentation too I think, if you want GPIO to be only usable
from device initcall level.

> Alas, it is not an easy change, and not expected to happen quickly
> or painlessly.
Sure, so let's do it in right order, shall we ?

Cheers.

-- 
Robert

PS: When I say "broken code", in the mioa701 case, my phone doesn't wakeup on
phone calls. Disturbing, isn't it ?

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-25 19:36             ` Robert Jarzmik
@ 2013-04-25 21:22               ` Linus Walleij
  2013-04-26 17:48                 ` Robert Jarzmik
  2013-04-26 12:38               ` Mike Dunn
  1 sibling, 1 reply; 13+ messages in thread
From: Linus Walleij @ 2013-04-25 21:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 25, 2013 at 9:36 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Now this patch breaks boards. I disagree in having my board code broken without
> notice. What I would find less intrusive would be to :
>  (a) revert the patch

This was done by pull request to Torvalds yesterday ...

>  (b) inform board maintainers that they must fix their board code (for example
>  give them 1 window)
>  (c) put back the patch. Board maintainers who did not amend their board
>  code cannot say anymore they didn't know it. Board maintainers will also have
>  time to rise objections if they think they cannot change their board code
>  (which I do not believe is possible, but who knows ...)

So when it comes to the PXA there is maybe this list of things that
would be nice to change, and this is likely on top of it ... moving the
PXA machines over to device tree would be more important if people
who like them want them so stay around, because non-device tree
platforms will at some point become deprecated, and fixing the platforms
to support device tree will inevitably lead to this problem being fixed
as well.

Yours,
Linus Walleij

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-25 19:36             ` Robert Jarzmik
  2013-04-25 21:22               ` Linus Walleij
@ 2013-04-26 12:38               ` Mike Dunn
  1 sibling, 0 replies; 13+ messages in thread
From: Mike Dunn @ 2013-04-26 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/25/2013 12:36 PM, Robert Jarzmik wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
> 
>> On Tue, Apr 23, 2013 at 9:42 PM, Mike Dunn <mikedunn@newsguy.com> wrote:
>>
>>> On a more general note... I'm surprised that gpiolib can not run earlier, for
>>> something so low-level.
>>
>> Well I think the patch we're discussing is making it run less early, and it
>> can actually run as arch_initcall() or similar if you wish (it's up to the
>> driver).
>>
>> However that is beside the point, Haojian is still doing the right thing
>> trying to push this to driver init (==module_init) level. The reason is
>> that basically all hardware drivers should be initialized at this level
>> because the other init levels are basically for other things and driver
>> deps are just shoehorned into them.
>>
>> Instead of relying on things to be set up early one shall rely on
>> deferred probe.
> Hi Linus,
> 
> Even if that is the long term goal, and I agree with that goal, let me quote
> Documentation/gpio.txt : "However, for spinlock-safe GPIOs it's OK to use them
> before tasking is enabled, as part of early board setup.".
> 
> When gpio abstraction was written, it was assumed GPIO usage was usable in early
> platform setup code. As this was granted, we (board maintainers) coded
> accordingly.
> 
> Now this patch breaks boards. I disagree in having my board code broken without
> notice. What I would find less intrusive would be to :
>  (a) revert the patch
>  (b) inform board maintainers that they must fix their board code (for example
>  give them 1 window)
>  (c) put back the patch. Board maintainers who did not amend their board
>  code cannot say anymore they didn't know it. Board maintainers will also have
>  time to rise objections if they think they cannot change their board code
>  (which I do not believe is possible, but who knows ...)


Yes, thank you Robert.  More than a few boards were broken.  Old architectures
get no respect ;)

Some drivers may need rework, not just board support code.  For the pxa27x lcd
driver (pxafb), I am thinking that the board will have to pass a callback
function to the driver by way of platform_data, which the driver will call from
its probe function.  Advice gratefully received!


> Ah and change the Documentation too I think, if you want GPIO to be only usable
> from device initcall level.


As Linus mentioned, it depends on the driver.  There is no standard initcall
level for them.  Do 'grep initcall drivers/gpio/*.c' and you'll see the variety.
 This is a problem with the gpio abstraction that should be addressed, at least
in the documentation.  I wouldn't mind helping, but I'm pretty clueless at this
point.

Thanks,
Mike

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

* gpio-pxa initcall level change and machine init breakage
  2013-04-25 21:22               ` Linus Walleij
@ 2013-04-26 17:48                 ` Robert Jarzmik
  0 siblings, 0 replies; 13+ messages in thread
From: Robert Jarzmik @ 2013-04-26 17:48 UTC (permalink / raw)
  To: linux-arm-kernel

Linus Walleij <linus.walleij@linaro.org> writes:

> On Thu, Apr 25, 2013 at 9:36 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> This was done by pull request to Torvalds yesterday ...
Thanks, that's a relief.

>
>>  (b) inform board maintainers that they must fix their board code (for example
>>  give them 1 window)
>>  (c) put back the patch. Board maintainers who did not amend their board
>>  code cannot say anymore they didn't know it. Board maintainers will also have
>>  time to rise objections if they think they cannot change their board code
>>  (which I do not believe is possible, but who knows ...)
>
> So when it comes to the PXA there is maybe this list of things that
> would be nice to change, and this is likely on top of it ... moving the
> PXA machines over to device tree would be more important if people
> who like them want them so stay around, because non-device tree
> platforms will at some point become deprecated, and fixing the platforms
> to support device tree will inevitably lead to this problem being fixed
> as well.
Yeah, probably. That's a good long term goal too.
Now you reverted, I'm under pressure to fix mioa701 to get rid of gpio calls in
its board setup code :)

As for device tree conversion, well that's another story.
For my case for example I'll have to redevelop the bootloader (IPL+SPL) in order
for suspend to RAM to work ... no world is perfecgt.

Cheers.

-- 
Robert

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

end of thread, other threads:[~2013-04-26 17:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-20 15:26 gpio-pxa initcall level change and machine init breakage Mike Dunn
2013-04-21  6:02 ` Haojian Zhuang
2013-04-21 22:23   ` Mike Dunn
2013-04-22  0:58     ` Haojian Zhuang
2013-04-23  7:26       ` Linus Walleij
2013-04-23  7:49         ` Haojian Zhuang
2013-04-23 19:42         ` Mike Dunn
2013-04-24 19:37           ` Linus Walleij
2013-04-25 19:36             ` Robert Jarzmik
2013-04-25 21:22               ` Linus Walleij
2013-04-26 17:48                 ` Robert Jarzmik
2013-04-26 12:38               ` Mike Dunn
2013-04-24 16:07   ` Mike Dunn

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.