diff mbox

[v2,1/2] arm64: dts: marvell: add CP110 uart peripherals

Message ID 03f1bec6e11c9f6fb9d5520c745eafca28597801.1517381798.git.baruch@tkos.co.il (mailing list archive)
State New, archived
Headers show

Commit Message

Baruch Siach Jan. 31, 2018, 6:56 a.m. UTC
The CP110 component has 4 uart peripherals. All of them use the same clock
gate for slow peripherals that is shared with the i2c and spi peripherals.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: No change
---
 arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 40 +++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Gregory CLEMENT Feb. 14, 2018, 10:42 a.m. UTC | #1
Hi Baruch,
 
 On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:

> The CP110 component has 4 uart peripherals. All of them use the same clock
> gate for slow peripherals that is shared with the i2c and spi peripherals.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Applied on mvebu/dt64

Thanks,

Gregory

> ---
> v2: No change
> ---
>  arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 40 +++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> index a8af4136dbe7..a422cd981a0b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> @@ -290,6 +290,46 @@
>  			status = "disabled";
>  		};
>  
> +		CP110_LABEL(uart0): serial@702000 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702000 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 122 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart1): serial@702100 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702100 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 123 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart2): serial@702200 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702200 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 124 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart3): serial@702300 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702300 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 125 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
>  		CP110_LABEL(nand): nand@720000 {
>  			/*
>  			* Due to the limitation of the pins available
> -- 
> 2.15.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Gregory CLEMENT Feb. 14, 2018, 10:42 a.m. UTC | #2
Hi Baruch,
 
 On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:

> The CP110 component has 4 uart peripherals. All of them use the same clock
> gate for slow peripherals that is shared with the i2c and spi peripherals.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Applied on mvebu/dt64

Thanks,

Gregory

> ---
> v2: No change
> ---
>  arch/arm64/boot/dts/marvell/armada-cp110.dtsi | 40 +++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> index a8af4136dbe7..a422cd981a0b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
> @@ -290,6 +290,46 @@
>  			status = "disabled";
>  		};
>  
> +		CP110_LABEL(uart0): serial@702000 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702000 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 122 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart1): serial@702100 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702100 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 123 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart2): serial@702200 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702200 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 124 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
> +		CP110_LABEL(uart3): serial@702300 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x702300 0x100>;
> +			reg-shift = <2>;
> +			interrupts = <ICU_GRP_NSR 125 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <1>;
> +			clocks = <&CP110_LABEL(clk) 1 21>;
> +			status = "disabled";
> +		};
> +
>  		CP110_LABEL(nand): nand@720000 {
>  			/*
>  			* Due to the limitation of the pins available
> -- 
> 2.15.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Baruch Siach Feb. 14, 2018, 10:56 a.m. UTC | #3
Hi Gregory,

On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
>  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
> 
> > The CP110 component has 4 uart peripherals. All of them use the same clock
> > gate for slow peripherals that is shared with the i2c and spi peripherals.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> Applied on mvebu/dt64

Thanks.

What about patch 2/2 in this series?

baruch
Gregory CLEMENT Feb. 14, 2018, 11:07 a.m. UTC | #4
Hi Baruch,
 
 On mer., févr. 14 2018, Baruch Siach <baruch@tkos.co.il> wrote:

> Hi Gregory,
>
> On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
>>  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
>> 
>> > The CP110 component has 4 uart peripherals. All of them use the same clock
>> > gate for slow peripherals that is shared with the i2c and spi peripherals.
>> >
>> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> 
>> Applied on mvebu/dt64
>
> Thanks.
>
> What about patch 2/2 in this series?

I thought I applied it, but now it's done.

Gregory

>
> baruch
>
> -- 
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Russell King (Oracle) Feb. 14, 2018, 11:07 a.m. UTC | #5
On Wed, Feb 14, 2018 at 12:56:53PM +0200, Baruch Siach wrote:
> Hi Gregory,
> 
> On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
> >  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
> > 
> > > The CP110 component has 4 uart peripherals. All of them use the same clock
> > > gate for slow peripherals that is shared with the i2c and spi peripherals.
> > >
> > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > 
> > Applied on mvebu/dt64
> 
> Thanks.
> 
> What about patch 2/2 in this series?

I'm not entirely convinced that it's something that should be done.
I know that some people are already using the UART headers for other
purposes (other than UART) and the later revision boards have the
placement of the microUSB fixed so it is accessible.

While you can tell Linux to use the other UART headers with this
patch, uboot won't use them, which means you can't configure the
boot loader without (in your case) taking the board out of the case.

I've a similar problem (with the mcbin in a rackmount case), and my
solution to that has been to put a single washer under the mounting
post near the microUSB to lift the board sufficiently to allow a
connector to be plugged in.  Sometimes simple hardware fixes are
better than software fixes.

Others have used a dremel to modify the case to access the microUSB.
Baruch Siach Feb. 14, 2018, 11:17 a.m. UTC | #6
Hi Ressell,

On Wed, Feb 14, 2018 at 11:07:51AM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 14, 2018 at 12:56:53PM +0200, Baruch Siach wrote:
> > On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
> > >  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
> > > 
> > > > The CP110 component has 4 uart peripherals. All of them use the same clock
> > > > gate for slow peripherals that is shared with the i2c and spi peripherals.
> > > >
> > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > > 
> > > Applied on mvebu/dt64
> > 
> > Thanks.
> > 
> > What about patch 2/2 in this series?
> 
> I'm not entirely convinced that it's something that should be done.
> I know that some people are already using the UART headers for other
> purposes (other than UART) and the later revision boards have the
> placement of the microUSB fixed so it is accessible.
> 
> While you can tell Linux to use the other UART headers with this
> patch, uboot won't use them, which means you can't configure the
> boot loader without (in your case) taking the board out of the case.
> 
> I've a similar problem (with the mcbin in a rackmount case), and my
> solution to that has been to put a single washer under the mounting
> post near the microUSB to lift the board sufficiently to allow a
> connector to be plugged in.  Sometimes simple hardware fixes are
> better than software fixes.
> 
> Others have used a dremel to modify the case to access the microUSB.

Just for the record, I'm fine with dropping 'status = "okay"' from the mcbin 
CP{0,1} UART nodes. This would still allow anyone who needs this functionality 
to enable it with a simple .dts modification, or a run-time dtb modification 
from the bootloader.

baruch
Russell King (Oracle) Feb. 14, 2018, 11:30 a.m. UTC | #7
On Wed, Feb 14, 2018 at 01:17:45PM +0200, Baruch Siach wrote:
> Hi Ressell,
> 
> On Wed, Feb 14, 2018 at 11:07:51AM +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 14, 2018 at 12:56:53PM +0200, Baruch Siach wrote:
> > > On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
> > > >  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
> > > > 
> > > > > The CP110 component has 4 uart peripherals. All of them use the same clock
> > > > > gate for slow peripherals that is shared with the i2c and spi peripherals.
> > > > >
> > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > > > 
> > > > Applied on mvebu/dt64
> > > 
> > > Thanks.
> > > 
> > > What about patch 2/2 in this series?
> > 
> > I'm not entirely convinced that it's something that should be done.
> > I know that some people are already using the UART headers for other
> > purposes (other than UART) and the later revision boards have the
> > placement of the microUSB fixed so it is accessible.
> > 
> > While you can tell Linux to use the other UART headers with this
> > patch, uboot won't use them, which means you can't configure the
> > boot loader without (in your case) taking the board out of the case.
> > 
> > I've a similar problem (with the mcbin in a rackmount case), and my
> > solution to that has been to put a single washer under the mounting
> > post near the microUSB to lift the board sufficiently to allow a
> > connector to be plugged in.  Sometimes simple hardware fixes are
> > better than software fixes.
> > 
> > Others have used a dremel to modify the case to access the microUSB.
> 
> Just for the record, I'm fine with dropping 'status = "okay"' from the mcbin 
> CP{0,1} UART nodes. This would still allow anyone who needs this functionality 
> to enable it with a simple .dts modification, or a run-time dtb modification 
> from the bootloader.

Talking more with Jon (who works for SolidRun, the board manufacturer),
the feeling there is:

1. Why enable both UART headers - it makes more sense (given your reason)
   to enable one as UART but keep the other as GPIO.  The labelling up of
   them being a UART is merely a historical artifact of the very early
   development of the board.

2. They are developer boards, not a final product.  Case modification is
   somewhat expected.

(2) is especially relevant when SFP support gets added - some of the
early revision boards have the SCL/SDA lines swapped on the I2C bus.
With that fixed, all boards have way too strong pull-ups on the I2C
bus which means some modules don't work - and worse than that, may
result in corrupted module EEPROMs.

I ended up with corruption here, and although I've a rev 1.3 board now,
I'm still using my self-modified rev 1.1 in preference to it, because
I don't want to have to deal with another corrupted EEPROM.

In order for SFP to be reliably functional, board modification is
required (either replacement of resistor packs, or in the case of the
early boards, cutting the I2C lines and rewiring them.)

IOW, board modification will be required for SFP on most mcbins.
Gregory CLEMENT Feb. 14, 2018, 12:14 p.m. UTC | #8
Hi,
 
 On mer., févr. 14 2018, Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Wed, Feb 14, 2018 at 01:17:45PM +0200, Baruch Siach wrote:
>> Hi Ressell,
>> 
>> On Wed, Feb 14, 2018 at 11:07:51AM +0000, Russell King - ARM Linux wrote:
>> > On Wed, Feb 14, 2018 at 12:56:53PM +0200, Baruch Siach wrote:
>> > > On Wed, Feb 14, 2018 at 11:42:52AM +0100, Gregory CLEMENT wrote:
>> > > >  On mer., janv. 31 2018, Baruch Siach <baruch@tkos.co.il> wrote:
>> > > > 
>> > > > > The CP110 component has 4 uart peripherals. All of them use the same clock
>> > > > > gate for slow peripherals that is shared with the i2c and spi peripherals.
>> > > > >
>> > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> > > > 
>> > > > Applied on mvebu/dt64
>> > > 
>> > > Thanks.
>> > > 
>> > > What about patch 2/2 in this series?
>> > 
>> > I'm not entirely convinced that it's something that should be done.
>> > I know that some people are already using the UART headers for other
>> > purposes (other than UART) and the later revision boards have the
>> > placement of the microUSB fixed so it is accessible.
>> > 
>> > While you can tell Linux to use the other UART headers with this
>> > patch, uboot won't use them, which means you can't configure the
>> > boot loader without (in your case) taking the board out of the case.
>> > 
>> > I've a similar problem (with the mcbin in a rackmount case), and my
>> > solution to that has been to put a single washer under the mounting
>> > post near the microUSB to lift the board sufficiently to allow a
>> > connector to be plugged in.  Sometimes simple hardware fixes are
>> > better than software fixes.
>> > 
>> > Others have used a dremel to modify the case to access the microUSB.
>> 
>> Just for the record, I'm fine with dropping 'status = "okay"' from the mcbin 
>> CP{0,1} UART nodes. This would still allow anyone who needs this functionality 
>> to enable it with a simple .dts modification, or a run-time dtb modification 
>> from the bootloader.
>
> Talking more with Jon (who works for SolidRun, the board manufacturer),
> the feeling there is:
>
> 1. Why enable both UART headers - it makes more sense (given your reason)
>    to enable one as UART but keep the other as GPIO.  The labelling up of
>    them being a UART is merely a historical artifact of the very early
>    development of the board.
>
> 2. They are developer boards, not a final product.  Case modification is
>    somewhat expected.

Just to let know that I applied the patch but I still can either ammend
it or drop it as it is not yet part of an immutable tag.

Once you will have agreed I will do the change.

Gregory


>
> (2) is especially relevant when SFP support gets added - some of the
> early revision boards have the SCL/SDA lines swapped on the I2C bus.
> With that fixed, all boards have way too strong pull-ups on the I2C
> bus which means some modules don't work - and worse than that, may
> result in corrupted module EEPROMs.
>
> I ended up with corruption here, and although I've a rev 1.3 board now,
> I'm still using my self-modified rev 1.1 in preference to it, because
> I don't want to have to deal with another corrupted EEPROM.
>
> In order for SFP to be reliably functional, board modification is
> required (either replacement of resistor packs, or in the case of the
> early boards, cutting the I2C lines and rewiring them.)
>
> IOW, board modification will be required for SFP on most mcbins.
>
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up
diff mbox

Patch

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
index a8af4136dbe7..a422cd981a0b 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110.dtsi
@@ -290,6 +290,46 @@ 
 			status = "disabled";
 		};
 
+		CP110_LABEL(uart0): serial@702000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x702000 0x100>;
+			reg-shift = <2>;
+			interrupts = <ICU_GRP_NSR 122 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <1>;
+			clocks = <&CP110_LABEL(clk) 1 21>;
+			status = "disabled";
+		};
+
+		CP110_LABEL(uart1): serial@702100 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x702100 0x100>;
+			reg-shift = <2>;
+			interrupts = <ICU_GRP_NSR 123 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <1>;
+			clocks = <&CP110_LABEL(clk) 1 21>;
+			status = "disabled";
+		};
+
+		CP110_LABEL(uart2): serial@702200 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x702200 0x100>;
+			reg-shift = <2>;
+			interrupts = <ICU_GRP_NSR 124 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <1>;
+			clocks = <&CP110_LABEL(clk) 1 21>;
+			status = "disabled";
+		};
+
+		CP110_LABEL(uart3): serial@702300 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x702300 0x100>;
+			reg-shift = <2>;
+			interrupts = <ICU_GRP_NSR 125 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <1>;
+			clocks = <&CP110_LABEL(clk) 1 21>;
+			status = "disabled";
+		};
+
 		CP110_LABEL(nand): nand@720000 {
 			/*
 			* Due to the limitation of the pins available