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

David Laight
2020-02-24T14:00:06Z

From: Geert Uytterhoeven
> Sent: 24 February 2020 12:54
> To: Krzysztof Kozlowski <krzk@kernel.org>
...
> > > > 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?

Or more likely change the definitions to use a struct for the layout.
That also stops the constants being used in the wrong place.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org