2022-07-11 20:56:18

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] pinctrl: ocelot: Fix pincfg

On Mon, Jul 11, 2022 at 10:23 PM Horatiu Vultur
<[email protected]> wrote:
>
> The 07/11/2022 21:51, Andy Shevchenko wrote:
> >
> > On Mon, Jul 11, 2022 at 9:17 PM Horatiu Vultur
> > <[email protected]> wrote:
> > >
> > > The blamed commit changed to use regmaps instead of __iomem. But it
> > > didn't update the register offsets to be at word offset, so it uses byte
> > > offset.
> > > Another issue with the same commit is that it has a limit of 32 registers
> > > which is incorrect. The sparx5 has 64 while lan966x has 77.
> >
> > ...
> >
> > > -static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
> > > +static struct regmap *ocelot_pinctrl_create_pincfg(struct ocelot_pinctrl *info,
> > > + struct platform_device *pdev)
> >
> > const?
> >
> > And I would leave pdev to be the first parameter, if there are no
> > other functions that have them like this.
>
> I will do that in the next version.
> Just for my understanding/knowledge why is this desire to have const or
> to keep the const?

For non-POD types it's a good coding practice to reduce surface of
attack, if any (the data will be located in the pages with RO flag
set, and attempt to write will give you a page fault or other
exception, it depends on architecture).
Also a common sense, if you don't change data (which is actually
initial configuration or so), then why shouldn't you use const?
Note, in cases when it's not initial data, but runtime stuff (like
really run time), const is obviously either can't or not needed to be
used.

--
With Best Regards,
Andy Shevchenko