diff mbox series

[4/4] ARM: dts: imx7d: sbc imx7: add uart5

Message ID 20181204110741.6943-5-hohatzel@jusst.de (mailing list archive)
State New, archived
Headers show
Series imx7 sbc board support extensions | expand

Commit Message

Hans Ole Hatzel Dec. 4, 2018, 11:07 a.m. UTC 
Adds uart5 connections as used on the sbc.
In the sbc's reference material this is known as uart1, it does however
connect to the som as uart5. This uart is connected via the soc's
pins 11 (TX) and 13 (RX). On the sbc it is pinned out on P5
using pins 15 (TX) and 17 (RX).

Signed-off-by: Hans Ole Hatzel <hohatzel@jusst.de>
Signed-off-by: Julian Scheel <jscheel@jusst.de>
---
 arch/arm/boot/dts/imx7d-cl-som-imx7.dts | 16 ++++++++++++++++
 arch/arm/boot/dts/imx7d-sbc-imx7.dts    |  4 ++++
 2 files changed, 20 insertions(+)

Comments

Shawn Guo Dec. 6, 2018, 3:15 a.m. UTC | #1 
On Tue, Dec 04, 2018 at 12:07:41PM +0100, Hans Ole Hatzel wrote:
> Adds uart5 connections as used on the sbc.
> In the sbc's reference material this is known as uart1, it does however
> connect to the som as uart5. This uart is connected via the soc's
> pins 11 (TX) and 13 (RX). On the sbc it is pinned out on P5
> using pins 15 (TX) and 17 (RX).
> 
> Signed-off-by: Hans Ole Hatzel <hohatzel@jusst.de>
> Signed-off-by: Julian Scheel <jscheel@jusst.de>
> ---
>  arch/arm/boot/dts/imx7d-cl-som-imx7.dts | 16 ++++++++++++++++
>  arch/arm/boot/dts/imx7d-sbc-imx7.dts    |  4 ++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
> index 6c2c844dc052..f7c002093c67 100644
> --- a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
> +++ b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
> @@ -223,6 +223,15 @@
>  	fsl;
>  };
>  
> +&uart5 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart5>;
> +	assigned-clocks = <&clks IMX7D_UART5_ROOT_SRC>;
> +	assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;

Can we have some comments in the commit log on why we have to use this
particular clock source? 

Shawn

