[RFC PATCH linux dev-4.13 2/3] dts: aspeed-g5: Expose VGA scratch registers

[Andrew Jeffery]

On Thu, 19 Apr 2018, at 12:57, Joel Stanley wrote:
> On 12 April 2018 at 13:21, Andrew Jeffery <andrew at aj.id.au> wrote:
> > Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> > ---
> >  arch/arm/boot/dts/aspeed-g5.dtsi | 48 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> > index 1468b4ad22dc..8321df50c593 100644
> > --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> > @@ -158,6 +158,11 @@
> >
> >                                 };
> >
> > +                               vga_scratch: scratch at 50 {
> > +                                       compatible = "aspeed,bmc-misc";
> 
> Convention is to use aspeed,<socname>-<hardware>, so this would be
> aspeed,ast2500-bmc-misc.

Yeah, this partly comes back to the naming problem you mentioned in the other patch. The functionality is generic, and so probably shouldn't mention aspeed at all, let alone the SoC revision.

> 
> 
> > +                                       reg = <0x50 0x20>;
> > +                               };
> > +
> >                                 hwrng at 78 {
> >                                         compatible = "timeriomem_rng";
> >                                         reg = <0x78 0x4>;
> > @@ -1425,3 +1430,46 @@
> >                 groups = "WDTRST2";
> >         };
> >  };
> > +
> > +&vga_scratch {
> > +       vga0 {
> 
> Do these names come out in the userspace API?

Yes, though you can override the use of the node name by adding a label property inside the node. That way if someone complains about the node naming we don't have to break userspace.

> 
> > +               offset = <0x50>;
> 
> Would reg <> be more devicetree-y than offset = <>?

There's a mix. I was looking through some other syscon-type stuff and some of those drivers used offset. This approach might avoid militant opinions around the relationship between the unit address and the reg property, as you "can't" have multiple nodes with the same unit address. There's some discussion on this in the PECI threads.

> 
> 
> > +               bit-mask = <0xffffffff>;
> > +               bit-shift = <0>;
> 
> Should we assume this is the default? Or something else? Having a
> default for the common case would be nice.

Whilst the nodes I've added in these examples expose an entire register, I think this would actually be a bit of an unusual case. More likely you'll want to expose a single bit from some register, in which case a default for mask (though maybe 0x1?) or shift isn't going to make any sense.

Cheers,

Andrew