pinctrl-single: num_maps in generic pinconf support?

pinctrl-single: num_maps in generic pinconf support?

[Drew Fustini]

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

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

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200526 12:22]:
> 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?

Only slightly related, but we should really eventually move omaps to use
#pinctrl-cells = <2> (or 3) instead of 1, and pass the pinconf seprately
from the mux mode. We already treat them separately with the new
AM33XX_PADCONF macro, so we'd only have to change one SoC at a time to
use updated #pinctrl-cells. But I think pinctrl-single might need some
changes before we can do that.

Regards,

Tony

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

[Drew Fustini]

On Wed, May 27, 2020 at 09:51:22AM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@beagleboard.org> [200526 12:22]:
> > 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?
> 
> Only slightly related, but we should really eventually move omaps to use
> #pinctrl-cells = <2> (or 3) instead of 1, and pass the pinconf seprately
> from the mux mode. 

Thanks for the insight, Tony.

I was not considering the situation where pinctrl-cells would be more
than 1.

I see now from pinctrl-single.txt bindings doc that:

- #pinctrl-cells : number of cells in addition to the index, set to 1
  for pinctrl-single,pins and 2 for pinctrl-single,bits

I am now wondering if it wrong for me to expect compatible string of 
"pinconf-single" to work with pinctrl-cells of 1.

I see that arch/arm/boot/dts/da850.dtsi has:

