Message ID | 20171013090200.31034-1-miquel.raynal@free-electrons.com |
---|---|
Headers | show |
Series | Support armada-37xx second UART port | expand |
Hi Miquel, On ven., oct. 13 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > - port = &mvebu_uart_ports[0]; > + /* Assume that all UART ports have a DT alias or none has */ > + id = of_alias_get_id(pdev->dev.of_node, "serial"); You stil need to check the pdev->dev.of_node before calling of_alias_get_id. Once it will be fixed, you can add: Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Thanks, Gregory > + if (!pdev->dev.of_node || id < 0) > + pdev->id = uart_num_counter++; > + else > + pdev->id = id; > + > + if (pdev->id >= MVEBU_NR_UARTS) { > + dev_err(&pdev->dev, "cannot have more than %d UART ports\n", > + MVEBU_NR_UARTS); > + return -EINVAL; > + } > + > + port = &mvebu_uart_ports[pdev->id]; > > spin_lock_init(&port->lock); > > @@ -572,7 +588,7 @@ static int mvebu_uart_probe(struct platform_device *pdev) > port->fifosize = 32; > port->iotype = UPIO_MEM32; > port->flags = UPF_FIXED_PORT; > - port->line = 0; /* single port: force line number to 0 */ > + port->line = pdev->id; > > port->irq = irq->start; > port->irqflags = 0; > -- > 2.11.0 >
Hi Gregory, On Fri, 13 Oct 2017 11:40:32 +0200 Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > Hi Miquel, > > On ven., oct. 13 2017, Miquel Raynal > <miquel.raynal@free-electrons.com> wrote: > > > - port = &mvebu_uart_ports[0]; > > + /* Assume that all UART ports have a DT alias or none has > > */ > > + id = of_alias_get_id(pdev->dev.of_node, "serial"); > > You stil need to check the pdev->dev.of_node before calling > of_alias_get_id. Thanks for your feedback but I don't think we still need to check it, because the of_alias_get_id() function will either return -ENOSYS if CONFIG_OF is not defined, or -ENODEV if the node is not found. As the function does not dereference pdev->dev.of_node at any moment but instead compares the value with another pointer, I think this call is safe like this. Thanks, Miquèl > > Once it will be fixed, you can add: > > Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > > Thanks, > > Gregory > > > > + if (!pdev->dev.of_node || id < 0) > > + pdev->id = uart_num_counter++; > > + else > > + pdev->id = id; > > + > > + if (pdev->id >= MVEBU_NR_UARTS) { > > + dev_err(&pdev->dev, "cannot have more than %d UART > > ports\n", > > + MVEBU_NR_UARTS); > > + return -EINVAL; > > + } > > + > > + port = &mvebu_uart_ports[pdev->id]; > > > > spin_lock_init(&port->lock); > > > > @@ -572,7 +588,7 @@ static int mvebu_uart_probe(struct > > platform_device *pdev) port->fifosize = 32; > > port->iotype = UPIO_MEM32; > > port->flags = UPF_FIXED_PORT; > > - port->line = 0; /* single port: force line number > > to 0 */ > > + port->line = pdev->id; > > > > port->irq = irq->start; > > port->irqflags = 0; > > -- > > 2.11.0 > > >
Hi Miquel, On ven., oct. 13 2017, Miquel RAYNAL <miquel.raynal@free-electrons.com> wrote: > Hi Gregory, > > On Fri, 13 Oct 2017 11:40:32 +0200 > Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > >> Hi Miquel, >> >> On ven., oct. 13 2017, Miquel Raynal >> <miquel.raynal@free-electrons.com> wrote: >> >> > - port = &mvebu_uart_ports[0]; >> > + /* Assume that all UART ports have a DT alias or none has >> > */ >> > + id = of_alias_get_id(pdev->dev.of_node, "serial"); >> >> You stil need to check the pdev->dev.of_node before calling >> of_alias_get_id. > > Thanks for your feedback but I don't think we still need to check it, > because the of_alias_get_id() function will either return -ENOSYS if > CONFIG_OF is not defined, or -ENODEV if the node is not found. As the > function does not dereference pdev->dev.of_node at any moment but > instead compares the value with another pointer, I think this call is > safe like this. Fair enough, I expected that the pointer was dereferenced but I was wrong, was so no need for a new version. Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Thanks, Gregory > > Thanks, > Miquèl > >> >> Once it will be fixed, you can add: >> >> Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> >> >> Thanks, >> >> Gregory >> >> >> > + if (!pdev->dev.of_node || id < 0) >> > + pdev->id = uart_num_counter++; >> > + else >> > + pdev->id = id; >> > + >> > + if (pdev->id >= MVEBU_NR_UARTS) { >> > + dev_err(&pdev->dev, "cannot have more than %d UART >> > ports\n", >> > + MVEBU_NR_UARTS); >> > + return -EINVAL; >> > + } >> > + >> > + port = &mvebu_uart_ports[pdev->id]; >> > >> > spin_lock_init(&port->lock); >> > >> > @@ -572,7 +588,7 @@ static int mvebu_uart_probe(struct >> > platform_device *pdev) port->fifosize = 32; >> > port->iotype = UPIO_MEM32; >> > port->flags = UPF_FIXED_PORT; >> > - port->line = 0; /* single port: force line number >> > to 0 */ >> > + port->line = pdev->id; >> > >> > port->irq = irq->start; >> > port->irqflags = 0; >> > -- >> > 2.11.0 >> > >> > > > > -- > Miquel Raynal, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
Hi Miquel, On ven., oct. 13 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Add the missing clock property to armada-3700 UART node. > > This clock will be used to derive the prescaler value to comply with > the requested baudrate. > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Applied on mvebu/dt64 Thanks, Gregory > --- > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > index b554cdaf5e53..a36d667f770e 100644 > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > @@ -135,6 +135,7 @@ > uart0: serial@12000 { > compatible = "marvell,armada-3700-uart"; > reg = <0x12000 0x200>; > + clocks = <&xtalclk>; > interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; > status = "disabled"; > }; > -- > 2.11.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Miquel, On ven., oct. 13 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Add a node in Armada 37xx DTSI file for the second UART, with a > different compatible due to its extended IP which has some > differences with the first UART already in place. > > Make use of this commit to also fully describe the first port and > use the same clear and named interrupt bindings for both ports. > > The standard UART (UART0) uses level-interrupts while the extended > UART (UART1) uses edge-triggered interrupts. > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Applied on mvebu/dt64 Thanks, Gregory > --- > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > index a36d667f770e..72b68f23c001 100644 > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi > @@ -55,6 +55,7 @@ > > aliases { > serial0 = &uart0; > + serial1 = &uart1; > }; > > cpus { > @@ -136,7 +137,22 @@ > compatible = "marvell,armada-3700-uart"; > reg = <0x12000 0x200>; > clocks = <&xtalclk>; > - interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; > + interrupts = > + <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "uart-sum", "uart-tx", "uart-rx"; > + status = "disabled"; > + }; > + > + uart1: serial@12200 { > + compatible = "marvell,armada-3700-uart-ext"; > + reg = <0x12200 0x30>; > + clocks = <&xtalclk>; > + interrupts = > + <GIC_SPI 30 IRQ_TYPE_EDGE_RISING>, > + <GIC_SPI 31 IRQ_TYPE_EDGE_RISING>; > + interrupt-names = "uart-tx", "uart-rx"; > status = "disabled"; > }; > > -- > 2.11.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Miquel, On ven., oct. 13 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Enable Armada-3720-DB second UART port by adding the corresponding > device tree node in the board DTS and enabling it. > > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> > Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com> > --- Applied on mvebu/dt64 Thanks, Gregory > arch/arm64/boot/dts/marvell/armada-3720-db.dts | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts > index 9df0f06ce607..15713c19b3d0 100644 > --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts > +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts > @@ -216,7 +216,7 @@ > > /* > * Exported on the micro USB connector CON30(V2.0)/CON32(V1.4) through > - * an FTDI > + * an FTDI (also on CON24(V2.0)/CON26(V1.4)). > */ > &uart0 { > pinctrl-names = "default"; > @@ -224,6 +224,13 @@ > status = "okay"; > }; > > +/* CON26(V2.0)/CON28(V1.4) */ > +&uart1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart2_pins>; > + status = "okay"; > +}; > + > /* CON27(V2.0)/CON29(V1.4) */ > &usb2 { > status = "okay"; > -- > 2.11.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Miquel, On ven., oct. 13 2017, Miquel Raynal <miquel.raynal@free-electrons.com> wrote: > Fill ESPRESSObin uart0 node with pinctrl information like in the > Armada-3720-DB device tree (which uses the same node). > > Also explain how to enable the second UART port available on the > headers. This second port is not enabled by default because both > headers are dedicated to expose general purpose pins and remapping > some of them to use the second UART would break existing users. > > Suggested-by: László ÁSHIN <laszlo@ashin.hu> > Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com> Applied on mvebu/dt64 Thanks, Gregory > --- > > Changes since v1: comment about UART1 node. > > arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > index 2ce52ba74f73..bdfb5553ddb5 100644 > --- a/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > +++ b/arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts > @@ -98,9 +98,21 @@ > > /* Exported on the micro USB connector J5 through an FTDI */ > &uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart1_pins>; > status = "okay"; > }; > > +/* > + * Connector J17 and J18 expose a number of different features. Some pins are > + * multiplexed. This is the case for instance for the following features: > + * - UART1 (pin 24 = RX, pin 26 = TX). See armada-3720-db.dts for an example of > + * how to enable it. Beware that the signals are 1.8V TTL. > + * - I2C > + * - SPI > + * - MMC > + */ > + > /* J7 */ > &usb3 { > status = "okay"; > -- > 2.11.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel