Re: [PATCH 7/7] OF/ACPI/I2C: Add generic match function for the aforementioned systems

From: Mark Brown
Date: Sat Jun 07 2014 - 05:43:15 EST


On Sat, Jun 07, 2014 at 12:42:53AM +0100, Grant Likely wrote:
> On Fri, 6 Jun 2014 13:36:53 +0100, Lee Jones <lee.jones@xxxxxxxxxx> wrote:
> > On Fri, 06 Jun 2014, Mark Brown wrote:

> > > > I've also been mulling over the idea of removing the second probe()
> > > > parameter, as suggested by Wolfram. However, this has quite deep
> > > > ramifications which would require a great deal of driver adaptions.

> > > If you're going to do that another option is to refactor the probe()
> > > function to take the driver_data as an argument and then have the core
> > > pass that from whatever table it matched from rather than the entire
> > > i2c_device_id structure. That way the driver just needs to supply all
> > > the ID tables mapping binding information to whatever it needs and the
> > > core can pass in the driver data from whatever table it matched against.

> > Unfortunately this means we're back to the aforementioned typing
> > issue. For struct {platform,i2c,spi,acpi,etc}_device_id the driver
> > data is a kernel ulong but the of_device_id's driver data attribute is
> > a void*.

> We're actually okay there. Each subsystem defines it's own convention
> about what those values mean. ulong and void* are the same size and
> every user I've seen stuffs the same data into the data field of both
> tables.

Indeed - if we're going to go with that approach it seems to me like we
should just pick one and put any casting in the core.

> > I've just started work on a migration over to a new probe(). I don't
> > think it's all that much work, but if there are any objections I'd
> > prefer to hear them now rather than waste any time.

> I have no problem with that approach.

Converting probe() makes sense to me. I think I would prefer to see the
ID lookup handled as an argument to probe() rather than with functions
in probe() but it's not the end of the world if it isn't.

Attachment: signature.asc
Description: Digital signature