On Fri, May 29, 2020 at 10:40:37AM -0700, Tony Lindgren wrote: > * Drew Fustini <drew@xxxxxxxxxxxxxxx> [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