diff mbox series

[v2,1/3] pinctrl: amd: Define and use PINCTRL_GRP

Message ID 20220524074007.2792445-2-Basavaraj.Natikar@amd.com
State Superseded
Headers show
Series [v2,1/3] pinctrl: amd: Define and use PINCTRL_GRP | expand

Commit Message

Basavaraj Natikar May 24, 2022, 7:40 a.m. UTC
AMD pingroup can be extended to support multi-function pins.
Hence define and use PINCTRL_GRP to manage and represent
larger number of pingroups inline.

Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/pinctrl/pinctrl-amd.h | 39 ++++++++---------------------------
 1 file changed, 9 insertions(+), 30 deletions(-)

Comments

Linus Walleij May 25, 2022, 6:05 a.m. UTC | #1
On Tue, May 24, 2022 at 4:39 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, May 24, 2022 at 10:13 AM Basavaraj Natikar
> <Basavaraj.Natikar@amd.com> wrote:
> >
> > AMD pingroup can be extended to support multi-function pins.
> > Hence define and use PINCTRL_GRP to manage and represent
> > larger number of pingroups inline.
>
> This is good idea, but please make it available for all pin control
> drivers, since the data structure like
>
>   pingroup {
>     *name;
>     *pins;
>     npins;
>   }
>
> is used in many drivers.
>
> That said, I think the AMD_ prefix will be misleading in this case.

Aha you mean like a utility macro? That's useful, don't know where to put it
though, include/linux/pinctrl/pinmux.h?

Yours,
Linus Walleij
Basavaraj Natikar May 26, 2022, 6:27 a.m. UTC | #2
On 5/25/2022 11:35 AM, Linus Walleij wrote:

> On Tue, May 24, 2022 at 4:39 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Tue, May 24, 2022 at 10:13 AM Basavaraj Natikar
>> <Basavaraj.Natikar@amd.com> wrote:
>>> AMD pingroup can be extended to support multi-function pins.
>>> Hence define and use PINCTRL_GRP to manage and represent
>>> larger number of pingroups inline.
>> This is good idea, but please make it available for all pin control
>> drivers, since the data structure like
>>
>>   pingroup {
>>     *name;
>>     *pins;
>>     npins;
>>   }
>>
>> is used in many drivers.
>>
>> That said, I think the AMD_ prefix will be misleading in this case.
> Aha you mean like a utility macro? That's useful, don't know where to put it
> though, include/linux/pinctrl/pinmux.h?

For me looks like we need to put this in include/linux/pinctrl/pinctrl.h for 
a generic usage. is this fine?

> Yours,
> Linus Walleij
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-amd.h b/drivers/pinctrl/pinctrl-amd.h
index 1d4317073654..de2bc9dddc9c 100644
--- a/drivers/pinctrl/pinctrl-amd.h
+++ b/drivers/pinctrl/pinctrl-amd.h
@@ -296,37 +296,16 @@  static const unsigned i2c3_pins[] = {19, 20};
 static const unsigned uart0_pins[] = {135, 136, 137, 138, 139};
 static const unsigned uart1_pins[] = {140, 141, 142, 143, 144};
 
+#define PINCTRL_GRP(_name, _pins, _npins) \
+	{ .name = _name, .pins = _pins, .npins = _npins}
+
 static const struct amd_pingroup kerncz_groups[] = {
-	{
-		.name = "i2c0",
-		.pins = i2c0_pins,
-		.npins = 2,
-	},
-	{
-		.name = "i2c1",
-		.pins = i2c1_pins,
-		.npins = 2,
-	},
-	{
-		.name = "i2c2",
-		.pins = i2c2_pins,
-		.npins = 2,
-	},
-	{
-		.name = "i2c3",
-		.pins = i2c3_pins,
-		.npins = 2,
-	},
-	{
-		.name = "uart0",
-		.pins = uart0_pins,
-		.npins = 5,
-	},
-	{
-		.name = "uart1",
-		.pins = uart1_pins,
-		.npins = 5,
-	},
+	PINCTRL_GRP("i2c0", i2c0_pins, 2),
+	PINCTRL_GRP("i2c1", i2c1_pins, 2),
+	PINCTRL_GRP("i2c2", i2c2_pins, 2),
+	PINCTRL_GRP("i2c3", i2c3_pins, 2),
+	PINCTRL_GRP("uart0", uart0_pins, 5),
+	PINCTRL_GRP("uart1", uart1_pins, 5),
 };
 
 #endif