Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Linus Walleij]

On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

I think you need to CC DT bindings to devicetree-discuss.

> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
> index c855240..2a3d55c 100644
> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
> @@ -10,6 +10,8 @@ Required properties:
>  - reg: must be 0x4b for i2c address
>  - interrupts: twl6040 has one interrupt line connecteded to the main SoC
>  - interrupt-parent: The parent interrupt controller
> +- gpio-controller:
> +- #gpio-cells = <1>: twl6040 provides GPO lines.
>  - twl6040,audpwron-gpio: Power on GPIO line for the twl6040
>
>  - vio-supply: Regulator for the twl6040 VIO supply
> @@ -29,6 +31,8 @@ Required properties:
>  - ti,viblmotor-res: Resistance parameter for left motor
>  - ti,viblmotor-res: Resistance parameter for right motor
>
> +- ti,use-gpo: Set it to <1> if the GPO functionality is in use
> +

It's not like I'm sort of expert on bindings, but I've seen the
pattern status = "okay" used for this kind of boolean stuff,
and if that node is not there it's implicitly not OK.

Can someone shed some light on this convention?

Yours,
Linus Walleij

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Peter Ujfalusi]

Hello Linus,

On 07/28/2012 01:50 AM, Linus Walleij wrote:
> On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
> I think you need to CC DT bindings to devicetree-discuss.
> 
>> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> index c855240..2a3d55c 100644
>> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
>> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> @@ -10,6 +10,8 @@ Required properties:
>>  - reg: must be 0x4b for i2c address
>>  - interrupts: twl6040 has one interrupt line connecteded to the main SoC
>>  - interrupt-parent: The parent interrupt controller
>> +- gpio-controller:
>> +- #gpio-cells = <1>: twl6040 provides GPO lines.
>>  - twl6040,audpwron-gpio: Power on GPIO line for the twl6040
>>
>>  - vio-supply: Regulator for the twl6040 VIO supply
>> @@ -29,6 +31,8 @@ Required properties:
>>  - ti,viblmotor-res: Resistance parameter for left motor
>>  - ti,viblmotor-res: Resistance parameter for right motor
>>
>> +- ti,use-gpo: Set it to <1> if the GPO functionality is in use
>> +
> 
> It's not like I'm sort of expert on bindings, but I've seen the
> pattern status = "okay" used for this kind of boolean stuff,
> and if that node is not there it's implicitly not OK.

Yes I have also seen status = "okay" in dts files, but usually they were
associated with sections with compatible flag.
Here I'm not sure how to use it to indicate that the core driver should create
the mfd child for the GPIO driver.
I do not have parameters for the GPIO driver.

I could, if this is working do this:

ti,twl6040-gpo {
	status = "okay";
};

If the board needs the gpo driver, but in the driver(s) I need to check for
the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
I think it is easier to just get the value of "ti,use-gpo", if it exist and it
is 1 I enable the GPIO driver otherwise I don't.


Regards,
Péter

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Mark Brown]

On Mon, Jul 30, 2012 at 05:13:17PM +0300, Peter Ujfalusi wrote:

> If the board needs the gpo driver, but in the driver(s) I need to check for
> the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
> I think it is easier to just get the value of "ti,use-gpo", if it exist and it
> is 1 I enable the GPIO driver otherwise I don't.

Will having the GPIO driver actually cause any problems if it's not in
use?  It's not like things like RTC which are directly visible to
userspace and so can create problems if they're non-functional, unless
it does something like grab resources that might be needed by another
subfunction driver there's not really a substantial cost in just
instantiating it (people who are really space pressured probably won't
build the driver in in the first place).

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Peter Ujfalusi]

On 08/09/2012 02:55 PM, Mark Brown wrote:
> On Mon, Jul 30, 2012 at 05:13:17PM +0300, Peter Ujfalusi wrote:
> 
>> If the board needs the gpo driver, but in the driver(s) I need to check for
>> the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
>> I think it is easier to just get the value of "ti,use-gpo", if it exist and it
>> is 1 I enable the GPIO driver otherwise I don't.
> 
> Will having the GPIO driver actually cause any problems if it's not in
> use?  It's not like things like RTC which are directly visible to
> userspace and so can create problems if they're non-functional, unless
> it does something like grab resources that might be needed by another
> subfunction driver there's not really a substantial cost in just
> instantiating it (people who are really space pressured probably won't
> build the driver in in the first place).

Fair enough, I'll make the change for the v2.

-- 
Péter