On Sat, Jan 09, 2021 at 02:22:07AM +0100, Linus Walleij wrote:

Hi Drew,

sorry for belated review. The approach is so uncommon so it had me
confused.

On Thu, Dec 24, 2020 at 9:36 PM Drew Fustini [off-list ref] wrote:

quoted

quoted

quoted

I used the compatible string "pinctrl,state-helper" but would appreciate
advice on how to best name this. Should I create a new vendor prefix?

Here is the first concern. Why does this require to be a driver with a
compatible string?

I have not been able to figure out how to have different active pinctrl
states for each header pins (for example P2 header pin 3) unless they
are represented as DT nodes with their own compatible for this helper
driver such as:

&ocp {
        P2_03_pinmux {
                compatible = "pinctrl,state-helper";
                pinctrl-names = "default", "gpio", "gpio_pu", "gpio_pd", "gpio_input", "pwm";
                pinctrl-0 = <&P2_03_default_pin>;
                pinctrl-1 = <&P2_03_gpio_pin>;
                pinctrl-2 = <&P2_03_gpio_pu_pin>;
                pinctrl-3 = <&P2_03_gpio_pd_pin>;
                pinctrl-4 = <&P2_03_gpio_input_pin>;
                pinctrl-5 = <&P2_03_pwm_pin>;
        };
}

I do not think the DT people are going to appreciate this pseudo-device.

Thank you for reviewing and commenting.

It is does seem like creating a platform device for each header pin and
binding to this proposed helper driver is not the correct approach.
 

Can you not just represent them as pin control hogs and have the debugfs
code with the other debugfs code in drivers/pinctrl/core.c?

I tried defining pinctrl states in the am33xx_pinmux DT node (which has 
compatible "pinctrl-single").  It does work to have default state
defined for pins.  However, I was not sure how represent having
different states active for independent header pins.

Instead of DT binds, maybe I need to use PIN_MAP_MUX_GROUP_HOG_DEFAULT()
in pinctrl-single code?

Normal drivers cannot play around with the state assigned to a
hog, but debugfs can certainly do that so go ahead and patch
the core.

Is there an existing debugfs file that you think would be appropriate to
allow the state of a hog to be changed?
 

quoted

I can assign pinctrl states in the pin controller DT node which has
compatible pinctrl-single (line 301 arch/arm/boot/dts/am33xx-l4.dtsi):

&am33xx_pinmux {

        pinctrl-names = "default", "gpio", "pwm";
        pinctrl-0 =   < &P2_03_default_pin &P1_34_default_pin &P2_19_default_pin &P2_24_default_pin
                        &P2_33_default_pin &P2_22_default_pin &P2_18_default_pin &P2_10_default_pin
                        &P2_06_default_pin &P2_04_default_pin &P2_02_default_pin &P2_08_default_pin
                        &P2_17_default_pin >;
        pinctrl-1 =   < &P2_03_gpio_pin &P1_34_gpio_pin &P2_19_gpio_pin &P2_24_gpio_pin
                        &P2_33_gpio_pin &P2_22_gpio_pin &P2_18_gpio_pin &P2_10_gpio_pin
                        &P2_06_gpio_pin &P2_04_gpio_pin &P2_02_gpio_pin &P2_08_gpio_pin
                        &P2_17_gpio_pin >;
        pinctrl-2 =   < &P2_03_pwm &P1_34_pwm &P2_19_pwm &P2_24_pwm
                        &P2_33_pwm &P2_22_pwm &P2_18_pwm &P2_10_pwm
                        &P2_06_pwm &P2_04_pwm &P2_02_pwm &P2_08_pwm
                        &P2_17_pwm >;

}

However, there is no way to later select "gpio" for P2.03 and select
"pwm" for P1.34 at the same time.  Thus, I can not figure out a way to
select independent states per pin unless I make a node for each pin that
binds to a helper driver.

It feels like there may be a simpler soluation but I can't see to figure
it out.  Suggestions welcome!

I think maybe there is no solution because you are solving a problem
that only pinctrl-single while trying to stay generic? The single
driver is special in that it requires all states of pins to be encoded
into the device tree, but for debugging that is kind of unfriendly
which was mentioned in its inception. For deep debugging it is good
to let the core know of all available functions and groups and
single does not IIUC.

Yours,
Linus Walleij

I discussed my use case and this patch on #armlinux earlier this week
and Alexandre Belloni suggested looking at the pinmux-pins debugfs file.

This made me think that a possible solution could be to define a store
function for pinmux-pins to handle something like "<pin#> <function#>".
I believe the ability to activate a pin function (or pin group) from
userspace would satisfy our beagleboard.org use-case.

Does that seem like a reasonable approach?

Thank you!
Drew