| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdXosvV=EuRtL69r6=UT0SO8Aq-XjWwJMJQpWAhT2z+ffA@mail.gmail.com>
Date: Tue, 2 Jan 2024 11:17:50 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc: Magnus Damm <magnus.damm@...il.com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Linus Walleij <linus.walleij@...aro.org>,
linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org,
linux-gpio@...r.kernel.org, Biju Das <biju.das.jz@...renesas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v3 1/3] pinctrl: renesas: rzg2l: Include pinmap in
RZG2L_GPIO_PORT_PACK() macro
Hi Prabhakar,
On Thu, Dec 21, 2023 at 10:04 PM Lad, Prabhakar
<prabhakar.csengg@...il.com> wrote:
> On Wed, Dec 6, 2023 at 1:13 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > On Fri, Dec 1, 2023 at 2:16 PM Prabhakar <prabhakar.csengg@...il.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > >
> > > Currently we assume all the port pins are sequential ie always PX_0 to
> > > PX_n (n=1..7) exist, but on RZ/Five SoC we have additional pins P19_1 to
> > > P28_5 which have holes in them, for example only one pin on port19 is
> > > available and that is P19_1 and not P19_0. So to handle such cases
> > > include pinmap for each port which would indicate the pin availability
> > > on each port. As the pincount can be calculated based on pinmap drop this
> > > from RZG2L_GPIO_PORT_PACK() macro and update RZG2L_GPIO_PORT_GET_PINCNT()
> > > macro.
> > >
> > > Previously we had a max of 7 pins on each port but on RZ/Five Port-20
> > > has 8 pins, so move the single pin configuration to BIT(63).
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > @@ -80,15 +80,17 @@
> > > * n indicates number of pins in the port, a is the register index
> > > * and f is pin configuration capabilities supported.
> > > */
> > > -#define RZG2L_GPIO_PORT_PACK(n, a, f) (((n) << 28) | ((a) << 20) | (f))
> > > -#define RZG2L_GPIO_PORT_GET_PINCNT(x) (((x) & GENMASK(30, 28)) >> 28)
> > > +#define RZG2L_GPIO_PORT_PACK(n, a, f) (((n) > 0 ? ((u64)(GENMASK_ULL(((n) - 1 + 28), 28))) : 0) | \
> >
> > The mask creation can be simplified to
> >
> > ((1ULL << (n)) - 1) << 28
> >
> OK.
>
> > but see below...
> >
> > > + ((a) << 20) | (f))
> > > +#define RZG2L_GPIO_PORT_GET_PINMAP(x) (((x) & GENMASK_ULL(35, 28)) >> 28)
> > > +#define RZG2L_GPIO_PORT_GET_PINCNT(x) (hweight8(RZG2L_GPIO_PORT_GET_PINMAP((x))))
> >
> > I think we've reached the point where it would be easier for the
> > casual reviewer to #define PIN_CFG_*_MASK for all fields, and use
> > FIELD_{PREP,GET}() to pack resp. extract values. That would also
> > make it more obvious which bits are in use, and how many bits are
> > still available for future use.
> >
> If I use the FIELD_PREP() macro like below I get build issues as below:
>
> #define RZG2L_GPIO_PORT_PIN_CNT_MASK GENMASK(31, 28)
> #define RZG2L_GPIO_PORT_PIN_REG_MASK GENMASK(27, 20)
> #define RZG2L_GPIO_PORT_PIN_CFG_MASK GENMASK(19, 0)
> #define RZG2L_GPIO_PORT_PACK(n, a, f)
> FIELD_PREP(RZG2L_GPIO_PORT_PIN_CNT_MASK, n) | \
> FIELD_PREP(RZG2L_GPIO_PORT_PIN_REG_MASK, a) | \
> FIELD_PREP(RZG2L_GPIO_PORT_PIN_CFG_MASK, f)
>
>
> drivers/pinctrl/renesas/pinctrl-rzg2l.c:91:41: note: in expansion of
> macro 'FIELD_PREP'
> 91 |
> FIELD_PREP(RZG2L_GPIO_PORT_PIN_CFG_MASK, f)
> | ^~~~~~~~~~
> drivers/pinctrl/renesas/pinctrl-rzg2l.c:1486:9: note: in expansion of
> macro 'RZG2L_GPIO_PORT_PACK'
> 1486 | RZG2L_GPIO_PORT_PACK(6, 0x2a,
> RZG3S_MPXED_PIN_FUNCS(A)), /* P18 */
> | ^~~~~~~~~~~~~~~~~~~~
>
> Do you have any pointers?
You left out the actual error :-(
include/linux/bitfield.h:113:9: error: braced-group within expression
allowed only inside a function
113 | ({
\
| ^
drivers/pinctrl/renesas/pinctrl-rzg2l.c:93:39: note: in expansion of
macro ‘FIELD_PREP’
93 | #define RZG2L_GPIO_PORT_PACK(n, a, f)
FIELD_PREP(RZG2L_GPIO_PORT_PIN_CNT_MASK, n) | \
| ^~~~~~~~~~
drivers/pinctrl/renesas/pinctrl-rzg2l.c:1555:9: note: in expansion of
macro ‘RZG2L_GPIO_PORT_PACK’
1555 | RZG2L_GPIO_PORT_PACK(2, 0x10, RZG2L_MPXED_PIN_FUNCS),
| ^~~~~~~~~~~~~~~~~~~~
Using FIELD_PREP_CONST() instead makes it build.
/**
* FIELD_PREP_CONST() - prepare a constant bitfield element
* @_mask: shifted mask defining the field's length and position
* @_val: value to put in the field
*
* FIELD_PREP_CONST() masks and shifts up the value. The result should
* be combined with other fields of the bitfield using logical OR.
*
* Unlike FIELD_PREP() this is a constant expression and can therefore
* be used in initializers. Error checking is less comfortable for this
* version, and non-constant masks cannot be used.
*/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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
Powered by blists - more mailing lists