Re: [PATCH] i2c: Driver to expose PowerNV platform i2c busses

Benjamin Herrenschmidt
2014-11-14T05:17:19Z

On Fri, 2014-11-14 at 07:53 +1100, Benjamin Herrenschmidt wrote:
> > > > > + adapter->dev.parent = &pdev->dev;
> > > > > + adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> > > > > + pname = of_get_property(pdev->dev.of_node, "port-name", NULL);
> > > >
> > > > I have never seen this binding before, it looks fishy. Where is it documented?
> > >
> > > We made it up, like pretty every SoC vendor out there. What's fishy
> > > about it ? It's a very good way to get fixed i2c port names on the
> > > system, the firmware defines them.
> >
> > But the SoC vendors prefix it with their company name and add
> > documentation for the binding.
>
> Why do we need to prefix arbitrary props for a very specific device ?
> When adding things to an existing more/less generic device it makes some
> sense but here I don't see much point. I can whip up a "binding"
> document for this adapter and make "port-name" be part of it if you
> want :) In fact a better name for the property might be "bus-id"...

Note: We've slipped in a last minute update to the FW which isn't
deployed yet to turn that into ibm,port-name and change the compatible
match to the more generic "ibm,opal-i2c" since the underlying HW
implementation doesn't have to be P8 (and in fact we'll soon also expose
the i2c bus off the Centaur memory buffer chips that goes to the DIMMs).

This is the only parts of the "binding" linux cares about. The actual
DT representation for that device is a bit more complete, there's a
parent node for the i2c master engine and a node per port, but clients
like Linux only care about the ports. The clock frequencies are in
standard properties but here too, only the FW use them for its own
internal use, this is an abstract FW API (server world ....)

Cheers,
Ben.