All of lore.kernel.org
 help / color / mirror / Atom feed
From: kiko@linaro.org (Christian Robottom Reis)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.
Date: Tue, 17 Apr 2012 11:13:09 -0300	[thread overview]
Message-ID: <20120417141309.GH11786@async.com.br> (raw)
In-Reply-To: <CAMXH7KH41kWMPu_dEpetZc8OUD3C0-mtxGbXX0Vqd4EODPWJUg@mail.gmail.com>

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> + ? ? drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.

Regardless of how you end up solving this, it's probably a good idea
to document the rationale, perhaps cribbing from what you describe
below..

> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

..in a comment; without it, the code indeed looks bizarre.
-- 
Christian Robottom Reis, Engineering VP
Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
Linaro.org: Open Source Software for ARM SoCs

WARNING: multiple messages have this Message-ID (diff)
From: Christian Robottom Reis <kiko@linaro.org>
To: Rob Lee <rob.lee@linaro.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>,
	linaro-dev@lists.linaro.org, patches@linaro.org,
	linux-kernel@vger.kernel.org, kernel@pengutronix.de,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality.
Date: Tue, 17 Apr 2012 11:13:09 -0300	[thread overview]
Message-ID: <20120417141309.GH11786@async.com.br> (raw)
In-Reply-To: <CAMXH7KH41kWMPu_dEpetZc8OUD3C0-mtxGbXX0Vqd4EODPWJUg@mail.gmail.com>

On Tue, Apr 17, 2012 at 08:54:03AM -0500, Rob Lee wrote:
> >> +void __init imx_cpuidle_set_driver(struct cpuidle_driver *p)
> >> +{
> >> +     drv = p;
> >> +}
> >
> > You like it complicated, eh? Why do you introduce a function which sets
> > a variable...
> 
> This complication is used to deal with the timing of various levels of
> init calls.  More explanation below.

Regardless of how you end up solving this, it's probably a good idea
to document the rationale, perhaps cribbing from what you describe
below..

> If I called imx_cpuidle_init directly from imx5 or imx6q init
> routines, it would be getting called before the coreinit_call of core
> cpuidle causing a failure.  There were various other directions to
> take and all seemed less desirable than this one.
> 
> One alternative would be to add a function to return the pointer to
> the cpuidle driver object based on the machine type.  Functionality
> exists to identify imx5 as a machine type but not imx6q, so I couldn't
> use that machine based method without adding that extra code.
> 
> Another alternative would be to add a general platform lateinit_call
> function to each platforms that support cpuidle.

..in a comment; without it, the code indeed looks bizarre.
-- 
Christian Robottom Reis, Engineering VP
Brazil (GMT-3) | [+55] 16 9112 6430 | [+1] 612 216 4935
Linaro.org: Open Source Software for ARM SoCs

  reply	other threads:[~2012-04-17 14:13 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-16 23:50 [PATCH 0/3] Add imx cpuidle Robert Lee
2012-04-16 23:50 ` Robert Lee
2012-04-16 23:50 ` [PATCH 1/3] ARM: imx: Add common imx cpuidle init functionality Robert Lee
2012-04-16 23:50   ` Robert Lee
2012-04-16 23:56   ` Jesper Juhl
2012-04-16 23:56     ` Jesper Juhl
2012-04-17  7:43   ` Sascha Hauer
2012-04-17  7:43     ` Sascha Hauer
2012-04-17 13:54     ` Rob Lee
2012-04-17 13:54       ` Rob Lee
2012-04-17 14:13       ` Christian Robottom Reis [this message]
2012-04-17 14:13         ` Christian Robottom Reis
2012-04-17 14:32         ` Rob Lee
2012-04-17 14:32           ` Rob Lee
2012-04-17 17:42       ` Sascha Hauer
2012-04-17 17:42         ` Sascha Hauer
2012-04-17 19:32         ` Rob Lee
2012-04-17 19:32           ` Rob Lee
2012-04-19  4:18           ` Rob Lee
2012-04-19  4:18             ` Rob Lee
2012-04-19  6:43             ` Sascha Hauer
2012-04-19  6:43               ` Sascha Hauer
2012-04-20  2:08               ` Shawn Guo
2012-04-20  2:08                 ` Shawn Guo
2012-04-23  4:44                 ` Rob Lee
2012-04-23  4:44                   ` Rob Lee
2012-04-23  5:18                   ` Shawn Guo
2012-04-23  5:18                     ` Shawn Guo
2012-04-23  6:27                     ` Sascha Hauer
2012-04-23  6:27                       ` Sascha Hauer
2012-04-23  6:53                       ` Shawn Guo
2012-04-23  6:53                         ` Shawn Guo
2012-04-23  6:56                         ` Sascha Hauer
2012-04-23  6:56                           ` Sascha Hauer
2012-04-23  7:10                           ` Shawn Guo
2012-04-23  7:10                             ` Shawn Guo
2012-04-23  7:48                             ` Sascha Hauer
2012-04-23  7:48                               ` Sascha Hauer
2012-04-23 15:45                               ` Rob Lee
2012-04-23 15:45                                 ` Rob Lee
2012-04-24  1:38                                 ` Shawn Guo
2012-04-24  1:38                                   ` Shawn Guo
2012-04-24  7:54                                   ` Russell King - ARM Linux
2012-04-24  7:54                                     ` Russell King - ARM Linux
2012-04-24  8:36                                     ` Shawn Guo
2012-04-24  8:36                                       ` Shawn Guo
2012-04-24 15:40                                       ` Rob Lee
2012-04-24 15:40                                         ` Rob Lee
2012-04-24 19:51                                         ` Russell King - ARM Linux
2012-04-24 19:51                                           ` Russell King - ARM Linux
2012-04-25  2:06                                           ` Shawn Guo
2012-04-25  2:06                                             ` Shawn Guo
2012-04-16 23:50 ` [PATCH 2/3] ARM: imx: Add imx5 cpuidle driver Robert Lee
2012-04-16 23:50   ` Robert Lee
2012-04-16 23:50 ` [PATCH 3/3] ARM: imx: Add imx6q " Robert Lee
2012-04-16 23:50   ` Robert Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120417141309.GH11786@async.com.br \
    --to=kiko@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.