Messages in this thread | | | Date | Sun, 23 Oct 2011 13:55:13 +0200 | From | Mark Brown <> | Subject | Re: [PATCH 1/3] mfd: Add S5M core driver |
| |
On Sun, Oct 23, 2011 at 07:39:36PM +0900, Sangbeom Kim wrote: > On Sun, Oct 23, 2011 at 05:37 PM +0900, Mark Brown wrote:
> > > +EXPORT_SYMBOL(s5m_bulk_read);
> > All this stuff should be _GPL as the regmap core is _GPL - you shouldn't > > wrap a _GPL function with a non-GPL one.
> You mean that EXPORT_SYMBOL should be replace with EXPORT_SYMBOL_GPL?
Yes.
> > > +static struct mfd_cell s5m87xx_devs[] = { > > > + { > > > + .name = "s5m8763-pmic", > > > + }, { > > > + .name = "s5m8767-pmic", > > > + }, {
> > It looks a bit odd to have both simultaneously but I guess this will > > become more obvious later on? I guess what I'd expect is one of these > > arrays per device variant.
> My intention of this mfd driver is supporting all samsung mfd. > It is desirable that a core driver handle various driver to prevent produce > similar code.
Yes, this is very similar to what the wm831x driver does to handle all the different chips we've got with the same register interface.
> So, If I want to implement like above concept, What kind of approach can be > advised?
What wm831x does is register a different set of devices depending on the device that gets registered - the per-device stuff is mostly just a set of tables of devices to register.
> > > + s5m87xx->dev = &i2c->dev; > > > + s5m87xx->i2c = i2c; > > > + s5m87xx->irq = i2c->irq;
> > Is SPI supported?
> SPI isn't supported by Samsung mfd
In that case it might be as well to just get things like the IRQ from the i2c client rather than keeping a copy of the variable.
| |