Re: pinctrl-single: num_maps in generic pinconf support?

[Drew Fustini <drew@beagleboard.org>]

On Tue, May 26, 2020 at 02:21:33PM +0200, Drew Fustini wrote:
> Hello Haojian and Linus,
> 
> For pcs_parse_one_pinctrl_entry() in drivers/pinctrl/pinctrl-single.c,
> I see that num_maps is set to 2 if PCS_HAS_PINCONF is enabled:
> 
> 1057         if (PCS_HAS_PINCONF && function) {
> 1058                 res = pcs_parse_pinconf(pcs, np, function, map);
> 1059                 if (res)
> 1060                         goto free_pingroups;
> 1061                 *num_maps = 2;
> 1062         } else {
> 1063                 *num_maps = 1;
> 1064         }
> 1065         mutex_unlock(&pcs->mutex);
> 
> git blame shows me that came from 9dddb4df90d13:
> "pinctrl: single: support generic pinconf"
> 
> Would you be able to provide any insight as to num_maps needs to be 2
> when pinconf is enabled?
> 
> thank you,
> drew

The BeagleBone fails to boot when I change the am33xx_pinmux compatible
from "pinctrl,single" to "pinconf,single":

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
index 5ed7f3c58c0f..b5bedd776ee6 100644
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -276,7 +276,7 @@ scm: scm@0 {
                                ranges = <0 0 0x2000>;
 
                                am33xx_pinmux: pinmux@800 {
-                                       compatible = "pinctrl-single";
+                                       compatible = "pinconf-single";
                                        reg = <0x800 0x238>;
                                        #pinctrl-cells = <1>;
                                        pinctrl-single,register-width = <32>;


From the full dmesg output [0], these lines seem the most relevant:

[    2.974958] pinctrl-single 44e10800.pinmux: 142 pins, size 568
[    3.847475] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x160 value: 0x2f
[    3.855556] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xfc value: 0x30
[    3.863520] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf8 value: 0x30
[    3.871483] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf4 value: 0x30
[    3.879444] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf0 value: 0x30
[    3.887404] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x104 value: 0x30
[    3.895455] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x100 value: 0x30
[    3.903505] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x1a0 value: 0x2c
[    3.911583] pinctrl core: add 2 pinctrl maps
[    3.915976] pinctrl core: failed to register map default (1): invalid type given
[    3.923594] omap_hsmmc: probe of 48060000.mmc failed with error -22
[    3.930403] omap_hsmmc 47810000.mmc: GPIO lookup for consumer cd
[    4.440389] Waiting for root device /dev/mmcblk0p1...

The error message:
"pinctrl core: failed to register map default (1): invalid type given"

comes from drivers/pinctrl/core.c:

1387 int pinctrl_register_mappings(const struct pinctrl_map *maps,
1388                               unsigned num_maps)
1389 {
1390         int i, ret;
1391         struct pinctrl_maps *maps_node;
1392 
1393         pr_debug("add %u pinctrl maps\n", num_maps);
1394 
1395         /* First sanity check the new mapping */
1396         for (i = 0; i < num_maps; i++) {
<snip>
1416                 switch (maps[i].type) {
1417                 case PIN_MAP_TYPE_DUMMY_STATE:
1418                         break;
1419                 case PIN_MAP_TYPE_MUX_GROUP:
1420                         ret = pinmux_validate_map(&maps[i], i);
1421                         if (ret < 0)
1422                                 return ret;
1423                         break;
1424                 case PIN_MAP_TYPE_CONFIGS_PIN:
1425                 case PIN_MAP_TYPE_CONFIGS_GROUP:
1426                         ret = pinconf_validate_map(&maps[i], i);
1427                         if (ret < 0)
1428                                 return ret;
1429                         break;
1430                 default:
1431                         pr_err("failed to register map %s (%d): invalid type given\n",
1432                                maps[i].name, i);
1433                         return -EINVAL;
1434                 }
1435         }


The invalid type error occurs when maps[i].type does not match any of
the case statements.

I have determined the system will boot ok [1] when num_maps is forced
to be 1.  Here are the related dmesg lines:

[    3.920484] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x160 value: 0x2f
[    3.928420] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xfc value: 0x30
[    3.936224] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf8 value: 0x30
[    3.944026] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf4 value: 0x30
[    3.951826] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0xf0 value: 0x30
[    3.959627] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x104 value: 0x30
[    3.967515] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x100 value: 0x30
[    3.975403] pinctrl-single 44e10800.pinmux: pinmux-mmc0-pins index: 0x1a0 value: 0x2c
[    3.983318] pinctrl core: add 1 pinctrl maps
[    3.987653] pinctrl-single 44e10800.pinmux: found group selector 4 for pinmux-mmc0-pins
[    3.995723] pinctrl-single 44e10800.pinmux: request pin 88 (PIN88) for 48060000.mmc
[    4.003434] pinctrl-single 44e10800.pinmux: request pin 63 (PIN63) for 48060000.mmc
[    4.011141] pinctrl-single 44e10800.pinmux: request pin 62 (PIN62) for 48060000.mmc
[    4.018847] pinctrl-single 44e10800.pinmux: request pin 61 (PIN61) for 48060000.mmc
[    4.026633] pinctrl-single 44e10800.pinmux: request pin 60 (PIN60) for 48060000.mmc
[    4.034351] pinctrl-single 44e10800.pinmux: request pin 65 (PIN65) for 48060000.mmc
[    4.042065] pinctrl-single 44e10800.pinmux: request pin 64 (PIN64) for 48060000.mmc
[    4.049774] pinctrl-single 44e10800.pinmux: request pin 104 (PIN104) for 48060000.mmc
[    4.057662] pinctrl-single 44e10800.pinmux: enabling (null) function4

Here is the patch for num_maps:

diiff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 1e0614daee9b..78a93336c711 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1058,7 +1058,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
                res = pcs_parse_pinconf(pcs, np, function, map);
                if (res)
                        goto free_pingroups;
-               *num_maps = 2;
+               *num_maps = 1;
        } else {
                *num_maps = 1;
        }

I am trying to understand why num_maps is supposed to be 2 when
PCS_HAS_PINCONF, and I would appreciate any comments.

Is there a bug somewhere in the code?

Or, is it supposed to be invalid to enable "pinconf,single" compatible
for the am33xx_pinmux node?


thank you,
drew

[0] https://gist.github.com/pdp7/293716fe98d90f031bb75950803952a1
[1] https://gist.github.com/pdp7/fc5186f46e34c3acc1b1a169be85d3a9