[Letux-kernel] [PATCH] arm: dts: gta04: enable inputs/mcbsp1 pinux

H. Nikolaus Schaller hns at goldelico.com
Sun Nov 25 14:41:26 CET 2018


Hi,

> Am 24.11.2018 um 18:40 schrieb Andreas Kemnade <andreas at kemnade.info>:
> 
> Hi,
> 
> On Sat, 24 Nov 2018 18:12:51 +0100
> "H. Nikolaus Schaller" <hns at goldelico.com> wrote:
> 
>> Hi,
>> 
>>> Am 24.11.2018 um 12:15 schrieb Andreas Kemnade <andreas at kemnade.info>:
>>> 
>>> To ensure proper clock handling, the mcbsp1 seems to need
>>> some feedback on its clocks, so enable inputs.
>>> 
>>> Signed-off-by: Andreas Kemnade <andreas at kemnade.info>
>>> ---
>>> Replaces: dtb: gta04: fix mcbsp1 fsr setting
>>> 
>>> arch/arm/boot/dts/omap3-gta04.dtsi | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
>>> index 4e15137ae08a..2d6222625e19 100644
>>> --- a/arch/arm/boot/dts/omap3-gta04.dtsi
>>> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi
>>> @@ -415,14 +415,14 @@
>>> 
>>> 	mcbsp1_pins: pinmux_mcbsp1_pins {
>>> 		pinctrl-single,pins = <
>>> -			OMAP3_CORE1_IOPAD(0x218c, PIN_INPUT | MUX_MODE4)	/* mcbsp1_clkr.mcbsp1_clkr - gpio_156 FM interrupt */
>>> -			OMAP3_CORE1_IOPAD(0x218e, PIN_OUTPUT | MUX_MODE0)	/* mcbsp1_clkr.mcbsp1_fsr */
>>> -			OMAP3_CORE1_IOPAD(0x2190, PIN_OUTPUT | MUX_MODE0)	/* mcbsp1_dx.mcbsp1_dx */
>>> +			OMAP3_CORE1_IOPAD(0x218c, PIN_INPUT_PULLUP | MUX_MODE4)	/* mcbsp1_clkr.mcbsp1_clkr - gpio_156 FM interrupt */  
>> 
>> Yes, it may be reasonable to have explicit pull-ups if the Si4721 is disabled (suspended or driver removed).
>> I have no idea if the chip actively drives the FM interrupt line - and do we use it?
>> On the other hand it probably does neither care or harm.
>> 
>>> +			OMAP3_CORE1_IOPAD(0x218e, PIN_INPUT_PULLUP | MUX_MODE0)	/* mcbsp1_clkr.mcbsp1_fsr */  
>> 
>> Yes, I remember strange things can happen if the pinmux was not defined as "input" although the pin was used as output.
>> Maybe we did have that in the board file in 2.6.32 or 3.7 up to 3.12.
>> 
> 3.7 did not touch it, it just expects uboot to  initialize it. Yes,
> there was a strange thing with the input. My devconf thing is the other
> way to work around. BUt if in doubt I do things like the 3.7 kernel.
> 
>>> +			OMAP3_CORE1_IOPAD(0x2190, PIN_OUTPUT_PULLDOWN | MUX_MODE0)	/* mcbsp1_dx.mcbsp1_dx */  
>> 
>> This is probably when driver is removed to keep the output in a defined state when it switches to input?
>> May be good to note as a reminder in the commit message.
>> 
>>> 			OMAP3_CORE1_IOPAD(0x2192, PIN_INPUT | MUX_MODE0)	/* mcbsp1_dx.mcbsp1_dr */
>>> 			/* mcbsp_clks is used as PENIRQ */
>>> 			/* OMAP3_CORE1_IOPAD(0x2194, PIN_INPUT | MUX_MODE0)	/* mcbsp_clks.mcbsp_clks */
>>> -			OMAP3_CORE1_IOPAD(0x2196, PIN_INPUT | MUX_MODE0)	/* mcbsp_clks.mcbsp1_fsx */
>>> -			OMAP3_CORE1_IOPAD(0x2198, PIN_INPUT | MUX_MODE0)	/* mcbsp1_clkx.mcbsp1_clkx */
>>> +			OMAP3_CORE1_IOPAD(0x2196, PIN_INPUT_PULLUP | MUX_MODE0)	/* mcbsp_clks.mcbsp1_fsx */
>>> +			OMAP3_CORE1_IOPAD(0x2198, PIN_INPUT_PULLDOWN | MUX_MODE0)	/* mcbsp1_clkx.mcbsp1_clkx */  
>> 
>> Is there a reason to mix pulldown and pullup especially on clocks?
>> 
> 
> It is a good point to think about.
> Maybe just ask the author of the following patch: ;-)

Hm. I am not sure if he still remembers the reasons... It is a > 5 years old patch :)

I have asked and got these potential reasons:
* copied from elsewhere without understanding the reasons
* tried and tested and not realized the drawbacks and non-obvious bugs and side-effects
* tossing a coin
* a cat was walking over the keyboard
* ...

BTW: having pull-up/down was there even before and this only changes the GPIO_156
pull.

BR,
Nikolaus

> 
> andi at aktux:~/gta04/gta04-uboot$ git show 05bdeba3ec58bb69677caa0fb274070e60ec3b69
> commit 05bdeba3ec58bb69677caa0fb274070e60ec3b69
> Author: H. Nikolaus Schaller <hns at goldelico.com>
> Date:   Thu Jun 20 10:03:03 2013 +0200
> 
>    fixed pinmux for McBSP1 - Si47xx connection
> 
>    Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>
> 
> diff --git a/u-boot/board/goldelico/gta04/gta04.h b/u-boot/board/goldelico/gta04/gta04.h
> index 82495181eb..2ddc514fdc 100644
> --- a/u-boot/board/goldelico/gta04/gta04.h
> +++ b/u-boot/board/goldelico/gta04/gta04.h
> @@ -457,13 +457,13 @@ MUX_VAL(CP(UART2_CTS),            (IEN  | PTU | DIS | M4)) /*GPIO_144 / UART2-CTS - ext An
> MUX_VAL(CP(UART2_RTS),         (IDIS | PTD | DIS | M4)) /*GPIO_145 / UART2-RTS - GPS ON(0)/OFF(1)*/\
> MUX_VAL(CP(UART2_TX),          (IDIS | PTU | DIS | M0)) /*GPIO_146 / UART2-TX - GPS_TX */\
> MUX_VAL(CP(UART2_RX),          (IEN  | PTU | DIS | M0)) /*GPIO_147 / UART2-RX - GPS_RX */\
> -MUX_VAL(CP(MCBSP1_CLKR),       (IDIS | PTD | DIS | M0)) /*GPIO_156 - FM TRX*/\
> -MUX_VAL(CP(MCBSP1_FSR),                (IEN  | PTU | EN  | M0)) /*GPIO_157 -  */\
> -MUX_VAL(CP(MCBSP1_DX),         (IDIS | PTD | EN  | M0)) /*GPIO_158 -  */\
> -MUX_VAL(CP(MCBSP1_DR),         (IEN  | PTU | DIS | M0)) /*GPIO_159 -  */\
> -MUX_VAL(CP(MCBSP_CLKS),                (IEN  | PTU | EN  | M4)) /*GPIO_160 - PENIRQ*/\
> -MUX_VAL(CP(MCBSP1_FSX),                (IDIS | PTU | EN  | M0)) /*GPIO_161 -  */\
> -MUX_VAL(CP(MCBSP1_CLKX),       (IDIS | PTD | EN  | M0)) /*GPIO_162 -  */\
> +MUX_VAL(CP(MCBSP1_CLKR),       (IEN  | PTU | EN  | M4)) /*GPIO_156 - FM TRX IRQ */\
> +MUX_VAL(CP(MCBSP1_FSR),                (IEN  | PTU | EN  | M0)) /*GPIO_157 - FM RX frame sync output */\
> +MUX_VAL(CP(MCBSP1_DX),         (IDIS | PTD | EN  | M0)) /*GPIO_158 - FM TX signal output */\
> +MUX_VAL(CP(MCBSP1_DR),         (IEN  | PTU | DIS | M0)) /*GPIO_159 - FM RX signal input */\
> +MUX_VAL(CP(MCBSP_CLKS),                (IEN  | PTU | EN  | M4)) /*GPIO_160 - used as PENIRQ*/\
> +MUX_VAL(CP(MCBSP1_FSX),                (IEN  | PTU | EN  | M0)) /*GPIO_161 - FM TX frame sync output */\
> +MUX_VAL(CP(MCBSP1_CLKX),       (IEN  | PTD | EN  | M0)) /*GPIO_162 - FM shared RX/TX clock output */\
> MUX_VAL(CP(MCBSP4_CLKX),       (IEN  | PTD | DIS | M0)) /*MCBSP4_CLKX*/\
> MUX_VAL(CP(MCBSP4_DR),         (IEN  | PTD | DIS | M0)) /*MCBSP4_DR*/\
> MUX_VAL(CP(MCBSP4_DX),         (IEN  | PTD | DIS | M0)) /*MCBSP4_DX*/\
> @@ -516,13 +516,13 @@ MUX_VAL(CP(MCBSP3_DX),            (IDIS | PTD | DIS | M0)) /*McBSP3 -> Bluetooth PCM */\
> MUX_VAL(CP(MCBSP3_DR),         (IEN  | PTD | DIS | M0)) /**/\
> MUX_VAL(CP(MCBSP3_CLKX),       (IDIS | PTD | DIS | M0)) /**/\
> MUX_VAL(CP(MCBSP3_FSX),                (IEN  | PTD | DIS | M0)) /**/\
> -MUX_VAL(CP(MCBSP1_CLKR),       (IDIS | PTD | DIS | M0)) /*GPIO_156 -> FM TRX*/\
> -MUX_VAL(CP(MCBSP1_FSR),                (IEN  | PTU | EN  | M0)) /*GPIO_157 -> MCBSP1_FSR */\
> -MUX_VAL(CP(MCBSP1_DX),         (IDIS | PTD | EN  | M0)) /*GPIO_158 -> MCBSP1_DX */\
> -MUX_VAL(CP(MCBSP1_DR),         (IEN  | PTU | DIS | M0)) /*GPIO_159 -> MCBSP1_DR */\
> -MUX_VAL(CP(MCBSP_CLKS),                (IEN  | PTU | DIS | M4)) /*GPIO_160 - PENIRQ*/\
> -MUX_VAL(CP(MCBSP1_FSX),                (IDIS | PTU | EN  | M0)) /*GPIO_161 -> MCBSP1_FSX */\
> -MUX_VAL(CP(MCBSP1_CLKX),       (IDIS | PTD | EN  | M0)) /*GPIO_162 -> MCBSP1_CLKX */\
> +MUX_VAL(CP(MCBSP1_CLKR),       (IEN  | PTU | EN  | M4)) /*GPIO_156 - FM TRX IRQ */\
> +MUX_VAL(CP(MCBSP1_FSR),                (IEN  | PTU | EN  | M0)) /*GPIO_157 - FM RX frame sync output */\
> +MUX_VAL(CP(MCBSP1_DX),         (IDIS | PTD | EN  | M0)) /*GPIO_158 - FM TX signal output */\
> +MUX_VAL(CP(MCBSP1_DR),         (IEN  | PTU | DIS | M0)) /*GPIO_159 - FM RX signal input */\
> +MUX_VAL(CP(MCBSP_CLKS),                (IEN  | PTU | EN  | M4)) /*GPIO_160 - used as PENIRQ*/\
> +MUX_VAL(CP(MCBSP1_FSX),                (IEN  | PTU | EN  | M0)) /*GPIO_161 - FM TX frame sync output */\
> +MUX_VAL(CP(MCBSP1_CLKX),       (IEN  | PTD | EN  | M0)) /*GPIO_162 - FM shared RX/TX clock output */\
> MUX_VAL(CP(MCBSP4_CLKX),       (IEN  | PTD | DIS | M0)) /*GPIO_152 / MCBSP4_CLKX*/\
> MUX_VAL(CP(MCBSP4_DR),         (IEN  | PTD | DIS | M0)) /*GPIO_153 / MCBSP4_DR*/\
> MUX_VAL(CP(MCBSP4_DX),         (IEN  | PTD | DIS | M0)) /*GPIO_154 / MCBSP4_DX*/\
> 
> Regards,
> Andreas



More information about the Letux-kernel mailing list