| Message ID | 1485367787-8109-4-git-send-email-jacopo+renesas@jmondi.org |
|---|---|
| State | New |
| Headers | show |
Hi Jacopo, On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > Add dt-bindings header for Renesas RZ pincontroller. > The header defines macros for pin description and alternate function > numbers. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > --- > include/dt-bindings/pinctrl/pinctrl-renesas-rz.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > create mode 100644 include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > > diff --git a/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > new file mode 100644 > index 0000000..92816d4 > --- /dev/null > +++ b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > @@ -0,0 +1,19 @@ > +/* > + * Defines macros and constants for Renesas RZ pin controller and muxer > + */ > + > +#ifndef __DT_BINDINGS_PINCTRL_RENESAS_RZ_H > +#define __DT_BINDINGS_PINCTRL_RENESAS_RZ_H > + > +#define RZ_PIN(b, p) b p And the advantage of this macro is? > +#define ALTERNATE_FUNC_1 0 > +#define ALTERNATE_FUNC_2 1 > +#define ALTERNATE_FUNC_3 2 > +#define ALTERNATE_FUNC_4 3 > +#define ALTERNATE_FUNC_5 4 > +#define ALTERNATE_FUNC_6 5 > +#define ALTERNATE_FUNC_7 6 > +#define ALTERNATE_FUNC_8 7 I have mixed feelings about these macros: 1. They're long to type, 2. They just map from n to n-1. Why not use plain numbers 1..8 (the alternate function numbering in the datasheet is 1-based), and subtract 1 in the C code? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 26 Jan 2017 20:52:33 Geert Uytterhoeven wrote: > On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi wrote: > > Add dt-bindings header for Renesas RZ pincontroller. > > The header defines macros for pin description and alternate function > > numbers. > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > --- > > > > include/dt-bindings/pinctrl/pinctrl-renesas-rz.h | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > create mode 100644 include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > > > > diff --git a/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > > b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h new file mode 100644 > > index 0000000..92816d4 > > --- /dev/null > > +++ b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h > > @@ -0,0 +1,19 @@ > > +/* > > + * Defines macros and constants for Renesas RZ pin controller and muxer > > + */ > > + > > +#ifndef __DT_BINDINGS_PINCTRL_RENESAS_RZ_H > > +#define __DT_BINDINGS_PINCTRL_RENESAS_RZ_H > > + > > +#define RZ_PIN(b, p) b p > > And the advantage of this macro is? > > > +#define ALTERNATE_FUNC_1 0 > > +#define ALTERNATE_FUNC_2 1 > > +#define ALTERNATE_FUNC_3 2 > > +#define ALTERNATE_FUNC_4 3 > > +#define ALTERNATE_FUNC_5 4 > > +#define ALTERNATE_FUNC_6 5 > > +#define ALTERNATE_FUNC_7 6 > > +#define ALTERNATE_FUNC_8 7 > > I have mixed feelings about these macros: > 1. They're long to type, > 2. They just map from n to n-1. > > Why not use plain numbers 1..8 (the alternate function numbering in the > datasheet is 1-based), and subtract 1 in the C code? I was about to mention the same. I think you can drop this patch and use the numbers directly.
Hi Geert, Laurent, On 30/01/2017 19:30, Laurent Pinchart wrote: > On Thursday 26 Jan 2017 20:52:33 Geert Uytterhoeven wrote: >> On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi wrote: >>> Add dt-bindings header for Renesas RZ pincontroller. >>> The header defines macros for pin description and alternate function >>> numbers. >>> >>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> >>> --- >>> >>> include/dt-bindings/pinctrl/pinctrl-renesas-rz.h | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> create mode 100644 include/dt-bindings/pinctrl/pinctrl-renesas-rz.h >>> >>> diff --git a/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h >>> b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h new file mode 100644 >>> index 0000000..92816d4 >>> --- /dev/null >>> +++ b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h >>> @@ -0,0 +1,19 @@ >>> +/* >>> + * Defines macros and constants for Renesas RZ pin controller and muxer >>> + */ >>> + >>> +#ifndef __DT_BINDINGS_PINCTRL_RENESAS_RZ_H >>> +#define __DT_BINDINGS_PINCTRL_RENESAS_RZ_H >>> + >>> +#define RZ_PIN(b, p) b p >> >> And the advantage of this macro is? >> Nothing beside being cute >>> +#define ALTERNATE_FUNC_1 0 >>> +#define ALTERNATE_FUNC_2 1 >>> +#define ALTERNATE_FUNC_3 2 >>> +#define ALTERNATE_FUNC_4 3 >>> +#define ALTERNATE_FUNC_5 4 >>> +#define ALTERNATE_FUNC_6 5 >>> +#define ALTERNATE_FUNC_7 6 >>> +#define ALTERNATE_FUNC_8 7 >> >> I have mixed feelings about these macros: >> 1. They're long to type, >> 2. They just map from n to n-1. >> >> Why not use plain numbers 1..8 (the alternate function numbering in the >> datasheet is 1-based), and subtract 1 in the C code? > > I was about to mention the same. I think you can drop this patch and use the > numbers directly. > Please be aware there are values we'll have to define here, such as the additional configurations we're talking about in some other part of this email thread. This may become something like: #define ALT_FUNC_1 0 #define ALT_FUNC_2 1 #define ALT_FUNC_3 2 #define ALT_FUNC_4 3 #define ALT_FUNC_5 4 #define ALT_FUNC_6 5 #define ALT_FUNC_7 6 #define ALT_FUNC_8 7 #define INPUT_MODE 1 #define BIDRECTIONAL 2 #define ... #define PIN(b, p) b p #define PIN_MUX(func, conf) \ ((func & 0xf) | ((conf) << 16)) and in DTS sources you would describe a pin as pins = <PIN(bank, pin) PIN_MUX(ALT_FUNC_1, INPUT | BIDIRECTIONAL)>; We can drop the PIN macro as it does not bring anything I agree, but it's nicer to see imho Thanks j -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jacopo, On Tue, Jan 31, 2017 at 10:12 AM, jacopo mondi <jacopo@jmondi.org> wrote: > On 30/01/2017 19:30, Laurent Pinchart wrote: >> On Thursday 26 Jan 2017 20:52:33 Geert Uytterhoeven wrote: >>> On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi wrote: >>>> Add dt-bindings header for Renesas RZ pincontroller. >>>> The header defines macros for pin description and alternate function >>>> numbers. >>>> --- /dev/null >>>> +++ b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h >>>> +#define RZ_PIN(b, p) b p >>> >>> And the advantage of this macro is? >>> > > Nothing beside being cute :-) >>>> +#define ALTERNATE_FUNC_1 0 >>>> +#define ALTERNATE_FUNC_2 1 >>>> +#define ALTERNATE_FUNC_3 2 >>>> +#define ALTERNATE_FUNC_4 3 >>>> +#define ALTERNATE_FUNC_5 4 >>>> +#define ALTERNATE_FUNC_6 5 >>>> +#define ALTERNATE_FUNC_7 6 >>>> +#define ALTERNATE_FUNC_8 7 >>> >>> I have mixed feelings about these macros: >>> 1. They're long to type, >>> 2. They just map from n to n-1. >>> >>> Why not use plain numbers 1..8 (the alternate function numbering in the >>> datasheet is 1-based), and subtract 1 in the C code? >> >> I was about to mention the same. I think you can drop this patch and use >> the >> numbers directly. > > Please be aware there are values we'll have to define here, such as the > additional configurations we're talking about in some other part of this > email thread. In that case you indeed have to OR values, but... > This may become something like: > > #define ALT_FUNC_1 0 > #define ALT_FUNC_2 1 > #define ALT_FUNC_3 2 > #define ALT_FUNC_4 3 > #define ALT_FUNC_5 4 > #define ALT_FUNC_6 5 > #define ALT_FUNC_7 6 > #define ALT_FUNC_8 7 > > #define INPUT_MODE 1 > #define BIDRECTIONAL 2 > #define ... ... if you integrate the shifts in the definitions above, e.g. #define INPUT_MODE (1 << 16) > #define PIN(b, p) b p > #define PIN_MUX(func, conf) \ > ((func & 0xf) | ((conf) << 16)) > > and in DTS sources you would describe a pin as > > pins = <PIN(bank, pin) PIN_MUX(ALT_FUNC_1, INPUT | BIDIRECTIONAL)>; > > We can drop the PIN macro as it does not bring anything I agree, but it's > nicer to see imho ... you can keep plain 1-based numbers for alternate functions, and use the definitions for flags, e.g.: pins = <PIN(bank, pin) (1 | INPUT_MODE | BIDIRECTIONAL)>; 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, January 31, 2017, Geert Uytterhoeven wrote: > > This may become something like: > > > > #define ALT_FUNC_1 0 > > #define ALT_FUNC_2 1 > > #define ALT_FUNC_3 2 > > #define ALT_FUNC_4 3 > > #define ALT_FUNC_5 4 > > #define ALT_FUNC_6 5 > > #define ALT_FUNC_7 6 > > #define ALT_FUNC_8 7 ALT_FUNC_1 is still too long if you're going to OR things on 1 line in the DT. I personally like: #define ALT_1 0 > > #define PIN(b, p) b p > > #define PIN_MUX(func, conf) \ > > ((func & 0xf) | ((conf) << 16)) > > > > and in DTS sources you would describe a pin as > > > > pins = <PIN(bank, pin) PIN_MUX(ALT_FUNC_1, INPUT | BIDIRECTIONAL)>; > > > > We can drop the PIN macro as it does not bring anything I agree, but > > it's nicer to see imho > > ... you can keep plain 1-based numbers for alternate functions, and use > the definitions for flags, e.g.: > > pins = <PIN(bank, pin) (1 | INPUT_MODE | BIDIRECTIONAL)>; To Geert's point, nothing is going to be "shorter" than just using '1','2','3',etc.. I also think "BIDIRECTIONAL" >> "BIDIR" Or....we could make life easy on people and #define the pins that are tricky: #define I2C BIDIRECTIONAL #define SDHI_DATA BIDIRECTIONAL #define SDHI_CTRL /* none */ #define RSPI /* none */ #define ETHERNET /* none */ Chris
diff --git a/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h new file mode 100644 index 0000000..92816d4 --- /dev/null +++ b/include/dt-bindings/pinctrl/pinctrl-renesas-rz.h @@ -0,0 +1,19 @@ +/* + * Defines macros and constants for Renesas RZ pin controller and muxer + */ + +#ifndef __DT_BINDINGS_PINCTRL_RENESAS_RZ_H +#define __DT_BINDINGS_PINCTRL_RENESAS_RZ_H + +#define RZ_PIN(b, p) b p + +#define ALTERNATE_FUNC_1 0 +#define ALTERNATE_FUNC_2 1 +#define ALTERNATE_FUNC_3 2 +#define ALTERNATE_FUNC_4 3 +#define ALTERNATE_FUNC_5 4 +#define ALTERNATE_FUNC_6 5 +#define ALTERNATE_FUNC_7 6 +#define ALTERNATE_FUNC_8 7 + +#endif
Add dt-bindings header for Renesas RZ pincontroller. The header defines macros for pin description and alternate function numbers. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- include/dt-bindings/pinctrl/pinctrl-renesas-rz.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 include/dt-bindings/pinctrl/pinctrl-renesas-rz.h