On Mon, Feb 27, 2017 at 02:41:10PM +0300, Siarhei Volkau wrote:
> Hi,
>
> 2017-02-27 12:28 GMT+03:00 Maxime Ripard <maxime.rip...@free-electrons.com>:
> >
> > Hi,
> >
> > On Fri, Feb 24, 2017 at 08:41:09AM +0300, lis8...@gmail.com wrote:
> > > +static const struct reg_field
> > > +sun4i_pwm_regfields[SUN4I_MAX_PWM_CHANNELS][NUM_FIELDS] = {
> > > + {
> > > + [FIELD_PRESCALER] = REG_FIELD(PWM_CTRL_REG, 0, 3),
> > > + [FIELD_POLARITY] = REG_FIELD(PWM_CTRL_REG, 5, 5),
> > > + [FIELD_CLK_GATING] = REG_FIELD(PWM_CTRL_REG, 6, 6),
> > > + [FIELD_READY] = REG_FIELD(PWM_CTRL_REG, 28, 28),
> > > + },
> > > + {
> > > + [FIELD_PRESCALER] = REG_FIELD(PWM_CTRL_REG, 15, 18),
> > > + [FIELD_POLARITY] = REG_FIELD(PWM_CTRL_REG, 20, 20),
> > > + [FIELD_CLK_GATING] = REG_FIELD(PWM_CTRL_REG, 21, 21),
> > > + [FIELD_READY] = REG_FIELD(PWM_CTRL_REG, 29, 29),
> > > + },
> > > +};
> > > +
> >
> > I'm not sure you need something that complicated.
> >
> > What about something like:
> >
> > struct sun4i_chan_fields {
> > struct reg_field prescaler;
> > struct reg_field polarity;
> > struct reg_field clk_gating;
> > struct reg_field ready;
> > };
> >
> > And in sun4i_pwm_data add an array of this struct.
> >
> > Maxime
> >
> > --
> > Maxime Ripard, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com
>
> Code with struct looks cleaner, but in this case need to be
> written separate code for each reg_field entry allocation,
> please look at the sunxi_alloc_reg_fields() function.
>
> Current solution allows adding new fields slightly easier.Yet, you also end up with a similar pattern in your later patches, with the pwmch_data structure. But since using that structure for that was not as easy as it was supposed to be, you just dropped reg_field entirely for those. Really, consistency is key, and having a structure like this, even if slightly less easy to initialise (but that's not rocket science either) provides that consistency that makes it easier to review, understand and maintain. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: PGP signature
