Hi Scott,
Please see my reply inline.
> -----Original Message-----
> From: Scott Wood <oss@xxxxxxxxxxxx>
> Sent: 2018年10月22日 13:21
> To: Andy Tang <andy.tang@xxxxxxx>; mturquette@xxxxxxxxxxxx
> Cc: sboyd@xxxxxxxxxx; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx;
> linux-clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock node
>
> On Mon, 2018-10-22 at 01:34 +0000, Andy Tang wrote:
> > Hi Scott,
> >
> > Please see my reply inline.
> >
> > > -----Original Message-----
> > > From: Scott Wood <oss@xxxxxxxxxxxx>
> > > Sent: 2018年10月21日 7:54
> > > To: Andy Tang <andy.tang@xxxxxxx>; mturquette@xxxxxxxxxxxx
> > > Cc: sboyd@xxxxxxxxxx; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx;
> > > linux-clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> > > Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock
> > > node
> > >
> > > On Wed, 2018-09-19 at 15:39 +0800, andy.tang@xxxxxxx wrote:
> > > > +clockgen: global-utilities@e1000 {
> > > > + compatible = "fsl,qoriq-clockgen";
> > >
> > > Where does this compatible string come from?
> > >
> > > > + compatible = "fsl,t1023-clockgen";
> > > > };
> > >
> > > And here you overwrite it with only the chip-specific compatible?
> > >
> > > Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and
> > > fsl,qoriq-clockgen- 2.0? The existing dts says 2.0; is that wrong?
> > >
> > > BTW, assuming it is 2.0 compatible and thus the use of
> > > qoriq-clockgen2.dtsi is correct, the best course of action is
> > > probably to to remove the legacy stuff from all fsl chips, rather
> > > than introduce a new dtsi.
> > > In fact it'd be nice to see it all removed in any case. :-)
> >
> > qoriq-clockgen*.dtsi are used by legacy bindings. The contents are all
> > of legacy bindings.
> > To use new framework, I introduce a new dtsi which contains new
> > bindings and used for all PPC soc.
> > A chip-specific compatible is needed because driver will use it to get
> > chip- specific clock tree information.
> > The clock information was defined in driver not in dts in new
> > framework, remember?
>
> I'm aware that a chip-specific compatible is required. What is unusual (on
> PPC) is providing *only* the chip-specific compatible. I was expecting
> something more along the lines of
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> atchwork.ozlabs.org%2Fpatch%2F486565%2F&data=02%7C01%7Can
> dy.tang%40nxp.com%7Ceaf46e43620b45a19eb408d637de20cb%7C686e
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636757824505667539
> &sdata=T5yRSWtHYO2yLMjsZRIxZT%2BM82xuGQm2VGX7MRB8MJA%
> 3D&reserved=0
>
> Granted, the driver requiring two different compatibles to be present is
> odd, and was a convenience based on what was already in the device
> trees, but the
> fsl,qoriq-clockgen-2.0 compatible is part of the new binding (albeit an
> optional part). I guess I don't mind no longer relying on it, but at least
> remove the "fsl,qoriq-clockgen" string.
After saw your previous patch, I plan to work out a similar patch. The fsl,qoriq-clockgen-2.0 and fsl,qoriq-clockgen-1.0 will be retained.
Thanks,
Andy
>
> -Scott
