On Tue, Oct 06, 2020 at 08:13:17AM +0200, Ricardo Cañuelo wrote: > Hi Rob, > > thanks for reviewing the patch. Find my comments below: > > On lun 05-10-2020 10:37:09, Rob Herring wrote: > > > + '#address-cells': > > > + enum: [1, 2] > > > + > > > + '#size-cells': > > > + enum: [0, 1] > > > > This doesn't really make sense. Either there's a size or there isn't. > > > > [...] > > > > > + "^regulator@[a-f0-9]+$": > > > + "^ec-codec@[a-f0-9]+$": > > > > What does the number space represent and is it the same for each of > > these? If not, then this is kind of broken. There's only 1 number > > space at a given level. > > I see what you mean. In the regulator, the unit-address means the identifier > for the voltage regulator and I guess it could also be defined as > simply "^regulator@[0-9]+$". In the codec, though, it's a physical base > address. > > The reg property in these has a different format, that's why I > defined #address-cells and #size-cells above to have a range of values > instead of fixed values. But in any given DT it is going to be fixed, so that doesn't help you. > >From your experience, what's the best course of action here? I can't > find a driver managing google,cros-ec-codec yet, although the binding > was submitted in January. Normally, you just add another layer with a 'regulators' and/or 'codecs' node. Do you really have more than 1 codec? Rob