154                 pmx_core: pinmux@14120 {
155                         compatible = "pinctrl-single";
156                         reg = <0x14120 0x50>;
157                         #pinctrl-cells = <2>;
158                         pinctrl-single,bit-per-mux;

and arch/arm/boot/dts/keystone-k2l.dtsi has:

108                 k2l_pmx: pinmux@2620690 {
109                         compatible = "pinctrl-single";
110                         reg = <0x02620690 0xc>;
111                         #address-cells = <1>;
112                         #size-cells = <0>;
113                         #pinctrl-cells = <2>;
114                         pinctrl-single,bit-per-mux;

> We already treat them separately with the new
> AM33XX_PADCONF macro, so we'd only have to change one SoC at a time to
> use updated #pinctrl-cells. But I think pinctrl-single might need some
> changes before we can do that.

Do you mean that it would be possible to make the change just for AM335x
to start with?

Do you think the changes would be limited to pinctrl-single.c and the
associated device tree files like am33xx-l4.dtsi ?


thanks,
drew

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

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200527 22:20]:
> On Wed, May 27, 2020 at 09:51:22AM -0700, Tony Lindgren wrote:
> > * Drew Fustini <drew@beagleboard.org> [200526 12:22]:
> > > 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?
> > 
> > Only slightly related, but we should really eventually move omaps to use
> > #pinctrl-cells = <2> (or 3) instead of 1, and pass the pinconf seprately
> > from the mux mode. 
> 
> Thanks for the insight, Tony.
> 
> I was not considering the situation where pinctrl-cells would be more
> than 1.
> 
> I see now from pinctrl-single.txt bindings doc that:
> 
> - #pinctrl-cells : number of cells in addition to the index, set to 1
>   for pinctrl-single,pins and 2 for pinctrl-single,bits
> 
> I am now wondering if it wrong for me to expect compatible string of 
> "pinconf-single" to work with pinctrl-cells of 1.

Ideally the #pinctrl-cells would be what makes sense for the
hardware. However, I'm guessing pinctrl-single.c needs patching
for that to happen.

> I see that arch/arm/boot/dts/da850.dtsi has:
> 
> 154                 pmx_core: pinmux@14120 {
> 155                         compatible = "pinctrl-single";
> 156                         reg = <0x14120 0x50>;
> 157                         #pinctrl-cells = <2>;
> 158                         pinctrl-single,bit-per-mux;
> 
> and arch/arm/boot/dts/keystone-k2l.dtsi has:
> 
> 108                 k2l_pmx: pinmux@2620690 {
> 109                         compatible = "pinctrl-single";
> 110                         reg = <0x02620690 0xc>;
> 111                         #address-cells = <1>;
> 112                         #size-cells = <0>;
> 113                         #pinctrl-cells = <2>;
> 114                         pinctrl-single,bit-per-mux;

Yeah so there's also the "bit-per-mux" variant. That should not
affect #pinctrl-cells use, we just need to make it more flexible.

> > We already treat them separately with the new
> > AM33XX_PADCONF macro, so we'd only have to change one SoC at a time to
> > use updated #pinctrl-cells. But I think pinctrl-single might need some
> > changes before we can do that.
> 
> Do you mean that it would be possible to make the change just for AM335x
> to start with?

Yes. So ideally we'd just fix up whatever is needed in pinctrl-single.c,
then just set #pinctrl-cells = <2> in am33xx-l4.dtsi, and update the
AM33XX_PADCONF accordingly.

> Do you think the changes would be limited to pinctrl-single.c and the
> associated device tree files like am33xx-l4.dtsi ?

Yes that should be the case. There should be no need to churn the board
specific dts files now that we have AM33XX_PADCONF.

Regards,

Tony

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

[Drew Fustini]

On Wed, May 27, 2020 at 03:41:08PM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@beagleboard.org> [200527 22:20]:
> > On Wed, May 27, 2020 at 09:51:22AM -0700, Tony Lindgren wrote:
> > > * Drew Fustini <drew@beagleboard.org> [200526 12:22]:
> > > > 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?
> > > 
> > > Only slightly related, but we should really eventually move omaps to use
> > > #pinctrl-cells = <2> (or 3) instead of 1, and pass the pinconf seprately
> > > from the mux mode. 
> > 
> > Thanks for the insight, Tony.
> > 
> > I was not considering the situation where pinctrl-cells would be more
> > than 1.
> > 
> > I see now from pinctrl-single.txt bindings doc that:
> > 
> > - #pinctrl-cells : number of cells in addition to the index, set to 1
> >   for pinctrl-single,pins and 2 for pinctrl-single,bits
> > 
> > I am now wondering if it wrong for me to expect compatible string of 
> > "pinconf-single" to work with pinctrl-cells of 1.
> 
> Ideally the #pinctrl-cells would be what makes sense for the
> hardware. However, I'm guessing pinctrl-single.c needs patching
> for that to happen.
> 
> > I see that arch/arm/boot/dts/da850.dtsi has:
> > 
> > 154                 pmx_core: pinmux@14120 {
> > 155                         compatible = "pinctrl-single";
> > 156                         reg = <0x14120 0x50>;
> > 157                         #pinctrl-cells = <2>;
> > 158                         pinctrl-single,bit-per-mux;
> > 
> > and arch/arm/boot/dts/keystone-k2l.dtsi has:
> > 
> > 108                 k2l_pmx: pinmux@2620690 {
> > 109                         compatible = "pinctrl-single";
> > 110                         reg = <0x02620690 0xc>;
> > 111                         #address-cells = <1>;
> > 112                         #size-cells = <0>;
> > 113                         #pinctrl-cells = <2>;
> > 114                         pinctrl-single,bit-per-mux;
> 
> Yeah so there's also the "bit-per-mux" variant. That should not
> affect #pinctrl-cells use, we just need to make it more flexible.
> 
> > > We already treat them separately with the new
> > > AM33XX_PADCONF macro, so we'd only have to change one SoC at a time to
> > > use updated #pinctrl-cells. But I think pinctrl-single might need some
> > > changes before we can do that.
> > 
> > Do you mean that it would be possible to make the change just for AM335x
> > to start with?
> 
> Yes. So ideally we'd just fix up whatever is needed in pinctrl-single.c,
> then just set #pinctrl-cells = <2> in am33xx-l4.dtsi, and update the
> AM33XX_PADCONF accordingly.

Would you be able to describe what you think AM33XX_PADCONF would look
like if the mux and conf are seperated?

Is there an example you know of for another SoC?
 
> > Do you think the changes would be limited to pinctrl-single.c and the
> > associated device tree files like am33xx-l4.dtsi ?
> 
> Yes that should be the case. There should be no need to churn the board
> specific dts files now that we have AM33XX_PADCONF.

Currently, the macro takes dir and mux:

include/dt-bindings/pinctrl/omap.h:
#define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))

For example, in arch/arm/boot/dts/am335x-bone-common.dtsi:
AM33XX_PADCONF(AM335X_PIN_I2C0_SDA, PIN_INPUT_PULLUP, MUX_MODE0)

I think it might be more accurate to rename 'dir' to 'conf'.

Table "9.3.1.50 conf_<module>_<pin> Register" in the AM335x TRM [0]
defines bits 0-2 as "Pad functional signal mux select." Bits 3-6 are
what I would consider pin configuration.  Only bit 5 is related to
direction: "Input enable value for the PAD".

> 
> Regards,
> 
> Tony

thanks,
drew

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

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200528 12:54]:
> Would you be able to describe what you think AM33XX_PADCONF would look
> like if the mux and conf are seperated?

Yes it would just slightly change, see your example below.

> Is there an example you know of for another SoC?

I think the other driver already keep the padconf and mux separate.

So not sure where all #pinctrl-cells could be used. It would make
pinctrl-single.c a bit nicer though, and probably would make it
easier to implement further features.

Some hardware may need it to have #pinctrl-cells = <3> if GPIO
features are there too, ideally pinctrl-single would not even
care but just work for what is configured for the hardware.

> Currently, the macro takes dir and mux:
> 
> include/dt-bindings/pinctrl/omap.h:
> #define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))

So after fixing up pinctrl-single.c, and changing the SoC dts
to have #pinctrl-cells = <2> instead of <1>, the macro would
then need to be:

#define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) (dir), (mux))

> For example, in arch/arm/boot/dts/am335x-bone-common.dtsi:
> AM33XX_PADCONF(AM335X_PIN_I2C0_SDA, PIN_INPUT_PULLUP, MUX_MODE0)

Yeah so no change needed for the use.

> I think it might be more accurate to rename 'dir' to 'conf'.

Sure makes sense to rename it in the macro.

Regards,

Tony

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

[Drew Fustini]

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

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

[Drew Fustini]

On Fri, May 29, 2020 at 07:55:44PM +0200, Drew Fustini wrote:
> 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
> 

I believe I may have found a bug in drivers/pinctrl/pinctrl-single.c

The function pcs_parse_one_pinctrl_entry() calls pcs_parse_pinconf()
if PCS_HAS_PINCONF is enabled.  The function pcs_parse_pinconf() 
returns 0 to indicate there was no error and num_maps is then set to 2:

 980 static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 981                                                 struct device_node *np,
 982                                                 struct pinctrl_map **map,
 983                                                 unsigned *num_maps,
 984                                                 const char **pgnames)
 985 {
<snip>
1053         (*map)->type = PIN_MAP_TYPE_MUX_GROUP;
1054         (*map)->data.mux.group = np->name;
1055         (*map)->data.mux.function = np->name;
1056
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);
1066
1067         return 0;

However, pcs_parse_pinconf() will also return 0 if !PCS_HAS_PINCONF or
!nconfs.  I believe these conditions should return non-zero.  Otherwise
pcs_parse_one_pinctrl_entry() will set num_maps = 2 even though no maps
were successfully added, as it does not reach "m++" on line 944:

 895 static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
 896                              struct pcs_function *func,
 897                              struct pinctrl_map **map)
 898
 899 {
 900         struct pinctrl_map *m = *map;
 901         int i = 0, nconfs = 0;
 902         unsigned long *settings = NULL, *s = NULL;
 903         struct pcs_conf_vals *conf = NULL;
 904         static const struct pcs_conf_type prop2[] = {
 905                 { "pinctrl-single,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, },
 906                 { "pinctrl-single,slew-rate", PIN_CONFIG_SLEW_RATE, },
 907                 { "pinctrl-single,input-schmitt", PIN_CONFIG_INPUT_SCHMITT, },
 908                 { "pinctrl-single,low-power-mode", PIN_CONFIG_LOW_POWER_MODE, },
 909         };
 910         static const struct pcs_conf_type prop4[] = {
 911                 { "pinctrl-single,bias-pullup", PIN_CONFIG_BIAS_PULL_UP, },
 912                 { "pinctrl-single,bias-pulldown", PIN_CONFIG_BIAS_PULL_DOWN, },
 913                 { "pinctrl-single,input-schmitt-enable",
 914                         PIN_CONFIG_INPUT_SCHMITT_ENABLE, },
 915         };
 916
 917         /* If pinconf isn't supported, don't parse properties in below. */
 918         if (!PCS_HAS_PINCONF)
 919                 return 0;
 920
 921         /* cacluate how much properties are supported in current node */
 922         for (i = 0; i < ARRAY_SIZE(prop2); i++) {
 923                 if (of_find_property(np, prop2[i].name, NULL))
 924                         nconfs++;
 925         }
 926         for (i = 0; i < ARRAY_SIZE(prop4); i++) {
 927                 if (of_find_property(np, prop4[i].name, NULL))
 928                         nconfs++;
 929         }
 930         if (!nconfs)
 919                 return 0;
 932
 933         func->conf = devm_kcalloc(pcs->dev,
 934                                   nconfs, sizeof(struct pcs_conf_vals),
 935                                   GFP_KERNEL);
 936         if (!func->conf)
 937                 return -ENOMEM;
 938         func->nconfs = nconfs;
 939         conf = &(func->conf[0]);
 940         m++;
 941         settings = devm_kcalloc(pcs->dev, nconfs, sizeof(unsigned long),
 942                                 GFP_KERNEL);
 943         if (!settings)
 944                 return -ENOMEM;
 945         s = &settings[0];
 946
 947         for (i = 0; i < ARRAY_SIZE(prop2); i++)
 948                 pcs_add_conf2(pcs, np, prop2[i].name, prop2[i].param,
 949                               &conf, &s);
 950         for (i = 0; i < ARRAY_SIZE(prop4); i++)
 951                 pcs_add_conf4(pcs, np, prop4[i].name, prop4[i].param,
 952                               &conf, &s);
 953         m->type = PIN_MAP_TYPE_CONFIGS_GROUP;
 954         m->data.configs.group_or_pin = np->name;
 955         m->data.configs.configs = settings;
 956         m->data.configs.num_configs = nconfs;
 957         return 0;
 958 }

This patch solves the boot failure on the BeagleBone Black when pinmux
compatible is "pinconf,single":

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 1e0614daee9b..18a02cd0c701 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -916,7 +916,7 @@ static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,

        /* If pinconf isn't supported, don't parse properties in below. */
        if (!PCS_HAS_PINCONF)
-               return 0;
+               return -ENOTSUPP; /* do not return 0 as no map added */

        /* cacluate how much properties are supported in current node */
        for (i = 0; i < ARRAY_SIZE(prop2); i++) {
@@ -928,7 +928,7 @@ static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
                        nconfs++;
        }
        if (!nconfs)
-               return 0;
+               return -ENOTSUPP; /* do not return 0 as no map added */

        func->conf = devm_kcalloc(pcs->dev,
                                  nconfs, sizeof(struct pcs_conf_vals),


Does this appear to be a reasonable solution?

I would appreciate any comments.


thank you,
drew

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

[Linus Walleij]

On Sun, May 31, 2020 at 2:17 AM Drew Fustini <drew@beagleboard.org> wrote:

> Does this appear to be a reasonable solution?
>
> I would appreciate any comments.

Looks reasonable to me. I suggest you send a proper patch. Tony?

Yours,
Linus Walleij

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

[Drew Fustini]

On Mon, Jun 8, 2020 at 2:10 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, May 31, 2020 at 2:17 AM Drew Fustini <drew@beagleboard.org> wrote:
>
> > Does this appear to be a reasonable solution?
> >
> > I would appreciate any comments.
>
> Looks reasonable to me. I suggest you send a proper patch. Tony?
>
> Yours,
> Linus Walleij

Thanks for reviewing, Linus.

I have posted a patch last week to which Tony replied that it looked
okay but would like Haojian Zhuang to review as neither of us have the
Hikey SoCs that enable pinconf for pinctrl-single [0].

On a related note, I'm about to send (from drew@beagleboard.org) a new
version that handles the case of when pincof is enabled and a group
has no pinconf properties.

-Drew

[0] https://lore.kernel.org/linux-gpio/20200601174803.GD1371046@x1/

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

[Drew Fustini]

On Fri, May 29, 2020 at 10:40:37AM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@beagleboard.org> [200528 12:54]:
> > Would you be able to describe what you think AM33XX_PADCONF would look
> > like if the mux and conf are seperated?
> 
> Yes it would just slightly change, see your example below.
> 
> > Is there an example you know of for another SoC?
> 
> I think the other driver already keep the padconf and mux separate.
> 
> So not sure where all #pinctrl-cells could be used. It would make
> pinctrl-single.c a bit nicer though, and probably would make it
> easier to implement further features.
> 
> Some hardware may need it to have #pinctrl-cells = <3> if GPIO
> features are there too, ideally pinctrl-single would not even
> care but just work for what is configured for the hardware.
> 
> > Currently, the macro takes dir and mux:
> > 
> > include/dt-bindings/pinctrl/omap.h:
> > #define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
> 
> So after fixing up pinctrl-single.c, and changing the SoC dts
> to have #pinctrl-cells = <2> instead of <1>, the macro would
> then need to be:
> 
> #define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) (dir), (mux))
> 
> > For example, in arch/arm/boot/dts/am335x-bone-common.dtsi:
> > AM33XX_PADCONF(AM335X_PIN_I2C0_SDA, PIN_INPUT_PULLUP, MUX_MODE0)
> 
> Yeah so no change needed for the use.
> 
> > I think it might be more accurate to rename 'dir' to 'conf'.
> 
> Sure makes sense to rename it in the macro.
> 
> Regards,
> 
> Tony

Tony - would this be how the macro would need to be changed?

diff --git a/include/dt-bindings/pinctrl/omap.h b/include/dt-bindings/pinctrl/omap.h
index 625718042413..2d2a8c737822 100644
--- a/include/dt-bindings/pinctrl/omap.h
+++ b/include/dt-bindings/pinctrl/omap.h
@@ -65,7 +65,7 @@
 #define DM814X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
 #define DM816X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
 #define AM33XX_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
-#define AM33XX_PADCONF(pa, dir, mux)   OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
+#define AM33XX_PADCONF(pa, conf, mux)  OMAP_IOPAD_OFFSET((pa), 0x0800) (conf) (mux)

 /*
  * Macros to allow using the offset from the padconf physical address

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

[Drew Fustini]

On Mon, Jun 08, 2020 at 08:05:43PM +0200, Drew Fustini wrote:
> On Fri, May 29, 2020 at 10:40:37AM -0700, Tony Lindgren wrote:
> > * Drew Fustini <drew@beagleboard.org> [200528 12:54]:
> > > Would you be able to describe what you think AM33XX_PADCONF would look
> > > like if the mux and conf are seperated?
> > 
> > Yes it would just slightly change, see your example below.
> > 
> > > Is there an example you know of for another SoC?
> > 
> > I think the other driver already keep the padconf and mux separate.
> > 
> > So not sure where all #pinctrl-cells could be used. It would make
> > pinctrl-single.c a bit nicer though, and probably would make it
> > easier to implement further features.
> > 
> > Some hardware may need it to have #pinctrl-cells = <3> if GPIO
> > features are there too, ideally pinctrl-single would not even
> > care but just work for what is configured for the hardware.
> > 
> > > Currently, the macro takes dir and mux:
> > > 
> > > include/dt-bindings/pinctrl/omap.h:
> > > #define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
> > 
> > So after fixing up pinctrl-single.c, and changing the SoC dts
> > to have #pinctrl-cells = <2> instead of <1>, the macro would
> > then need to be:
> > 
> > #define AM33XX_PADCONF(pa, dir, mux) OMAP_IOPAD_OFFSET((pa), 0x0800) (dir), (mux))
> > 
> > > For example, in arch/arm/boot/dts/am335x-bone-common.dtsi:
> > > AM33XX_PADCONF(AM335X_PIN_I2C0_SDA, PIN_INPUT_PULLUP, MUX_MODE0)
> > 
> > Yeah so no change needed for the use.
> > 
> > > I think it might be more accurate to rename 'dir' to 'conf'.
> > 
> > Sure makes sense to rename it in the macro.
> > 
> > Regards,
> > 
> > Tony
> 
> Tony - would this be how the macro would need to be changed?
> 
> diff --git a/include/dt-bindings/pinctrl/omap.h b/include/dt-bindings/pinctrl/omap.h
> index 625718042413..2d2a8c737822 100644
> --- a/include/dt-bindings/pinctrl/omap.h
> +++ b/include/dt-bindings/pinctrl/omap.h
> @@ -65,7 +65,7 @@
>  #define DM814X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
>  #define DM816X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
>  #define AM33XX_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
> -#define AM33XX_PADCONF(pa, dir, mux)   OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
> +#define AM33XX_PADCONF(pa, conf, mux)  OMAP_IOPAD_OFFSET((pa), 0x0800) (conf) (mux)
> 
>  /*
>   * Macros to allow using the offset from the padconf physical address

I've been looking at pinctrl-single.c further and now I am wondering if
it makes sense to modify pcs_parse_one_pinctrl_entry() or create a new
function.  Currently it does:

1020                 /* Index plus one value cell */
1021                 offset = pinctrl_spec.args[0];
1022                 vals[found].reg = pcs->base + offset;
1023                 vals[found].val = pinctrl_spec.args[1];

I believe it will need something similar to what is done in
pcs_parse_bits_in_pinctrl_entry():

1136                 /* Index plus two value cells */
1137                 offset = pinctrl_spec.args[0];
1138                 val = pinctrl_spec.args[1];
1139                 mask = pinctrl_spec.args[2];

So using #pinctrl-cells = <2> would require would require:

offset = pinctrl_spec.args[0];
vals[found].reg = pcs->base + offset;
vals[found].conf = pinctrl_spec.args[1];
vals[found].mux = pinctrl_spec.args[2];

However, the type of vals:

struct pcs_func_vals {
	void __iomem *reg;
	unsigned val;
	unsigned mask;
};

represents the combined mux + conf value in the register, so I think
pcs_conf_vals would need to used:

struct pcs_conf_vals {
	enum pin_config_param param;
	unsigned val;
	unsigned enable;
	unsigned disable;
	unsigned mask;
};

Thoughts?

thanks,
drew

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

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200608 18:06]:
> Tony - would this be how the macro would need to be changed?
> 
> diff --git a/include/dt-bindings/pinctrl/omap.h b/include/dt-bindings/pinctrl/omap.h
> index 625718042413..2d2a8c737822 100644
> --- a/include/dt-bindings/pinctrl/omap.h
> +++ b/include/dt-bindings/pinctrl/omap.h
> @@ -65,7 +65,7 @@
>  #define DM814X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
>  #define DM816X_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
>  #define AM33XX_IOPAD(pa, val)          OMAP_IOPAD_OFFSET((pa), 0x0800) (val)
> -#define AM33XX_PADCONF(pa, dir, mux)   OMAP_IOPAD_OFFSET((pa), 0x0800) ((dir) | (mux))
> +#define AM33XX_PADCONF(pa, conf, mux)  OMAP_IOPAD_OFFSET((pa), 0x0800) (conf) (mux)

Yes looks right to me :)

Tony

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

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200608 20:23]:
> I've been looking at pinctrl-single.c further and now I am wondering if
> it makes sense to modify pcs_parse_one_pinctrl_entry() or create a new
> function.  Currently it does:
> 
> 1020                 /* Index plus one value cell */
> 1021                 offset = pinctrl_spec.args[0];
> 1022                 vals[found].reg = pcs->base + offset;
> 1023                 vals[found].val = pinctrl_spec.args[1];
> 
> I believe it will need something similar to what is done in
> pcs_parse_bits_in_pinctrl_entry():
> 
> 1136                 /* Index plus two value cells */
> 1137                 offset = pinctrl_spec.args[0];
> 1138                 val = pinctrl_spec.args[1];
> 1139                 mask = pinctrl_spec.args[2];
> 
> So using #pinctrl-cells = <2> would require would require:
> 
> offset = pinctrl_spec.args[0];
> vals[found].reg = pcs->base + offset;
> vals[found].conf = pinctrl_spec.args[1];
> vals[found].mux = pinctrl_spec.args[2];
> 
> However, the type of vals:
> 
> struct pcs_func_vals {
> 	void __iomem *reg;
> 	unsigned val;
> 	unsigned mask;
> };
> 
> represents the combined mux + conf value in the register, so I think
> pcs_conf_vals would need to used:
> 
> struct pcs_conf_vals {
> 	enum pin_config_param param;
> 	unsigned val;
> 	unsigned enable;
> 	unsigned disable;
> 	unsigned mask;
> };
> 
> Thoughts?

Yes sounds about right to me. On write the driver would just write
the register based on mux and conf and orr the values together
using configured shifts.

Not sure if #pinctrl-cells would ever be more than 2 in this
case, but having things generic where possible makes sense.

For example, the omap padconf wakeirq specific bits could
potentially configured with #pinctrl-cells = <3>. We now
handle them automatically as interrupts so there's really
nothing to configure there.

I don't if other SoCs might need to configure other options
beyond conf and mux in the dts. I guess it's safe to assume
nothing else needs to be configured and have the folks needing
more support add it as needed.

Regards,

Tony