On 5/29/2014 7:40 PM, Lee Jones wrote: > [...] > >> +static int intel_soc_pmic_find_gpio_irq(struct device *dev) >> +{ >> + struct gpio_desc *desc; >> + int irq; >> + >> + desc = devm_gpiod_get_index(dev, KBUILD_MODNAME, 0); > > What does "KBUILD_MODNAME" translate to?
It translates into "intel_soc_pmic". > >> + if (IS_ERR(desc)) { >> + dev_dbg(dev, "Not using GPIO as interrupt.\n"); > > You can't have a debug print, then return an err - use dev_err(). Actually returning ENOENT here is just a hardware difference. On some boards the PMIC interrupt is from a GPIO line exposed by the CPU, on the rest (e.g. Asus T100TA) it's not. When -ENOENT is returned, probe() will simply use the IRQ provided by the I2C. I will remove this line completely, and put a comment before the function. > > [...] > >> +static int intel_soc_pmic_i2c_probe(struct i2c_client *i2c, >> + const struct i2c_device_id *id) >> +{ >> + struct device *dev = &i2c->dev; >> + struct intel_soc_pmic_config *config = >> + (struct intel_soc_pmic_config *)id->driver_data; >> + struct intel_soc_pmic *pmic; >> + int ret; >> + int irq; >> + >> + pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL); >> + dev_set_drvdata(dev, pmic); >> + >> + pmic->regmap = devm_regmap_init_i2c(i2c, config->regmap_config); >> + >> + irq = intel_soc_pmic_find_gpio_irq(dev); >> + if (irq < 0) >> + pmic->irq = i2c->irq; >> + else >> + pmic->irq = irq; > > pmic->irq = (irq < 0) ? i2c->irq : irq; I'll fix it. >> + ret = regmap_add_irq_chip(pmic->regmap, pmic->irq, >> + config->irq_flags | IRQF_ONESHOT, >> + 0, config->irq_chip, >> + &pmic->irq_chip_data); >> + if (ret) >> + goto err; > > Just return ret here and remove the 'err:' label. I'll fix it. > > [...] > >> +static const struct i2c_device_id intel_soc_pmic_i2c_id[] = { >> + {"INT33FD:00", (kernel_ulong_t)&intel_soc_pmic_config_crc}, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, intel_soc_pmic_i2c_id); >> + >> +static struct acpi_device_id intel_soc_pmic_acpi_match[] = { >> + {"INT33FD", (kernel_ulong_t)&intel_soc_pmic_config_crc}, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(acpi, intel_soc_pmic_acpi_match); > > Does ACPI have a match function to extact it's .driver_data attribute? > > If so, are you using it here? If not, why not? > The ACPI table is used in i2c_device_match(), and the i2c table is used in i2c_device_probe(), so the id in the i2c table is actually fed to intel_soc_pmic_probe(). But I only found out now that having the i2c table alone is enough, because i2c_device_match will fallback to the i2c table if there's no ACPI table. So to keep it simple, I'll remove the ACPI table completely. By the way, the GPIO child driver got reviewed-by from Linus Walleij, but can't be merged because it depends on intel_soc_pmic.h. May I include it in next version of the patch set and have it merged along with the MFD driver? Best Regards Lejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/