On Tue, Dec 17, 2024 at 12:33:45PM +0000, Biju Das wrote:
> Hi Rob,
>
> > -----Original Message-----
> > From: Rob Herring <robh@xxxxxxxxxx>
> > Sent: 17 December 2024 11:58
> > Subject: Re: [PATCH v4 1/7] dt-bindings: pinctrl: renesas: Add alpha-numerical port support for RZ/V2H
> >
> > On Mon, Dec 16, 2024 at 07:53:11PM +0000, Biju Das wrote:
> > > RZ/V2H has ports P0-P9 and PA-PB. Add support for defining
> > > alpha-numerical ports in DT using RZV2H_* macros.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > ---
> > > v3->v4:
> > > * Added new header file with separate RZV2H_P* definitions.
> > > v3:
> > > * New patch.
> > > ---
> > > .../pinctrl/renesas,r9a09g057-pinctrl.h | 31 +++++++++++++++++++
> > > 1 file changed, 31 insertions(+)
> > > create mode 100644
> > > include/dt-bindings/pinctrl/renesas,r9a09g057-pinctrl.h
> > >
> > > diff --git a/include/dt-bindings/pinctrl/renesas,r9a09g057-pinctrl.h
> > > b/include/dt-bindings/pinctrl/renesas,r9a09g057-pinctrl.h
> > > new file mode 100644
> > > index 000000000000..9008a7e71609
> > > --- /dev/null
> > > +++ b/include/dt-bindings/pinctrl/renesas,r9a09g057-pinctrl.h
> > > @@ -0,0 +1,31 @@
> > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > > +/*
> > > + * This header provides constants for Renesas RZ/V2H family pinctrl bindings.
> > > + *
> > > + * Copyright (C) 2024 Renesas Electronics Corp.
> > > + *
> > > + */
> > > +
> > > +#ifndef __DT_BINDINGS_RZV2H_PINCTRL_H #define
> > > +__DT_BINDINGS_RZV2H_PINCTRL_H
> > > +
> > > +#include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
> > > +
> > > +/* RZV2H_Px = Offset address of PFC_P_mn - 0x20 */
> > > +#define RZV2H_P0 0
> > > +#define RZV2H_P1 1
> > > +#define RZV2H_P2 2
> > > +#define RZV2H_P3 3
> > > +#define RZV2H_P4 4
> > > +#define RZV2H_P5 5
> > > +#define RZV2H_P6 6
> > > +#define RZV2H_P7 7
> > > +#define RZV2H_P8 8
> > > +#define RZV2H_P9 9
> > > +#define RZV2H_PA 10
> > > +#define RZV2H_PB 11
> >
> > I'm not a fan of defines which are just 'FOO_n n'. And these are if you speak hex.
>
> For RZ/V2H, ports are P{0..9} and P{A..B}, the port name are with in hexa decimal range
>
> whereas for RZ/G3E SoC,
>
> Ports are P{0..8},P{A..H},P{J..M},PS
>
> RZ/V2H and RZ/G3E are similar SoCs.
>
> >
> > > +
> > > +#define RZV2H_PORT_PINMUX(b, p, f) RZG2L_PORT_PINMUX(RZV2H_P##b, p, f)
> > > +#define RZV2H_GPIO(port, pin) RZG2L_GPIO(RZV2H_P##port, pin)
> >
> > So the user does RZV2H_GPIO(A, 123) instead of RZV2H_GPIO(0xA, 123)? Not sure the bounds checking the
> > port is worth it. pin or function can still be crap.
>
> Previously we were using plain number in DT for RZ/V2H, RZG2L_GPIO(10, pin) = RZG2L_GPIO(0xA, pin)
> The port names for RZ/G2L are plane number, whereas for RZ/G3E and RZ/V2H it's alpha numeric.
>
> Since RZ/V2H ports are within the hexadecimal range, maybe this header file can be dropped
> at least for RZ/V2H??
>
> Any way we are doing bounds check in driver.
If you are doing it elsewhere, then I guess it is fine for some
consistency.
Rob
