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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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).

-- 
Regards,

Laurent Pinchart