Re: [patch v1 1/2] mfd: Add Mellanox regmap core driver

[Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

Hi!

> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/mellanox,mlxreg-core
> > > @@ -0,0 +1,347 @@
> > > +Mellanox programmable device control.
> > > +-------------------------------------
> > > +This binding defines the device control interface over I2C bus for
> > > +Mellanox BMC based switches.
> > 
> > You'll need to cc dt maintainers here.
> 
> Hi Pavel,
> 
> Thank you very much for review.
> 
> I have in to: Rob Herring and in c: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Aha, sorry, did not notice that.

> Should it be separate mail, or it's OK?

They prefer separate mails, yes.

> > > +	for (i = 0; i < priv->item[grp->type]->count; i++, id++, data++) {
> > > +		priv->mlxreg_core_attr[id] =
> > > +				&priv-
> > >mlxreg_core_dev_attr[id].dev_attr.attr;
> > > +		if (grp->use_grp_name)
> > > +			priv->mlxreg_core_attr[id]->name =
> > > +				devm_kasprintf(priv->dev, GFP_KERNEL,
> > > +					       "%s%u", grp->name, id %
> > > +						priv->item[grp->type]-
> > >count
> > > +						+ 1);
> > > +		else
> > > +			priv->mlxreg_core_attr[id]->name =
> > > +				devm_kasprintf(priv->dev, GFP_KERNEL,
> > > +					       data->label);
> > 
> > Is it really neccessary to dynamically generate it? Static table could be
> > simpler...
> 
> But I don't know how many groups and attributes can be defined for particular device.
> On the system I am working on know I have four programmable device with different purposes and different set of attributes.
>

I guess that's good reason. But if you could make that code simpler /
easier to follow, it would be nice.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]