Hello Lee,
On 09/24/2015 06:58 PM, Lee Jones wrote:
[snip]
>>
>>> Drivers will know if they either only supply an I2C or OF table, so
>>> they will know which call to use in order to obtain their
>>
>> Yes but that is not true for drivers that support both OF and legacy board
>> files. For those drivers, there will be a lot of boiler plate code duplicated
>> that would look something like:
>>
>> unsigned long data;
>> struct of_device_id *match;
>> struct i2c_devicd_id *id;
>>
>> if (i2c->dev.of_node) {
>> match = i2c_of_match_device(of_match_table, i2c);
>> if (!match)
>> return -EINVAL;
>>
>> data = (unsigned long)match->data;
>> } else {
>> id = i2c_match_id(id_table, i2c);
>> if (!id)
>> return -EINVAL;
>>
>> data = id->driver_data;
>> }
>>
>> While it would be nice to have something like:
>>
>> data = i2c_get_data(i2c);
>>
>> and let the core handle which table should be looked up depending on
>> which mechanism was used to register the i2c device (legacy or OF).
>
> I'm fine with a new API for this stuff. I'm even happy to go ahead
> and code it up, but it's important to note that this is work which
> should be based on this set and not a blocker for this set to be
> accepted.
>
I didn't mean this should be a blocker and yes can be done as a follow up.
>>> .driver_data|.data. attributes. We can generify the call if you think
>>> that makes things easier, but I don't see a need for it ATM.
>>>
>>
>> As I explained above, it will make easier for drivers but I raised the
>> point to discuss if the table data should be looked up by the driver
>> or if the core should get it and pass to the probe() function as it is
>> made right now for the I2C device ID table. i.e:
>>
>> static int foo_i2c_probe(struct i2c_client *i2c, const void *data)
>>
>> If the correct approach is the former, then this series is the right
>> direction and as you said a generic match function can be added later.
>>
>> But if the correct approach is the latter, then this series is not
>> the right direction and a different approach is needed. I don't have
>> a strong opinion but wanted to mention that we have two options here.
>
> The correct approach is the former. One of the aims of this set was
> to bring the I2C .probe() call-back more into line with the majority
> of the other .probe() calls in the kernel i.e. with only a single
> parameter. I'm really not a fan of passing some random void pointer
> in. Using a look-up call to fetch ACPI/OF/I2C/etc data is the current
> norm and is a very viable option.
>
Ok, as I said I don't have a strong opinion and you are right that this
set will make I2C to be more aligned with other subsystems (i.e: SPI that
the I2C implementation is very similar to).
> Wolfram, please (finally :D) take this set.
>
Indeed :)
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
