Re: [PATCH 1/3] PCI: rcar: Replace unsigned long with u32 for register values

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Marek Vasut <marek.vasut@gmail.com>
Cc: linux-pci <linux-pci@vger.kernel.org>,
	Marek Vasut <marek.vasut+renesas@gmail.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Phil Edworthy <phil.edworthy@renesas.com>,
	Simon Horman <horms+renesas@verge.net.au>,
	Wolfram Sang <wsa@the-dreams.de>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH 1/3] PCI: rcar: Replace unsigned long with u32 for register values
Date: Thu, 21 Mar 2019 08:31:41 +0100	[thread overview]
Message-ID: <CAMuHMdUYM=Yux+uoAuwGgKzzdvkr-3-JSHHWisZVPHiH=GqEiQ@mail.gmail.com> (raw)
In-Reply-To: <c7fa1a3c-f3cb-2c9c-3321-c8b53da609f2@gmail.com>

Hi Marek,

On Thu, Mar 21, 2019 at 5:14 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> On 3/18/19 9:47 AM, Geert Uytterhoeven wrote:
> > On Sun, Mar 17, 2019 at 1:06 AM <marek.vasut@gmail.com> wrote:
> >> From: Marek Vasut <marek.vasut+renesas@gmail.com>
> >>
> >> Replace unsigned long with u32 type for variables holding
> >> register values, since the registers are 32bit. Note that
> >> rcar_pcie_msi_irq() still uses unsigned long because both
> >> find_first_bit() and __fls() require unsigned long as an
> >> argument.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> >> Cc: Phil Edworthy <phil.edworthy@renesas.com>
> >> Cc: Simon Horman <horms+renesas@verge.net.au>
> >> Cc: Wolfram Sang <wsa@the-dreams.de>
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> To: linux-pci@vger.kernel.org
> >> ---
> >>  drivers/pci/controller/pcie-rcar.c | 10 +++++-----
> >>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> >> index 1408c8aa758b..857d88fd03d5 100644
> >> --- a/drivers/pci/controller/pcie-rcar.c
> >> +++ b/drivers/pci/controller/pcie-rcar.c
> >> @@ -169,7 +169,7 @@ enum {
> >>
> >>  static void rcar_rmw32(struct rcar_pcie *pcie, int where, u32 mask, u32 data)
> >>  {
> >> -       int shift = 8 * (where & 3);
> >> +       u32 shift = 8 * (where & 3);
> >
> > shift is not a register value, so IMHO the original type is fine (the "int"
> > comes from the pci_ops API, BTW).
>
> I presume it should be at least unsigned ?

Yes, and "where" too.

Note that the other uses of "where" are also int, and IMHO should be
unsigned, too. But changing that means changing the PCI API and all
drivers, sigh...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds