Re: [PATCH V3 1/2] dt-bindings: pinctrl: support specifying pins, groups & functions

["Rafał Miłecki" <zajec5@gmail.com>]

On 14.12.2021 21:10, Rafał Miłecki wrote:
> On 14.12.2021 20:59, Rob Herring wrote:
>> On Sat, Dec 11, 2021 at 12:16:25PM +0100, Rafał Miłecki wrote:
>>> Rob: please kindly comment on this idea of storing pins/groups/functions
>>> in DT.
>>
>> I was never a fan of stuffing pin mux/ctrl into DT for what's mostly a
>> one time stuffing of register values. And given how many things run
>> before getting to the kernel, doing proper pin configuration in the
>> kernel is much too late (or redundant because it was actually already
>> done).
> 
> OK, thanks for sharing that. Given a pretty limited optimism on this
> approach I'll simply drop it and do things the old good way.

I feel I need to post one more comment though.

***

What I find a really clean DT code for defining some BCM4908 groups:

groups {
	led_0_grp {
		pins = <&pin0 3>;
	};

	led_1_grp {
		pins = <&pin1 3>;
	};

	nand_grp {
		pins = <&pin32 0>, <&pin33 0>, <&pin34 0>, <&pin43 0>, <&pin44 0>, <&pin45 0>, <&pin56 1>;
	};
};

***

Gets a bit cumbersome (for me) when using ANSI C structs. I remain
unconvinced about ANSI C being a good place for storing such data.

Maybe I'm just getting too old & grumpy ;)

struct bcm4908_pinctrl_pin_setup {
	unsigned number;
	unsigned function;
};

static const struct bcm4908_pinctrl_pin_setup led_0_pins[] = {
	{ 0, 3 },
};

static const struct bcm4908_pinctrl_pin_setup led_1_pins[] = {
	{ 0, 3 },
};

static const struct bcm4908_pinctrl_pin_setup nand_pins[] = {
	{ 32, 0 }, { 33, 0 }, { 34, 0 }, { 43, 0 }, { 44, 0 }, { 45, 0 }, { 56, 1 },
};

struct bcm4908_pinctrl_grp {
	const char *name;
	const struct bcm4908_pinctrl_pin_setup *pins;
	const unsigned int num_pins;
};

static const struct bcm4908_pinctrl_grp bcm4908_pinctrl_grps[] = {
	{ "led_0_grp", led_0_pins, ARRAY_SIZE(led_0_pins) },
	{ "led_1_grp", led_1_pins, ARRAY_SIZE(led_1_pins) },
	{ "nand_grp", nand_pins, ARRAY_SIZE(nand_pins) },
};