On Wed, Jul 29, 2020 at 9:06 PM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > Hi, > > On Sat, Jul 25, 2020 at 02:18:39PM -0500, Samuel Holland wrote: > > On 7/17/20 11:07 AM, Maxime Ripard wrote: > > > Hi! > > > > > > On Wed, Jul 15, 2020 at 07:54:12PM +0800, Frank Lee wrote: > > >> From: Yangtao Li <frank@xxxxxxxxxxxxxxxxx> > > >> > > >> The sunxi gpio binding defines a few custom cells for its gpio specifier. > > >> Provide bank name for those. > > >> > > >> Signed-off-by: Yangtao Li <frank@xxxxxxxxxxxxxxxxx> > > > > > > Thanks for working on this, I wanted to do it at some point but it kept > > > getting pushed further into my todo list. > > > > > >> --- > > >> include/dt-bindings/gpio/sunxi-gpio.h | 29 +++++++++++++++++++++++++++ > > >> 1 file changed, 29 insertions(+) > > >> create mode 100644 include/dt-bindings/gpio/sunxi-gpio.h > > >> > > >> diff --git a/include/dt-bindings/gpio/sunxi-gpio.h b/include/dt-bindings/gpio/sunxi-gpio.h > > >> new file mode 100644 > > >> index 000000000000..c692b4360da6 > > >> --- /dev/null > > >> +++ b/include/dt-bindings/gpio/sunxi-gpio.h > > > > > > So generally we've been using the compatible name as the file name. You > > > should follow that convention too, and since it was added with the A10, > > > using the A10 compatible. > > > > > >> @@ -0,0 +1,29 @@ > > >> +/* SPDX-License-Identifier: GPL-2.0 */ > > >> +/* > > >> + * GPIO definitions for Allwinner SoCs > > >> + * > > >> + * Copyright (C) 2020 Yangtao Li <frank@xxxxxxxxxxxxxxxxx> > > >> + */ > > >> + > > >> +#ifndef _DT_BINDINGS_SUNXI_GPIO_H > > >> +#define _DT_BINDINGS_SUNXI_GPIO_H > > >> + > > >> +#include <dt-bindings/gpio/gpio.h> > > >> + > > >> +/* pio */ > > >> +#define PA 0 > > >> +#define PB 1 > > >> +#define PC 2 > > >> +#define PD 3 > > >> +#define PE 4 > > >> +#define PF 5 > > >> +#define PG 6 > > >> +#define PH 7 > > >> +#define PI 8 > > >> + > > >> +/* r-pio */ > > >> +#define PL 0 > > >> +#define PM 1 > > >> +#define PN 2 > > >> + > > >> +#endif /* _DT_BINDINGS_SUNXI_GPIO_H */ > > > > > > Maybe we can go one step further and use a macro to have something like > > > PIN(A, 12) ? > > > > Since we have separate cells for the bank and pin, I don't think it would be > > appropriate to have a single macro generating both. > > Yeah, but it's "just" an encoding issue though, it's not a major concern > if it makes our life easier. > > > And I'm not sure what the benefit of the macro would be, if all it > > does is forward its arguments. Are you concerned that P[A-M] could > > conflict with something else in the device tree? > > There's indeed a bunch of names that are fairly generic and could be > conflicting with others (PD for power domain is the first one that comes > to my mind). Using a prefix would make the GPIO descriptors pretty long, > so it wasn't ideal either. A macro makes it readable without increasing > too much the risks of conflicts I tried to use macros, but failed. I have a look at some other GPIO macros, which have a prefix in front of them. Maybe we can do the same? It's all numbers. It's not intuitive. Yangtao
