[RESEND PATCH v2 9/9] ath5k: Constify ioreadX() iomem argument (as in generic implementation)

[Krzysztof Kozlowski]

On Mon, Feb 24, 2020 at 01:54:00PM +0100, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Mon, Feb 24, 2020 at 1:47 PM Krzysztof Kozlowski <krzk at kernel.org> wrote:
> > On Thu, Feb 20, 2020 at 10:48:33AM +0100, Jiri Slaby wrote:
> > > On 19. 02. 20, 18:50, Krzysztof Kozlowski wrote:
> > > > The ioreadX() helpers have inconsistent interface.  On some architectures
> > > > void *__iomem address argument is a pointer to const, on some not.
> > > >
> > > > Implementations of ioreadX() do not modify the memory under the address
> > > > so they can be converted to a "const" version for const-safety and
> > > > consistency among architectures.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <krzk at kernel.org>
> > > > Acked-by: Kalle Valo <kvalo at codeaurora.org>
> > > > ---
> > > >  drivers/net/wireless/ath/ath5k/ahb.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/ath/ath5k/ahb.c b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > index 2c9cec8b53d9..8bd01df369fb 100644
> > > > --- a/drivers/net/wireless/ath/ath5k/ahb.c
> > > > +++ b/drivers/net/wireless/ath/ath5k/ahb.c
> > > > @@ -138,18 +138,18 @@ static int ath_ahb_probe(struct platform_device *pdev)
> > > >
> > > >     if (bcfg->devid >= AR5K_SREV_AR2315_R6) {
> > > >             /* Enable WMAC AHB arbitration */
> > > > -           reg = ioread32((void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > > > +           reg = ioread32((const void __iomem *) AR5K_AR2315_AHB_ARB_CTL);
> > >
> > > While I understand why the parameter of ioread32 should be const, I
> > > don't see a reason for these casts on the users' side. What does it
> > > bring except longer code to read?
> >
> > Because the argument is an int:
> >
> > drivers/net/wireless/ath/ath5k/ahb.c: In function ‘ath_ahb_probe’:
> > drivers/net/wireless/ath/ath5k/ahb.c:141:18: warning: passing argument 1 of ‘ioread32’ makes pointer from integer without a cast [-Wint-conversion]
> >    reg = ioread32(AR5K_AR2315_AHB_ARB_CTL);
> 
> That's an argument for keeping the cast to "void __iomem *", not for
> adding the "const", right?

Yes, correct. Maybe I misunderstood the question... The const on the
other hand does not have to be in the cast. It is merely for making it
consistent with interface. It is not required. I also mentioned it in
the cover letter:
"PAtches 5-9 are optional cleanup, without actual impact"

Feel free to ignore this one if it is not worth the effort.

Best regards,
Krzysztof