From: Linus Walleij <linus.walleij@linaro.org>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>,
Nicolas Ferre <nicolas.ferre@microchip.com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] pinctrl: at91-pio4: implement .get_multiple and .set_multiple
Date: Thu, 12 Sep 2019 14:46:32 +0100 [thread overview]
Message-ID: <CACRpkdbziti1yBY_mZaGPg+jHVPR8vtRm8oNZBqt6e+m2wh67A@mail.gmail.com> (raw)
In-Reply-To: <20190911091101.GC21254@piout.net>
On Wed, Sep 11, 2019 at 10:11 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 11/09/2019 01:27:10+0100, Linus Walleij wrote:
> > > +#if ATMEL_PIO_NPINS_PER_BANK != BITS_PER_LONG
> >
> > Should it not be > rather than != ?
>
> Realistically, the only case that could happen would be
> ATMEL_PIO_NPINS_PER_BANK == 32 and BITS_PER_LONG ==64. so I would go for
> ATMEL_PIO_NPINS_PER_BANK < BITS_PER_LONG
OK I see.
> > > + word = BIT_WORD(bank * ATMEL_PIO_NPINS_PER_BANK);
> > > + offset = bank * ATMEL_PIO_NPINS_PER_BANK % BITS_PER_LONG;
> > > +#endif
> >
> > This doesn't look good for multiplatform kernels.
>
> I don't think we have multiplatform kernels that run both in 32 and 64
> bits. I don't believe ATMEL_PIO_NPINS_PER_BANK will ever change, it has
> been 32 on all the atmel SoCs since 2001.
So there is a bit missing from the commit message: the info that
the same driver is being used on 32 and 64 bit builds, and that is
the reason we allow compile-time ifdef things.
Can you add this to the commit message, or maybe
inline in the code, or both?
It confused me so it will confuse others.
Yours,
Linus Walleij
next prev parent reply other threads:[~2019-09-12 13:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 14:13 [PATCH] pinctrl: at91-pio4: implement .get_multiple and .set_multiple Alexandre Belloni
2019-09-05 14:33 ` Claudiu.Beznea
2019-09-11 0:27 ` Linus Walleij
2019-09-11 9:11 ` Alexandre Belloni
2019-09-12 13:46 ` Linus Walleij [this message]
2019-09-14 19:36 ` Ludovic Desroches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACRpkdbziti1yBY_mZaGPg+jHVPR8vtRm8oNZBqt6e+m2wh67A@mail.gmail.com \
--to=linus.walleij@linaro.org \
--cc=alexandre.belloni@bootlin.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ludovic.desroches@microchip.com \
--cc=nicolas.ferre@microchip.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).