lkml.org 
[lkml]   [2012]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V5 2/2] mfd: stmpe: Update DT support in stmpe driver
From
Ping!!!

On 1 December 2012 00:33, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 30 November 2012 21:15, Lee Jones <lee.jones@linaro.org> wrote:
>> But ... I don't see how the changes in the -i2c and -spi files
>> are of benefit either. When I boot without the ID table I still
>> get "stmpe-i2c 0-0040: stmpe1601 detected, chip id: 0x212".
>>
>> What is it that actually uses the IDs?
>>
>> Perhaps Viresh can shine some light on the matter?
>
> As you can see, i wasn't the author of this patch and when you asked
> this question, i didn't had an answer to it. I went through code and
> formed some theory/story :) .
>
> @Grant: i need your help to check if my theory is correct or not. Question
> is about adding below code in any i2c client driver:
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id stmpe_dt_ids[] = {
> + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], },
> + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], },
> + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], },
> + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], },
> + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], },
> + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
> +#endif
> +
> static struct i2c_driver stmpe_i2c_driver = {
> .driver = {
> .name = "stmpe-i2c",
> @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = {
> #ifdef CONFIG_PM
> .pm = &stmpe_dev_pm_ops,
> #endif
> + .of_match_table = of_match_ptr(stmpe_dt_ids),
>
> So, what is the use of this table when we already have i2c_driver.id_table
> populated.
>
> This is my theory:
> ---------------------
> Adapter drivers supporting DT will call:
> of_i2c_register_devices()
> {
> for_each_child_of_node(adap->dev.of_node, node) {
> if (of_modalias_node(node, info.type, sizeof(info.type)) < 0)
> error condition
>
> ...
> result = i2c_new_device(adap, &info);
>
> ...
> }
>
> of_modalias_node(): expects compatible in child node, i.e. stmpe node in our
> case. If it is not there, then that node is skipped. then it copies
> string after ','
> to info.type. So, for us only "stmpe810" out of "st,stmpe810" is copied.
>
> Now this name, i.e. "stmpe810" is copied as client.name in i2c_new_device()
> and device_register() is called, which will eventually call:
>
> i2c_device_match()
> {
> /* Attempt an OF style match */
> if (of_driver_match_device(dev, drv))
> return 1;
>
> driver = to_i2c_driver(drv);
> /* match on an id table if there is one */
> if (driver->id_table)
> return i2c_match_id(driver->id_table, client) != NULL;
> }
>
> This first tries to match the table my patch added, _BUT_ the string will
> never match as we had "st,stmpe810" in table and "stmpe810" in dev.
>
> So, we fall back to i2c_match_id(), which will match it against
> i2c_driver.id_table present in driver, which has entry for "stmpe810" and
> so strings matched.
>
> @Lee: This is what happened in your case. :)
>
> So, whether its DT or non DT, true is returned from here if something
> matched.
>
> Later on, this will be called:
>
> static int i2c_device_probe(struct device *dev)
> {
> .....
> status = driver->probe(client, i2c_match_id(driver->id_table, client));
> ....
> }
>
> Which will again match the legacy table to find correct struct i2c_device_id *id
> to pass to probe().
>
> So, the final question: WTF is of_match_table for?
>
> Then i thought maybe it is used when we don't have child nodes inside parent,
> something related to the phandle way ? I grep'd i2c in arch/arm/boot/dts and
> couldn't find anything of that sort, the way i2c clients are added is:
>
> in dtsi file:
>
> i2c0: i2c@address {
> ...
> }
>
> in dts file:
> &i2c0 {
> stmpe810 {
> ........
> }
> }
>
> which i believe will be taken care by dtc and will fold client nodes
> as child nodes
> of i2c0.
>
> @Grant: Please throw some light here :)
>
> --
> viresh


\
 
 \ /
  Last update: 2012-12-05 14:42    [W:0.094 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site