Re: [PATCH/RFC 2/6] boot-mode-reg: Add R-Car Gen2 driver

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

Hi Simon,

On Monday 26 October 2015 14:50:08 Simon Horman wrote:
> On Fri, Oct 23, 2015 at 03:37:46PM +0300, Laurent Pinchart wrote:
> > On Thursday 15 October 2015 16:59:18 Simon Horman wrote:
> >> On Thu, Oct 15, 2015 at 09:09:13AM +0200, Geert Uytterhoeven wrote:
> >>> On Thu, Oct 15, 2015 at 8:59 AM, Simon Horman wrote:
> >>>> --- /dev/null
> >>>> +++ b/drivers/misc/boot-mode-reg/rcar-gen2.c
> >>>> @@ -0,0 +1,61 @@
> >>>> +/*
> >>>> + * R-Car Gen2 Boot Mode Register Driver
> >>>> 
> >>>> +#define MODEMR 0xe6160060
> >>> 
> >>> Shouldn't this come from DT?
> >> 
> >> If its a property of the SoC then I'm not sure that it needs to as its a
> >> known value for the supported SoCs.
> > 
> > Hypervisors (at least Xen) use DT to initialize memory mappings. I believe
> > we should thus describe the RST IP in DT and create an rcar-rst driver
> > that includes the code from this patch.
> 
> So we would add a binding, say a compat string and a register range.
> That might look like this:
> 
> 	rst: reset-controller@e6160000 {
> 		compatible = "renesas,rst-r8a7795", "syscon";
> 		reg = <0 0xe6160000 0 0x0200>;
> 	};
> 
> The above is copped from Geerts earlier work
> "[PATCH 1/6]  reset: Add renesas,rst DT bindings"
> 
> http://www.spinics.net/lists/linux-sh/msg44800.html:
> 
> Is that binding what we are after?

That's exactly it, except that as we'll have a proper API to retrieve the boot 
mode we won't need the "syscon" compatible string.

> Would the driver do anything beyond what it currently does + using an offset
> to the base of its register range instead of the hardcoded MODEMR value (and
> changes discussed elswhere in this thread, e.g. to use soemthing like
> CLK_OF_DECLARE) ?

It should handle CPU reset in the future, but for now your description is all 
that would need to be implemented.

-- 
Regards,

Laurent Pinchart