[RFC,3/5] arm: dts: dt-bindings: Add Renesas RZ pinctrl header
diff mbox

Message ID 1485367787-8109-4-git-send-email-jacopo+renesas@jmondi.org
State New
Headers show

Commit Message

Jacopo Mondi Jan. 25, 2017, 6:09 p.m. UTC
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

Comments

Geert Uytterhoeven Jan. 26, 2017, 7:52 p.m. UTC | #1
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
Laurent Pinchart Jan. 30, 2017, 6:30 p.m. UTC | #2
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.
Jacopo Mondi Jan. 31, 2017, 9:12 a.m. UTC | #3
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
Geert Uytterhoeven Jan. 31, 2017, 9:31 a.m. UTC | #4
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
Chris Brandt Jan. 31, 2017, 2 p.m. UTC | #5
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

Patch
diff mbox

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