Messages in this thread | | | Subject | Re: [PATCH v3 1/4] pinctrl: aspeed: Read and write bits in LPC and GFX controllers | From | Andrew Jeffery <> | Date | Thu, 15 Dec 2016 10:30:43 +1030 |
| |
On Wed, 2016-12-14 at 10:46 -0600, Rob Herring wrote: > >> > + compatible = "aspeed,g5-pinctrl"; > >> > >> There's no register range for pinctrl? > > > > This may be a mistake on my part; when I wrote this I had no experience > > with writing devicetree bindings (and still don't have a lot). > > > > The SCU does have register regions for pinctrl but on reflection I feel > > neither the mfd nor syscon bindings describe how children's resources > > should be treated in general. The example in the mfd bindings is for > > hardware that is register-bit-led,compatible, whose bindings use the > > 'offset' property rather than 'reg', which still describes where, but > > not using the reg property. Given my uncertainty with reg in an mfd > > child, I wrote the pinctrl/pinmux driver using offsets from the base of > > the SCU's syscon rather than describing the exact region(s) of the > > syscon that should be used. > > > > The issue you raise here occurred to me when writing the LPC Host > > Controller bindings, but there I wasn't convinced using the ranges > > property to give offsets was the right thing to do either. > > > > Regardless, whilst there are two dedicated regions of pinmux registers, > > the mux state also depends on bits in SCU registers outside of these > > regions. Assuming we define an appropriate ranges property for the SCU > > syscon the pinctrl reg property would look like: > > > > reg = <0x2c 0x1>, <0x3c 0x1>, <0x48 0x1>, <0x70 0x1>, <0x7c 0x1>, <0x80 0x18>, <0xa0 0x10>, <0xd0 0x1>; > > > > This is the list of registers affecting the mux taken from the pinctrl- > > aspeed.h. > > With other registers in the holes, right?
Yes.
> If it is sparse like that, > then yes you probably just want to have reg in the parent for the > whole block.
Alright, we will leave it as-is.
> > > What action do you recommend here? The pinctrl dts patches for the > > Aspeed SoCs are yet to be applied, so changing the bindings to require > > a reg property can't break any existing in-tree users as there are > > none. The pinctrl driver can be patched to respect the reg property > > after the fact, though actually using the region descriptions might be > > interesting. > > > >> > >> > + aspeed,external-nodes = <&gfx, &lpchc>; > > > > Did you have feedback on this approach? I queried you about it in the > > previous revision, but never received a reply: > > It seems okay. At least, I don't have a better suggestion.
Thanks.
Andrew[unhandled content-type:application/pgp-signature] | |