Re: [PATCH 10/11] OMAP: GPIO: Implement GPIO as a platform device

[Kevin Hilman <khilman@deeprootsystems.com>]

"Cousson, Benoit" <b-cousson@ti.com> writes:

> Hi Charu
>
>>From: Varadarajan, Charulatha
>>
>>> From: Cousson, Benoit
>>> Sent: Tuesday, May 18, 2010 10:55 PM
>>>
>>> Hi Charu,
>>>
>>> On 5/18/2010 4:50 PM, Charulatha V wrote:
>>>
>>> [snip]
>>>
>>> > diff --git a/arch/arm/mach-omap2/clock2420_data.c b/arch/arm/mach-
>>> omap2/clock2420_data.c
>>> > index d932b14..a693403 100644
>>> > --- a/arch/arm/mach-omap2/clock2420_data.c
>>> > +++ b/arch/arm/mach-omap2/clock2420_data.c
>>> > @@ -1802,8 +1802,14 @@ static struct omap_clk omap2420_clks[] = {
>>> >          CLK(NULL,       "uart2_fck",&uart2_fck,     CK_242X),
>>> >          CLK(NULL,       "uart3_ick",&uart3_ick,     CK_242X),
>>> >          CLK(NULL,       "uart3_fck",&uart3_fck,     CK_242X),
>>> > -       CLK(NULL,       "gpios_ick",&gpios_ick,     CK_242X),
>>> > -       CLK(NULL,       "gpios_fck",&gpios_fck,     CK_242X),
>>> > +       CLK("omap-gpio.0",      "ick",&gpios_ick,     CK_242X),
>>> > +       CLK("omap-gpio.1",      "ick",&gpios_ick,     CK_242X),
>>> > +       CLK("omap-gpio.2",      "ick",&gpios_ick,     CK_242X),
>>> > +       CLK("omap-gpio.3",      "ick",&gpios_ick,     CK_242X),
>>> > +       CLK("omap-gpio.0",      "fck",&gpios_fck,     CK_242X),
>>> > +       CLK("omap-gpio.1",      "fck",&gpios_fck,     CK_242X),
>>> > +       CLK("omap-gpio.2",      "fck",&gpios_fck,     CK_242X),
>>> > +       CLK("omap-gpio.3",      "fck",&gpios_fck,     CK_242X),
>>>
>>> Are you sure that this is still needed? In theory Paul removed the
>>> dependency a couple of months ago.
>>> HWMOD does not need that anymore at least, it is relying on the clock
>>> name only.
>>
>>You are right. These are not required if hwmod is used. But
>>here we use pm_runtime APIs which needs clk_get/ clk_enable in
>>case of CONFIG_PM_RUNTIME is not defined.
>>Also in OMAP3&4, for dbck we need clk_get/clk_enable to be
>>used as opt_clk is
>>taken care by the driver. Hence the above changes are required.
>
> That still looks wrong to me. We cannot keep exposing manual clock management method in case runtime_pm is not used.
> It makes the whole hwmod isolation stuff a little bit useless, it forces us to maintain some dummy clocks node...
> In that case you should use the omap_device to enable the device.

I agree, the !CONFIG_PM_RUNTIME case can still use omap_device API.

> Dbck are listed as opt_clock in the hwmod struct and thus can be accesses.

Agreed.

Kevin