On Tue, 22 Aug 2017 11:37:45 +0200, Andy Shevchenko wrote: > > On Tue, 2017-08-22 at 07:57 +0200, Takashi Iwai wrote: > > This patch adds the MFD driver for Dollar Cove TI PMIC (ACPI INT33F5) > > that is found on some Intel Cherry Trail devices. > > The driver is based on the original work by Intel, found at: > > https://github.com/01org/ProductionKernelQuilts > > > > This is a minimal version for adding the basic resources. Currently, > > only ACPI PMIC opregion and the external power-button are used. > > Overall looks good. Few comments below. > > I hope Hans de Goede had been involved somehow to this. He did enormous > work to examine all this PMIC/ACPI/I2C stuff on CHT platforms. Sure, he was of a great help, although he didn't touch directly about these codes. > > +#include <linux/module.h> > > +#include <linux/mfd/core.h> > > +#include <linux/i2c.h> > > +#include <linux/interrupt.h> > > +#include <linux/gpio/consumer.h> > > +#include <linux/acpi.h> > > +#include <linux/regmap.h> > > +#include <linux/mfd/intel_soc_pmic.h> > > Alphabetical order? OK, will rearrange. Also, gpio stuff was removed from this version, so it's superfluous. Will rip off. > > +static const struct i2c_device_id dc_ti_i2c_id[] = { > > + { } > > +}; > > No need anymore. Moreover, you already are using ->probe_new(). OK. > > +static struct i2c_driver dc_ti_i2c_driver = { > > + .driver = { > > + .name = KBUILD_MODNAME, > > + .pm = &dc_ti_pm_ops, > > + .acpi_match_table = ACPI_PTR(dc_ti_acpi_ids), > > ACPI_PTR is redundant here, driver is solely for ACPI case. Right. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
