[PATCH 0/3] pinctrl: single: support #pinctrl-cells = 2

[PATCH 0/3] pinctrl: single: support #pinctrl-cells = 2

[Drew Fustini]

Currently, pinctrl-single only allows #pinctrl-cells = 1.

This series will allow pinctrl-single to also support #pinctrl-cells = 2

If "pinctrl-single,pins" has 3 arguments (offset, conf, mux) then
pcs_parse_one_pinctrl_entry() does an OR operation on to get the
value to store in the register.
    
To take advantage of #pinctrl-cells = 2, the AM33XX_PADCONF macro in
omap.h is modified to keep pin conf and pin mux values separate.

Drew Fustini (3):
  ARM: dts: change AM33XX_PADCONF macro separate conf and mux
  ARM: dts: am33xx-l4: change #pinctrl-cells from 1 to 2
  pinctrl: single: parse #pinctrl-cells = 2

 arch/arm/boot/dts/am33xx-l4.dtsi   |  2 +-
 drivers/pinctrl/pinctrl-single.c   | 11 +++++++++--
 include/dt-bindings/pinctrl/omap.h |  2 +-
 3 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 1/3] ARM: dts: change AM33XX_PADCONF macro separate conf and mux

[Drew Fustini]

AM33XX_PADCONF macro is modified to keep pin conf and pin mux separate.

This requires #pinctrl-cells = 2 in am33xx-l4.dtsi

pinctrl-single.c but also be changed to support "pinctrl-single,pins"
with 3 arguments (offset, conf, mux)

Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
 include/dt-bindings/pinctrl/omap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/pinctrl/omap.h b/include/dt-bindings/pinctrl/omap.h
index 625718042413..a0974923bb1b 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
-- 
2.25.1

[PATCH 2/3] ARM: dts: am33xx-l4: change #pinctrl-cells from 1 to 2

[Drew Fustini]

This requires AM33XX_PADCONF macro in omap.h to be modified to keep pin
conf and pin mux values separate.

pinctrl-single.c must also be changed to support "pinctrl-single,pins"
with 3 arguments (offset, conf, mux).

Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
 arch/arm/boot/dts/am33xx-l4.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
index 340ea331e54d..f9b99cfa93b6 100644
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -290,7 +290,7 @@ scm: scm@0 {
 				am33xx_pinmux: pinmux@800 {
 					compatible = "pinctrl-single";
 					reg = <0x800 0x238>;
-					#pinctrl-cells = <1>;
+					#pinctrl-cells = <2>;
 					pinctrl-single,register-width = <32>;
 					pinctrl-single,function-mask = <0x7f>;
 				};
-- 
2.25.1

[PATCH 3/3] pinctrl: single: parse #pinctrl-cells = 2

[Drew Fustini]

If "pinctrl-single,pins" has 3 arguments (offset, conf, mux) then
pcs_parse_one_pinctrl_entry() does an OR operation on to get the
value to store in the register.

Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
 drivers/pinctrl/pinctrl-single.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 02f677eb1d53..e6d1cf25782c 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1017,10 +1017,17 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 			break;
 		}
 
-		/* Index plus one value cell */
 		offset = pinctrl_spec.args[0];
 		vals[found].reg = pcs->base + offset;
-		vals[found].val = pinctrl_spec.args[1];
+
+		switch (pinctrl_spec.args_count) {
+		case 2:
+			vals[found].val = pinctrl_spec.args[1];
+			break;
+		case 3:
+			vals[found].val = (pinctrl_spec.args[1] | pinctrl_spec.args[2]);
+			break;
+		}
 
 		dev_dbg(pcs->dev, "%pOFn index: 0x%x value: 0x%x\n",
 			pinctrl_spec.np, offset, pinctrl_spec.args[1]);
-- 
2.25.1

Re: [PATCH 1/3] ARM: dts: change AM33XX_PADCONF macro separate conf and mux

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

Hi Drew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on omap/for-next pinctrl/devel v5.8-rc1 next-20200618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Drew-Fustini/pinctrl-single-support-pinctrl-cells-2/20200618-205624
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/am335x-baltos.dtsi:53.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-igep0033.dtsi:57.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-bone-common.dtsi:71.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-osd335x-common.dtsi:39.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-chilisom.dtsi:30.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-cm-t335.dts:93.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-evm.dts:170.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-evmsk.dts:207.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-icev2.dts:157.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-lxm.dts:46.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
--
>> Error: arch/arm/boot/dts/am335x-moxa-uc-2100-common.dtsi:36.39-40 syntax error
   FATAL ERROR: Unable to parse input tree
