ARM: support for ICP DAS LP-8x4x

 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/Makefile                         |   3 +
 arch/arm/boot/dts/pxa27x-lp8x4x-i105.dts           |  50 ++++
 arch/arm/boot/dts/pxa27x-lp8x4x.dts                | 259 +++++++++++++++++++++
 arch/arm/configs/lp8x4x_defconfig                  | 176 ++++++++++++++

Quoted 8 lines · On Tuesday 15 December 2015 19:27:50 Sergei Ianovich wrote:

?.../devicetree/bindings/vendor-prefixes.txt????????|???1 +
?arch/arm/boot/dts/Makefile?????????????????????????|???3 +
?arch/arm/boot/dts/pxa27x-lp8x4x-i105.dts???????????|??50 ++++
?arch/arm/boot/dts/pxa27x-lp8x4x.dts????????????????| 259

+++++++++++++++++++++
?arch/arm/configs/lp8x4x_defconfig??????????????????| 176
++++++++++++++

I had not noticed earlier that you are adding a new defconfig file. PXA is
already the platform with the most defconfig files, and I'd rather like
to see that reduced than increased.

Is there a chance you could merge this one with some of the existing files
into one configuration that handles them all?

Quoted 16 lines · Arnd Bergmann

Quoted 8 lines · On Tuesday 15 December 2015 19:27:50 Sergei Ianovich wrote:

 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/Makefile                         |   3 +
 arch/arm/boot/dts/pxa27x-lp8x4x-i105.dts           |  50 ++++
 arch/arm/boot/dts/pxa27x-lp8x4x.dts                | 259
+++++++++++++++++++++
 arch/arm/configs/lp8x4x_defconfig                  | 176

++++++++++++++

I had not noticed earlier that you are adding a new defconfig file. PXA is
already the platform with the most defconfig files, and I'd rather like
to see that reduced than increased.

Quoted 3 lines

Is there a chance you could merge this one with some of the existing files
into one configuration that handles them all?

There are several board-specific devices on LP8x4x: custom FPGA, custom UART, custom IRQ on FPGA, custom parallel bus for industrial IO. The defconfig file could alert potential users to this fact. If this is not a sufficient reason to have a defconfig file, it can be dropped.

I use the full .config anyway

Quoted 8 lines · On Tuesday 15 December 2015 19:42:07 Sergei Ianovich wrote:

There are several board-specific devices on LP8x4x: custom FPGA, custom
UART, custom IRQ on FPGA, custom parallel bus for industrial IO. The defconfig file could alert potential users to this fact. If this is not
a sufficient reason to have a defconfig file, it can be dropped.

I use the full .config anyway

I would like those drivers to be enabled in some defconfig, so we get compile-time coverage, but we generally stopped having one-config-per- board
files.

Maybe we can have a pxa_defconfig file that enables lots of boards and then we remove the individual configs? We don't have to remove them all at once, but it would make me very happy if we could at least kill off some of the ones that are not used regularly.

Quoted 15 lines · Arnd Bergmann

Quoted 8 lines · On Tuesday 15 December 2015 19:42:07 Sergei Ianovich wrote:

There are several board-specific devices on LP8x4x: custom FPGA, custom
UART, custom IRQ on FPGA, custom parallel bus for industrial IO. The defconfig file could alert potential users to this fact. If this is not
a sufficient reason to have a defconfig file, it can be dropped.

I use the full .config anyway

I would like those drivers to be enabled in some defconfig, so we get compile-time coverage, but we generally stopped having one-config-per- board
files.

Quoted 4 lines

Maybe we can have a pxa_defconfig file that enables lots of boards and then we remove the individual configs? We don't have to remove them all at once, but it would make me very happy if we could at least kill off some of the ones that are not used regularly.

lp8x4x seems to be the first pxa board which requires DT.

We can create pxa27x-dt_defconfig which selects:
1. PXA27X_DT
2. PXA_FB and 8250_PXA to enable console
3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC

4. all optional pxa stuff as modules
5. all stuff on supported pxa boards as modules

If #5 is extended when support for new boards is added, it should be possible to run any supported pxa27x board with pxa27x_defconfig.

?

Quoted 4 lines

Quoted 4 lines

Maybe we can have a pxa_defconfig file that enables lots of boards and then we remove the individual configs? We don't have to remove them all at once, but it would make me very happy if we could at least kill off some of the ones that are not used regularly.

Yeah, I'd be happy too, that would simplify also my life.

Actually I was thinking of 2 defconfigs :
?- one for platform_data based boards pxa2xx
?- one for DT only boards pxa2xx

Quoted 1 line

