lkml.org 
[lkml]   [2014]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 1/3] mfd: intel_soc_pmic: Core driver
    > >> +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".

    Can you just put that instead?

    > >> + 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.

    That'll do, thanks.

    > >> +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.

    Actually, can you do it the other way round? Minimise the i2c table
    and populate the ACPI one. I'm just about to work on a separate
    patch-set which deprecates the use of the i2c table on DT and/or ACPI
    only registered devices.

    > 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?

    Yes, if it's okay with Linus and you aapply his Ack.

    --
    Lee Jones
    Linaro STMicroelectronics Landing Team Lead
    Linaro.org │ Open source software for ARM SoCs
    Follow Linaro: Facebook | Twitter | Blog
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2014-05-30 11:01    [W:3.176 / U:0.908 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site