On 08/04/16(Fri) 17:14, Patrick Wildt wrote:
> On Fri, Apr 08, 2016 at 04:34:25PM +0200, Martin Pieuchot wrote:
> > On 08/04/16(Fri) 14:46, Patrick Wildt wrote:
> > > Hi,
> > > 
> > > so that we can easily check if a node is compatible or to find nodes
> > > that are compatible, I would like to add helpers to the fdt routines.
> > > 
> > > This way the drivers can check if they "match" to a node by simply
> > > calling:
> > > 
> > >   if (fdt_node_compatible(ma->ma_node, "samsung,exynos4210-ehci"))
> > >           return (1);
> > > 
> > > Sometimes it's helpful to find any node that is compatible.  It can
> > > be helpful for instance on finding the early uart.  Simplified example:
> > > 
> > >   if ((node = fdt_find_compatible("arm,pl011")) != NULL)
> > >           pl011cnattach(...);
> > > 
> > > Thoughts? ok?
> > 
> > Always hard to comment without seeing which code would use that.
> > 
> > Is a custom function really needed?  Why not do like sparc64 and macppc,
> > for example your ehci_match() could be:
> > 
> > {
> >     char compat[32];
> >     ...
> > 
> >     if (OF_getprop(ma->ma_node, "compatible", compat, sizeof(compat)) == -1)
> >             return 0;
> >     
> >     if (strcmp(compat, "samsung,exynos4210-ehci") == 0
> >             return 1;
> >     
> >     return 0;
> > }
> > 
> 
> The compatible attribute can be a list of strings:

Yes, that's also true for macppc.

> Implementing parsing multiple strings in a buffer and checking it in
> every _match function seemed a bit too much overhead.

Seems or is overhead?  How is the reality?  From my experience dealing
with Apple device-tree most of the time you just use one of the
compatible entry.

>                                                       Imagine 20 drivers
> doing the same code over and over again.

So?  What I find really nice about FDT is that you can use the OF_* API.
That means we can write a driver for ARMv7 the same way we do it for
sparc64 and macppc.  So it's easier to learn an maintain.  If you
introduce fdt-only functions, we lose this advantage.

Now there might be a real benefit to having such function, but in that
case I'd suggest to implement it in the OF_* API.

For the moment I haven't seen any code that in my opinion justify this.

> Especially if you're also are not looking for one compatible, but
> multiple compatibles:

You have some example, I don't find the code hard to follow:

macppc:         aoa, wdc@obio, mpcpci
sparc64:        com@ebus, comkbd, comms, ofwiic, pcfiic, pyro, schizo 

Reply via email to