> +	status = "okay";
> +	fsl;
> +};
> +
>  &usbotg1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_usbotg1>;
> @@ -330,6 +339,13 @@
>  		>;
>  	};
>  
> +	pinctrl_uart5: uart5grp {
> +		fsl,pins = <
> +			MX7D_PAD_I2C4_SCL__UART5_DCE_RX		0x79
> +			MX7D_PAD_I2C4_SDA__UART5_DCE_TX		0x79
> +		>;
> +	};
> +
>  	pinctrl_usdhc2: usdhc2grp {
>  		fsl,pins = <
>  			MX7D_PAD_SD2_CMD__SD2_CMD			0x59
> diff --git a/arch/arm/boot/dts/imx7d-sbc-imx7.dts b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
> index 74904127fbc6..d23d62aceb82 100644
> --- a/arch/arm/boot/dts/imx7d-sbc-imx7.dts
> +++ b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
> @@ -30,6 +30,10 @@
>  	status = "okay";
>  };
>  
> +&uart5 {
> +	status = "okay";
> +};
> +
>  &iomuxc {
>  	pinctrl_usdhc1: usdhc1grp {
>  		fsl,pins = <
> -- 
> 2.19.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hans Ole Hatzel Dec. 10, 2018, 10:51 a.m. UTC | #2 
On 12/6/18 4:15 AM, Shawn Guo wrote:
> On Tue, Dec 04, 2018 at 12:07:41PM +0100, Hans Ole Hatzel wrote:
>> Adds uart5 connections as used on the sbc.
>> In the sbc's reference material this is known as uart1, it does however
>> connect to the som as uart5. This uart is connected via the soc's
>> pins 11 (TX) and 13 (RX). On the sbc it is pinned out on P5
>> using pins 15 (TX) and 17 (RX).
>>
>> Signed-off-by: Hans Ole Hatzel <hohatzel@jusst.de>
>> Signed-off-by: Julian Scheel <jscheel@jusst.de>
>> ---
>>   arch/arm/boot/dts/imx7d-cl-som-imx7.dts | 16 ++++++++++++++++
>>   arch/arm/boot/dts/imx7d-sbc-imx7.dts    |  4 ++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> index 6c2c844dc052..f7c002093c67 100644
>> --- a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> +++ b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
>> @@ -223,6 +223,15 @@
>>   	fsl;
>>   };
>>   
>> +&uart5 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_uart5>;
>> +	assigned-clocks = <&clks IMX7D_UART5_ROOT_SRC>;
>> +	assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
> 
> Can we have some comments in the commit log on why we have to use this
> particular clock source?
> 
> Shawn
> 

imx7d-pico.dts does this the same way. Is that good enough of a reason? 
If so, should it be included in the commit message?

Hans

>> +	status = "okay";
>> +	fsl;
>> +};
>> +
>>   &usbotg1 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&pinctrl_usbotg1>;
>> @@ -330,6 +339,13 @@
>>   		>;
>>   	};
>>   
>> +	pinctrl_uart5: uart5grp {
>> +		fsl,pins = <
>> +			MX7D_PAD_I2C4_SCL__UART5_DCE_RX		0x79
>> +			MX7D_PAD_I2C4_SDA__UART5_DCE_TX		0x79
>> +		>;
>> +	};
>> +
>>   	pinctrl_usdhc2: usdhc2grp {
>>   		fsl,pins = <
>>   			MX7D_PAD_SD2_CMD__SD2_CMD			0x59
>> diff --git a/arch/arm/boot/dts/imx7d-sbc-imx7.dts b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> index 74904127fbc6..d23d62aceb82 100644
>> --- a/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> +++ b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
>> @@ -30,6 +30,10 @@
>>   	status = "okay";
>>   };
>>   
>> +&uart5 {
>> +	status = "okay";
>> +};
>> +
>>   &iomuxc {
>>   	pinctrl_usdhc1: usdhc1grp {
>>   		fsl,pins = <
>> -- 
>> 2.19.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Fabio Estevam Dec. 10, 2018, 11:43 a.m. UTC | #3 
Hi Hans,

On Mon, Dec 10, 2018 at 8:52 AM Hans Ole Hatzel <hohatzel@jusst.de> wrote:

> imx7d-pico.dts does this the same way. Is that good enough of a reason?
> If so, should it be included in the commit message?

The UART clock parent initialization has been removed from the imx7d
clock driver since commit (in linux-next):

commit ea662d2f804ad13c3c92c75c7dc1abad30e31c31
Author: Anson Huang <anson.huang@nxp.com>
Date:   Fri Oct 19 01:05:36 2018 +0000

    clk: imx7d: remove UART1 clock setting

    There are clock assignments in all i.MX7D dtb files for UART1,
    below is the example in imx7d-sdb.dts, so setting UART1 clock
    in clock driver is NOT necessary, actually, module clocks setting
    should be done in module driver.

    &uart1 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_uart1>;
        assigned-clocks = <&clks IMX7D_UART1_ROOT_SRC>;
        assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
        status = "okay";
    };

    Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
    Signed-off-by: Stephen Boyd <sboyd@kernel.org>

So the UART clock parent should be set in the device tree.
Shawn Guo Dec. 11, 2018, 2:06 a.m. UTC | #4 
On Mon, Dec 10, 2018 at 09:43:12AM -0200, Fabio Estevam wrote:
> Hi Hans,
> 
> On Mon, Dec 10, 2018 at 8:52 AM Hans Ole Hatzel <hohatzel@jusst.de> wrote:
> 
> > imx7d-pico.dts does this the same way. Is that good enough of a reason?
> > If so, should it be included in the commit message?
> 
> The UART clock parent initialization has been removed from the imx7d
> clock driver since commit (in linux-next):
> 
> commit ea662d2f804ad13c3c92c75c7dc1abad30e31c31
> Author: Anson Huang <anson.huang@nxp.com>
> Date:   Fri Oct 19 01:05:36 2018 +0000
> 
>     clk: imx7d: remove UART1 clock setting
> 
>     There are clock assignments in all i.MX7D dtb files for UART1,
>     below is the example in imx7d-sdb.dts, so setting UART1 clock
>     in clock driver is NOT necessary, actually, module clocks setting
>     should be done in module driver.
> 
>     &uart1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_uart1>;
>         assigned-clocks = <&clks IMX7D_UART1_ROOT_SRC>;
>         assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
>         status = "okay";
>     };
> 
>     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>     Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> 
> So the UART clock parent should be set in the device tree.

Okay, but would it be better to set the default parent clock in soc
level of dtsi, since the configuration doesn't seem to be board
specific?

Shawn
Fabio Estevam Dec. 11, 2018, 2:35 a.m. UTC | #5 
Hi Shawn,

On Tue, Dec 11, 2018 at 12:07 AM Shawn Guo <shawnguo@kernel.org> wrote:

> Okay, but would it be better to set the default parent clock in soc
> level of dtsi, since the configuration doesn't seem to be board
> specific?

Yes, I think this is a good idea.
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
index 6c2c844dc052..f7c002093c67 100644
--- a/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
+++ b/arch/arm/boot/dts/imx7d-cl-som-imx7.dts
@@ -223,6 +223,15 @@ 
 	fsl;
 };
 
+&uart5 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart5>;
+	assigned-clocks = <&clks IMX7D_UART5_ROOT_SRC>;
+	assigned-clock-parents = <&clks IMX7D_PLL_SYS_MAIN_240M_CLK>;
+	status = "okay";
+	fsl;
+};
+
 &usbotg1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usbotg1>;
@@ -330,6 +339,13 @@ 
 		>;
 	};
 
+	pinctrl_uart5: uart5grp {
+		fsl,pins = <
+			MX7D_PAD_I2C4_SCL__UART5_DCE_RX		0x79
+			MX7D_PAD_I2C4_SDA__UART5_DCE_TX		0x79
+		>;
+	};
+
 	pinctrl_usdhc2: usdhc2grp {
 		fsl,pins = <
 			MX7D_PAD_SD2_CMD__SD2_CMD			0x59
diff --git a/arch/arm/boot/dts/imx7d-sbc-imx7.dts b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
index 74904127fbc6..d23d62aceb82 100644
--- a/arch/arm/boot/dts/imx7d-sbc-imx7.dts
+++ b/arch/arm/boot/dts/imx7d-sbc-imx7.dts
@@ -30,6 +30,10 @@ 
 	status = "okay";
 };
 
+&uart5 {
+	status = "okay";
+};
+
 &iomuxc {
 	pinctrl_usdhc1: usdhc1grp {
 		fsl,pins = <