Re: [PATCH 2/3] 82xx: Parse SMC serial device node in DTS

[Mark Zhan <rongkai.zhan@windriver.com>]

On Tue, 2007-07-17 at 14:06 +0200, Arnd Bergmann wrote:
> On Tuesday 17 July 2007, Mark Zhan wrote:
> > On Tue, 2007-07-17 at 03:02 +0200, Arnd Bergmann wrote:
> > > On Monday 16 July 2007, Mark Zhan wrote:
> > > > -               cpm_uart_data.uart_clk = ppc_proc_freq;
> > > > +               if (strstr(model, "SMC")) {
> > > > +                       cpm_uart_dev = platform_device_register_simple("fsl-cpm-smc:uart",
> > > > +                                                       i, &r[0], 3);
> > > > +               } else if (strstr(model, "SCC")) {
> > > > +                       cpm_uart_dev = platform_device_register_simple("fsl-cpm-scc:uart",
> > > > +                                                       i, &r[0], 3);
> > > > +               }
> > >
> > > You should probably use of_device_is_compatible() to check
> > > if a given device can be used by a particular driver.
> >
> > I think, the function of_find_compatible_node(), which is called in the
> > for loop, has already done that. So definitely, no need to call
> > of_device_is_compatible() any more.
> 
> It's a little more complicated than that. The "compatible" property should
> indicate the exact interface of that device, so the "model" does not really
> matter here. I don't know the difference between smc and scc, but if you
> need to register them as different pplatform devices, they should
> normally also have different names in "compatible", possibly in addition
> to the existing one.
> 
> If the compatible property contains "fsl,cpm-smc\0cpm_uart", you can scan for
> either of them. The loop will iterate over all cpm_uart compatible devices,
> while the later test will look for an fsl,cpm-smc compatible device.

Arnd,

Yes, basically I agree what you say. That should be the right way. 

but, the current situation is that: in all DTS files that are using smc
or scc as uart device, all device node definitions have the same
"compatible" property -- "cpm_uart"

So what I do here is just following the upstream source tree.

Thanks
Mark Zhan