[2/7] gpio: rcar: Add optional functional clock to bindings

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: linux-gpio@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/gpio/renesas,gpio-rcar.txt |    4 ++++
 1 file changed, 4 insertions(+)

Hi Geert,

Thank you for the patch.

On Thursday 27 March 2014 21:47:37 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: linux-gpio@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/gpio/renesas,gpio-rcar.txt |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt index
> f61cef74a212..9ccbe50dcec2 100644
> --- a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> +++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> @@ -21,6 +21,10 @@ Required Properties:
>      GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
>    - gpio-ranges: Range of pins managed by the GPIO controller.
> 
> +Optional properties:
> +
> +  - clocks: Must contain a reference to the functional clock.
> +

I would make the property mandatory. Obviously the driver needs to consider it 
as optional in order not to break the DT ABI, but the specification should 
make it mandatory in order to ensure that all future implementations will 
specify the clock.

>  Please refer to gpio.txt in this directory for details of gpio-ranges
> property and the common GPIO bindings used by client devices.

Hi Laurent,

On Fri, Mar 28, 2014 at 5:53 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> --- a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
>> +++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
>> @@ -21,6 +21,10 @@ Required Properties:
>>      GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
>>    - gpio-ranges: Range of pins managed by the GPIO controller.
>>
>> +Optional properties:
>> +
>> +  - clocks: Must contain a reference to the functional clock.
>> +
>
> I would make the property mandatory. Obviously the driver needs to consider it
> as optional in order not to break the DT ABI, but the specification should
> make it mandatory in order to ensure that all future implementations will
> specify the clock.

I think it has to stay optional: unless I misinterpreted the datasheet, r8a7778
doesn't have MSTP bits for the GPIO modules. I guess it's the same for
other R-Car Gen1 like r8a7779. So it looks like the bits were added in
R-Car Gen2.

Hence I'll just add ", if present", unless the Best Practice is to put such
properties under "Required properties" or "Recommended properties"?

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Geert,

On Monday 31 March 2014 11:55:52 Geert Uytterhoeven wrote:
> On Fri, Mar 28, 2014 at 5:53 PM, Laurent Pinchart wrote:
> >> --- a/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> >> +++ b/Documentation/devicetree/bindings/gpio/renesas,gpio-rcar.txt
> >> 
> >> @@ -21,6 +21,10 @@ Required Properties:
> >>      GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
> >>    - gpio-ranges: Range of pins managed by the GPIO controller.
> >> 
> >> +Optional properties:
> >> +
> >> +  - clocks: Must contain a reference to the functional clock.
> >> +
> > 
> > I would make the property mandatory. Obviously the driver needs to
> > consider it as optional in order not to break the DT ABI, but the
> > specification should make it mandatory in order to ensure that all future
> > implementations will specify the clock.
> 
> I think it has to stay optional: unless I misinterpreted the datasheet,
> r8a7778 doesn't have MSTP bits for the GPIO modules. I guess it's the same
> for other R-Car Gen1 like r8a7779. So it looks like the bits were added in
> R-Car Gen2.

Good point. It might also be that older SoCs have MSTP bits for the GPIO cores 
but don't document them. Anyway, we have no information, so we can't specify a 
clock.

> Hence I'll just add ", if present", unless the Best Practice is to put such
> properties under "Required properties" or "Recommended properties"?

I'm fine with keeping in under "Optional Properties". I would phrase it as 
"Must contain a reference to the functional clock. The property is mandatory 
if the hardware implements a controllable functional clock for the GPIO 
instance." (or something similar).