Messages in this thread | | | Date | Thu, 11 Sep 2014 15:23:40 -0700 | From | Jacob Pan <> | Subject | Re: [PATCH 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic |
| |
On Tue, 9 Sep 2014 15:37:04 +0200 Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Tue, Sep 09, 2014 at 05:45:17AM -0700, Jacob Pan wrote: > > > > -static int axp20x_i2c_probe(struct i2c_client *i2c, > > > > - const struct i2c_device_id *id) > > > > +static int axp2xx_match_device(struct axp2xx_dev *axp2xx, > > > > struct device *dev) { > > > > - struct axp20x_dev *axp20x; > > > > + const struct acpi_device_id *acpi_id; > > > > const struct of_device_id *of_id; > > > > - int ret; > > > > > > > > - axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), > > > > GFP_KERNEL); > > > > - if (!axp20x) > > > > - return -ENOMEM; > > > > + of_id = of_match_device(axp2xx_of_match, dev); > > > > + if (of_id) { > > > > + axp2xx->variant = (long) of_id->data; > > > > + goto found_match; > > > > + } > > > > > > > > - of_id = of_match_device(axp20x_of_match, &i2c->dev); > > > > - if (!of_id) { > > > > - dev_err(&i2c->dev, "Unable to setup AXP20X > > > > data\n"); > > > > + acpi_id = > > > > acpi_match_device(dev->driver->acpi_match_table, dev); > > > > + if (!acpi_id || !acpi_id->driver_data) { > > > > + dev_err(dev, "Unable to setup AXP2XX ACPI > > > > data\n"); return -ENODEV; > > > > } > > > > - axp20x->variant = (long) of_id->data; > > > > + axp2xx->variant = (long) acpi_id->driver_data; > > > > > > Shouldn't that be in the if statement above? I guess acpi_id will > > > be null on a DT-based system. > > > > > I am not sure what you mean. if acpi_id == NULL, then it will return > > -ENODEV since of_match_device() already found no match. If > > acpi_id != NULL, then id must contain variant info. > > Hmm, never mind. I read it backward and thought you were still in this > code path. I guess I need more coffee. > > That goto isn't very intuitive though. > > Maybe something like > > if (of_id) { > /* DT case */ > } else if (acpi_id) { > /* ACPI case */ > } else { > return; > } > > would be better? > yes, will be in v3.
thanks. > Maxime >
[Jacob Pan]
| |