Re: [PATCH 05/13] thermal: rcar: Document SoC-specific bindings

[Simon Horman <horms@verge.net.au>]

On Tue, Jul 15, 2014 at 07:16:06PM -0700, Kuninori Morimoto wrote:
> 
> Hi
> 
> > > > The documentation only mentioned the generic fallback compatible property.
> > > > Add the missing SoC-specific compatible properties, some of which are
> > > > already in use.
> > > > 
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > Cc: Zhang Rui <rui.zhang@intel.com>
> > > > Cc: Eduardo Valentin <eduardo.valentin@ti.com>
> > > > Cc: linux-pm@vger.kernel.org
> > > 
> > > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > 
> > Kuninori,
> > 
> > what' your opinion of this patch?
> > 
> > thanks,
> > rui
> > > 
> > > > ---
> > > >  .../devicetree/bindings/thermal/rcar-thermal.txt       | 18 ++++++++++++------
> > > >  1 file changed, 12 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > > index 28ef498a66e5..0ef00be44b01 100644
> > > > --- a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > > +++ b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> > > > @@ -1,7 +1,13 @@
> > > >  * Renesas R-Car Thermal
> > > >  
> > > >  Required properties:
> > > > -- compatible		: "renesas,rcar-thermal"
> > > > +- compatible		: "renesas,thermal-<soctype>", "renesas,rcar-thermal"
> > > > +			  as fallback.
> > > > +			  Examples with soctypes are:
> > > > +			    - "renesas,thermal-r8a73a4" (R-Mobile AP6)
> > > > +			    - "renesas,thermal-r8a7779" (R-Car H1)
> > > > +			    - "renesas,thermal-r8a7790" (R-Car H2)
> > > > +			    - "renesas,thermal-r8a7791" (R-Car M2)
> > > >  - reg			: Address range of the thermal registers.
> > > >  			  The 1st reg will be recognized as common register
> > > >  			  if it has "interrupts".
> > > > @@ -12,18 +18,18 @@ Option properties:
> > > >  
> > > >  Example (non interrupt support):
> > > >  
> > > > -thermal@e61f0100 {
> > > > -	compatible = "renesas,rcar-thermal";
> > > > -	reg = <0xe61f0100 0x38>;
> > > > +thermal@ffc48000 {
> > > > +	compatible = "renesas,thermal-r8a7779", "renesas,rcar-thermal";
> > > > +	reg = <0xffc48000 0x38>;
> > > >  };
> > > >  
> > > >  Example (interrupt support):
> > > >  
> > > >  thermal@e61f0000 {
> > > > -	compatible = "renesas,rcar-thermal";
> > > > +	compatible = "renesas,thermal-r8a73a4", "renesas,rcar-thermal";
> > > >  	reg = <0xe61f0000 0x14
> > > >  		0xe61f0100 0x38
> > > >  		0xe61f0200 0x38
> > > >  		0xe61f0300 0x38>;
> > > > -	interrupts = <0 69 4>;
> > > > +	interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
> > > >  };
> 
> This patch and [12/13] are adding SoC-specific compatible name.
> Of course we don't know future request,
> and, adding SoC-specific compatible name for
> fallbacking is nice safety for us.
> So, I don't have strong objection about it.
> 
> But, thermal driver side do nothing for each
> SoC-specific compatible name at this point.
> This means
> 
>  -> There is no trouble in driver/SoC
>  -> Add new (and not used) compatible name
>  -> Nothing happen in driver/SoC
> 
> My questions are...
>  1) How to verify this patch ?
>  2) Do we need to update example SoC "specific name" list in rcar-thermal.txt.
>     Few example codes are very enough ?

Hi Morimoto-san,

I believe that the approach taken with this patch is similar to the
approach that has been taken for several other drivers used by Renesas
SoCs. The implication of this approach is:

1. That the (in this case thermal) IP on the SoC's listed is known
   to work with the driver using the generic (in this case
   renesas,rcar-thermal) compatibility string.

2. That if some incompatibility is subsequently found such that the
   IP on SoC does function correctly using the generic compatibility
   string or some new feature is to be enabled which is not generic
   then it the driver should be updated with code that is triggered
   by the SoC-specific compat string.

3. That Soc dts(i) files should list the more specific SoC compat string
   followed bu the generic compat string. In this way so long as the
   driver only matches on the generic compat string it will be used. But
   if the driver is updated match on the SoC-specific compat string
   then it will be used instead. In this way dtbs should be forwards
   compatible with driver updates.

   I believe that this series includes patches to update the relevant
   dtsi files accordingly.

In relation to verification, I believe all the SoCs listed in this patch
are known to work with the generic compat string. And they should continue
to work with this change because its only an documentation change. In the
future, if a SoC specific compat string is added to the driver code then
verification would need to occur.

>From my point of view the documentation in rcar-thermal.txt is consistent
with the documentation for other drivers that use this binding scheme
(at least the ones that are documented :). I would not have any problems
examples but I don't think its entirely necessary.