Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1483220 > unrolled thread
| Started by | Zhichang Yuan <yuanzhichang@hisilicon.com> |
|---|---|
| First post | 2016-09-14 14:00 +0200 |
| Last post | 2016-09-21 21:40 +0200 |
| Articles | 13 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH V3 0/4] ARM64 LPC: legacy ISA I/O support Zhichang Yuan <yuanzhichang@hisilicon.com> - 2016-09-14 14:00 +0200
[PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced Zhichang Yuan <yuanzhichang@hisilicon.com> - 2016-09-14 14:10 +0200
Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced Arnd Bergmann <arnd@arndb.de> - 2016-09-14 14:30 +0200
Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced "zhichang.yuan" <yuanzhichang@hisilicon.com> - 2016-09-14 16:20 +0200
Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced Arnd Bergmann <arnd@arndb.de> - 2016-09-14 16:30 +0200
Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced zhichang <zhichang.yuan02@gmail.com> - 2016-09-18 05:40 +0200
Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced zhichang <zhichang.yuan02@gmail.com> - 2016-09-21 11:30 +0200
[PATCH V3 4/4] ARM64 LPC: support earlycon for UART connected to LPC Zhichang Yuan <yuanzhichang@hisilicon.com> - 2016-09-14 14:10 +0200
[PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count Zhichang Yuan <yuanzhichang@hisilicon.com> - 2016-09-14 14:10 +0200
Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count Arnd Bergmann <arnd@arndb.de> - 2016-09-14 14:30 +0200
Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count "zhichang.yuan" <yuanzhichang@hisilicon.com> - 2016-09-14 17:10 +0200
Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count Arnd Bergmann <arnd@arndb.de> - 2016-09-14 23:40 +0200
Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count Arnd Bergmann <arnd@arndb.de> - 2016-09-21 21:40 +0200
| From | Zhichang Yuan <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 14:00 +0200 |
| Subject | [PATCH V3 0/4] ARM64 LPC: legacy ISA I/O support |
| Message-ID | <shh18-no-5@gated-at.bofh.it> |
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
This patch supports the 16550 compatible UART attached to the Low-Pin-Count
interface mplemented on Hisilicon Hip06 SoC. The periperals attached this LPC
include UART, BT, KCS, and so on.
-----------
| LPC host|
| |
-----------
|
_____________V_______________LPC
| |
V V
----------- ------------
| UART | | BT(ipmi)|
----------- ------------
When master accesses those periperals beneath the Hip06 LPC, a specific LPC
driver is needed to make LPC host generate the standard LPC I/O cycles with
the target periperals'I/O port addresses. But on curent arm64 world, there is
no real I/O accesses. All the I/O operations through in/out pair are based on
MMIO which is not satisfied the I/O mechanism on Hip06 LPC.
To solve this issue and keep the relevant existing peripherals' driver
unchanged, this patch set redefines the in/out pair to support both the IO
operations for Hip06 LPC and the original MMIO. The way specific to Hip06 is
named as indirect-IO in this patchset.
This patch set is built based on mainline v4.8-rc6;
Changes from V2:
- Support the PIO retrieval from the linux PIO generated by
pci_address_to_pio. This method replace the 4K PIO reservation in V2;
- Support the flat-tree earlycon;
- Some revises based on Arnd's remarks;
- Make sure the linux PIO range allocated to Hip06 LPC peripherals starts
from non-ZERO;
Changes from V1:
- Support the ACPI LPC device;
- Optimize the dts LPC driver in ISA compatible mode;
- Reserve the IO range below 4K in avoid the possible conflict with PCI host
IO ranges;
- Support the LPC uart and relevant earlycon;
Signed-off-by: Zhichang Yuan <yuanzhichang@hisilicon.com>
zhichang.yuan (4):
ARM64 LPC: Indirect ISA port IO introduced
ARM64 LPC: LPC driver implementation on Hip06
ARM64 LPC: support serial based on low-pin-count
ARM64 LPC: support earlycon for UART connected to LPC
.../arm/hisilicon/hisilicon-low-pin-count.txt | 35 +
.../devicetree/bindings/serial/hisi-lpc-uart.txt | 60 ++
arch/arm64/Kconfig | 6 +
arch/arm64/include/asm/io.h | 90 +++
drivers/bus/Kconfig | 8 +
drivers/bus/Makefile | 2 +
drivers/bus/extio.c | 66 ++
drivers/bus/hisi_lpc.c | 766 +++++++++++++++++++++
drivers/of/address.c | 9 +
drivers/tty/serial/8250/8250_early.c | 26 +-
drivers/tty/serial/8250/8250_hisi_lpc.c | 171 +++++
drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
include/linux/extio.h | 49 ++
14 files changed, 1296 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
create mode 100644 Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
create mode 100644 drivers/bus/extio.c
create mode 100644 drivers/bus/hisi_lpc.c
create mode 100644 drivers/tty/serial/8250/8250_hisi_lpc.c
create mode 100644 include/linux/extio.h
--
1.9.1
[toc] | [next] | [standalone]
| From | Zhichang Yuan <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 14:10 +0200 |
| Subject | [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <shhaN-GA-1@gated-at.bofh.it> |
| In reply to | #1483220 |
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
For arm64, there is no I/O space as other architectural platforms, such as
X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
such as Hip06, when accessing some legacy ISA devices connected to LPC, those
known port addresses are used to control the corresponding target devices, for
example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
normal MMIO mode in using.
To drive these devices, this patch introduces a method named indirect-IO.
In this method the in/out pair in arch/arm64/include/asm/io.h will be
redefined. When upper layer drivers call in/out with those known legacy port
addresses to access the peripherals, the hooking functions corrresponding to
those target peripherals will be called. Through this way, those upper layer
drivers which depend on in/out can run on Hip06 without any changes.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
arch/arm64/Kconfig | 6 +++
arch/arm64/include/asm/io.h | 90 +++++++++++++++++++++++++++++++++++++++++++++
drivers/bus/extio.c | 66 +++++++++++++++++++++++++++++++++
include/linux/extio.h | 49 ++++++++++++++++++++++++
4 files changed, 211 insertions(+)
create mode 100644 drivers/bus/extio.c
create mode 100644 include/linux/extio.h
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index bc3f00f..9579479 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -161,6 +161,12 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
config ARCH_MMAP_RND_COMPAT_BITS_MAX
default 16
+config ARM64_INDIRECT_PIO
+ def_bool n
+ help
+ Support to access the ISA I/O devices with the legacy X86 I/O port
+ addresses in some SoCs, such as Hisilicon Hip06.
+
config NO_IOPORT_MAP
def_bool y if !PCI
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 9b6e408..d3acf1f 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -34,6 +34,10 @@
#include <xen/xen.h>
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+#include <linux/extio.h>
+#endif
+
/*
* Generic IO read/write. These perform native-endian accesses.
*/
@@ -142,6 +146,38 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#define writel(v,c) ({ __iowmb(); writel_relaxed((v),(c)); })
#define writeq(v,c) ({ __iowmb(); writeq_relaxed((v),(c)); })
+
+#define BUILDS_RW(bwl, type) \
+static inline void reads##bwl(const volatile void __iomem *addr, \
+ void *buffer, unsigned int count) \
+{ \
+ if (count) { \
+ type *buf = buffer; \
+ \
+ do { \
+ type x = __raw_read##bwl(addr); \
+ *buf++ = x; \
+ } while (--count); \
+ } \
+} \
+ \
+static inline void writes##bwl(volatile void __iomem *addr, \
+ const void *buffer, unsigned int count) \
+{ \
+ if (count) { \
+ const type *buf = buffer; \
+ \
+ do { \
+ __raw_write##bwl(*buf++, addr); \
+ } while (--count); \
+ } \
+}
+
+BUILDS_RW(b, u8)
+#define readsb readsb
+#define writesb writesb
+
+
/*
* I/O port access primitives.
*/
@@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
#define PCI_IOBASE ((void __iomem *)PCI_IO_START)
+
+/*
+ * redefine the in(s)b/out(s)b for indirect-IO.
+ */
+#define inb inb
+static inline u8 inb(unsigned long addr)
+{
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+ if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
+ addr <= arm64_extio_ops->end)
+ return extio_inb(addr);
+#endif
+ return readb(PCI_IOBASE + addr);
+}
+
+
+#define outb outb
+static inline void outb(u8 value, unsigned long addr)
+{
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+ if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
+ addr <= arm64_extio_ops->end)
+ extio_outb(value, addr);
+ else
+#endif
+ writeb(value, PCI_IOBASE + addr);
+}
+
+#define insb insb
+static inline void insb(unsigned long addr, void *buffer, unsigned int count)
+{
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+ if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
+ addr <= arm64_extio_ops->end)
+ extio_insb(addr, buffer, count);
+ else
+#endif
+ readsb(PCI_IOBASE + addr, buffer, count);
+}
+
+#define outsb outsb
+static inline void outsb(unsigned long addr, const void *buffer,
+ unsigned int count)
+{
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+ if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
+ addr <= arm64_extio_ops->end)
+ extio_outsb(addr, buffer, count);
+ else
+#endif
+ writesb(PCI_IOBASE + addr, buffer, count);
+}
+
+
/*
* String version of I/O memory access operations.
*/
diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
new file mode 100644
index 0000000..1e7a9c5
--- /dev/null
+++ b/drivers/bus/extio.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
+ * Author: Zou Rongrong <@huawei.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/io.h>
+
+
+struct extio_ops *arm64_extio_ops;
+
+
+u8 __weak extio_inb(unsigned long addr)
+{
+ return arm64_extio_ops->pfin ?
+ arm64_extio_ops->pfin(arm64_extio_ops->devpara,
+ addr + arm64_extio_ops->ptoffset, NULL,
+ sizeof(u8), 1) : -1;
+}
+
+void __weak extio_outb(u8 value, unsigned long addr)
+{
+ if (!arm64_extio_ops->pfout)
+ return;
+
+ arm64_extio_ops->pfout(arm64_extio_ops->devpara,
+ addr + arm64_extio_ops->ptoffset, &value,
+ sizeof(u8), 1);
+}
+
+
+void __weak extio_insb(unsigned long addr, void *buffer,
+ unsigned int count)
+{
+ if (!arm64_extio_ops->pfin)
+ return;
+
+ arm64_extio_ops->pfin(arm64_extio_ops->devpara,
+ addr + arm64_extio_ops->ptoffset, buffer,
+ sizeof(u8), count);
+}
+
+void __weak extio_outsb(unsigned long addr, const void *buffer,
+ unsigned int count)
+{
+ if (!arm64_extio_ops->pfout)
+ return;
+
+ arm64_extio_ops->pfout(arm64_extio_ops->devpara,
+ addr + arm64_extio_ops->ptoffset, buffer,
+ sizeof(u8), count);
+}
+
+
diff --git a/include/linux/extio.h b/include/linux/extio.h
new file mode 100644
index 0000000..08d1fca
--- /dev/null
+++ b/include/linux/extio.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
+ * Author: Zou Rongrong <@huawei.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LINUX_EXTIO_H
+#define __LINUX_EXTIO_H
+
+
+typedef u64 (*inhook)(void *devobj, unsigned long ptaddr, void *inbuf,
+ size_t dlen, unsigned int count);
+typedef void (*outhook)(void *devobj, unsigned long ptaddr,
+ const void *outbuf, size_t dlen,
+ unsigned int count);
+
+struct extio_ops {
+ unsigned long start;/* inclusive, sys io addr */
+ unsigned long end;/* inclusive, sys io addr */
+ unsigned long ptoffset;/* port Io - system Io */
+
+ inhook pfin;
+ outhook pfout;
+ void *devpara;
+};
+
+
+extern struct extio_ops *arm64_extio_ops;
+
+extern u8 extio_inb(unsigned long addr);
+extern void extio_outb(u8 value, unsigned long addr);
+extern void extio_insb(unsigned long addr, void *buffer, unsigned int count);
+extern void extio_outsb(unsigned long addr, const void *buffer,
+ unsigned int count);
+
+
+#endif /* __LINUX_EXTIO_H*/
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-09-14 14:30 +0200 |
| Subject | Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <shhu9-PJ-9@gated-at.bofh.it> |
| In reply to | #1483225 |
On Wednesday, September 14, 2016 8:15:51 PM CEST Zhichang Yuan wrote:
> From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
>
> For arm64, there is no I/O space as other architectural platforms, such as
> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> known port addresses are used to control the corresponding target devices, for
> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> normal MMIO mode in using.
>
> To drive these devices, this patch introduces a method named indirect-IO.
> In this method the in/out pair in arch/arm64/include/asm/io.h will be
> redefined. When upper layer drivers call in/out with those known legacy port
> addresses to access the peripherals, the hooking functions corrresponding to
> those target peripherals will be called. Through this way, those upper layer
> drivers which depend on in/out can run on Hip06 without any changes.
>
> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
Looks ok overall, but I have a couple of comments for details.
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index bc3f00f..9579479 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -161,6 +161,12 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
> config ARCH_MMAP_RND_COMPAT_BITS_MAX
> default 16
>
> +config ARM64_INDIRECT_PIO
> + def_bool n
'def_bool n' is the same as the shorter and more common 'bool'.
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 9b6e408..d3acf1f 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -34,6 +34,10 @@
>
> #include <xen/xen.h>
>
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> +#include <linux/extio.h>
> +#endif
No need to guard includes with an #ifdef.
> +#define BUILDS_RW(bwl, type) \
> +static inline void reads##bwl(const volatile void __iomem *addr, \
> + void *buffer, unsigned int count) \
> +{ \
> + if (count) { \
> + type *buf = buffer; \
> + \
> + do { \
> + type x = __raw_read##bwl(addr); \
> + *buf++ = x; \
> + } while (--count); \
> + } \
> +} \
> + \
> +static inline void writes##bwl(volatile void __iomem *addr, \
> + const void *buffer, unsigned int count) \
> +{ \
> + if (count) { \
> + const type *buf = buffer; \
> + \
> + do { \
> + __raw_write##bwl(*buf++, addr); \
> + } while (--count); \
> + } \
> +}
> +
> +BUILDS_RW(b, u8)
Why is this in here?
> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>
> +
> +/*
> + * redefine the in(s)b/out(s)b for indirect-IO.
> + */
> +#define inb inb
> +static inline u8 inb(unsigned long addr)
> +{
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
> + addr <= arm64_extio_ops->end)
> + return extio_inb(addr);
> +#endif
> + return readb(PCI_IOBASE + addr);
> +}
> +
Looks ok, but you only seem to do this for the 8-bit
accessors, when it should be done for 16-bit and 32-bit
ones as well for consistency.
> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
> new file mode 100644
> index 0000000..1e7a9c5
> --- /dev/null
> +++ b/drivers/bus/extio.c
> @@ -0,0 +1,66 @@
This is in a globally visible directory
> +
> +struct extio_ops *arm64_extio_ops;
But the identifier uses an architecture specific prefix. Either
move the whole file into arch/arm64, or make the naming so that
it can be used for everything.
> +u8 __weak extio_inb(unsigned long addr)
> +{
> + return arm64_extio_ops->pfin ?
> + arm64_extio_ops->pfin(arm64_extio_ops->devpara,
> + addr + arm64_extio_ops->ptoffset, NULL,
> + sizeof(u8), 1) : -1;
> +}
No need for the __weak attribute, just make sure that the
code is always built-in when needed.
Also, it doesn't seem necessary to have an extern function if
all it does is call the one callback that you have already
checked earlier. Either put it all into the inline
definition in asm/io.h, or put it all into the extern
version like this.
#ifdef CONFIG_ARM64_INDIRECT_PIO /* otherwise use default from asm-generic */
#define inb inb
extern u8 inb(unsigned long addr);
#endif
u8 inb(unsigned long addr)
{
if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
addr <= arm64_extio_ops->end)
arm64_extio_ops->pfin(arm64_extio_ops->devpara,addr + arm64_extio_ops->ptoffset, NULL,sizeof(u8), 1) : -1;
return extio_inb(addr);
}
> +#define inb inb
> +static inline u8 inb(unsigned long addr)
> +{
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
> + addr <= arm64_extio_ops->end)
> + return extio_inb(addr);
> +#endif
> + return readb(PCI_IOBASE + addr);
> +}
> diff --git a/include/linux/extio.h b/include/linux/extio.h
> new file mode 100644
> index 0000000..08d1fca
> --- /dev/null
> +++ b/include/linux/extio.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
> + * Author: Zou Rongrong <@huawei.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __LINUX_EXTIO_H
> +#define __LINUX_EXTIO_H
> +
> +
> +typedef u64 (*inhook)(void *devobj, unsigned long ptaddr, void *inbuf,
> + size_t dlen, unsigned int count);
> +typedef void (*outhook)(void *devobj, unsigned long ptaddr,
> + const void *outbuf, size_t dlen,
> + unsigned int count);
I would drop the typedef and just declare the types directly in the
only place that references them.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | "zhichang.yuan" <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 16:20 +0200 |
| Subject | Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <shjcB-1Ui-29@gated-at.bofh.it> |
| In reply to | #1483234 |
Hi, Arnd
On 2016/9/14 20:24, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 8:15:51 PM CEST Zhichang Yuan wrote:
>> From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
>>
>> For arm64, there is no I/O space as other architectural platforms, such as
>> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
>> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
>> known port addresses are used to control the corresponding target devices, for
>> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
>> normal MMIO mode in using.
>>
>> To drive these devices, this patch introduces a method named indirect-IO.
>> In this method the in/out pair in arch/arm64/include/asm/io.h will be
>> redefined. When upper layer drivers call in/out with those known legacy port
>> addresses to access the peripherals, the hooking functions corrresponding to
>> those target peripherals will be called. Through this way, those upper layer
>> drivers which depend on in/out can run on Hip06 without any changes.
>>
>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>
> Looks ok overall, but I have a couple of comments for details.
>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index bc3f00f..9579479 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -161,6 +161,12 @@ config ARCH_MMAP_RND_COMPAT_BITS_MIN
>> config ARCH_MMAP_RND_COMPAT_BITS_MAX
>> default 16
>>
>> +config ARM64_INDIRECT_PIO
>> + def_bool n
>
> 'def_bool n' is the same as the shorter and more common 'bool'.
Yes. Will modify as bool "access peripherals with legacy I/O port"
>
>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>> index 9b6e408..d3acf1f 100644
>> --- a/arch/arm64/include/asm/io.h
>> +++ b/arch/arm64/include/asm/io.h
>> @@ -34,6 +34,10 @@
>>
>> #include <xen/xen.h>
>>
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> +#include <linux/extio.h>
>> +#endif
>
> No need to guard includes with an #ifdef.
If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
How about removing everything about the configure item "ARM64_INDIRECT_PIO"?
This will make the indirect-IO mechanism global on ARM64.
I worry about this mechanism is not so common, so using "ARM64_INDIRECT_PIO" make this feature optional.
>
>> +#define BUILDS_RW(bwl, type) \
>> +static inline void reads##bwl(const volatile void __iomem *addr, \
>> + void *buffer, unsigned int count) \
>> +{ \
>> + if (count) { \
>> + type *buf = buffer; \
>> + \
>> + do { \
>> + type x = __raw_read##bwl(addr); \
>> + *buf++ = x; \
>> + } while (--count); \
>> + } \
>> +} \
>> + \
>> +static inline void writes##bwl(volatile void __iomem *addr, \
>> + const void *buffer, unsigned int count) \
>> +{ \
>> + if (count) { \
>> + const type *buf = buffer; \
>> + \
>> + do { \
>> + __raw_write##bwl(*buf++, addr); \
>> + } while (--count); \
>> + } \
>> +}
>> +
>> +BUILDS_RW(b, u8)
>
> Why is this in here?
the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
those function needed here....
Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
#ifdef CONFIG_ARM64_INDIRECT_PIO
#define inb inb
extern u8 inb(unsigned long addr);
#define outb outb
extern void outb(u8 value, unsigned long addr);
#define insb insb
extern void insb(unsigned long addr, void *buffer, unsigned int count);
#define outsb outsb
extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
#endif
and definitions of all these functions are in extio.c :
u8 inb(unsigned long addr)
{
if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
arm64_extio_ops->end < addr)
return readb(PCI_IOBASE + addr);
else
return arm64_extio_ops->pfin ?
arm64_extio_ops->pfin(arm64_extio_ops->devpara,
addr + arm64_extio_ops->ptoffset, NULL,
sizeof(u8), 1) : -1;
}
.....
>
>> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>
>> +
>> +/*
>> + * redefine the in(s)b/out(s)b for indirect-IO.
>> + */
>> +#define inb inb
>> +static inline u8 inb(unsigned long addr)
>> +{
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>> + addr <= arm64_extio_ops->end)
>> + return extio_inb(addr);
>> +#endif
>> + return readb(PCI_IOBASE + addr);
>> +}
>> +
>
> Looks ok, but you only seem to do this for the 8-bit
> accessors, when it should be done for 16-bit and 32-bit
> ones as well for consistency.
Hip06 LPC only support 8-bit I/O operations on the designated port.
>
>> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
>> new file mode 100644
>> index 0000000..1e7a9c5
>> --- /dev/null
>> +++ b/drivers/bus/extio.c
>> @@ -0,0 +1,66 @@
>
> This is in a globally visible directory
>
>> +
>> +struct extio_ops *arm64_extio_ops;
>
> But the identifier uses an architecture specific prefix. Either
> move the whole file into arch/arm64, or make the naming so that
> it can be used for everything.
I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
>
>> +u8 __weak extio_inb(unsigned long addr)
>> +{
>> + return arm64_extio_ops->pfin ?
>> + arm64_extio_ops->pfin(arm64_extio_ops->devpara,
>> + addr + arm64_extio_ops->ptoffset, NULL,
>> + sizeof(u8), 1) : -1;
>> +}
>
> No need for the __weak attribute, just make sure that the
> code is always built-in when needed.
>
> Also, it doesn't seem necessary to have an extern function if
> all it does is call the one callback that you have already
> checked earlier. Either put it all into the inline
> definition in asm/io.h, or put it all into the extern
> version like this.
>
> #ifdef CONFIG_ARM64_INDIRECT_PIO /* otherwise use default from asm-generic */
> #define inb inb
> extern u8 inb(unsigned long addr);
> #endif
>
Yes. This is good!
Although the in(s)/out(s) are not inline anymore.
I had applied this way above.
> u8 inb(unsigned long addr)
> {
> if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
> addr <= arm64_extio_ops->end)
> arm64_extio_ops->pfin(arm64_extio_ops->devpara,addr + arm64_extio_ops->ptoffset, NULL,sizeof(u8), 1) : -1;
> return extio_inb(addr);
> }
>
>> +#define inb inb
>> +static inline u8 inb(unsigned long addr)
>> +{
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>> + addr <= arm64_extio_ops->end)
>> + return extio_inb(addr);
>> +#endif
>> + return readb(PCI_IOBASE + addr);
>> +}
>
>> diff --git a/include/linux/extio.h b/include/linux/extio.h
>> new file mode 100644
>> index 0000000..08d1fca
>> --- /dev/null
>> +++ b/include/linux/extio.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
>> + * Author: Zou Rongrong <@huawei.com>
>> + *
>> + * 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.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef __LINUX_EXTIO_H
>> +#define __LINUX_EXTIO_H
>> +
>> +
>> +typedef u64 (*inhook)(void *devobj, unsigned long ptaddr, void *inbuf,
>> + size_t dlen, unsigned int count);
>> +typedef void (*outhook)(void *devobj, unsigned long ptaddr,
>> + const void *outbuf, size_t dlen,
>> + unsigned int count);
>
> I would drop the typedef and just declare the types directly in the
> only place that references them.
>
Ok. Will apply it.
Thanks!
Zhichang
> Arnd
>
> .
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-09-14 16:30 +0200 |
| Subject | Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <shjmi-1XX-47@gated-at.bofh.it> |
| In reply to | #1483359 |
On Wednesday, September 14, 2016 10:16:28 PM CEST zhichang.yuan wrote:
> >
> > No need to guard includes with an #ifdef.
> If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
> extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
There is no problem with making declarations visible for functions that
are not part of the kernel, we do that all the time.
> >> +#define BUILDS_RW(bwl, type) \
> >> +static inline void reads##bwl(const volatile void __iomem *addr, \
> >> + void *buffer, unsigned int count) \
> >> +{ \
> >> + if (count) { \
> >> + type *buf = buffer; \
> >> + \
> >> + do { \
> >> + type x = __raw_read##bwl(addr); \
> >> + *buf++ = x; \
> >> + } while (--count); \
> >> + } \
> >> +} \
> >> + \
> >> +static inline void writes##bwl(volatile void __iomem *addr, \
> >> + const void *buffer, unsigned int count) \
> >> +{ \
> >> + if (count) { \
> >> + const type *buf = buffer; \
> >> + \
> >> + do { \
> >> + __raw_write##bwl(*buf++, addr); \
> >> + } while (--count); \
> >> + } \
> >> +}
> >> +
> >> +BUILDS_RW(b, u8)
> >
> > Why is this in here?
> the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
> to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
>
> It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
> those function needed here....
>
> Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
>
> #ifdef CONFIG_ARM64_INDIRECT_PIO
> #define inb inb
> extern u8 inb(unsigned long addr);
>
> #define outb outb
> extern void outb(u8 value, unsigned long addr);
>
> #define insb insb
> extern void insb(unsigned long addr, void *buffer, unsigned int count);
>
> #define outsb outsb
> extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
> #endif
>
> and definitions of all these functions are in extio.c :
>
> u8 inb(unsigned long addr)
> {
> if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
> arm64_extio_ops->end < addr)
> return readb(PCI_IOBASE + addr);
> else
> return arm64_extio_ops->pfin ?
> arm64_extio_ops->pfin(arm64_extio_ops->devpara,
> addr + arm64_extio_ops->ptoffset, NULL,
> sizeof(u8), 1) : -1;
> }
> .....
Yes, sounds good.
> >> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
> >> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
> >> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
> >>
> >> +
> >> +/*
> >> + * redefine the in(s)b/out(s)b for indirect-IO.
> >> + */
> >> +#define inb inb
> >> +static inline u8 inb(unsigned long addr)
> >> +{
> >> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> >> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
> >> + addr <= arm64_extio_ops->end)
> >> + return extio_inb(addr);
> >> +#endif
> >> + return readb(PCI_IOBASE + addr);
> >> +}
> >> +
> >
> > Looks ok, but you only seem to do this for the 8-bit
> > accessors, when it should be done for 16-bit and 32-bit
> > ones as well for consistency.
> Hip06 LPC only support 8-bit I/O operations on the designated port.
That is an interesting limitation. Maybe still call the extio operations
and have them do WARN_ON_ONCE() instead?
If you get a driver that calls inw/outw on the range that is owned
by the LPC bus, you otherwise get an unhandled page fault in kernel
space, which is not as nice.
> >> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
> >> new file mode 100644
> >> index 0000000..1e7a9c5
> >> --- /dev/null
> >> +++ b/drivers/bus/extio.c
> >> @@ -0,0 +1,66 @@
> >
> > This is in a globally visible directory
> >
> >> +
> >> +struct extio_ops *arm64_extio_ops;
> >
> > But the identifier uses an architecture specific prefix. Either
> > move the whole file into arch/arm64, or make the naming so that
> > it can be used for everything.
>
> I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
Ok, that simplifies it a lot, you can just do everything in asm/io.h then.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | zhichang <zhichang.yuan02@gmail.com> |
|---|---|
| Date | 2016-09-18 05:40 +0200 |
| Subject | Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <siB7r-2Pf-1@gated-at.bofh.it> |
| In reply to | #1483382 |
Hi, Arnd,
On 2016年09月14日 22:23, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 10:16:28 PM CEST zhichang.yuan wrote:
>>>
>>> No need to guard includes with an #ifdef.
>> If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
>> extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
>
> There is no problem with making declarations visible for functions that
> are not part of the kernel, we do that all the time.
>
>>>> +#define BUILDS_RW(bwl, type) \
>>>> +static inline void reads##bwl(const volatile void __iomem *addr, \
>>>> + void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + type x = __raw_read##bwl(addr); \
>>>> + *buf++ = x; \
>>>> + } while (--count); \
>>>> + } \
>>>> +} \
>>>> + \
>>>> +static inline void writes##bwl(volatile void __iomem *addr, \
>>>> + const void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + const type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + __raw_write##bwl(*buf++, addr); \
>>>> + } while (--count); \
>>>> + } \
>>>> +}
>>>> +
>>>> +BUILDS_RW(b, u8)
>>>
>>> Why is this in here?
>> the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
>> to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
>>
>> It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
>> those function needed here....
>>
>> Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
>>
>> #ifdef CONFIG_ARM64_INDIRECT_PIO
>> #define inb inb
>> extern u8 inb(unsigned long addr);
>>
>> #define outb outb
>> extern void outb(u8 value, unsigned long addr);
>>
>> #define insb insb
>> extern void insb(unsigned long addr, void *buffer, unsigned int count);
>>
>> #define outsb outsb
>> extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
>> #endif
>>
>> and definitions of all these functions are in extio.c :
>>
>> u8 inb(unsigned long addr)
>> {
>> if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
>> arm64_extio_ops->end < addr)
>> return readb(PCI_IOBASE + addr);
>> else
>> return arm64_extio_ops->pfin ?
>> arm64_extio_ops->pfin(arm64_extio_ops->devpara,
>> addr + arm64_extio_ops->ptoffset, NULL,
>> sizeof(u8), 1) : -1;
>> }
>> .....
>
> Yes, sounds good.
>
>>>> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>>>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>>>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>>>
>>>> +
>>>> +/*
>>>> + * redefine the in(s)b/out(s)b for indirect-IO.
>>>> + */
>>>> +#define inb inb
>>>> +static inline u8 inb(unsigned long addr)
>>>> +{
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>>>> + addr <= arm64_extio_ops->end)
>>>> + return extio_inb(addr);
>>>> +#endif
>>>> + return readb(PCI_IOBASE + addr);
>>>> +}
>>>> +
>>>
>>> Looks ok, but you only seem to do this for the 8-bit
>>> accessors, when it should be done for 16-bit and 32-bit
>>> ones as well for consistency.
>> Hip06 LPC only support 8-bit I/O operations on the designated port.
>
> That is an interesting limitation. Maybe still call the extio operations
> and have them do WARN_ON_ONCE() instead?
>
> If you get a driver that calls inw/outw on the range that is owned
> by the LPC bus, you otherwise get an unhandled page fault in kernel
> space, which is not as nice.
>
Yes. It probably cause kernel panic.
Will define the extio operations for other IO length and add the corresponding WARNINGS.
Best,
Zhichang
>>>> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
>>>> new file mode 100644
>>>> index 0000000..1e7a9c5
>>>> --- /dev/null
>>>> +++ b/drivers/bus/extio.c
>>>> @@ -0,0 +1,66 @@
>>>
>>> This is in a globally visible directory
>>>
>>>> +
>>>> +struct extio_ops *arm64_extio_ops;
>>>
>>> But the identifier uses an architecture specific prefix. Either
>>> move the whole file into arch/arm64, or make the naming so that
>>> it can be used for everything.
>>
>> I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
>
> Ok, that simplifies it a lot, you can just do everything in asm/io.h then.
>
> Arnd
>
[toc] | [prev] | [next] | [standalone]
| From | zhichang <zhichang.yuan02@gmail.com> |
|---|---|
| Date | 2016-09-21 11:30 +0200 |
| Subject | Re: [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced |
| Message-ID | <sjM0N-7bE-1@gated-at.bofh.it> |
| In reply to | #1483382 |
Hi, Arnd,
On 2016年09月14日 22:23, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 10:16:28 PM CEST zhichang.yuan wrote:
>>>
>>> No need to guard includes with an #ifdef.
>> If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
>> extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
>
> There is no problem with making declarations visible for functions that
> are not part of the kernel, we do that all the time.
>
>>>> +#define BUILDS_RW(bwl, type) \
>>>> +static inline void reads##bwl(const volatile void __iomem *addr, \
>>>> + void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + type x = __raw_read##bwl(addr); \
>>>> + *buf++ = x; \
>>>> + } while (--count); \
>>>> + } \
>>>> +} \
>>>> + \
>>>> +static inline void writes##bwl(volatile void __iomem *addr, \
>>>> + const void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + const type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + __raw_write##bwl(*buf++, addr); \
>>>> + } while (--count); \
>>>> + } \
>>>> +}
>>>> +
>>>> +BUILDS_RW(b, u8)
>>>
>>> Why is this in here?
>> the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
>> to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
>>
>> It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
>> those function needed here....
>>
>> Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
>>
>> #ifdef CONFIG_ARM64_INDIRECT_PIO
>> #define inb inb
>> extern u8 inb(unsigned long addr);
>>
>> #define outb outb
>> extern void outb(u8 value, unsigned long addr);
>>
>> #define insb insb
>> extern void insb(unsigned long addr, void *buffer, unsigned int count);
>>
>> #define outsb outsb
>> extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
>> #endif
>>
>> and definitions of all these functions are in extio.c :
>>
>> u8 inb(unsigned long addr)
>> {
>> if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
>> arm64_extio_ops->end < addr)
>> return readb(PCI_IOBASE + addr);
>> else
>> return arm64_extio_ops->pfin ?
>> arm64_extio_ops->pfin(arm64_extio_ops->devpara,
>> addr + arm64_extio_ops->ptoffset, NULL,
>> sizeof(u8), 1) : -1;
>> }
>> .....
>
> Yes, sounds good.
>
>>>> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>>>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>>>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>>>
>>>> +
>>>> +/*
>>>> + * redefine the in(s)b/out(s)b for indirect-IO.
>>>> + */
>>>> +#define inb inb
>>>> +static inline u8 inb(unsigned long addr)
>>>> +{
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>>>> + addr <= arm64_extio_ops->end)
>>>> + return extio_inb(addr);
>>>> +#endif
>>>> + return readb(PCI_IOBASE + addr);
>>>> +}
>>>> +
>>>
>>> Looks ok, but you only seem to do this for the 8-bit
>>> accessors, when it should be done for 16-bit and 32-bit
>>> ones as well for consistency.
>> Hip06 LPC only support 8-bit I/O operations on the designated port.
>
> That is an interesting limitation. Maybe still call the extio operations
> and have them do WARN_ON_ONCE() instead?
>
> If you get a driver that calls inw/outw on the range that is owned
> by the LPC bus, you otherwise get an unhandled page fault in kernel
> space, which is not as nice.
As for this issue, I provided a wrong reply in the last email.
After double-checking with SoC guys, the inw(l)/outw(l) are OK with multiple 8-bit transfers to consecutive
I/O addresses.
Sorry for the wrong information!
Will support inw(l)/outw(l) in V4.
Best,
Zhichang
>
>>>> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
>>>> new file mode 100644
>>>> index 0000000..1e7a9c5
>>>> --- /dev/null
>>>> +++ b/drivers/bus/extio.c
>>>> @@ -0,0 +1,66 @@
>>>
>>> This is in a globally visible directory
>>>
>>>> +
>>>> +struct extio_ops *arm64_extio_ops;
>>>
>>> But the identifier uses an architecture specific prefix. Either
>>> move the whole file into arch/arm64, or make the naming so that
>>> it can be used for everything.
>>
>> I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
>
> Ok, that simplifies it a lot, you can just do everything in asm/io.h then.
>
> Arnd
>
[toc] | [prev] | [next] | [standalone]
| From | Zhichang Yuan <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 14:10 +0200 |
| Subject | [PATCH V3 4/4] ARM64 LPC: support earlycon for UART connected to LPC |
| Message-ID | <shhaN-GA-7@gated-at.bofh.it> |
| In reply to | #1483220 |
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
This patch support the earlycon for UART connected to LPC on Hip06.
This patch is depended on the LPC driver.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
drivers/bus/hisi_lpc.c | 113 +++++++++++++++++++++++++++++++++++
drivers/tty/serial/8250/8250_early.c | 26 +++++++-
2 files changed, 137 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 9b364d0..2269828 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -360,6 +360,119 @@ void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
/**
+ * hisilpc_early_in - read/input operation specific for hisi LPC earlycon.
+ * @devobj: pointer to device relevant information of the caller.
+ * @inbuf: the buffer where the read back data is populated.
+ *
+ * for earlycon, dlen and count should be one.
+ *
+ * Return the data read from earlycon on success, error ID on fail.
+ *
+ */
+static unsigned int __init hisilpc_early_in(struct uart_port *port, int offset)
+{
+ unsigned int backval = 0;
+ unsigned int ret = 0;
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+
+ if (!port->mapbase || !port->iobase || !port->membase)
+ return -EINVAL;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = 1;
+ lpcdev.membase = port->membase;
+
+ ret = hisilpc_target_in(&lpcdev, ¶,
+ port->iobase + (offset << port->regshift),
+ (unsigned char *)&backval, 1);
+ return (ret) ? : backval;
+}
+
+/**
+ * hisilpc_early_out - write/output operation specific for hisi LPC earlycon.
+ * @port: pointer to uart_port of eralycon
+ *
+ * for earlycon, dlen and count should be one.
+ *
+ */
+static void __init hisilpc_early_out(struct uart_port *port, int offset,
+ int value)
+{
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+
+ if (!port->mapbase || !port->iobase || !port->membase)
+ return;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = 1;
+ lpcdev.membase = port->membase;
+
+ (void)hisilpc_target_out(&lpcdev, ¶,
+ port->iobase + (offset << port->regshift),
+ (unsigned char *)&value, 1);
+}
+
+
+/**
+ * early_hisilpc8250_setup - initilize the lpc earlycon
+ * @device: pointer to the elarycon device
+ * @options: a option string from earlycon kernel-parameter
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int __init early_hisilpc8250_setup(struct earlycon_device *device,
+ const char *options)
+{
+ char *p;
+ int ret;
+
+ if (!device->port.membase)
+ return -ENODEV;
+
+ if (device->port.iotype != UPIO_MEM)
+ return -EINVAL;
+
+ if (device->options) {
+ p = strchr(device->options, ',');
+ if (p && (p + 1) != '\0') {
+ ret = kstrtoul(++p, 0,
+ (unsigned long *)&device->port.iobase);
+ if (ret || device->port.iobase == 0)
+ return ret ?: -EFAULT;
+ } else
+ device->port.iobase = 0x2f8;
+ } else {
+ device->port.iobase = 0x2f8;
+ device->baud = 0;
+ }
+
+ device->port.serial_in = hisilpc_early_in;
+ device->port.serial_out = hisilpc_early_out;
+ /* must convert iotype to UPIO_PORT for Hip06 indirect-io */
+ device->port.iotype = UPIO_PORT;
+
+ /* disable interrupts from LPC */
+ writel(LPC_IRQ_CLEAR, device->port.membase + LPC_REG_IRQ_ST);
+ /* ensure the LPC is available */
+ while (!(readl(device->port.membase + LPC_REG_OP_STATUS) &
+ LPC_STATUS_IDLE))
+ cpu_relax();
+
+ return early_serial8250_setup(device, options);
+}
+
+
+
+EARLYCON_DECLARE(hisilpcuart, early_hisilpc8250_setup);
+OF_EARLYCON_DECLARE(hisilpcuart, "hisilicon,lpc-uart",
+ early_hisilpc8250_setup);
+
+
+
+/**
* hisilpc_ischild_ipmi - check whether the designated device is ipmi
* @dev: the device to be checked.
* @data: the value used to match the acpi device in checking.
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index 85a12f0..3b5d0a8 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -37,7 +37,7 @@
#include <asm/io.h>
#include <asm/serial.h>
-static unsigned int __init serial8250_early_in(struct uart_port *port, int offset)
+static unsigned int __init serial8250_early_in_raw(struct uart_port *port, int offset)
{
offset <<= port->regshift;
@@ -57,7 +57,7 @@ static unsigned int __init serial8250_early_in(struct uart_port *port, int offse
}
}
-static void __init serial8250_early_out(struct uart_port *port, int offset, int value)
+static void __init serial8250_early_out_raw(struct uart_port *port, int offset, int value)
{
offset <<= port->regshift;
@@ -80,6 +80,28 @@ static void __init serial8250_early_out(struct uart_port *port, int offset, int
}
}
+static inline void __init serial8250_early_out(struct uart_port *port,
+ int offset, int value)
+{
+ if (port->serial_out)
+ port->serial_out(port, offset, value);
+ else {
+ port->serial_out = serial8250_early_out_raw;
+ serial8250_early_out_raw(port, offset, value);
+ }
+}
+
+static inline unsigned int __init serial8250_early_in(struct uart_port *port,
+ int offset)
+{
+ if (port->serial_in)
+ return port->serial_in(port, offset);
+
+ port->serial_in = serial8250_early_in_raw;
+ return serial8250_early_in_raw(port, offset);
+}
+
+
#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
static void __init serial_putc(struct uart_port *port, int c)
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Zhichang Yuan <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 14:10 +0200 |
| Subject | [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count |
| Message-ID | <shhaN-GA-13@gated-at.bofh.it> |
| In reply to | #1483220 |
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
On Hip06 platform, a 16550 compatible UART is connected to low-pin-count and
controlled through the LPC I/O cycles. After registering the LPC uart specific
serial_in/serial_out to 8250 core driver, serial data can be read/written
through the LPC.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
.../devicetree/bindings/serial/hisi-lpc-uart.txt | 60 ++++++++
drivers/tty/serial/8250/8250_hisi_lpc.c | 171 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 9 ++
drivers/tty/serial/8250/Makefile | 1 +
4 files changed, 241 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
create mode 100644 drivers/tty/serial/8250/8250_hisi_lpc.c
diff --git a/Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt b/Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
new file mode 100644
index 0000000..0f8ec2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
@@ -0,0 +1,60 @@
+* Hisilicon hip06 UART through low-pin-count
+
+Required properties:
+- compatible : "hisilicon,lpc-uart"
+- reg : offset and length of the I/O port set for the device.
+- reg-names : name the uart resources.
+ "lpc_mem" represents memory resource of the LPC parent. When
+ flat-tree earlycon is needed, this memory resource and
+ resource name are mandatory. Otherwise they are optional;
+ "dev_io" represents I/O resource of this device. It is
+ mandatory to access this device;
+
+Clock handling:
+ The clock rate of this device is same as the 8250 default clock rate, that
+ is 1843200. No need to define the clock rate in device tree.
+
+Note:
+ This device depends on its parent device whose compatible string is
+ "hisilicon,low-pin-count".
+
+ The format of "reg" property follows the I/O space definition in ISA/EISA
+ binding specification linked to:
+ http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
+
+Example:
+
+ uart0: lpc-uart@2f8 {
+ compatible = "hisilicon,lpc-uart";
+ reg = <0x01 0x2f8 0x08>;
+ status = "disabled";
+ };
+
+
+Example with low-pin-count parent device:
+
+ isa@a01b0000 {
+ compatible = "hisilicon,low-pin-count";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b0000 0x0 0x1000>;
+ ranges = <0x00 0xa01b0000 0x00 0xa01b0000 0x1000>,
+ <0x01 0xe4 0x0 0xe4 0x04>,
+ <0x01 0x2f8 0x0 0x2f8 0x08>;
+
+ ipmi0: bt@e4 {
+ compatible = "ipmi-bt";
+ device_type = "ipmi";
+ reg = <0x01 0xe4 0x04>;
+ reg-names = "dev_io";
+ status = "disabled";
+ };
+
+ uart0: lpc-uart@2f8 {
+ compatible = "hisilicon,lpc-uart";
+ reg = <0x00 0xa01b0000 0x1000>,
+ <0x01 0x2f8 0x08>;
+ ret-names = "lpc_mem", "dev_io";
+ status = "disabled";
+ };
+ };
diff --git a/drivers/tty/serial/8250/8250_hisi_lpc.c b/drivers/tty/serial/8250/8250_hisi_lpc.c
new file mode 100644
index 0000000..a8ba1ca
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_hisi_lpc.c
@@ -0,0 +1,171 @@
+/*
+ * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved.
+ * Author: Zhichang Yuan <yuanzhichang@hisilicon.com>
+ * Author: Zou Rongrong <@huawei.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/serial_8250.h>
+#include <asm-generic/serial.h>
+#include <linux/of_address.h>
+
+
+/**
+ * hisilpc_serial_inb - read/input data from the designated serial port.
+ * @p: the serial port where the data read from
+ * @offset: the target I/O port address where the read is from
+ *
+ * Returns the byte data from this serial port.
+ * -1 means some failures.
+ *
+ */
+static unsigned int hisilpc_serial_inb(struct uart_port *p, int offset)
+{
+ struct extio_ops *parentops;
+
+ parentops = p->private_data;
+ if (!parentops || !parentops->pfin)
+ return -1;
+
+ return parentops->pfin(parentops->devpara,
+ p->iobase + (offset << p->regshift),
+ NULL, sizeof(u8), 1);
+}
+
+/**
+ * hisilpc_serial_outb - write/output data from the designated serial port.
+ * @p: the serial port where the data is written to
+ * @offset: the target I/O port address where the write is from
+ *
+ */
+static void hisilpc_serial_outb(struct uart_port *p, int offset, int value)
+{
+ struct extio_ops *parentops;
+
+ parentops = p->private_data;
+ if (!parentops || !parentops->pfout)
+ return;
+
+ parentops->pfout(parentops->devpara,
+ p->iobase + (offset << p->regshift),
+ &value, sizeof(u8), 1);
+}
+
+
+static int hisilpc8250_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port uart = {};
+ struct uart_port *port = &uart.port;
+ int err = 0;
+ struct resource *iores;
+ struct extio_ops *platdata;
+
+ if (!pdev->dev.parent)
+ return -ENODEV;
+ dev_info(&pdev->dev, "##probe entering\n");
+
+ /* To support the earlycon in bootargs, the first reg must be MEM */
+ iores = platform_get_resource_byname(pdev, IORESOURCE_IO,
+ "dev_io");
+ if (!iores) {
+ dev_err(&pdev->dev, "can not find the IO0\n");
+ return -ENXIO;
+ }
+
+ /*
+ * save the platform data from parent in uart_port for serial_in,
+ * serial_out
+ */
+ platdata = dev_get_platdata(&pdev->dev);
+ port->private_data = (void *)platdata;
+ if (!port->private_data) {
+ dev_err(&pdev->dev, "no platform data!\n");
+ return -ENODEV;
+ }
+
+ if (platdata->start != iores->start || platdata->end != iores->end) {
+ dev_err(&pdev->dev, "PIO range[0x%lx - %lx] isn't fit!\n",
+ (unsigned long)iores->start,
+ (unsigned long)iores->end);
+ return -ENXIO;
+ }
+ port->iobase = (unsigned long)iores->start + platdata->ptoffset;
+ dev_info(&pdev->dev, "real port start is 0x%lx\n", port->iobase);
+
+ port->irq = 0;
+ port->flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
+ port->dev = &pdev->dev;
+ port->iotype = UPIO_PORT;
+ port->regshift = 0;
+ port->uartclk = BASE_BAUD * 16;
+
+ spin_lock_init(&port->lock);
+
+ port->serial_in = hisilpc_serial_inb;
+ port->serial_out = hisilpc_serial_outb;
+
+ err = serial8250_register_8250_port(&uart);
+ if (err < 0) {
+ dev_err(&pdev->dev, "register uart FAIL(%d)!\n", -err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, (void *)&err);
+ dev_info(&pdev->dev, "##probing OK(%d)\n", err);
+ return 0;
+}
+
+static int hisilpc8250_remove(struct platform_device *pdev)
+{
+ int line = *((int *)platform_get_drvdata(pdev));
+
+ serial8250_unregister_port(line);
+
+ return 0;
+}
+
+
+static const struct of_device_id hs8250_of_match[] = {
+ { .compatible = "hisilicon,lpc-uart" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, hs8250_of_match);
+
+static const struct acpi_device_id hs8250_acpi_match[] = {
+ /*{ "PNP0501", 0 },*/
+ { "HISI1031", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, hs8250_acpi_match);
+
+static struct platform_driver hs_lpc8250_driver = {
+ .driver = {
+ .name = "hisi-lpc-uart",
+ .of_match_table = hs8250_of_match,
+ .acpi_match_table = ACPI_PTR(hs8250_acpi_match),
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
+ .probe = hisilpc8250_probe,
+ .remove = hisilpc8250_remove,
+};
+
+module_platform_driver(hs_lpc8250_driver);
+
+
+MODULE_AUTHOR("Rongrong Zou");
+MODULE_DESCRIPTION("8250 serial probe module for Hisilicon LPC UART");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("v1.0");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 7c6f7af..c2e42f7 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -246,6 +246,15 @@ config SERIAL_8250_HUB6
To compile this driver as a module, choose M here: the module
will be called 8250_hub6.
+config SERIAL_8250_HISI_LPC
+ tristate "Support Hisilicon Hip0X UART through LPC"
+ depends on SERIAL_8250 !=n && HISILICON_LPC
+ help
+ Say Y here if you have a hip06 board.
+
+ To compile this driver as a module, choose M here: the module
+ will be called 8250_hisi_lpc.
+
#
# Misc. options/drivers.
#
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 367d403..1f2915b 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554) += 8250_exar_st16c554.o
obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
+obj-$(CONFIG_SERIAL_8250_HISI_LPC) += 8250_hisi_lpc.o
obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o
obj-$(CONFIG_SERIAL_8250_EM) += 8250_em.o
obj-$(CONFIG_SERIAL_8250_OMAP) += 8250_omap.o
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-09-14 14:30 +0200 |
| Subject | Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count |
| Message-ID | <shhua-PJ-11@gated-at.bofh.it> |
| In reply to | #1483228 |
On Wednesday, September 14, 2016 8:15:53 PM CEST Zhichang Yuan wrote: > From: "zhichang.yuan" <yuanzhichang@hisilicon.com> > > On Hip06 platform, a 16550 compatible UART is connected to low-pin-count and > controlled through the LPC I/O cycles. After registering the LPC uart specific > serial_in/serial_out to 8250 core driver, serial data can be read/written > through the LPC. > > Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> > I still think this should be handled by 8250_of.c after the addition of support for IORESOURCE_IO. Arnd
[toc] | [prev] | [next] | [standalone]
| From | "zhichang.yuan" <yuanzhichang@hisilicon.com> |
|---|---|
| Date | 2016-09-14 17:10 +0200 |
| Subject | Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count |
| Message-ID | <shjYZ-2qC-9@gated-at.bofh.it> |
| In reply to | #1483235 |
On 2016/9/14 20:25, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 8:15:53 PM CEST Zhichang Yuan wrote:
>> From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
>>
>> On Hip06 platform, a 16550 compatible UART is connected to low-pin-count and
>> controlled through the LPC I/O cycles. After registering the LPC uart specific
>> serial_in/serial_out to 8250 core driver, serial data can be read/written
>> through the LPC.
>>
>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>>
>
> I still think this should be handled by 8250_of.c after the addition of
> support for IORESOURCE_IO.
The 8250_hisi_lpc.c support both ACPI and dts similar to 8250_dw :
+static struct platform_driver hs_lpc8250_driver = {
+ .driver = {
+ .name = "hisi-lpc-uart",
+ .of_match_table = hs8250_of_match,
+ .acpi_match_table = ACPI_PTR(hs8250_acpi_match),
So, I am a little confused why we need to support dts in 8250_of.c and support ACPI in another
driver file.
best,
Zhichang
>
> Arnd
>
>
> .
>
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-09-14 23:40 +0200 |
| Subject | Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count |
| Message-ID | <shq4p-6gS-5@gated-at.bofh.it> |
| In reply to | #1483425 |
On Wednesday, September 14, 2016 11:04:33 PM CEST zhichang.yuan wrote:
> The 8250_hisi_lpc.c support both ACPI and dts similar to 8250_dw :
>
> +static struct platform_driver hs_lpc8250_driver = {
> + .driver = {
> + .name = "hisi-lpc-uart",
> + .of_match_table = hs8250_of_match,
> + .acpi_match_table = ACPI_PTR(hs8250_acpi_match),
>
> So, I am a little confused why we need to support dts in 8250_of.c and support ACPI in another
> driver file.
>
After looking again, I'm pretty sure that drivers/tty/serial/8250/8250_pnp.c
will handle the ACPI case without modifications, you just need to adapt
the 8250_of driver to handle IORESOURCE_IO ports. This will be required
anyway with the next version once the LPC bridge is on the PCI bus.
Arnd
[toc] | [prev] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-09-21 21:40 +0200 |
| Subject | Re: [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count |
| Message-ID | <sjVx8-4Eu-21@gated-at.bofh.it> |
| In reply to | #1483697 |
On Wednesday, September 21, 2016 6:12:28 PM CEST zhichang wrote:
> On 2016年09月15日 05:33, Arnd Bergmann wrote:
> > On Wednesday, September 14, 2016 11:04:33 PM CEST zhichang.yuan wrote:
> >> The 8250_hisi_lpc.c support both ACPI and dts similar to 8250_dw :
> >>
> >> +static struct platform_driver hs_lpc8250_driver = {
> >> + .driver = {
> >> + .name = "hisi-lpc-uart",
> >> + .of_match_table = hs8250_of_match,
> >> + .acpi_match_table = ACPI_PTR(hs8250_acpi_match),
> >>
> >> So, I am a little confused why we need to support dts in 8250_of.c and support ACPI in another
> >> driver file.
> >>
> >
> > After looking again, I'm pretty sure that drivers/tty/serial/8250/8250_pnp.c
> > will handle the ACPI case without modifications, you just need to adapt
> > the 8250_of driver to handle IORESOURCE_IO ports. This will be required
> > anyway with the next version once the LPC bridge is on the PCI bus.
> >
>
> Yes. You are right.
>
> When the indirectIO patch is accepted, no much changes needed in these two file to support LPC UART.
>
> BTW, what is your target to change these two files to support LPC uart?
> Do you think we don't need to add many new c files??
I think by modifying 8250_of.c, you should be able to make any UART on
some LPC bus work, regardless of what controller is used.
The ACPI version (8250_pnp.c) shouldn't need any changes at all if I
read that correctly.
Arnd
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web