Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220711192113.3522664-1-horatiu.vultur@microchip.com>
 <20220711192113.3522664-3-horatiu.vultur@microchip.com> <CAHp75VdeZSP62qoOdQf=g4b7AheFd4=jNxfjMh-_T7Q1Zi=LbA@mail.gmail.com>
 <20220711202646.om65vrksyifvkfkw@soft-dev3-1.localhost> <CAHp75VeJgUFdFgBpG5avmKCn-CGNOJ6wZAhc0a4f2MHfLbvXmA@mail.gmail.com>
In-Reply-To: <CAHp75VeJgUFdFgBpG5avmKCn-CGNOJ6wZAhc0a4f2MHfLbvXmA@mail.gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Mon, 11 Jul 2022 22:33:11 +0200
Message-ID: <CAHp75VdTUqP+Ho2cmeMiOwNtu_HhOMoo3cCUQ27vdPftcJ-xUA@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] pinctrl: ocelot: Fix pincfg
To:     Horatiu Vultur <horatiu.vultur@microchip.com>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        kavyasree.kotagiri@microchip.com,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Colin Foster <colin.foster@in-advantage.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Michael Walle <michael@walle.cc>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jul 11, 2022 at 10:29 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jul 11, 2022 at 10:23 PM Horatiu Vultur
> <horatiu.vultur@microchip.com> wrote:
> >
> > The 07/11/2022 21:51, Andy Shevchenko wrote:
> > >
> > > On Mon, Jul 11, 2022 at 9:17 PM Horatiu Vultur
> > > <horatiu.vultur@microchip.com> 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.

One more specifically for drivers (related to the first item above),
it allows one to scope the point of failure in case of wrong
configuration comes in. The device might misbehave badly because of
some garbage somewhere. Also, the driver won't write data to that
area, which is just a good preventive programming practice (but this I
already implied by the second item above).


-- 
With Best Regards,
Andy Shevchenko