lp8x4x seems to be the first pxa board which requires DT.

Most probably AFAIK.

Quoted 4 lines

We can create pxa27x-dt_defconfig which selects:
1. PXA27X_DT
2. PXA_FB and 8250_PXA to enable console
3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC

If you put MMC_PXA, one can argue why not any MTD device used on pxa devices,
such as pxa2xx-flash or docg3/docg4, etc ... I won't argue, I'm just pondering
about the right choice.

Quoted 5 lines

4. all optional pxa stuff as modules
5. all stuff on supported pxa boards as modules

If #5 is extended when support for new boards is added, it should be possible to run any supported pxa27x board with pxa27x_defconfig.

I'm very eager to see a patch on that. I can feed my Jenkins with it, it would
greatly help me catch issues earlier. Moreover it would for free test it on
lubbock, mainstone and mioa701. If there was one also for pxa3xx, I would launch
it on zylonite cm-x300 ... (that's a bonus, I know :))

Quoted 6 lines · Robert Jarzmik

Quoted 5 lines

4. all optional pxa stuff as modules
5. all stuff on supported pxa boards as modules

If #5 is extended when support for new boards is added, it should be possible to run any supported pxa27x board with pxa27x_defconfig.

Quoted 8 lines

I'm very eager to see a patch on that. I can feed my Jenkins with it, it would
greatly help me catch issues earlier. Moreover it would for free test it on
lubbock, mainstone and mioa701. If there was one also for pxa3xx, I would launch
it on zylonite cm-x300 ... (that's a bonus, I know :))

Updated plan:
1. MACH_PXA27X_DT
2. PXA_FB and 8250_PXA to enable console
3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC
3.1. MTD, MTD_CFI, MTD_PHYSMAP_OF and JFFS2_FS to enable boot from MTD
4. all optional pxa stuff as modules
5. all stuff on supported pxa boards as modules
6. supported boards should boot the kernel built with pxa27x-dt_defconfig after make olddefconfig

It is probably a good idea to put this plan somewhere in Documentation/ and to have a comment about that in the defconfig itself.

Is the plan acceptable?

Quoted 4 lines · On Tuesday 15 December 2015 21:50:13 Sergei Ianovich wrote:

On Tue, 2015-12-15 at 19:06 +0100, Robert Jarzmik wrote:
Updated plan:
1. MACH_PXA27X_DT
2. PXA_FB and 8250_PXA to enable console

Quoted 2 lines

3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC
3.1. MTD, MTD_CFI, MTD_PHYSMAP_OF and JFFS2_FS to enable boot from MTD

Quoted 9 lines

4. all optional pxa stuff as modules
5. all stuff on supported pxa boards as modules
6. supported boards should boot the kernel built with pxa27x-dt_defconfig after make olddefconfig

It is probably a good idea to put this plan somewhere in Documentation/ and to have a comment about that in the defconfig itself.

Is the plan acceptable?

Sounds good to me.

Quoted 5 lines · Arnd Bergmann

Quoted 4 lines · On Tuesday 15 December 2015 21:50:13 Sergei Ianovich wrote:

On Tue, 2015-12-15 at 19:06 +0100, Robert Jarzmik wrote:
Updated plan:
1. MACH_PXA27X_DT
2. PXA_FB and 8250_PXA to enable console

What about gpio-pxa ? And maybe pinctrl-pxa27x (only in linux-next by now) ? Can a pxafb work without gpio-pxa ? Also usually for pxafb you have pwm and pwm_bl to see something.

Quoted 2 lines

Quoted 2 lines

3. MMC, MMC_PXA and EXT4_FS to enable boot from MMC
3.1. MTD, MTD_CFI, MTD_PHYSMAP_OF and JFFS2_FS to enable boot from MTD

Ok. Is pxa2xx-pcmcia out of scope ? I seem to remember we have several boards where pcmcia is the rootfs (even if that is a small mess right now I still have to work on). Or do we consider pcmcia as obsolete ?

I would like those drivers to be enabled in some defconfig, so we get compile-time coverage, but we generally stopped having one-config-per-board files.

Maybe we can have a pxa_defconfig file that enables lots of boards and then we remove the individual configs? We don't have to remove them all at once, but it would make me very happy if we could at least kill off some of the ones that are not used regularly.

ICP DAS calls LP-8x4x 'programmable automation controller'. It is an industrial computer based on PXA270 SoC. They ship it with a 2.6.19 kernel and proprietary kernel module and userspace library to access its industrial IO.

arch/arm/mach-pxa/lp8x4x.c new file +13 -0

+static unsigned long lp8x4x_pin_config[] = {
+       /* MMC */
+       GPIO32_MMC_CLK,
+       GPIO112_MMC_CMD,
+       GPIO92_MMC_DAT_0,
+       GPIO109_MMC_DAT_1,
+       GPIO110_MMC_DAT_2,
+       GPIO111_MMC_DAT_3,
+
+       /* USB Host Port 1 */
+       GPIO88_USBH1_PWR,
+       GPIO89_USBH1_PEN,
+};

Argh! Now you're adding another user for a legacy custom pin control implementation. But if noone is going to modernize PXA2xx what can we do :-/

Quoted 3 lines · Linus Walleij

Argh! Now you're adding another user for a legacy custom pin control implementation. But if noone is going to modernize PXA2xx what can we do :-/

I've rewritten the device to use device tree and posted patch series for that.

Quoted 4 lines · Sergei Ianovich

ICP DAS calls LP-8x4x 'programmable automation controller'. It is an industrial computer based on PXA270 SoC. They ship it with a 2.6.19 kernel and proprietary kernel module and userspace library to access its industrial IO.

OK... so we only have device tree support for PXA3xx and noone is working on PXA2xx. And now we pile up some more legacy code. Such is life. But this:

Quoted 13 lines

arch/arm/mach-pxa/lp8x4x.c new file +13 -0

+static unsigned long lp8x4x_pin_config[] = {
+       /* MMC */
+       GPIO32_MMC_CLK,
+       GPIO112_MMC_CMD,
+       GPIO92_MMC_DAT_0,
+       GPIO109_MMC_DAT_1,
+       GPIO110_MMC_DAT_2,
+       GPIO111_MMC_DAT_3,
+
+       /* USB Host Port 1 */
+       GPIO88_USBH1_PWR,
+       GPIO89_USBH1_PEN,
+};

(...)

arch/arm/mach-pxa/lp8x4x.c line 416

Quoted 3 lines

arch/arm/mach-pxa/lp8x4x.c new file +3 -0

+static void __init lp8x4x_init(void)
+{
+       pxa2xx_mfp_config(ARRAY_AND_SIZE(lp8x4x_pin_config));

(...)

Argh! Now you're adding another user for a legacy custom pin control implementation. But if noone is going to modernize PXA2xx what can we do :-/

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 3 lines · Linus Walleij

Argh! Now you're adding another user for a legacy custom pin control implementation. But if noone is going to modernize PXA2xx what can we do :-/

I tried a ~year ago, but it's not so trivial. PXA2xx has no separate pin control module, it's
highly integrated into GPIO controller. I've asked the maillist what should I do for that case, but AFAIR no one answered.

Argh! Now you're adding another user for a legacy custom pin control implementation. But if noone is going to modernize PXA2xx what can we do :-/

The patch adds support for 3 additional LP-8x4x built-in serial ports.

The device can also host up to 8 extension cards with 4 serial ports on each card for a total of 35 ports. However, I don't have the hardware to test extension cards, so they are not supported, yet.

Signed-off-by Sergei Ianovich <[email protected]>

---
 arch/arm/configs/lp8x4x_defconfig       |   1 +
 arch/arm/mach-pxa/include/mach/lp8x4x.h |   6 ++
 drivers/tty/serial/8250/8250_lp8x4x.c   | 181 ++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig         |  11 ++
 drivers/tty/serial/8250/Makefile        |   1 +
 5 files changed, 200 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_lp8x4x.c

arch/arm/configs/lp8x4x_defconfig +1 -0

index 03c85bc..d297a67 100644
@@ -1141,6 +1141,7 @@ CONFIG_SERIAL_8250_NR_UARTS=36
 CONFIG_SERIAL_8250_RUNTIME_UARTS=36
 CONFIG_SERIAL_8250_EXTENDED=y
 CONFIG_SERIAL_8250_MANY_PORTS=y
+CONFIG_SERIAL_8250_LP8X4X=m
 CONFIG_SERIAL_8250_SHARE_IRQ=y
 # CONFIG_SERIAL_8250_DETECT_IRQ is not set
 # CONFIG_SERIAL_8250_RSA is not set

arch/arm/mach-pxa/include/mach/lp8x4x.h +6 -0

index a49df22..4d5474e 100644
@@ -50,6 +50,12 @@
 #define LP8X4X_CLRFALLINT	LP8X4X_P2V(0x1700901a)
 #define LP8X4X_RWRTC		LP8X4X_P2V(0x1700901c)
 #define LP8X4X_SRAMBANK		0x1700901e
+#define LP8X4X_TTYS0_QUIRK	0x17009030
+#define LP8X4X_TTYS1_QUIRK	0x17009032
+#define LP8X4X_TTYS2_QUIRK	0x17009034
+#define LP8X4X_TTYS0_IOMEM	0x17009050
+#define LP8X4X_TTYS1_IOMEM	0x17009060
+#define LP8X4X_TTYS2_IOMEM	0x17009070
 #define LP8X4X_SRAM		0x1700a000
 
 /* board specific IRQs */

drivers/tty/serial/8250/8250_lp8x4x.c new file +37 -0

new file mode 100644
index 0000000..27b01f0b
@@ -0,0 +1,181 @@
+/*  linux/drivers/tty/serial/8250/8250_lp8x4x.c
+ *
+ *  Support for 16550 serial ports on ICP DAS LP-8x4x
+ *
+ *  Copyright (C) 2013 Sergei Ianovich <[email protected]>
+ *  Framework taken from linux/drivers/tty/serial/8250/8250_accent.c
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/delay.h>
+#include <linux/serial_8250.h>
+#include <mach/lp8x4x.h>
+#include <linux/io.h>
+
+#define QUIRK_PORT(_base, _irq)					\
+	{							\
+		.iobase		= _base,			\
+		.membase	= (void *) _base,		\
+		.mapbase	= _base,			\
+		.irq		= _irq,				\
+		.uartclk	= 14745600,			\
+		.regshift	= 1,				\
+		.iotype		= UPIO_MEM,			\
+		.flags		= UPF_IOREMAP,			\
+		.set_termios	= lp8x4x_set_termios,		\
+		.serial_in	= lp8x4x_serial_in,		\
+		.serial_out	= lp8x4x_serial_out,		\
+	}
+
+static void lp8x4x_set_termios(struct uart_port *port,
+		struct ktermios *termios, struct ktermios *old)
+{

drivers/tty/serial/8250/8250_lp8x4x.c new file +71 -0

+}
+
+static unsigned int lp8x4x_serial_in(struct uart_port *p, int offset)
+{
+	unsigned int b;
+	udelay(30);
+	offset = offset << p->regshift;
+	b = readb(p->membase + offset);
+	return b;
+}
+
+static void lp8x4x_serial_out(struct uart_port *p, int offset, int value)
+{
+	offset = offset << p->regshift;
+	writeb(value, p->membase + offset);
+}
+
+static struct plat_serial8250_port lp8x4x_data[] = {
+	QUIRK_PORT(LP8X4X_TTYS0_IOMEM, LP8X4X_TTYS0_IRQ),
+	QUIRK_PORT(LP8X4X_TTYS1_IOMEM, LP8X4X_TTYS1_IRQ),
+	QUIRK_PORT(LP8X4X_TTYS2_IOMEM, LP8X4X_TTYS2_IRQ),
+	{ },
+};
+
+/* Total number of ports can be 35. The first 3 ports are on
+ * the device, the rest are on extension slots. Only the first 3
+ * require termios quirk */
+#define LP8X4X_QUIRK_PORTS	3
+
+static unsigned int extra_mem[LP8X4X_QUIRK_PORTS] = {
+	LP8X4X_TTYS0_QUIRK,
+	LP8X4X_TTYS1_QUIRK,
+	LP8X4X_TTYS2_QUIRK
+};
+
+static int request_and_remap(int i)
+{
+	if (!request_mem_region(extra_mem[i], 1, "serial"))
+		return -EBUSY;
+
+	lp8x4x_data[i].private_data = ioremap(extra_mem[i], 1);
+	if (lp8x4x_data[i].private_data)
+		return 0;
+
+	release_mem_region(extra_mem[i], 1);
+	return -ENODEV;
+}
+
+static void release_and_unmap(int i)
+{
+	iounmap((void *) lp8x4x_data[i].private_data);
+	release_mem_region(extra_mem[i], 1);
+}
+
+static struct platform_device lp8x4x_device = {
+	.name			= "serial8250",
+	.id			= PLAT8250_DEV_ACCENT,
+	.dev			= {
+		.platform_data	= lp8x4x_data,
+	},
+};
+
+static int __init lp8x4x_init(void)
+{
+	int i = 0;
+	int err = 0;
+
+	for (i = 0; i < LP8X4X_QUIRK_PORTS; i++) {
+		err = request_and_remap(i);
+		if (err == 0)
+			continue;

Quoted 23 lines · Sergei Ianovich

The patch adds support for 3 additional LP-8x4x built-in serial ports.

The device can also host up to 8 extension cards with 4 serial ports on each card for a total of 35 ports. However, I don't have the hardware to test extension cards, so they are not supported, yet.

arch/arm/mach-pxa/include/mach/lp8x4x.h +10 -0

@@ -50,6 +50,12 @@
 #define LP8X4X_CLRFALLINT	LP8X4X_P2V(0x1700901a)
 #define LP8X4X_RWRTC		LP8X4X_P2V(0x1700901c)
 #define LP8X4X_SRAMBANK		0x1700901e
+#define LP8X4X_TTYS0_QUIRK	0x17009030
+#define LP8X4X_TTYS1_QUIRK	0x17009032
+#define LP8X4X_TTYS2_QUIRK	0x17009034
+#define LP8X4X_TTYS0_IOMEM	0x17009050
+#define LP8X4X_TTYS1_IOMEM	0x17009060
+#define LP8X4X_TTYS2_IOMEM	0x17009070
+
+static void lp8x4x_set_termios(struct uart_port *port,
+		struct ktermios *termios, struct ktermios *old)
+{

<snip>

arch/arm/mach-pxa/include/mach/lp8x4x.h

Quoted 11 lines

arch/arm/mach-pxa/include/mach/lp8x4x.h +11 -0

+static int request_and_remap(int i)
+{
+	if (!request_mem_region(extra_mem[i], 1, "serial"))
+		return -EBUSY;
+
+	lp8x4x_data[i].private_data = ioremap(extra_mem[i], 1);
+	if (lp8x4x_data[i].private_data)
+		return 0;
+
+	release_mem_region(extra_mem[i], 1);
+	return -ENODEV;

Instead of registering a platform device for serial8250 here, you need to simply register a platform driver for something like lp8x4x_serial. The platform code will to registers the platform devices for it. You will use serial8250_register_8250_port() in your probe to register a new port. Use 8250_em.c and 8250_dw.c under drivers/tty/serial/8250/ as an example.

You don't need to introduce plat_serial8250_port so you won't need the QUIRK_PORT stuff. You deliver the iomem and the irq as normal resources that the driver uses when you create the platform device. That of course also means the driver does not need to care about the instances. The platform code will generate a platform device for as many ports you have and set to resources accordingly.

Quoted 20 lines · Heikki Krogerus

drivers/tty/serial/8250/8250_lp8x4x.c line 142

Quoted 11 lines · Sergei Ianovich

drivers/tty/serial/8250/8250_lp8x4x.c new file +11 -0

+static int request_and_remap(int i)
+{
+	if (!request_mem_region(extra_mem[i], 1, "serial"))
+		return -EBUSY;
+
+	lp8x4x_data[i].private_data = ioremap(extra_mem[i], 1);
+	if (lp8x4x_data[i].private_data)
+		return 0;
+
+	release_mem_region(extra_mem[i], 1);
+	return -ENODEV;

Instead of registering a platform device for serial8250 here, you need to simply register a platform driver for something like lp8x4x_serial. The platform code will to registers the platform devices for it. You will use serial8250_register_8250_port() in your probe to register a new port. Use 8250_em.c and 8250_dw.c under drivers/tty/serial/8250/ as an example.

Quoted 6 lines

You don't need to introduce plat_serial8250_port so you won't need the QUIRK_PORT stuff. You deliver the iomem and the irq as normal resources that the driver uses when you create the platform device. That of course also means the driver does not need to care about the instances. The platform code will generate a platform device for as many ports you have and set to resources accordingly.

8250_core.c doesn't use platform infrastructure to request and map IO memory. There will be a conflict (and an error in serial8250_request_std_resource()), if main IO memory is requested by the platform device, won't it?

drivers/tty/serial/8250/8250_lp8x4x.c line 153

Quoted 3 lines · Sergei Ianovich

drivers/tty/serial/8250/8250_lp8x4x.c new file +3 -0

+static struct platform_device lp8x4x_device = {
+	.name			= "serial8250",
+	.id			= PLAT8250_DEV_ACCENT,

You should not re-use the enum value here. The enum is designed as a method to provide the platform devices with a unique id, not as a means to identify the manufacturer or anything else like that.

Quoted 8 lines · Russell King - ARM Linux

drivers/tty/serial/8250/8250_lp8x4x.c line 153

Quoted 3 lines · Sergei Ianovich

drivers/tty/serial/8250/8250_lp8x4x.c new file +3 -0

+static struct platform_device lp8x4x_device = {
+	.name			= "serial8250",
+	.id			= PLAT8250_DEV_ACCENT,

You should not re-use the enum value here. The enum is designed as a method to provide the platform devices with a unique id, not as a means to identify the manufacturer or anything else like that.

Should I add a new enum?

Add a new enum.