Re: [PATCH 2/2] ARM: dts: omap4: panda: cleanup bluetooth

[Roger Quadros <rogerq@kernel.org>]

Hello Andreas,

On 30/12/2024 01:01, Andreas Kemnade wrote:
> Bluetooth is available on the other Panda board versions, too, so move
> stuff to common and specify the needed clock properly.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  .../boot/dts/ti/omap/omap4-panda-common.dtsi  | 30 +++++++++++++++--
>  arch/arm/boot/dts/ti/omap/omap4-panda-es.dts  | 32 -------------------
>  2 files changed, 28 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi b/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
> index c860b590142a..c048ab9af053 100644
> --- a/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap4-panda-common.dtsi
> @@ -368,9 +368,7 @@ OMAP4_IOPAD(0x130, PIN_INPUT_PULLUP | MUX_MODE0)	/* i2c4_sda */
>  	wl12xx_gpio: wl12xx-gpio-pins {
>  		pinctrl-single,pins = <
>  			OMAP4_IOPAD(0x066, PIN_OUTPUT | MUX_MODE3)		/* gpmc_a19.gpio_43 */

We could add function name in comment? e.g. /* gpmc_a19.gpio_43 - WLAN_EN */

> -			OMAP4_IOPAD(0x06c, PIN_OUTPUT | MUX_MODE3)		/* gpmc_a22.gpio_46 */
>  			OMAP4_IOPAD(0x070, PIN_OUTPUT_PULLUP | MUX_MODE3)	/* gpmc_a24.gpio_48 */

This one is FM_EN and has nothing to do with WLAN.

> -			OMAP4_IOPAD(0x072, PIN_OUTPUT_PULLUP | MUX_MODE3)	/* gpmc_a25.gpio_49 */
>  		>;
>  	};
>  
> @@ -393,6 +391,22 @@ button_pins: button-pins {
>  			OMAP4_IOPAD(0x114, PIN_INPUT_PULLUP | MUX_MODE3)	/* gpio_121 */
>  		>;
>  	};
> +
> +	bt_pins: bt-pins {
> +		pinctrl-single,pins = <
> +			OMAP4_IOPAD(0x06c, PIN_OUTPUT | MUX_MODE3)	  /* BTEN */
> +			OMAP4_IOPAD(0x072, PIN_OUTPUT_PULLUP | MUX_MODE3) /* BTWAKEUP */

Could we please use comment style <pin name>.<pinmux name> - Function
			OMAP4_IOPAD(0x06c, PIN_OUTPUT | MUX_MODE3)		/* gpmc_a22.gpio_46 - BTEN */
			OMAP4_IOPAD(0x072, PIN_OUTPUT_PULLUP | MUX_MODE3)	/* gpmc_a25.gpio_49 - BTWAKEUP */

> +		>;
> +	};
> +
> +	uart2_pins: uart2-pins {
> +		pinctrl-single,pins = <
> +			OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0)  /* uart2_cts - HCI */
> +			OMAP4_IOPAD(0x11a, PIN_OUTPUT | MUX_MODE0)	  /* uart2_rts */
> +			OMAP4_IOPAD(0x11c, PIN_INPUT_PULLUP | MUX_MODE0)  /* uart2_rx */
> +			OMAP4_IOPAD(0x11e, PIN_OUTPUT | MUX_MODE0)	  /* uart2_tx */

Need to fix comment style to <pin name>.<pinmux name> - Function

> +		>;
> +	};
>  };
>  
>  &omap4_pmx_wkup {
> @@ -531,8 +545,20 @@ &twl_usb_comparator {
>  };
>  
>  &uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart2_pins>;
> +

Unnecessary new line.
>  	interrupts-extended = <&wakeupgen GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH
>  			       &omap4_pmx_core OMAP4_UART2_RX>;
> +

Here too?

> +	bluetooth {
> +		compatible = "ti,wl1271-st";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_pins>;
> +		enable-gpios = <&gpio2 14 GPIO_ACTIVE_HIGH>;	/* GPIO_46 */
> +		clocks = <&twl 0>;
> +		clock-names = "ext_clock";
> +	};
>  };
>  
>  &uart3 {
> diff --git a/arch/arm/boot/dts/ti/omap/omap4-panda-es.dts b/arch/arm/boot/dts/ti/omap/omap4-panda-es.dts
> index fe7b156d10ed..a933fe560834 100644
> --- a/arch/arm/boot/dts/ti/omap/omap4-panda-es.dts
> +++ b/arch/arm/boot/dts/ti/omap/omap4-panda-es.dts
> @@ -49,22 +49,6 @@ button_pins: button-pins {
>  			OMAP4_IOPAD(0x0fc, PIN_INPUT_PULLUP | MUX_MODE3) /* gpio_113 */
>  		>;
>  	};
> -
> -	bt_pins: bt-pins {
> -		pinctrl-single,pins = <
> -			OMAP4_IOPAD(0x06c, PIN_OUTPUT | MUX_MODE3)		/* gpmc_a22.gpio_46 - BTEN */
> -			OMAP4_IOPAD(0x072, PIN_OUTPUT_PULLUP | MUX_MODE3)	/* gpmc_a25.gpio_49 - BTWAKEUP */
> -		>;
> -	};
> -
> -	uart2_pins: uart2-pins {
> -		pinctrl-single,pins = <
> -			OMAP4_IOPAD(0x118, PIN_INPUT_PULLUP | MUX_MODE0)	/* uart2_cts.uart2_cts - HCI */
> -			OMAP4_IOPAD(0x11a, PIN_OUTPUT | MUX_MODE0)		/* uart2_rts.uart2_rts */
> -			OMAP4_IOPAD(0x11c, PIN_INPUT_PULLUP | MUX_MODE0)	/* uart2_rx.uart2_rx */
> -			OMAP4_IOPAD(0x11e, PIN_OUTPUT | MUX_MODE0)		/* uart2_tx.uart2_tx */
> -		>;
> -	};
>  };
>  
>  &led_wkgpio_pins {
> @@ -96,19 +80,3 @@ buttonS2 {
>  &gpio1_target {
>  	 ti,no-reset-on-init;
>  };
> -
> -&wl12xx_gpio {
> -	pinctrl-single,pins = <
> -		OMAP4_IOPAD(0x066, PIN_OUTPUT | MUX_MODE3)		/* gpmc_a19.gpio_43 */
> -		OMAP4_IOPAD(0x070, PIN_OUTPUT_PULLUP | MUX_MODE3)	/* gpmc_a24.gpio_48 */
> -	>;
> -};
> -
> -&uart2 {
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&uart2_pins &bt_pins>;
> -	bluetooth: tiwi {
> -		compatible = "ti,wl1271-st";
> -		enable-gpios = <&gpio2 14 GPIO_ACTIVE_HIGH>;	/* GPIO_46 */
> -	};
> -};

-- 
cheers,
-roger