..

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75664 bytes --]

Re: [PATCH 0/3] pinctrl: single: support #pinctrl-cells = 2

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200618 05:52]:
> Currently, pinctrl-single only allows #pinctrl-cells = 1.
> 
> This series will allow pinctrl-single to also support #pinctrl-cells = 2
> 
> If "pinctrl-single,pins" has 3 arguments (offset, conf, mux) then
> pcs_parse_one_pinctrl_entry() does an OR operation on to get the
> value to store in the register.
>     
> To take advantage of #pinctrl-cells = 2, the AM33XX_PADCONF macro in
> omap.h is modified to keep pin conf and pin mux values separate.

Hmm to me it looks like the order of the patches is the
wrong way around here. Don't we need to first change
pinctrl-single.c, and then only after that update the
dts? And make sure the pinctrl-single.c change does not
break anything without changing the dts :)

Regards,

Tony

Re: [PATCH 0/3] pinctrl: single: support #pinctrl-cells = 2

[Drew Fustini]

On Mon, Jun 22, 2020 at 09:11:32AM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@beagleboard.org> [200618 05:52]:
> > Currently, pinctrl-single only allows #pinctrl-cells = 1.
> > 
> > This series will allow pinctrl-single to also support #pinctrl-cells = 2
> > 
> > If "pinctrl-single,pins" has 3 arguments (offset, conf, mux) then
> > pcs_parse_one_pinctrl_entry() does an OR operation on to get the
> > value to store in the register.
> >     
> > To take advantage of #pinctrl-cells = 2, the AM33XX_PADCONF macro in
> > omap.h is modified to keep pin conf and pin mux values separate.
> 
> Hmm to me it looks like the order of the patches is the
> wrong way around here. Don't we need to first change
> pinctrl-single.c, and then only after that update the
> dts? And make sure the pinctrl-single.c change does not
> break anything without changing the dts :)
> 
> Regards,
> 
> Tony

Thanks for pointing this out.

I'll submit new version where:
[PATCH 1/3] pinctrl: single: parse #pinctrl-cells = 2
[PATCH 2/3] ARM: dts: change AM33XX_PADCONF macro separate conf and mux
[PATCH 3/3] ARM: dts: am33xx-l4: change #pinctrl-cells from 1 to 2

-drew

Re: [PATCH 0/3] pinctrl: single: support #pinctrl-cells = 2

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200622 16:47]:
> On Mon, Jun 22, 2020 at 09:11:32AM -0700, Tony Lindgren wrote:
> > * Drew Fustini <drew@beagleboard.org> [200618 05:52]:
> > > Currently, pinctrl-single only allows #pinctrl-cells = 1.
> > > 
> > > This series will allow pinctrl-single to also support #pinctrl-cells = 2
> > > 
> > > If "pinctrl-single,pins" has 3 arguments (offset, conf, mux) then
> > > pcs_parse_one_pinctrl_entry() does an OR operation on to get the
> > > value to store in the register.
> > >     
> > > To take advantage of #pinctrl-cells = 2, the AM33XX_PADCONF macro in
> > > omap.h is modified to keep pin conf and pin mux values separate.
> > 
> > Hmm to me it looks like the order of the patches is the
> > wrong way around here. Don't we need to first change
> > pinctrl-single.c, and then only after that update the
> > dts? And make sure the pinctrl-single.c change does not
> > break anything without changing the dts :)
> > 
> > Regards,
> > 
> > Tony
> 
> Thanks for pointing this out.
> 
> I'll submit new version where:
> [PATCH 1/3] pinctrl: single: parse #pinctrl-cells = 2
> [PATCH 2/3] ARM: dts: change AM33XX_PADCONF macro separate conf and mux
> [PATCH 3/3] ARM: dts: am33xx-l4: change #pinctrl-cells from 1 to 2

OK thanks. Otherwise sounds like git bisect will break for booting.

Regards,

Tony