* [PATCH V2 0/4] ARM64 LPC: legacy ISA I/O support
@ 2016-09-07 13:33 ` Zhichang Yuan
0 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linux-arm-kernel
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-rc5;
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 | 27 +
.../devicetree/bindings/serial/hisi-lpc-uart.txt | 50 ++
arch/arm64/Kconfig | 6 +
arch/arm64/include/asm/io.h | 109 ++++
arch/arm64/include/asm/pci.h | 1 -
drivers/bus/Kconfig | 8 +
drivers/bus/Makefile | 1 +
drivers/bus/hisi_lpc.c | 625 +++++++++++++++++++++
drivers/of/address.c | 3 +-
drivers/pci/pci.c | 6 +-
drivers/tty/serial/8250/8250_hisi_lpc.c | 104 ++++
drivers/tty/serial/8250/Kconfig | 9 +
drivers/tty/serial/8250/Makefile | 1 +
13 files changed, 945 insertions(+), 5 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/hisi_lpc.c
create mode 100644 drivers/tty/serial/8250/8250_hisi_lpc.c
--
1.9.1
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 13:33 ` Zhichang Yuan
-1 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linuxarm, linux-arm-kernel, linux-kernel
Cc: arnd, minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
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 explicitly 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 | 109 +++++++++++++++++++++++++++++++++++++++++++
arch/arm64/include/asm/pci.h | 1 -
drivers/of/address.c | 3 +-
drivers/pci/pci.c | 6 +--
5 files changed, 120 insertions(+), 5 deletions(-)
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..0e0c4db 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -142,6 +142,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 +181,83 @@ 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)
+#define PCIBIOS_MIN_IO 0x1000
+
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+
+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 {
+ inhook pfin;
+ outhook pfout;
+ void *devpara;
+};
+
+extern struct extio_ops *arm64_simops __refdata;
+
+/*Up to now, only applied to Hip06 LPC. Define as static here.*/
+static inline void arm64_set_simops(struct extio_ops *ops)
+{
+ if (ops)
+ WRITE_ONCE(arm64_simops, ops);
+}
+
+
+#define BUILDIO(bw, type) \
+static inline type in##bw(unsigned long addr) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ return read##bw(PCI_IOBASE + addr); \
+ return (arm64_simops && arm64_simops->pfin) ? \
+ arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
+ sizeof(type), 1) : -1; \
+} \
+ \
+static inline void out##bw(type value, unsigned long addr) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ write##bw(value, PCI_IOBASE + addr); \
+ else \
+ if (arm64_simops && arm64_simops->pfout) \
+ arm64_simops->pfout(arm64_simops->devpara, addr,\
+ &value, sizeof(type), 1); \
+} \
+ \
+static inline void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ reads##bw(PCI_IOBASE + addr, buffer, count); \
+ else \
+ if (arm64_simops && arm64_simops->pfin) \
+ arm64_simops->pfin(arm64_simops->devpara, addr,\
+ buffer, sizeof(type), count); \
+} \
+ \
+static inline void outs##bw(unsigned long addr, const void *buffer, \
+ unsigned int count) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ writes##bw(PCI_IOBASE + addr, buffer, count); \
+ else \
+ if (arm64_simops && arm64_simops->pfin) \
+ arm64_simops->pfout(arm64_simops->devpara, addr,\
+ buffer, sizeof(type), count); \
+}
+
+
+BUILDIO(b, u8)
+#define inb inb
+#define outb outb
+#define insb insb
+#define outsb outsb
+
+#endif
+
+
/*
* String version of I/O memory access operations.
*/
diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b9a7ba9..5cd3738a 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -8,7 +8,6 @@
#include <asm/io.h>
-#define PCIBIOS_MIN_IO 0x1000
#define PCIBIOS_MIN_MEM 0
/*
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..4092a99 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -688,8 +688,9 @@ static int __of_address_to_resource(struct device_node *dev,
if (taddr == OF_BAD_ADDR)
return -EINVAL;
memset(r, 0, sizeof(struct resource));
- if (flags & IORESOURCE_IO) {
+ if (flags & IORESOURCE_IO && of_bus_pci_match(dev)) {
unsigned long port;
+
port = pci_address_to_pio(taddr);
if (port == (unsigned long)-1)
return -EINVAL;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index aab9d51..ac2e569 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3221,7 +3221,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = PCIBIOS_MIN_IO;
/* check if the range hasn't been previously recorded */
spin_lock(&io_range_lock);
@@ -3270,7 +3270,7 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = PCIBIOS_MIN_IO;
if (pio > IO_SPACE_LIMIT)
return address;
@@ -3293,7 +3293,7 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
#ifdef PCI_IOBASE
struct io_range *res;
- resource_size_t offset = 0;
+ resource_size_t offset = PCIBIOS_MIN_IO;
unsigned long addr = -1;
spin_lock(&io_range_lock);
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-07 13:33 ` Zhichang Yuan
0 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linux-arm-kernel
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 explicitly 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 | 109 +++++++++++++++++++++++++++++++++++++++++++
arch/arm64/include/asm/pci.h | 1 -
drivers/of/address.c | 3 +-
drivers/pci/pci.c | 6 +--
5 files changed, 120 insertions(+), 5 deletions(-)
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..0e0c4db 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -142,6 +142,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 +181,83 @@ 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)
+#define PCIBIOS_MIN_IO 0x1000
+
+#ifdef CONFIG_ARM64_INDIRECT_PIO
+
+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 {
+ inhook pfin;
+ outhook pfout;
+ void *devpara;
+};
+
+extern struct extio_ops *arm64_simops __refdata;
+
+/*Up to now, only applied to Hip06 LPC. Define as static here.*/
+static inline void arm64_set_simops(struct extio_ops *ops)
+{
+ if (ops)
+ WRITE_ONCE(arm64_simops, ops);
+}
+
+
+#define BUILDIO(bw, type) \
+static inline type in##bw(unsigned long addr) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ return read##bw(PCI_IOBASE + addr); \
+ return (arm64_simops && arm64_simops->pfin) ? \
+ arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
+ sizeof(type), 1) : -1; \
+} \
+ \
+static inline void out##bw(type value, unsigned long addr) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ write##bw(value, PCI_IOBASE + addr); \
+ else \
+ if (arm64_simops && arm64_simops->pfout) \
+ arm64_simops->pfout(arm64_simops->devpara, addr,\
+ &value, sizeof(type), 1); \
+} \
+ \
+static inline void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ reads##bw(PCI_IOBASE + addr, buffer, count); \
+ else \
+ if (arm64_simops && arm64_simops->pfin) \
+ arm64_simops->pfin(arm64_simops->devpara, addr,\
+ buffer, sizeof(type), count); \
+} \
+ \
+static inline void outs##bw(unsigned long addr, const void *buffer, \
+ unsigned int count) \
+{ \
+ if (addr >= PCIBIOS_MIN_IO) \
+ writes##bw(PCI_IOBASE + addr, buffer, count); \
+ else \
+ if (arm64_simops && arm64_simops->pfin) \
+ arm64_simops->pfout(arm64_simops->devpara, addr,\
+ buffer, sizeof(type), count); \
+}
+
+
+BUILDIO(b, u8)
+#define inb inb
+#define outb outb
+#define insb insb
+#define outsb outsb
+
+#endif
+
+
/*
* String version of I/O memory access operations.
*/
diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b9a7ba9..5cd3738a 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -8,7 +8,6 @@
#include <asm/io.h>
-#define PCIBIOS_MIN_IO 0x1000
#define PCIBIOS_MIN_MEM 0
/*
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..4092a99 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -688,8 +688,9 @@ static int __of_address_to_resource(struct device_node *dev,
if (taddr == OF_BAD_ADDR)
return -EINVAL;
memset(r, 0, sizeof(struct resource));
- if (flags & IORESOURCE_IO) {
+ if (flags & IORESOURCE_IO && of_bus_pci_match(dev)) {
unsigned long port;
+
port = pci_address_to_pio(taddr);
if (port == (unsigned long)-1)
return -EINVAL;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index aab9d51..ac2e569 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3221,7 +3221,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = PCIBIOS_MIN_IO;
/* check if the range hasn't been previously recorded */
spin_lock(&io_range_lock);
@@ -3270,7 +3270,7 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
#ifdef PCI_IOBASE
struct io_range *range;
- resource_size_t allocated_size = 0;
+ resource_size_t allocated_size = PCIBIOS_MIN_IO;
if (pio > IO_SPACE_LIMIT)
return address;
@@ -3293,7 +3293,7 @@ unsigned long __weak pci_address_to_pio(phys_addr_t address)
{
#ifdef PCI_IOBASE
struct io_range *res;
- resource_size_t offset = 0;
+ resource_size_t offset = PCIBIOS_MIN_IO;
unsigned long addr = -1;
spin_lock(&io_range_lock);
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 15:06 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 15:06 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong
On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> +
> +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 {
> + inhook pfin;
> + outhook pfout;
> + void *devpara;
> +};
> +
> +extern struct extio_ops *arm64_simops __refdata;
> +
> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
> +static inline void arm64_set_simops(struct extio_ops *ops)
> +{
> + if (ops)
> + WRITE_ONCE(arm64_simops, ops);
> +}
> +
> +
> +#define BUILDIO(bw, type) \
> +static inline type in##bw(unsigned long addr) \
> +{ \
> + if (addr >= PCIBIOS_MIN_IO) \
> + return read##bw(PCI_IOBASE + addr); \
> + return (arm64_simops && arm64_simops->pfin) ? \
> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
> + sizeof(type), 1) : -1; \
> +} \
>
Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
compile time means that only the dynamically registered PIO support
is possible for I/O port ranges 0-0xfff.
I think the runtime check should better test if simops was defined
first and fall back to normal PIO otherwise, in order to allow
LPC implementations on a PCI-LPC bridge.
How about allowing an I/O port range to be defined along with
the operations and check against that?
u8 intb(unsigned long port)
{
if (arm64_simops &&
(port >= arm64_simops->min) &&
(port <= arm64_simops->max))
return arm64_simops->pfin(arm64_simops, port, 1);
else
return readb(PCI_IOBASE + addr);
}
The other advantage of that is that you can dynamically register
a translation for the LPC port range into the Linux I/O port range
like PCI hosts do.
We may also want to move the inb/outb definitions into a .c file
as they are getting rather big.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-07 15:06 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 15:06 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> +
> +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 {
> + inhook pfin;
> + outhook pfout;
> + void *devpara;
> +};
> +
> +extern struct extio_ops *arm64_simops __refdata;
> +
> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
> +static inline void arm64_set_simops(struct extio_ops *ops)
> +{
> + if (ops)
> + WRITE_ONCE(arm64_simops, ops);
> +}
> +
> +
> +#define BUILDIO(bw, type) \
> +static inline type in##bw(unsigned long addr) \
> +{ \
> + if (addr >= PCIBIOS_MIN_IO) \
> + return read##bw(PCI_IOBASE + addr); \
> + return (arm64_simops && arm64_simops->pfin) ? \
> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
> + sizeof(type), 1) : -1; \
> +} \
>
Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
compile time means that only the dynamically registered PIO support
is possible for I/O port ranges 0-0xfff.
I think the runtime check should better test if simops was defined
first and fall back to normal PIO otherwise, in order to allow
LPC implementations on a PCI-LPC bridge.
How about allowing an I/O port range to be defined along with
the operations and check against that?
u8 intb(unsigned long port)
{
if (arm64_simops &&
(port >= arm64_simops->min) &&
(port <= arm64_simops->max))
return arm64_simops->pfin(arm64_simops, port, 1);
else
return readb(PCI_IOBASE + addr);
}
The other advantage of that is that you can dynamically register
a translation for the LPC port range into the Linux I/O port range
like PCI hosts do.
We may also want to move the inb/outb definitions into a .c file
as they are getting rather big.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-07 15:06 ` Arnd Bergmann
(?)
@ 2016-09-08 7:45 ` zhichang.yuan
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-08 7:45 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: linuxarm, linux-kernel, lorenzo.pieralisi, minyard, benh,
gabriele.paoloni, john.garry, liviu.dudau, zhichang.yuan02,
zourongrong, linux-pci
Hi, Arnd,
Thanks for your remarks!
On 2016/9/7 23:06, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> +
>> +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 {
>> + inhook pfin;
>> + outhook pfout;
>> + void *devpara;
>> +};
>> +
>> +extern struct extio_ops *arm64_simops __refdata;
>> +
>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>> +static inline void arm64_set_simops(struct extio_ops *ops)
>> +{
>> + if (ops)
>> + WRITE_ONCE(arm64_simops, ops);
>> +}
>> +
>> +
>> +#define BUILDIO(bw, type) \
>> +static inline type in##bw(unsigned long addr) \
>> +{ \
>> + if (addr >= PCIBIOS_MIN_IO) \
>> + return read##bw(PCI_IOBASE + addr); \
>> + return (arm64_simops && arm64_simops->pfin) ? \
>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>> + sizeof(type), 1) : -1; \
>> +} \
>>
>
> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
> compile time means that only the dynamically registered PIO support
> is possible for I/O port ranges 0-0xfff.
Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
register, you also mention below, will discuss there.
>
> I think the runtime check should better test if simops was defined
> first and fall back to normal PIO otherwise, in order to allow
> LPC implementations on a PCI-LPC bridge.
Do you mean check arm64_simops first?
I don't understand clearly what is the benefit about that.
It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> How about allowing an I/O port range to be defined along with
> the operations and check against that?
>
> u8 intb(unsigned long port)
> {
> if (arm64_simops &&
> (port >= arm64_simops->min) &&
> (port <= arm64_simops->max))
> return arm64_simops->pfin(arm64_simops, port, 1);
> else
> return readb(PCI_IOBASE + addr);
> }
>
> The other advantage of that is that you can dynamically register
> a translation for the LPC port range into the Linux I/O port range
> like PCI hosts do.
Yes. an IO port range along with the operations is more generic and extensible.
Do you want to define extio_ops like that:
struct extio_ops {
unsigned long start;
unsigned long end;
unsigned long ptoffset;/* port IO - linux io */
inhook pfin;
outhook pfout;
void *devpara;
};
With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
extio_ops where arm64_simops points to, we can only register one operation.
Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
the trouble will happen.
Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
1. setup a list where all indirect-IO devices' operations are linked to
struct extio_range {
unsigned long start;/* inclusive, sys io addr */
unsigned long end;/* inclusive, sys io addr */
unsigned long ptoffset;/* port Io - system Io */
};
struct extio_node {
struct list_head ranlink;
struct extio_range iores;
/*pointer to the device provided services*/
struct extio_ops *regops;
};
when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
complete the IO.
static inline type inb(unsigned long addr)
{
struct extio_node *extop;
unsigned long offset;
/* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
extop = extio_range_getops(addr, &offset);
if (!extop)
return read##bw(PCI_IOBASE + addr);
if (extop->regops && extop->regops->pfin)
return extop->regops->pfin(extop->regops->devpara,
addr + offset, NULL, sizeof(type), 1);
return -1;
}
The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
extio_ops structure. Probably this is your suggestion.
But if the PIO ranges are discrete, it seems we have to reserve a bigger PIO range which probably conflict with other PIO devices...
2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
#define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
#define PCI_IOBASE ((void __iomem *)PCI_IO_START)
current PCI_IO_SIZE is 16M.
It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
the structure for this way is:
#define EXTIO_VECTOR_MAX 32
struct extio_vector {
struct mutex seglock;
/* one bit corresponds with one segment */
DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
struct extio_ops *opsvec;
};
when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
static inline type inb(unsigned long addr)
{
if (!(addr & (0x01 << 16))) /* only check bit 16 */
return readb(PCI_IOBASE + addr);
/* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
return extio_inb(addr);
}
This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> We may also want to move the inb/outb definitions into a .c file
> as they are getting rather big.
The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-08 7:45 ` zhichang.yuan
0 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-08 7:45 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Arnd,
Thanks for your remarks!
On 2016/9/7 23:06, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> +
>> +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 {
>> + inhook pfin;
>> + outhook pfout;
>> + void *devpara;
>> +};
>> +
>> +extern struct extio_ops *arm64_simops __refdata;
>> +
>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>> +static inline void arm64_set_simops(struct extio_ops *ops)
>> +{
>> + if (ops)
>> + WRITE_ONCE(arm64_simops, ops);
>> +}
>> +
>> +
>> +#define BUILDIO(bw, type) \
>> +static inline type in##bw(unsigned long addr) \
>> +{ \
>> + if (addr >= PCIBIOS_MIN_IO) \
>> + return read##bw(PCI_IOBASE + addr); \
>> + return (arm64_simops && arm64_simops->pfin) ? \
>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>> + sizeof(type), 1) : -1; \
>> +} \
>>
>
> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
> compile time means that only the dynamically registered PIO support
> is possible for I/O port ranges 0-0xfff.
Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
register, you also mention below, will discuss there.
>
> I think the runtime check should better test if simops was defined
> first and fall back to normal PIO otherwise, in order to allow
> LPC implementations on a PCI-LPC bridge.
Do you mean check arm64_simops first?
I don't understand clearly what is the benefit about that.
It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> How about allowing an I/O port range to be defined along with
> the operations and check against that?
>
> u8 intb(unsigned long port)
> {
> if (arm64_simops &&
> (port >= arm64_simops->min) &&
> (port <= arm64_simops->max))
> return arm64_simops->pfin(arm64_simops, port, 1);
> else
> return readb(PCI_IOBASE + addr);
> }
>
> The other advantage of that is that you can dynamically register
> a translation for the LPC port range into the Linux I/O port range
> like PCI hosts do.
Yes. an IO port range along with the operations is more generic and extensible.
Do you want to define extio_ops like that:
struct extio_ops {
unsigned long start;
unsigned long end;
unsigned long ptoffset;/* port IO - linux io */
inhook pfin;
outhook pfout;
void *devpara;
};
With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
extio_ops where arm64_simops points to, we can only register one operation.
Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
the trouble will happen.
Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
1. setup a list where all indirect-IO devices' operations are linked to
struct extio_range {
unsigned long start;/* inclusive, sys io addr */
unsigned long end;/* inclusive, sys io addr */
unsigned long ptoffset;/* port Io - system Io */
};
struct extio_node {
struct list_head ranlink;
struct extio_range iores;
/*pointer to the device provided services*/
struct extio_ops *regops;
};
when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
complete the IO.
static inline type inb(unsigned long addr)
{
struct extio_node *extop;
unsigned long offset;
/* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
extop = extio_range_getops(addr, &offset);
if (!extop)
return read##bw(PCI_IOBASE + addr);
if (extop->regops && extop->regops->pfin)
return extop->regops->pfin(extop->regops->devpara,
addr + offset, NULL, sizeof(type), 1);
return -1;
}
The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
extio_ops structure. Probably this is your suggestion.
But if the PIO ranges are discrete, it seems we have to reserve a bigger PIO range which probably conflict with other PIO devices...
2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
#define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
#define PCI_IOBASE ((void __iomem *)PCI_IO_START)
current PCI_IO_SIZE is 16M.
It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
the structure for this way is:
#define EXTIO_VECTOR_MAX 32
struct extio_vector {
struct mutex seglock;
/* one bit corresponds with one segment */
DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
struct extio_ops *opsvec;
};
when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
static inline type inb(unsigned long addr)
{
if (!(addr & (0x01 << 16))) /* only check bit 16 */
return readb(PCI_IOBASE + addr);
/* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
return extio_inb(addr);
}
This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> We may also want to move the inb/outb definitions into a .c file
> as they are getting rather big.
The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-08 7:45 ` zhichang.yuan
0 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-08 7:45 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: lorenzo.pieralisi, minyard, gabriele.paoloni, linux-pci, benh,
john.garry, liviu.dudau, linuxarm, linux-kernel, zhichang.yuan02,
zourongrong
Hi, Arnd,
Thanks for your remarks!
On 2016/9/7 23:06, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>> +
>> +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 {
>> + inhook pfin;
>> + outhook pfout;
>> + void *devpara;
>> +};
>> +
>> +extern struct extio_ops *arm64_simops __refdata;
>> +
>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>> +static inline void arm64_set_simops(struct extio_ops *ops)
>> +{
>> + if (ops)
>> + WRITE_ONCE(arm64_simops, ops);
>> +}
>> +
>> +
>> +#define BUILDIO(bw, type) \
>> +static inline type in##bw(unsigned long addr) \
>> +{ \
>> + if (addr >= PCIBIOS_MIN_IO) \
>> + return read##bw(PCI_IOBASE + addr); \
>> + return (arm64_simops && arm64_simops->pfin) ? \
>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>> + sizeof(type), 1) : -1; \
>> +} \
>>
>
> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
> compile time means that only the dynamically registered PIO support
> is possible for I/O port ranges 0-0xfff.
Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
register, you also mention below, will discuss there.
>
> I think the runtime check should better test if simops was defined
> first and fall back to normal PIO otherwise, in order to allow
> LPC implementations on a PCI-LPC bridge.
Do you mean check arm64_simops first?
I don't understand clearly what is the benefit about that.
It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> How about allowing an I/O port range to be defined along with
> the operations and check against that?
>
> u8 intb(unsigned long port)
> {
> if (arm64_simops &&
> (port >= arm64_simops->min) &&
> (port <= arm64_simops->max))
> return arm64_simops->pfin(arm64_simops, port, 1);
> else
> return readb(PCI_IOBASE + addr);
> }
>
> The other advantage of that is that you can dynamically register
> a translation for the LPC port range into the Linux I/O port range
> like PCI hosts do.
Yes. an IO port range along with the operations is more generic and extensible.
Do you want to define extio_ops like that:
struct extio_ops {
unsigned long start;
unsigned long end;
unsigned long ptoffset;/* port IO - linux io */
inhook pfin;
outhook pfout;
void *devpara;
};
With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
extio_ops where arm64_simops points to, we can only register one operation.
Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
the trouble will happen.
Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
1. setup a list where all indirect-IO devices' operations are linked to
struct extio_range {
unsigned long start;/* inclusive, sys io addr */
unsigned long end;/* inclusive, sys io addr */
unsigned long ptoffset;/* port Io - system Io */
};
struct extio_node {
struct list_head ranlink;
struct extio_range iores;
/*pointer to the device provided services*/
struct extio_ops *regops;
};
when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
complete the IO.
static inline type inb(unsigned long addr)
{
struct extio_node *extop;
unsigned long offset;
/* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
extop = extio_range_getops(addr, &offset);
if (!extop)
return read##bw(PCI_IOBASE + addr);
if (extop->regops && extop->regops->pfin)
return extop->regops->pfin(extop->regops->devpara,
addr + offset, NULL, sizeof(type), 1);
return -1;
}
The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
extio_ops structure. Probably this is your suggestion.
But if the PIO ranges are discrete, it seems we have to reserve a bigger PIO range which probably conflict with other PIO devices...
2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
#define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
#define PCI_IOBASE ((void __iomem *)PCI_IO_START)
current PCI_IO_SIZE is 16M.
It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
the structure for this way is:
#define EXTIO_VECTOR_MAX 32
struct extio_vector {
struct mutex seglock;
/* one bit corresponds with one segment */
DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
struct extio_ops *opsvec;
};
when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
static inline type inb(unsigned long addr)
{
if (!(addr & (0x01 << 16))) /* only check bit 16 */
return readb(PCI_IOBASE + addr);
/* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
return extio_inb(addr);
}
This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> We may also want to move the inb/outb definitions into a .c file
> as they are getting rather big.
The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> Arnd
>
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-08 7:45 ` zhichang.yuan
@ 2016-09-08 13:23 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 13:23 UTC (permalink / raw)
To: zhichang.yuan
Cc: linux-arm-kernel, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong, linux-pci
On Thursday, September 8, 2016 3:45:21 PM CEST zhichang.yuan wrote:
> On 2016/9/7 23:06, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
> >> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> >> +
> >> +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 {
> >> + inhook pfin;
> >> + outhook pfout;
> >> + void *devpara;
> >> +};
> >> +
> >> +extern struct extio_ops *arm64_simops __refdata;
> >> +
> >> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
> >> +static inline void arm64_set_simops(struct extio_ops *ops)
> >> +{
> >> + if (ops)
> >> + WRITE_ONCE(arm64_simops, ops);
> >> +}
> >> +
> >> +
> >> +#define BUILDIO(bw, type) \
> >> +static inline type in##bw(unsigned long addr) \
> >> +{ \
> >> + if (addr >= PCIBIOS_MIN_IO) \
> >> + return read##bw(PCI_IOBASE + addr); \
> >> + return (arm64_simops && arm64_simops->pfin) ? \
> >> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
> >> + sizeof(type), 1) : -1; \
> >> +} \
> >>
> >
> > Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
> > compile time means that only the dynamically registered PIO support
> > is possible for I/O port ranges 0-0xfff.
> Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
> support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
> register, you also mention below, will discuss there.
I think having only one range is enough, but it may be best not to
assume that this is mapped at a fixed location in the Linux PIO
port address space.
As I understand, you list all the child devices in DT, so those
port numbers should all be translatable from bus specific (0x0-0xfff)
into the larger Linux range that also contains PCI devices.
> > I think the runtime check should better test if simops was defined
> > first and fall back to normal PIO otherwise, in order to allow
> > LPC implementations on a PCI-LPC bridge.
> Do you mean check arm64_simops first?
> I don't understand clearly what is the benefit about that.
> It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
No, this is about having devices at hardcoded PIO addresses behind PCI
on another (non-hisilicon) machine running the same kernel binary.
> > How about allowing an I/O port range to be defined along with
> > the operations and check against that?
> >
> > u8 intb(unsigned long port)
> > {
> > if (arm64_simops &&
> > (port >= arm64_simops->min) &&
> > (port <= arm64_simops->max))
> > return arm64_simops->pfin(arm64_simops, port, 1);
> > else
> > return readb(PCI_IOBASE + addr);
> > }
> >
> > The other advantage of that is that you can dynamically register
> > a translation for the LPC port range into the Linux I/O port range
> > like PCI hosts do.
> Yes. an IO port range along with the operations is more generic and extensible.
> Do you want to define extio_ops like that:
>
> struct extio_ops {
> unsigned long start;
> unsigned long end;
> unsigned long ptoffset;/* port IO - linux io */
> inhook pfin;
> outhook pfout;
> void *devpara;
> };
>
> With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
> extio_ops where arm64_simops points to, we can only register one operation.
> Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
> In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
> PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
> is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
> the trouble will happen.
I don't think we can solve all the possible cases. When a driver asks
for a hardcoded address, we can either route that to the first PCI bus
that registers, or always route it to LPC, but there may always be
corner cases that don't work.
Fortunately, it is very rare for hardcoded PIO addresses to be required
in particular on non-x86 architectures, so it might not matter too much
in practice:
Having the extio range live on ports 0-0x1000 by default is probably
reasonable, as long as that range is also usable for PCI on other
platforms. Having it registered dynamically after the PCI bus should
also be ok.
> Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
>
> 1. setup a list where all indirect-IO devices' operations are linked to
>
>
> struct extio_range {
> unsigned long start;/* inclusive, sys io addr */
> unsigned long end;/* inclusive, sys io addr */
> unsigned long ptoffset;/* port Io - system Io */
> };
>
> struct extio_node {
> struct list_head ranlink;
>
> struct extio_range iores;
>
> /*pointer to the device provided services*/
> struct extio_ops *regops;
> };
>
> when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
> complete the IO.
>
> static inline type inb(unsigned long addr)
> {
> struct extio_node *extop;
> unsigned long offset;
> /* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
> extop = extio_range_getops(addr, &offset);
> if (!extop)
> return read##bw(PCI_IOBASE + addr);
> if (extop->regops && extop->regops->pfin)
> return extop->regops->pfin(extop->regops->devpara,
> addr + offset, NULL, sizeof(type), 1);
> return -1;
> }
>
> The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
> If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
> extio_ops structure. Probably this is your suggestion.
I wouldn't go this far: just assume that there is either one set of
operations registered or none at all, but make it possible to have it
at an arbitrary address.
> 2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
>
> the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
>
> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>
> current PCI_IO_SIZE is 16M.
>
> It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
> IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
> by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
> to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
>
> the structure for this way is:
>
> #define EXTIO_VECTOR_MAX 32
> struct extio_vector {
> struct mutex seglock;
>
> /* one bit corresponds with one segment */
> DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
> struct extio_ops *opsvec;
> };
>
>
> when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
>
> static inline type inb(unsigned long addr)
> {
> if (!(addr & (0x01 << 16))) /* only check bit 16 */
> return readb(PCI_IOBASE + addr);
> /* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
> return extio_inb(addr);
> }
>
> This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
No, I don't think this is necessary either.
> > We may also want to move the inb/outb definitions into a .c file
> > as they are getting rather big.
> The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
The current implementation turns into a single CPU instruction, my idea
was not to grow that too much but instead turn it into a branch instruction
when your code is enabled at compile-time. When it's disabled, we should
still use the existing behavior.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-08 13:23 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 13:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, September 8, 2016 3:45:21 PM CEST zhichang.yuan wrote:
> On 2016/9/7 23:06, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
> >> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> >> +
> >> +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 {
> >> + inhook pfin;
> >> + outhook pfout;
> >> + void *devpara;
> >> +};
> >> +
> >> +extern struct extio_ops *arm64_simops __refdata;
> >> +
> >> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
> >> +static inline void arm64_set_simops(struct extio_ops *ops)
> >> +{
> >> + if (ops)
> >> + WRITE_ONCE(arm64_simops, ops);
> >> +}
> >> +
> >> +
> >> +#define BUILDIO(bw, type) \
> >> +static inline type in##bw(unsigned long addr) \
> >> +{ \
> >> + if (addr >= PCIBIOS_MIN_IO) \
> >> + return read##bw(PCI_IOBASE + addr); \
> >> + return (arm64_simops && arm64_simops->pfin) ? \
> >> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
> >> + sizeof(type), 1) : -1; \
> >> +} \
> >>
> >
> > Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
> > compile time means that only the dynamically registered PIO support
> > is possible for I/O port ranges 0-0xfff.
> Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
> support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
> register, you also mention below, will discuss there.
I think having only one range is enough, but it may be best not to
assume that this is mapped at a fixed location in the Linux PIO
port address space.
As I understand, you list all the child devices in DT, so those
port numbers should all be translatable from bus specific (0x0-0xfff)
into the larger Linux range that also contains PCI devices.
> > I think the runtime check should better test if simops was defined
> > first and fall back to normal PIO otherwise, in order to allow
> > LPC implementations on a PCI-LPC bridge.
> Do you mean check arm64_simops first?
> I don't understand clearly what is the benefit about that.
> It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
No, this is about having devices at hardcoded PIO addresses behind PCI
on another (non-hisilicon) machine running the same kernel binary.
> > How about allowing an I/O port range to be defined along with
> > the operations and check against that?
> >
> > u8 intb(unsigned long port)
> > {
> > if (arm64_simops &&
> > (port >= arm64_simops->min) &&
> > (port <= arm64_simops->max))
> > return arm64_simops->pfin(arm64_simops, port, 1);
> > else
> > return readb(PCI_IOBASE + addr);
> > }
> >
> > The other advantage of that is that you can dynamically register
> > a translation for the LPC port range into the Linux I/O port range
> > like PCI hosts do.
> Yes. an IO port range along with the operations is more generic and extensible.
> Do you want to define extio_ops like that:
>
> struct extio_ops {
> unsigned long start;
> unsigned long end;
> unsigned long ptoffset;/* port IO - linux io */
> inhook pfin;
> outhook pfout;
> void *devpara;
> };
>
> With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
> extio_ops where arm64_simops points to, we can only register one operation.
> Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
> In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
> PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
> is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
> the trouble will happen.
I don't think we can solve all the possible cases. When a driver asks
for a hardcoded address, we can either route that to the first PCI bus
that registers, or always route it to LPC, but there may always be
corner cases that don't work.
Fortunately, it is very rare for hardcoded PIO addresses to be required
in particular on non-x86 architectures, so it might not matter too much
in practice:
Having the extio range live on ports 0-0x1000 by default is probably
reasonable, as long as that range is also usable for PCI on other
platforms. Having it registered dynamically after the PCI bus should
also be ok.
> Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
>
> 1. setup a list where all indirect-IO devices' operations are linked to
>
>
> struct extio_range {
> unsigned long start;/* inclusive, sys io addr */
> unsigned long end;/* inclusive, sys io addr */
> unsigned long ptoffset;/* port Io - system Io */
> };
>
> struct extio_node {
> struct list_head ranlink;
>
> struct extio_range iores;
>
> /*pointer to the device provided services*/
> struct extio_ops *regops;
> };
>
> when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
> complete the IO.
>
> static inline type inb(unsigned long addr)
> {
> struct extio_node *extop;
> unsigned long offset;
> /* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
> extop = extio_range_getops(addr, &offset);
> if (!extop)
> return read##bw(PCI_IOBASE + addr);
> if (extop->regops && extop->regops->pfin)
> return extop->regops->pfin(extop->regops->devpara,
> addr + offset, NULL, sizeof(type), 1);
> return -1;
> }
>
> The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
> If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
> extio_ops structure. Probably this is your suggestion.
I wouldn't go this far: just assume that there is either one set of
operations registered or none at all, but make it possible to have it
at an arbitrary address.
> 2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
>
> the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
>
> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>
> current PCI_IO_SIZE is 16M.
>
> It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
> IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
> by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
> to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
>
> the structure for this way is:
>
> #define EXTIO_VECTOR_MAX 32
> struct extio_vector {
> struct mutex seglock;
>
> /* one bit corresponds with one segment */
> DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
> struct extio_ops *opsvec;
> };
>
>
> when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
>
> static inline type inb(unsigned long addr)
> {
> if (!(addr & (0x01 << 16))) /* only check bit 16 */
> return readb(PCI_IOBASE + addr);
> /* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
> return extio_inb(addr);
> }
>
> This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
No, I don't think this is necessary either.
> > We may also want to move the inb/outb definitions into a .c file
> > as they are getting rather big.
> The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
The current implementation turns into a single CPU instruction, my idea
was not to grow that too much but instead turn it into a branch instruction
when your code is enabled at compile-time. When it's disabled, we should
still use the existing behavior.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-08 13:23 ` Arnd Bergmann
@ 2016-09-13 6:08 ` zhichang
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-13 6:08 UTC (permalink / raw)
To: Arnd Bergmann, zhichang.yuan
Cc: linux-arm-kernel, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zourongrong, linux-pci
Hi, Arnd,
On 2016年09月08日 21:23, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 3:45:21 PM CEST zhichang.yuan wrote:
>> On 2016/9/7 23:06, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> +
>>>> +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 {
>>>> + inhook pfin;
>>>> + outhook pfout;
>>>> + void *devpara;
>>>> +};
>>>> +
>>>> +extern struct extio_ops *arm64_simops __refdata;
>>>> +
>>>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>>>> +static inline void arm64_set_simops(struct extio_ops *ops)
>>>> +{
>>>> + if (ops)
>>>> + WRITE_ONCE(arm64_simops, ops);
>>>> +}
>>>> +
>>>> +
>>>> +#define BUILDIO(bw, type) \
>>>> +static inline type in##bw(unsigned long addr) \
>>>> +{ \
>>>> + if (addr >= PCIBIOS_MIN_IO) \
>>>> + return read##bw(PCI_IOBASE + addr); \
>>>> + return (arm64_simops && arm64_simops->pfin) ? \
>>>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>>>> + sizeof(type), 1) : -1; \
>>>> +} \
>>>>
>>>
>>> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
>>> compile time means that only the dynamically registered PIO support
>>> is possible for I/O port ranges 0-0xfff.
>> Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
>> support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
>> register, you also mention below, will discuss there.
>
> I think having only one range is enough, but it may be best not to
> assume that this is mapped at a fixed location in the Linux PIO
> port address space.
ok. I will add the linux PIO range into struct extio_ops. With this new added PIO range, we can register a
variable linux PIO range to the global arm64_simops for our LPC.
>
> As I understand, you list all the child devices in DT, so those
> port numbers should all be translatable from bus specific (0x0-0xfff)
> into the larger Linux range that also contains PCI devices.
>
>>> I think the runtime check should better test if simops was defined
>>> first and fall back to normal PIO otherwise, in order to allow
>>> LPC implementations on a PCI-LPC bridge.
>> Do you mean check arm64_simops first?
>> I don't understand clearly what is the benefit about that.
>> It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> No, this is about having devices at hardcoded PIO addresses behind PCI
> on another (non-hisilicon) machine running the same kernel binary.
>
What about defining inb like that:
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);
}
The CONFIG_ARM64_INDIRECT_PIO is still using. When indirect-IO is needed, ARM64_INDIRECT_PIO will be selected.
I don't want to define arm64_extio_ops in some build-in source file, such as setup.c; keeping
CONFIG_ARM64_INDIRECT_PIO is for the specific devices which need indirect-IO.
extio_inb is defined as weak function in c file.
All these revise will be presented in V3 patch soon.
>>> How about allowing an I/O port range to be defined along with
>>> the operations and check against that?
>>>
>>> u8 intb(unsigned long port)
>>> {
>>> if (arm64_simops &&
>>> (port >= arm64_simops->min) &&
>>> (port <= arm64_simops->max))
>>> return arm64_simops->pfin(arm64_simops, port, 1);
>>> else
>>> return readb(PCI_IOBASE + addr);
>>> }
>>>
>>> The other advantage of that is that you can dynamically register
>>> a translation for the LPC port range into the Linux I/O port range
>>> like PCI hosts do.
>> Yes. an IO port range along with the operations is more generic and extensible.
>> Do you want to define extio_ops like that:
>>
>> struct extio_ops {
>> unsigned long start;
>> unsigned long end;
>> unsigned long ptoffset;/* port IO - linux io */
>> inhook pfin;
>> outhook pfout;
>> void *devpara;
>> };
>>
>> With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
>> extio_ops where arm64_simops points to, we can only register one operation.
>> Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
>> In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
>> PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
>> is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
>> the trouble will happen.
>
> I don't think we can solve all the possible cases. When a driver asks
> for a hardcoded address, we can either route that to the first PCI bus
> that registers, or always route it to LPC, but there may always be
> corner cases that don't work.
>
> Fortunately, it is very rare for hardcoded PIO addresses to be required
> in particular on non-x86 architectures, so it might not matter too much
> in practice:
>
> Having the extio range live on ports 0-0x1000 by default is probably
> reasonable, as long as that range is also usable for PCI on other
> platforms. Having it registered dynamically after the PCI bus should
> also be ok.
>
In the coming patch V3, we don't reserve the fixed PIO range of 0-0x1000. The extio range is totally depended
on the device IO resource configuration.
Several changes will be done in V3:
1) adopt the translation for all IO ranges including those from Hip06 LPC by pci_address_to_pio. Which means
that all physical IO ranges will be converted to fully different linux PIO ranges. Based on this, I think the
PCI PIO ranges can co-exist with the PIO ranges from other bus, including Hip06 LPC;
2) Since only one extio range is supported in this patch-set, we only register the linux PIO range for IPMI bt
in arm64_simops to work based on indirect-IO; In this way, ipmi bt can work well without any changes on
current ipmi driver.
>> Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
>>
>> 1. setup a list where all indirect-IO devices' operations are linked to
>>
>>
>> struct extio_range {
>> unsigned long start;/* inclusive, sys io addr */
>> unsigned long end;/* inclusive, sys io addr */
>> unsigned long ptoffset;/* port Io - system Io */
>> };
>>
>> struct extio_node {
>> struct list_head ranlink;
>>
>> struct extio_range iores;
>>
>> /*pointer to the device provided services*/
>> struct extio_ops *regops;
>> };
>>
>> when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
>> complete the IO.
>>
>> static inline type inb(unsigned long addr)
>> {
>> struct extio_node *extop;
>> unsigned long offset;
>> /* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
>> extop = extio_range_getops(addr, &offset);
>> if (!extop)
>> return read##bw(PCI_IOBASE + addr);
>> if (extop->regops && extop->regops->pfin)
>> return extop->regops->pfin(extop->regops->devpara,
>> addr + offset, NULL, sizeof(type), 1);
>> return -1;
>> }
>>
>> The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
>
>> If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
>> extio_ops structure. Probably this is your suggestion.
>
> I wouldn't go this far: just assume that there is either one set of
> operations registered or none at all, but make it possible to have it
> at an arbitrary address.
>
>> 2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
>>
>> the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
>>
>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>
>> current PCI_IO_SIZE is 16M.
>>
>> It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
>> IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
>> by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
>> to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
>>
>> the structure for this way is:
>>
>> #define EXTIO_VECTOR_MAX 32
>> struct extio_vector {
>> struct mutex seglock;
>>
>> /* one bit corresponds with one segment */
>> DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
>> struct extio_ops *opsvec;
>> };
>>
>>
>> when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
>>
>> static inline type inb(unsigned long addr)
>> {
>> if (!(addr & (0x01 << 16))) /* only check bit 16 */
>> return readb(PCI_IOBASE + addr);
>> /* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
>> return extio_inb(addr);
>> }
>>
>> This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> No, I don't think this is necessary either.
Ok. Will make the code simpler. One extio range is enough for hip06 LPC at this moment.
Thanks,
Zhichang
>
>>> We may also want to move the inb/outb definitions into a .c file
>>> as they are getting rather big.
>> The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> The current implementation turns into a single CPU instruction, my idea
> was not to grow that too much but instead turn it into a branch instruction
> when your code is enabled at compile-time. When it's disabled, we should
> still use the existing behavior.
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
@ 2016-09-13 6:08 ` zhichang
0 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-13 6:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Arnd,
On 2016?09?08? 21:23, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 3:45:21 PM CEST zhichang.yuan wrote:
>> On 2016/9/7 23:06, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> +
>>>> +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 {
>>>> + inhook pfin;
>>>> + outhook pfout;
>>>> + void *devpara;
>>>> +};
>>>> +
>>>> +extern struct extio_ops *arm64_simops __refdata;
>>>> +
>>>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>>>> +static inline void arm64_set_simops(struct extio_ops *ops)
>>>> +{
>>>> + if (ops)
>>>> + WRITE_ONCE(arm64_simops, ops);
>>>> +}
>>>> +
>>>> +
>>>> +#define BUILDIO(bw, type) \
>>>> +static inline type in##bw(unsigned long addr) \
>>>> +{ \
>>>> + if (addr >= PCIBIOS_MIN_IO) \
>>>> + return read##bw(PCI_IOBASE + addr); \
>>>> + return (arm64_simops && arm64_simops->pfin) ? \
>>>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>>>> + sizeof(type), 1) : -1; \
>>>> +} \
>>>>
>>>
>>> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
>>> compile time means that only the dynamically registered PIO support
>>> is possible for I/O port ranges 0-0xfff.
>> Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
>> support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
>> register, you also mention below, will discuss there.
>
> I think having only one range is enough, but it may be best not to
> assume that this is mapped at a fixed location in the Linux PIO
> port address space.
ok. I will add the linux PIO range into struct extio_ops. With this new added PIO range, we can register a
variable linux PIO range to the global arm64_simops for our LPC.
>
> As I understand, you list all the child devices in DT, so those
> port numbers should all be translatable from bus specific (0x0-0xfff)
> into the larger Linux range that also contains PCI devices.
>
>>> I think the runtime check should better test if simops was defined
>>> first and fall back to normal PIO otherwise, in order to allow
>>> LPC implementations on a PCI-LPC bridge.
>> Do you mean check arm64_simops first?
>> I don't understand clearly what is the benefit about that.
>> It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> No, this is about having devices at hardcoded PIO addresses behind PCI
> on another (non-hisilicon) machine running the same kernel binary.
>
What about defining inb like that:
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);
}
The CONFIG_ARM64_INDIRECT_PIO is still using. When indirect-IO is needed, ARM64_INDIRECT_PIO will be selected.
I don't want to define arm64_extio_ops in some build-in source file, such as setup.c; keeping
CONFIG_ARM64_INDIRECT_PIO is for the specific devices which need indirect-IO.
extio_inb is defined as weak function in c file.
All these revise will be presented in V3 patch soon.
>>> How about allowing an I/O port range to be defined along with
>>> the operations and check against that?
>>>
>>> u8 intb(unsigned long port)
>>> {
>>> if (arm64_simops &&
>>> (port >= arm64_simops->min) &&
>>> (port <= arm64_simops->max))
>>> return arm64_simops->pfin(arm64_simops, port, 1);
>>> else
>>> return readb(PCI_IOBASE + addr);
>>> }
>>>
>>> The other advantage of that is that you can dynamically register
>>> a translation for the LPC port range into the Linux I/O port range
>>> like PCI hosts do.
>> Yes. an IO port range along with the operations is more generic and extensible.
>> Do you want to define extio_ops like that:
>>
>> struct extio_ops {
>> unsigned long start;
>> unsigned long end;
>> unsigned long ptoffset;/* port IO - linux io */
>> inhook pfin;
>> outhook pfout;
>> void *devpara;
>> };
>>
>> With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
>> extio_ops where arm64_simops points to, we can only register one operation.
>> Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
>> In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
>> PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
>> is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
>> the trouble will happen.
>
> I don't think we can solve all the possible cases. When a driver asks
> for a hardcoded address, we can either route that to the first PCI bus
> that registers, or always route it to LPC, but there may always be
> corner cases that don't work.
>
> Fortunately, it is very rare for hardcoded PIO addresses to be required
> in particular on non-x86 architectures, so it might not matter too much
> in practice:
>
> Having the extio range live on ports 0-0x1000 by default is probably
> reasonable, as long as that range is also usable for PCI on other
> platforms. Having it registered dynamically after the PCI bus should
> also be ok.
>
In the coming patch V3, we don't reserve the fixed PIO range of 0-0x1000. The extio range is totally depended
on the device IO resource configuration.
Several changes will be done in V3:
1) adopt the translation for all IO ranges including those from Hip06 LPC by pci_address_to_pio. Which means
that all physical IO ranges will be converted to fully different linux PIO ranges. Based on this, I think the
PCI PIO ranges can co-exist with the PIO ranges from other bus, including Hip06 LPC;
2) Since only one extio range is supported in this patch-set, we only register the linux PIO range for IPMI bt
in arm64_simops to work based on indirect-IO; In this way, ipmi bt can work well without any changes on
current ipmi driver.
>> Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
>>
>> 1. setup a list where all indirect-IO devices' operations are linked to
>>
>>
>> struct extio_range {
>> unsigned long start;/* inclusive, sys io addr */
>> unsigned long end;/* inclusive, sys io addr */
>> unsigned long ptoffset;/* port Io - system Io */
>> };
>>
>> struct extio_node {
>> struct list_head ranlink;
>>
>> struct extio_range iores;
>>
>> /*pointer to the device provided services*/
>> struct extio_ops *regops;
>> };
>>
>> when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
>> complete the IO.
>>
>> static inline type inb(unsigned long addr)
>> {
>> struct extio_node *extop;
>> unsigned long offset;
>> /* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
>> extop = extio_range_getops(addr, &offset);
>> if (!extop)
>> return read##bw(PCI_IOBASE + addr);
>> if (extop->regops && extop->regops->pfin)
>> return extop->regops->pfin(extop->regops->devpara,
>> addr + offset, NULL, sizeof(type), 1);
>> return -1;
>> }
>>
>> The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
>
>> If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
>> extio_ops structure. Probably this is your suggestion.
>
> I wouldn't go this far: just assume that there is either one set of
> operations registered or none at all, but make it possible to have it
> at an arbitrary address.
>
>> 2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
>>
>> the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
>>
>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>
>> current PCI_IO_SIZE is 16M.
>>
>> It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
>> IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
>> by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
>> to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
>>
>> the structure for this way is:
>>
>> #define EXTIO_VECTOR_MAX 32
>> struct extio_vector {
>> struct mutex seglock;
>>
>> /* one bit corresponds with one segment */
>> DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
>> struct extio_ops *opsvec;
>> };
>>
>>
>> when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
>>
>> static inline type inb(unsigned long addr)
>> {
>> if (!(addr & (0x01 << 16))) /* only check bit 16 */
>> return readb(PCI_IOBASE + addr);
>> /* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
>> return extio_inb(addr);
>> }
>>
>> This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> No, I don't think this is necessary either.
Ok. Will make the code simpler. One extio range is enough for hip06 LPC at this moment.
Thanks,
Zhichang
>
>>> We may also want to move the inb/outb definitions into a .c file
>>> as they are getting rather big.
>> The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> The current implementation turns into a single CPU instruction, my idea
> was not to grow that too much but instead turn it into a branch instruction
> when your code is enabled at compile-time. When it's disabled, we should
> still use the existing behavior.
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 15:21 ` kbuild test robot
-1 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2016-09-07 15:21 UTC (permalink / raw)
To: Zhichang Yuan
Cc: kbuild-all, linuxarm, linux-arm-kernel, linux-kernel, arnd,
minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
Hi zhichang.yuan,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc5 next-20160907]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160907-212837
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
drivers/of/address.c: In function '__of_address_to_resource':
>> drivers/of/address.c:691:2: error: implicit declaration of function 'of_bus_pci_match'
vim +/of_bus_pci_match +691 drivers/of/address.c
685 if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
686 return -EINVAL;
687 taddr = of_translate_address(dev, addrp);
688 if (taddr == OF_BAD_ADDR)
689 return -EINVAL;
690 memset(r, 0, sizeof(struct resource));
> 691 if (flags & IORESOURCE_IO && of_bus_pci_match(dev)) {
692 unsigned long port;
693
694 port = pci_address_to_pio(taddr);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 7194 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 13:33 ` Zhichang Yuan
-1 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linuxarm, linux-arm-kernel, linux-kernel
Cc: arnd, minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
On Hip06, the accesses to LPC peripherals work in an indirect way. A
corresponding LPC driver need to configure some registers in LPC master
direclty, then the real accesses on LPC slave devices were finished by the
master controller.
This patch implement the relevant driver for Hip06 LPC.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
.../arm/hisilicon/hisilicon-low-pin-count.txt | 27 ++
drivers/bus/Kconfig | 8 +
drivers/bus/Makefile | 1 +
drivers/bus/hisi_lpc.c | 496 +++++++++++++++++++++
4 files changed, 532 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
create mode 100644 drivers/bus/hisi_lpc.c
diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
new file mode 100644
index 0000000..eb54d82
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
@@ -0,0 +1,27 @@
+Hisilicon Hip06 low-pin-count device
+ Usually LPC controller is part of PCI host bridge, so the legacy ISA ports
+ locate on LPC bus can be accessed direclty. But some SoCs have independent
+ LPC controller, and access the legacy ports by triggering LPC I/O cycles.
+ Hisilicon Hip06 implements this LPC device.
+
+Required properties:
+- compatible: should be "hisilicon,low-pin-count"
+- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
+- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
+- reg: base address and length of the register set for the device.
+- ranges: define a 1:1 mapping between the I/O space of the child device and
+ the parent.
+
+Note:
+ The node name before '@' must be "isa" to represent the binding stick to the
+ ISA/EISA binding specification.
+
+Example:
+
+isa@a01b0000 {
+ compatible = "hisilicom,low-pin-count";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b0000 0x0 0x1000>;
+ ranges = <0x01 0x0 0x0 0x0 0x1000>;
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 3b205e2..fdb232b 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -64,6 +64,14 @@ config BRCMSTB_GISB_ARB
arbiter. This driver provides timeout and target abort error handling
and internal bus master decoding.
+config HISILICON_LPC
+ bool "Workaround for nonstandard ISA I/O space on Hisilicon Hip0X"
+ depends on (ARCH_HISI || COMPILE_TEST) && ARM64
+ select ARM64_INDIRECT_PIO
+ help
+ Driver needed for some legacy ISA devices attached to Low-Pin-Count
+ on Hisilicon Hip0X Soc.
+
config IMX_WEIM
bool "Freescale EIM DRIVER"
depends on ARCH_MXC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ac84cc4..8b79b75 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ARM_CCN) += arm-ccn.o
obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o
obj-$(CONFIG_IMX_WEIM) += imx-weim.o
+obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o
obj-$(CONFIG_MIPS_CDMM) += mips_cdmm.o
obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
new file mode 100644
index 0000000..7ac0551
--- /dev/null
+++ b/drivers/bus/hisi_lpc.c
@@ -0,0 +1,496 @@
+/*
+ * 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/acpi.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/serial_8250.h>
+#include <linux/slab.h>
+
+
+struct hisilpc_dev;
+
+/* This flag is specific to differentiate earlycon operations and the others */
+#define FG_EARLYCON_LPC (0x01U << 0)
+/*
+ * this bit set means each IO operation will target to different port address;
+ * 0 means repeatly IO operations will be sticked on the same port, such as BT;
+ */
+#define FG_INCRADDR_LPC (0x01U << 1)
+
+struct lpc_cycle_para {
+ unsigned int opflags;
+ unsigned int csize;/*the data length of each operation*/
+};
+
+struct lpc_io_ops {
+ unsigned int periosz;
+ int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
+ unsigned long ptaddr, unsigned char *buf,
+ unsigned long dlen);
+ int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
+ unsigned long ptaddr,
+ const unsigned char *buf,
+ unsigned long dlen);
+};
+
+struct hisilpc_dev {
+ spinlock_t cycle_lock;
+ void __iomem *membase;
+ struct extio_ops *io_ops;
+ struct platform_device *pltdev;
+ struct lpc_io_ops target_io;
+};
+
+
+/* The maximum continous operations*/
+#define LPC_MAX_OPCNT 16
+
+#define LPC_REG_START (0x00)/*start a new LPC cycle*/
+#define LPC_REG_OP_STATUS (0x04)/*the current LPC status*/
+#define LPC_REG_IRQ_ST (0x08)/*interrupt enable&status*/
+#define LPC_REG_OP_LEN (0x10)/*how many LPC cycles each start*/
+#define LPC_REG_CMD (0x14)/*command for the required LPC cycle*/
+#define LPC_REG_ADDR (0x20)/*LPC target address*/
+#define LPC_REG_WDATA (0x24)/*data to be written*/
+#define LPC_REG_RDATA (0x28)/*data coming from peer*/
+
+
+/* The command register fields*/
+#define LPC_CMD_SAMEADDR_SING (0x00000008)
+#define LPC_CMD_SAMEADDR_INC (0x00000000)
+#define LPC_CMD_TYPE_IO (0x00000000)
+#define LPC_CMD_TYPE_MEM (0x00000002)
+#define LPC_CMD_TYPE_FWH (0x00000004)
+#define LPC_CMD_WRITE (0x00000001)
+#define LPC_CMD_READ (0x00000000)
+
+#define LPC_IRQ_CLEAR (0x02)
+#define LPC_IRQ_OCCURRED (0x02)
+
+#define LPC_STATUS_IDLE (0x01)
+#define LPC_OP_FINISHED (0x02)
+
+#define START_WORK (0x01)
+
+/*
+ * The minimal waiting interval... Suggest it is not less than 10.
+ * Bigger value probably will lower the performance.
+ */
+#define LPC_NSEC_PERWAIT 100
+/*
+ * The maximum waiting time is about 128us.
+ * The fastest IO cycle time is about 390ns, but the worst case will wait
+ * for extra 256 lpc clocks, so (256 + 13) * 30ns = 8 us. The maximum
+ * burst cycles is 16. So, the maximum waiting time is about 128us under
+ * worst case.
+ * choose 1300 as the maximum.
+ */
+#define LPC_MAX_WAITCNT 1300
+/* About 10us. This is specfic for single IO operation, such as inb. */
+#define LPC_PEROP_WAITCNT 100
+
+
+struct extio_ops *arm64_simops __refdata;
+
+
+static inline int wait_lpc_idle(unsigned char *mbase,
+ unsigned int waitcnt) {
+ u32 opstatus = 0;
+
+ while (waitcnt--) {
+ ndelay(LPC_NSEC_PERWAIT);
+ opstatus = readl(mbase + LPC_REG_OP_STATUS);
+ if (opstatus & LPC_STATUS_IDLE)
+ return (opstatus & LPC_OP_FINISHED) ? 0 : (-EIO);
+ }
+ return -ETIME;
+}
+
+
+/**
+ * hisilpc_target_in - trigger a series of lpc cycles to read required data
+ * from target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the read back data is stored
+ * @opcnt: how many I/O operations required in this calling
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_in(struct hisilpc_dev *pdev,
+ struct lpc_cycle_para *para,
+ unsigned long ptaddr, unsigned char *buf,
+ unsigned long opcnt)
+{
+ unsigned int cmd_word;
+ unsigned int waitcnt;
+ int retval;
+ /*initialized as 0 to remove compile warning */
+ unsigned long flags = 0;
+
+ if (!buf || !opcnt || !para || !pdev)
+ return -EINVAL;
+
+ if (para->csize != 1 || opcnt > LPC_MAX_OPCNT)
+ return -EINVAL;
+
+ cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_READ;
+ waitcnt = (LPC_PEROP_WAITCNT);
+ if (!(para->opflags & FG_INCRADDR_LPC)) {
+ cmd_word |= LPC_CMD_SAMEADDR_SING;
+ waitcnt = LPC_MAX_WAITCNT;
+ }
+
+ /* whole operation must be atomic */
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+ writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+
+ writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+ writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+ writel(START_WORK, pdev->membase + LPC_REG_START);
+
+ /* whether the operation is finished */
+ retval = wait_lpc_idle(pdev->membase, waitcnt);
+ if (!retval) {
+ for (; opcnt--; buf++)
+ *buf = readl(pdev->membase + LPC_REG_RDATA);
+ }
+
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+ return retval;
+}
+
+/**
+ * hisilpc_target_out - trigger a series of lpc cycles to write required data
+ * to target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the data to be written is stored
+ * @opcnt: how many I/O operations required
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_out(struct hisilpc_dev *pdev,
+ struct lpc_cycle_para *para,
+ unsigned long ptaddr,
+ const unsigned char *buf,
+ unsigned long opcnt)
+{
+ unsigned int cmd_word;
+ unsigned int waitcnt;
+ int retval;
+ /* initialized as 0 to remove compile warning */
+ unsigned long flags = 0;
+
+
+ if (!buf || !opcnt || !para || !pdev)
+ return -EINVAL;
+
+ if (para->csize != 1 || opcnt > LPC_MAX_OPCNT)
+ return -EINVAL;
+
+ cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE;
+ waitcnt = (LPC_PEROP_WAITCNT);
+ if (!(para->opflags & FG_INCRADDR_LPC)) {
+ cmd_word |= LPC_CMD_SAMEADDR_SING;
+ waitcnt = LPC_MAX_WAITCNT;
+ }
+
+ /* whole operation must be atomic */
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+ writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+ for (; opcnt--; buf++)
+ writel(*buf, pdev->membase + LPC_REG_WDATA);
+
+ writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+ writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+ writel(START_WORK, pdev->membase + LPC_REG_START);
+
+ /* whether the operation is finished */
+ retval = wait_lpc_idle(pdev->membase, waitcnt);
+
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+ return retval;
+}
+
+/**
+ * hisilpc_comm_inb - read/input the data from the I/O peripheral through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data read is stored at.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required to read from the target I/O port.
+ * @count: how many I/O operations required in this calling. >1 is for ins.
+ *
+ * For this lpc, only support inb/insb now.
+ *
+ * For inbs, returns 0 on success, -1 on fail.
+ * when succeed, the data read back is stored in buffer pointed by inbuf.
+ * For inb, return the data read from I/O or -1 when error occur.
+ */
+u64 hisilpc_comm_inb(void *devobj, unsigned long ptaddr,
+ void *inbuf, size_t dlen,
+ unsigned int count)
+{
+ struct hisilpc_dev *lpcdev;
+ struct lpc_cycle_para iopara;
+ unsigned int loopcnt, cntleft;
+ unsigned int rd_data;
+ unsigned char *newbuf;
+ int ret = 0;
+ /* only support data unit length is 1 now... */
+ if (!count || (!inbuf && count != 1) || !devobj || dlen != 1)
+ return -1;
+
+ newbuf = (unsigned char *)inbuf;
+ /*
+ * the operation data len is 4 bytes, need to ensure the buffer
+ * is big enough.
+ */
+ if (!inbuf || count < sizeof(u32))
+ newbuf = (unsigned char *)&rd_data;
+
+ lpcdev = (struct hisilpc_dev *)devobj;
+ dev_dbg(&lpcdev->pltdev->dev, "In-IO(0x%lx), count=%u\n", ptaddr,
+ count);
+
+ iopara.opflags = FG_INCRADDR_LPC;
+ /*
+ * to improve performance, support repeatly rd at same target
+ * address.
+ */
+ if (count > 1)
+ iopara.opflags &= ~FG_INCRADDR_LPC;
+
+ iopara.csize = dlen;
+
+ cntleft = count;
+ do {
+ loopcnt = (cntleft > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : cntleft;
+ ret = lpcdev->target_io.lpc_iord(lpcdev,
+ &iopara, ptaddr, newbuf, loopcnt);
+ if (ret)
+ return -1;
+ newbuf += loopcnt;
+ cntleft -= loopcnt;
+ } while (cntleft);
+
+ /* for inb */
+ if (!inbuf)
+ return rd_data;
+ /* for insb, copy the data to the return variable */
+ if (inbuf != newbuf)
+ memcpy(inbuf, &rd_data, count);
+
+ return 0;
+}
+
+/**
+ * hisilpc_comm_outb - write/output the data in out buffer to the I/O peripheral
+ * through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data to be written is stored.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required writing to the target I/O port .
+ * @count: how many I/O operations required in this calling. >1 is for outs.
+ *
+ * For this lpc, only support outb/outsb now.
+ *
+ */
+void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
+ const void *outbuf, size_t dlen,
+ unsigned int count)
+{
+ struct hisilpc_dev *lpcdev;
+ struct lpc_cycle_para iopara;
+ unsigned int loopcnt;
+ const unsigned char *newbuf;
+ int ret = 0;
+
+ if (!count || !outbuf || !devobj)
+ return;
+
+ newbuf = (const unsigned char *)outbuf;
+ lpcdev = (struct hisilpc_dev *)devobj;
+
+ dev_dbg(&lpcdev->pltdev->dev, "Out-IO(0x%lx), cnt=%u\n", ptaddr, count);
+
+ iopara.opflags = FG_INCRADDR_LPC;
+ /* to improve performance, support repeatly wr same target address */
+ if (count > 1)
+ iopara.opflags &= ~FG_INCRADDR_LPC;
+
+ iopara.csize = 1;
+
+ do {
+ loopcnt = (count > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : count;
+ ret = lpcdev->target_io.lpc_iowr(lpcdev,
+ &iopara, ptaddr, newbuf, loopcnt);
+ if (ret)
+ return;
+ newbuf += loopcnt;
+ count -= loopcnt;
+ } while (count);
+}
+
+
+/**
+ * hisilpc_probe - the probe callback function for hisi lpc device,
+ * will finish all the intialization.
+ * @pdev: the platform device corresponding to hisi lpc
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_probe(struct platform_device *pdev)
+{
+ struct resource *iores;
+ struct hisilpc_dev *lpcdev;
+
+ dev_dbg(&pdev->dev, "hslpc start probing...\n");
+
+ lpcdev = devm_kzalloc(&pdev->dev,
+ sizeof(struct hisilpc_dev), GFP_KERNEL);
+ if (!lpcdev)
+ return -ENOMEM;
+
+ spin_lock_init(&lpcdev->cycle_lock);
+
+ iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ lpcdev->membase = devm_ioremap_resource(&pdev->dev, iores);
+ if (IS_ERR(lpcdev->membase)) {
+ dev_err(&pdev->dev, "No mem resource\n");
+ return PTR_ERR(lpcdev->membase);
+ }
+
+ lpcdev->target_io.periosz = 0x01;
+ lpcdev->target_io.lpc_iord = hisilpc_target_in;
+ lpcdev->target_io.lpc_iowr = hisilpc_target_out;
+
+
+ lpcdev->pltdev = pdev;
+ platform_set_drvdata(pdev, lpcdev);
+
+ lpcdev->io_ops = devm_kzalloc(&pdev->dev,
+ sizeof(*lpcdev->io_ops),
+ GFP_KERNEL);
+ if (!lpcdev->io_ops) {
+ dev_err(&pdev->dev, "memory allocate fail!\n");
+ return -ENOMEM;
+ }
+
+ lpcdev->io_ops->pfin = hisilpc_comm_inb;
+ lpcdev->io_ops->pfout = hisilpc_comm_outb;
+ lpcdev->io_ops->devpara = (void *)lpcdev;
+
+ if (!has_acpi_companion(&pdev->dev)) {
+ struct device_node *root, *child;
+
+ root = pdev->dev.of_node;
+ for_each_available_child_of_node(root, child) {
+ if (!of_platform_device_create(child, NULL, &pdev->dev)) {
+ dev_err(&pdev->dev, "create platform device fail for %s\n",
+ child->name);
+ return -EFAULT;
+ }
+ dev_info(&pdev->dev, "create platform device OK for %s\n",
+ child->name);
+ }
+ }
+
+ /*should lock the earlycon callings at first*/
+ console_lock();
+ arm64_set_simops(lpcdev->io_ops);
+ console_unlock();
+
+ dev_dbg(&pdev->dev, "hslpc finish probing... extio_ops = %p\n",
+ arm64_simops);
+
+ return 0;
+}
+
+/**
+ * hisilpc_remove - the remove callback function for hisi lpc device.
+ * @pdev: the platform device corresponding to hisi lpc that is to be removed.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+
+static const struct of_device_id hisilpc_of_match[] = {
+ {
+ .compatible = "hisilicon,low-pin-count",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, hisilpc_of_match);
+
+static const struct acpi_device_id hisilpc_acpi_match[] = {
+ {"HISI0191", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(acpi, hisilpc_acpi_match);
+
+static struct platform_driver hisilpc_driver = {
+ .driver = {
+ .name = "hisi_lpc",
+ .of_match_table = hisilpc_of_match,
+ .acpi_match_table = hisilpc_acpi_match,
+ },
+ .probe = hisilpc_probe,
+ .remove = hisilpc_remove,
+};
+
+
+module_platform_driver(hisilpc_driver);
+
+MODULE_AUTHOR("Zhichang Yuan");
+MODULE_DESCRIPTION("The LPC driver for Hip06 SoC based on indirect-IO");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("v1.0");
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-07 13:33 ` Zhichang Yuan
0 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linux-arm-kernel
From: "zhichang.yuan" <yuanzhichang@hisilicon.com>
On Hip06, the accesses to LPC peripherals work in an indirect way. A
corresponding LPC driver need to configure some registers in LPC master
direclty, then the real accesses on LPC slave devices were finished by the
master controller.
This patch implement the relevant driver for Hip06 LPC.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
.../arm/hisilicon/hisilicon-low-pin-count.txt | 27 ++
drivers/bus/Kconfig | 8 +
drivers/bus/Makefile | 1 +
drivers/bus/hisi_lpc.c | 496 +++++++++++++++++++++
4 files changed, 532 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
create mode 100644 drivers/bus/hisi_lpc.c
diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
new file mode 100644
index 0000000..eb54d82
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt
@@ -0,0 +1,27 @@
+Hisilicon Hip06 low-pin-count device
+ Usually LPC controller is part of PCI host bridge, so the legacy ISA ports
+ locate on LPC bus can be accessed direclty. But some SoCs have independent
+ LPC controller, and access the legacy ports by triggering LPC I/O cycles.
+ Hisilicon Hip06 implements this LPC device.
+
+Required properties:
+- compatible: should be "hisilicon,low-pin-count"
+- #address-cells: must be 2 which stick to the ISA/EISA binding doc.
+- #size-cells: must be 1 which stick to the ISA/EISA binding doc.
+- reg: base address and length of the register set for the device.
+- ranges: define a 1:1 mapping between the I/O space of the child device and
+ the parent.
+
+Note:
+ The node name before '@' must be "isa" to represent the binding stick to the
+ ISA/EISA binding specification.
+
+Example:
+
+isa at a01b0000 {
+ compatible = "hisilicom,low-pin-count";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b0000 0x0 0x1000>;
+ ranges = <0x01 0x0 0x0 0x0 0x1000>;
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 3b205e2..fdb232b 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -64,6 +64,14 @@ config BRCMSTB_GISB_ARB
arbiter. This driver provides timeout and target abort error handling
and internal bus master decoding.
+config HISILICON_LPC
+ bool "Workaround for nonstandard ISA I/O space on Hisilicon Hip0X"
+ depends on (ARCH_HISI || COMPILE_TEST) && ARM64
+ select ARM64_INDIRECT_PIO
+ help
+ Driver needed for some legacy ISA devices attached to Low-Pin-Count
+ on Hisilicon Hip0X Soc.
+
config IMX_WEIM
bool "Freescale EIM DRIVER"
depends on ARCH_MXC
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ac84cc4..8b79b75 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_ARM_CCN) += arm-ccn.o
obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o
obj-$(CONFIG_IMX_WEIM) += imx-weim.o
+obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o
obj-$(CONFIG_MIPS_CDMM) += mips_cdmm.o
obj-$(CONFIG_MVEBU_MBUS) += mvebu-mbus.o
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
new file mode 100644
index 0000000..7ac0551
--- /dev/null
+++ b/drivers/bus/hisi_lpc.c
@@ -0,0 +1,496 @@
+/*
+ * 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/acpi.h>
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/serial_8250.h>
+#include <linux/slab.h>
+
+
+struct hisilpc_dev;
+
+/* This flag is specific to differentiate earlycon operations and the others */
+#define FG_EARLYCON_LPC (0x01U << 0)
+/*
+ * this bit set means each IO operation will target to different port address;
+ * 0 means repeatly IO operations will be sticked on the same port, such as BT;
+ */
+#define FG_INCRADDR_LPC (0x01U << 1)
+
+struct lpc_cycle_para {
+ unsigned int opflags;
+ unsigned int csize;/*the data length of each operation*/
+};
+
+struct lpc_io_ops {
+ unsigned int periosz;
+ int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
+ unsigned long ptaddr, unsigned char *buf,
+ unsigned long dlen);
+ int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
+ unsigned long ptaddr,
+ const unsigned char *buf,
+ unsigned long dlen);
+};
+
+struct hisilpc_dev {
+ spinlock_t cycle_lock;
+ void __iomem *membase;
+ struct extio_ops *io_ops;
+ struct platform_device *pltdev;
+ struct lpc_io_ops target_io;
+};
+
+
+/* The maximum continous operations*/
+#define LPC_MAX_OPCNT 16
+
+#define LPC_REG_START (0x00)/*start a new LPC cycle*/
+#define LPC_REG_OP_STATUS (0x04)/*the current LPC status*/
+#define LPC_REG_IRQ_ST (0x08)/*interrupt enable&status*/
+#define LPC_REG_OP_LEN (0x10)/*how many LPC cycles each start*/
+#define LPC_REG_CMD (0x14)/*command for the required LPC cycle*/
+#define LPC_REG_ADDR (0x20)/*LPC target address*/
+#define LPC_REG_WDATA (0x24)/*data to be written*/
+#define LPC_REG_RDATA (0x28)/*data coming from peer*/
+
+
+/* The command register fields*/
+#define LPC_CMD_SAMEADDR_SING (0x00000008)
+#define LPC_CMD_SAMEADDR_INC (0x00000000)
+#define LPC_CMD_TYPE_IO (0x00000000)
+#define LPC_CMD_TYPE_MEM (0x00000002)
+#define LPC_CMD_TYPE_FWH (0x00000004)
+#define LPC_CMD_WRITE (0x00000001)
+#define LPC_CMD_READ (0x00000000)
+
+#define LPC_IRQ_CLEAR (0x02)
+#define LPC_IRQ_OCCURRED (0x02)
+
+#define LPC_STATUS_IDLE (0x01)
+#define LPC_OP_FINISHED (0x02)
+
+#define START_WORK (0x01)
+
+/*
+ * The minimal waiting interval... Suggest it is not less than 10.
+ * Bigger value probably will lower the performance.
+ */
+#define LPC_NSEC_PERWAIT 100
+/*
+ * The maximum waiting time is about 128us.
+ * The fastest IO cycle time is about 390ns, but the worst case will wait
+ * for extra 256 lpc clocks, so (256 + 13) * 30ns = 8 us. The maximum
+ * burst cycles is 16. So, the maximum waiting time is about 128us under
+ * worst case.
+ * choose 1300 as the maximum.
+ */
+#define LPC_MAX_WAITCNT 1300
+/* About 10us. This is specfic for single IO operation, such as inb. */
+#define LPC_PEROP_WAITCNT 100
+
+
+struct extio_ops *arm64_simops __refdata;
+
+
+static inline int wait_lpc_idle(unsigned char *mbase,
+ unsigned int waitcnt) {
+ u32 opstatus = 0;
+
+ while (waitcnt--) {
+ ndelay(LPC_NSEC_PERWAIT);
+ opstatus = readl(mbase + LPC_REG_OP_STATUS);
+ if (opstatus & LPC_STATUS_IDLE)
+ return (opstatus & LPC_OP_FINISHED) ? 0 : (-EIO);
+ }
+ return -ETIME;
+}
+
+
+/**
+ * hisilpc_target_in - trigger a series of lpc cycles to read required data
+ * from target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the read back data is stored
+ * @opcnt: how many I/O operations required in this calling
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_in(struct hisilpc_dev *pdev,
+ struct lpc_cycle_para *para,
+ unsigned long ptaddr, unsigned char *buf,
+ unsigned long opcnt)
+{
+ unsigned int cmd_word;
+ unsigned int waitcnt;
+ int retval;
+ /*initialized as 0 to remove compile warning */
+ unsigned long flags = 0;
+
+ if (!buf || !opcnt || !para || !pdev)
+ return -EINVAL;
+
+ if (para->csize != 1 || opcnt > LPC_MAX_OPCNT)
+ return -EINVAL;
+
+ cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_READ;
+ waitcnt = (LPC_PEROP_WAITCNT);
+ if (!(para->opflags & FG_INCRADDR_LPC)) {
+ cmd_word |= LPC_CMD_SAMEADDR_SING;
+ waitcnt = LPC_MAX_WAITCNT;
+ }
+
+ /* whole operation must be atomic */
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+ writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+
+ writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+ writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+ writel(START_WORK, pdev->membase + LPC_REG_START);
+
+ /* whether the operation is finished */
+ retval = wait_lpc_idle(pdev->membase, waitcnt);
+ if (!retval) {
+ for (; opcnt--; buf++)
+ *buf = readl(pdev->membase + LPC_REG_RDATA);
+ }
+
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+ return retval;
+}
+
+/**
+ * hisilpc_target_out - trigger a series of lpc cycles to write required data
+ * to target periperal.
+ * @pdev: pointer to hisi lpc device
+ * @para: some paramerters used to control the lpc I/O operations
+ * @ptaddr: the lpc I/O target port address
+ * @buf: where the data to be written is stored
+ * @opcnt: how many I/O operations required
+ *
+ * only one byte data is read each I/O operation.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_target_out(struct hisilpc_dev *pdev,
+ struct lpc_cycle_para *para,
+ unsigned long ptaddr,
+ const unsigned char *buf,
+ unsigned long opcnt)
+{
+ unsigned int cmd_word;
+ unsigned int waitcnt;
+ int retval;
+ /* initialized as 0 to remove compile warning */
+ unsigned long flags = 0;
+
+
+ if (!buf || !opcnt || !para || !pdev)
+ return -EINVAL;
+
+ if (para->csize != 1 || opcnt > LPC_MAX_OPCNT)
+ return -EINVAL;
+
+ cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE;
+ waitcnt = (LPC_PEROP_WAITCNT);
+ if (!(para->opflags & FG_INCRADDR_LPC)) {
+ cmd_word |= LPC_CMD_SAMEADDR_SING;
+ waitcnt = LPC_MAX_WAITCNT;
+ }
+
+ /* whole operation must be atomic */
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_lock_irqsave(&pdev->cycle_lock, flags);
+
+ writel(opcnt, pdev->membase + LPC_REG_OP_LEN);
+ for (; opcnt--; buf++)
+ writel(*buf, pdev->membase + LPC_REG_WDATA);
+
+ writel(cmd_word, pdev->membase + LPC_REG_CMD);
+
+ writel(ptaddr, pdev->membase + LPC_REG_ADDR);
+
+ writel(START_WORK, pdev->membase + LPC_REG_START);
+
+ /* whether the operation is finished */
+ retval = wait_lpc_idle(pdev->membase, waitcnt);
+
+ if (!(para->opflags & FG_EARLYCON_LPC))
+ spin_unlock_irqrestore(&pdev->cycle_lock, flags);
+
+ return retval;
+}
+
+/**
+ * hisilpc_comm_inb - read/input the data from the I/O peripheral through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data read is stored at.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required to read from the target I/O port.
+ * @count: how many I/O operations required in this calling. >1 is for ins.
+ *
+ * For this lpc, only support inb/insb now.
+ *
+ * For inbs, returns 0 on success, -1 on fail.
+ * when succeed, the data read back is stored in buffer pointed by inbuf.
+ * For inb, return the data read from I/O or -1 when error occur.
+ */
+u64 hisilpc_comm_inb(void *devobj, unsigned long ptaddr,
+ void *inbuf, size_t dlen,
+ unsigned int count)
+{
+ struct hisilpc_dev *lpcdev;
+ struct lpc_cycle_para iopara;
+ unsigned int loopcnt, cntleft;
+ unsigned int rd_data;
+ unsigned char *newbuf;
+ int ret = 0;
+ /* only support data unit length is 1 now... */
+ if (!count || (!inbuf && count != 1) || !devobj || dlen != 1)
+ return -1;
+
+ newbuf = (unsigned char *)inbuf;
+ /*
+ * the operation data len is 4 bytes, need to ensure the buffer
+ * is big enough.
+ */
+ if (!inbuf || count < sizeof(u32))
+ newbuf = (unsigned char *)&rd_data;
+
+ lpcdev = (struct hisilpc_dev *)devobj;
+ dev_dbg(&lpcdev->pltdev->dev, "In-IO(0x%lx), count=%u\n", ptaddr,
+ count);
+
+ iopara.opflags = FG_INCRADDR_LPC;
+ /*
+ * to improve performance, support repeatly rd at same target
+ * address.
+ */
+ if (count > 1)
+ iopara.opflags &= ~FG_INCRADDR_LPC;
+
+ iopara.csize = dlen;
+
+ cntleft = count;
+ do {
+ loopcnt = (cntleft > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : cntleft;
+ ret = lpcdev->target_io.lpc_iord(lpcdev,
+ &iopara, ptaddr, newbuf, loopcnt);
+ if (ret)
+ return -1;
+ newbuf += loopcnt;
+ cntleft -= loopcnt;
+ } while (cntleft);
+
+ /* for inb */
+ if (!inbuf)
+ return rd_data;
+ /* for insb, copy the data to the return variable */
+ if (inbuf != newbuf)
+ memcpy(inbuf, &rd_data, count);
+
+ return 0;
+}
+
+/**
+ * hisilpc_comm_outb - write/output the data in out buffer to the I/O peripheral
+ * through LPC.
+ * @devobj: pointer to the device information relevant to LPC controller.
+ * @outbuf: a buffer where the data to be written is stored.
+ * @ptaddr: the target I/O port address.
+ * @dlen: the data length required writing to the target I/O port .
+ * @count: how many I/O operations required in this calling. >1 is for outs.
+ *
+ * For this lpc, only support outb/outsb now.
+ *
+ */
+void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
+ const void *outbuf, size_t dlen,
+ unsigned int count)
+{
+ struct hisilpc_dev *lpcdev;
+ struct lpc_cycle_para iopara;
+ unsigned int loopcnt;
+ const unsigned char *newbuf;
+ int ret = 0;
+
+ if (!count || !outbuf || !devobj)
+ return;
+
+ newbuf = (const unsigned char *)outbuf;
+ lpcdev = (struct hisilpc_dev *)devobj;
+
+ dev_dbg(&lpcdev->pltdev->dev, "Out-IO(0x%lx), cnt=%u\n", ptaddr, count);
+
+ iopara.opflags = FG_INCRADDR_LPC;
+ /* to improve performance, support repeatly wr same target address */
+ if (count > 1)
+ iopara.opflags &= ~FG_INCRADDR_LPC;
+
+ iopara.csize = 1;
+
+ do {
+ loopcnt = (count > LPC_MAX_OPCNT) ? LPC_MAX_OPCNT : count;
+ ret = lpcdev->target_io.lpc_iowr(lpcdev,
+ &iopara, ptaddr, newbuf, loopcnt);
+ if (ret)
+ return;
+ newbuf += loopcnt;
+ count -= loopcnt;
+ } while (count);
+}
+
+
+/**
+ * hisilpc_probe - the probe callback function for hisi lpc device,
+ * will finish all the intialization.
+ * @pdev: the platform device corresponding to hisi lpc
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_probe(struct platform_device *pdev)
+{
+ struct resource *iores;
+ struct hisilpc_dev *lpcdev;
+
+ dev_dbg(&pdev->dev, "hslpc start probing...\n");
+
+ lpcdev = devm_kzalloc(&pdev->dev,
+ sizeof(struct hisilpc_dev), GFP_KERNEL);
+ if (!lpcdev)
+ return -ENOMEM;
+
+ spin_lock_init(&lpcdev->cycle_lock);
+
+ iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ lpcdev->membase = devm_ioremap_resource(&pdev->dev, iores);
+ if (IS_ERR(lpcdev->membase)) {
+ dev_err(&pdev->dev, "No mem resource\n");
+ return PTR_ERR(lpcdev->membase);
+ }
+
+ lpcdev->target_io.periosz = 0x01;
+ lpcdev->target_io.lpc_iord = hisilpc_target_in;
+ lpcdev->target_io.lpc_iowr = hisilpc_target_out;
+
+
+ lpcdev->pltdev = pdev;
+ platform_set_drvdata(pdev, lpcdev);
+
+ lpcdev->io_ops = devm_kzalloc(&pdev->dev,
+ sizeof(*lpcdev->io_ops),
+ GFP_KERNEL);
+ if (!lpcdev->io_ops) {
+ dev_err(&pdev->dev, "memory allocate fail!\n");
+ return -ENOMEM;
+ }
+
+ lpcdev->io_ops->pfin = hisilpc_comm_inb;
+ lpcdev->io_ops->pfout = hisilpc_comm_outb;
+ lpcdev->io_ops->devpara = (void *)lpcdev;
+
+ if (!has_acpi_companion(&pdev->dev)) {
+ struct device_node *root, *child;
+
+ root = pdev->dev.of_node;
+ for_each_available_child_of_node(root, child) {
+ if (!of_platform_device_create(child, NULL, &pdev->dev)) {
+ dev_err(&pdev->dev, "create platform device fail for %s\n",
+ child->name);
+ return -EFAULT;
+ }
+ dev_info(&pdev->dev, "create platform device OK for %s\n",
+ child->name);
+ }
+ }
+
+ /*should lock the earlycon callings at first*/
+ console_lock();
+ arm64_set_simops(lpcdev->io_ops);
+ console_unlock();
+
+ dev_dbg(&pdev->dev, "hslpc finish probing... extio_ops = %p\n",
+ arm64_simops);
+
+ return 0;
+}
+
+/**
+ * hisilpc_remove - the remove callback function for hisi lpc device.
+ * @pdev: the platform device corresponding to hisi lpc that is to be removed.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static int hisilpc_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+
+static const struct of_device_id hisilpc_of_match[] = {
+ {
+ .compatible = "hisilicon,low-pin-count",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, hisilpc_of_match);
+
+static const struct acpi_device_id hisilpc_acpi_match[] = {
+ {"HISI0191", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(acpi, hisilpc_acpi_match);
+
+static struct platform_driver hisilpc_driver = {
+ .driver = {
+ .name = "hisi_lpc",
+ .of_match_table = hisilpc_of_match,
+ .acpi_match_table = hisilpc_acpi_match,
+ },
+ .probe = hisilpc_probe,
+ .remove = hisilpc_remove,
+};
+
+
+module_platform_driver(hisilpc_driver);
+
+MODULE_AUTHOR("Zhichang Yuan");
+MODULE_DESCRIPTION("The LPC driver for Hip06 SoC based on indirect-IO");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("v1.0");
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 15:27 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 15:27 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong
On Wednesday, September 7, 2016 9:33:51 PM CEST Zhichang Yuan wrote:
> +
> +struct hisilpc_dev;
> +
> +/* This flag is specific to differentiate earlycon operations and the others */
> +#define FG_EARLYCON_LPC (0x01U << 0)
> +/*
> + * this bit set means each IO operation will target to different port address;
> + * 0 means repeatly IO operations will be sticked on the same port, such as BT;
> + */
> +#define FG_INCRADDR_LPC (0x01U << 1)
Better express the constants as
#define FG_EARLYCON_LPC 0x0001
#define FG_INCRADDR_LPC 0x0002
> +struct lpc_io_ops {
> + unsigned int periosz;
> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> + unsigned long ptaddr, unsigned char *buf,
> + unsigned long dlen);
> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> + unsigned long ptaddr,
> + const unsigned char *buf,
> + unsigned long dlen);
> +};
The operations are not needed unless we also put the earlycon support
in, so maybe leave them out from the first patch and only add them
later (or drop the earlycon support if possible).
> +/**
> + * hisilpc_remove - the remove callback function for hisi lpc device.
> + * @pdev: the platform device corresponding to hisi lpc that is to be removed.
> + *
> + * Returns 0 on success, non-zero on fail.
> + *
> + */
> +static int hisilpc_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
It seems that it should not be possible to remove this driver,
please use builtin_platform_driver() then and remove this callback.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-07 15:27 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 15:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 7, 2016 9:33:51 PM CEST Zhichang Yuan wrote:
> +
> +struct hisilpc_dev;
> +
> +/* This flag is specific to differentiate earlycon operations and the others */
> +#define FG_EARLYCON_LPC (0x01U << 0)
> +/*
> + * this bit set means each IO operation will target to different port address;
> + * 0 means repeatly IO operations will be sticked on the same port, such as BT;
> + */
> +#define FG_INCRADDR_LPC (0x01U << 1)
Better express the constants as
#define FG_EARLYCON_LPC 0x0001
#define FG_INCRADDR_LPC 0x0002
> +struct lpc_io_ops {
> + unsigned int periosz;
> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> + unsigned long ptaddr, unsigned char *buf,
> + unsigned long dlen);
> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> + unsigned long ptaddr,
> + const unsigned char *buf,
> + unsigned long dlen);
> +};
The operations are not needed unless we also put the earlycon support
in, so maybe leave them out from the first patch and only add them
later (or drop the earlycon support if possible).
> +/**
> + * hisilpc_remove - the remove callback function for hisi lpc device.
> + * @pdev: the platform device corresponding to hisi lpc that is to be removed.
> + *
> + * Returns 0 on success, non-zero on fail.
> + *
> + */
> +static int hisilpc_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
It seems that it should not be possible to remove this driver,
please use builtin_platform_driver() then and remove this callback.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-07 15:27 ` Arnd Bergmann
@ 2016-09-08 8:06 ` zhichang.yuan
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-08 8:06 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: linuxarm, linux-kernel, lorenzo.pieralisi, minyard, benh,
gabriele.paoloni, john.garry, liviu.dudau, zhichang.yuan02,
zourongrong
Hi, Arnd
On 2016/9/7 23:27, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:51 PM CEST Zhichang Yuan wrote:
>
>> +
>> +struct hisilpc_dev;
>> +
>> +/* This flag is specific to differentiate earlycon operations and the others */
>> +#define FG_EARLYCON_LPC (0x01U << 0)
>> +/*
>> + * this bit set means each IO operation will target to different port address;
>> + * 0 means repeatly IO operations will be sticked on the same port, such as BT;
>> + */
>> +#define FG_INCRADDR_LPC (0x01U << 1)
>
> Better express the constants as
>
> #define FG_EARLYCON_LPC 0x0001
> #define FG_INCRADDR_LPC 0x0002
Ok. Will revise.
>
>> +struct lpc_io_ops {
>> + unsigned int periosz;
>> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>> + unsigned long ptaddr, unsigned char *buf,
>> + unsigned long dlen);
>> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>> + unsigned long ptaddr,
>> + const unsigned char *buf,
>> + unsigned long dlen);
>> +};
>
> The operations are not needed unless we also put the earlycon support
> in, so maybe leave them out from the first patch and only add them
> later (or drop the earlycon support if possible).
Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
I think we can not do that.
These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
for 8250 serial.
I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
meaningful.
>
>> +/**
>> + * hisilpc_remove - the remove callback function for hisi lpc device.
>> + * @pdev: the platform device corresponding to hisi lpc that is to be removed.
>> + *
>> + * Returns 0 on success, non-zero on fail.
>> + *
>> + */
>> +static int hisilpc_remove(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>
> It seems that it should not be possible to remove this driver,
> please use builtin_platform_driver() then and remove this callback.
Yes. Will use builtin_platform_driver for the unnecessary remove callback.
Best,
Zhichang
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-08 8:06 ` zhichang.yuan
0 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-08 8:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Arnd
On 2016/9/7 23:27, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:51 PM CEST Zhichang Yuan wrote:
>
>> +
>> +struct hisilpc_dev;
>> +
>> +/* This flag is specific to differentiate earlycon operations and the others */
>> +#define FG_EARLYCON_LPC (0x01U << 0)
>> +/*
>> + * this bit set means each IO operation will target to different port address;
>> + * 0 means repeatly IO operations will be sticked on the same port, such as BT;
>> + */
>> +#define FG_INCRADDR_LPC (0x01U << 1)
>
> Better express the constants as
>
> #define FG_EARLYCON_LPC 0x0001
> #define FG_INCRADDR_LPC 0x0002
Ok. Will revise.
>
>> +struct lpc_io_ops {
>> + unsigned int periosz;
>> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>> + unsigned long ptaddr, unsigned char *buf,
>> + unsigned long dlen);
>> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>> + unsigned long ptaddr,
>> + const unsigned char *buf,
>> + unsigned long dlen);
>> +};
>
> The operations are not needed unless we also put the earlycon support
> in, so maybe leave them out from the first patch and only add them
> later (or drop the earlycon support if possible).
Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
I think we can not do that.
These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
for 8250 serial.
I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
meaningful.
>
>> +/**
>> + * hisilpc_remove - the remove callback function for hisi lpc device.
>> + * @pdev: the platform device corresponding to hisi lpc that is to be removed.
>> + *
>> + * Returns 0 on success, non-zero on fail.
>> + *
>> + */
>> +static int hisilpc_remove(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>
> It seems that it should not be possible to remove this driver,
> please use builtin_platform_driver() then and remove this callback.
Yes. Will use builtin_platform_driver for the unnecessary remove callback.
Best,
Zhichang
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-08 8:06 ` zhichang.yuan
@ 2016-09-08 10:00 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 10:00 UTC (permalink / raw)
To: zhichang.yuan
Cc: linux-arm-kernel, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong
On Thursday, September 8, 2016 4:06:01 PM CEST zhichang.yuan wrote:
> >
> >> +struct lpc_io_ops {
> >> + unsigned int periosz;
> >> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> >> + unsigned long ptaddr, unsigned char *buf,
> >> + unsigned long dlen);
> >> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> >> + unsigned long ptaddr,
> >> + const unsigned char *buf,
> >> + unsigned long dlen);
> >> +};
> >
> > The operations are not needed unless we also put the earlycon support
> > in, so maybe leave them out from the first patch and only add them
> > later (or drop the earlycon support if possible).
>
> Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
> I think we can not do that.
>
> These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
> when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
> for 8250 serial.
>
> I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
> struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
> type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
> meaningful.
My point was that the indirect function calls are not needed at
until patch four, so you can call the functions directly here,
and make the logic for indirect function calls part of that
later patch.
What are the other LPC cycle types that could be supported?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-08 10:00 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 10:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, September 8, 2016 4:06:01 PM CEST zhichang.yuan wrote:
> >
> >> +struct lpc_io_ops {
> >> + unsigned int periosz;
> >> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> >> + unsigned long ptaddr, unsigned char *buf,
> >> + unsigned long dlen);
> >> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
> >> + unsigned long ptaddr,
> >> + const unsigned char *buf,
> >> + unsigned long dlen);
> >> +};
> >
> > The operations are not needed unless we also put the earlycon support
> > in, so maybe leave them out from the first patch and only add them
> > later (or drop the earlycon support if possible).
>
> Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
> I think we can not do that.
>
> These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
> when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
> for 8250 serial.
>
> I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
> struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
> type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
> meaningful.
My point was that the indirect function calls are not needed at
until patch four, so you can call the functions directly here,
and make the logic for indirect function calls part of that
later patch.
What are the other LPC cycle types that could be supported?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-08 10:00 ` Arnd Bergmann
@ 2016-09-13 6:31 ` zhichang
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-13 6:31 UTC (permalink / raw)
To: Arnd Bergmann, zhichang.yuan
Cc: linux-arm-kernel, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zourongrong
On 2016年09月08日 18:00, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 4:06:01 PM CEST zhichang.yuan wrote:
>>>
>>>> +struct lpc_io_ops {
>>>> + unsigned int periosz;
>>>> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr, unsigned char *buf,
>>>> + unsigned long dlen);
>>>> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr,
>>>> + const unsigned char *buf,
>>>> + unsigned long dlen);
>>>> +};
>>>
>>> The operations are not needed unless we also put the earlycon support
>>> in, so maybe leave them out from the first patch and only add them
>>> later (or drop the earlycon support if possible).
>>
>> Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
>> I think we can not do that.
>>
>> These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
>> when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
>> for 8250 serial.
>>
>> I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
>> struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
>> type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
>> meaningful.
>
> My point was that the indirect function calls are not needed at
> until patch four, so you can call the functions directly here,
> and make the logic for indirect function calls part of that
> later patch.
O. I think I got your meaning now.
In patch V2, the lpc_io_ops is not needed even if earlycon is supported. The early_in/early_out operation is
defined in hisi_lpc.c too, we can directly call the hisilpc_target_in/hisilpc_target_out to finish the LPC IO
operations.
At this moment, all the IO functions specific to the child devices of hip06 LPC have their own indirect call
method. This lpc_io_ops will be removed in V3.
>
> What are the other LPC cycle types that could be supported?
O. memory and firmware operations are supported too. But at this moment, we only use IO cycle.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-13 6:31 ` zhichang
0 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-13 6:31 UTC (permalink / raw)
To: linux-arm-kernel
On 2016?09?08? 18:00, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 4:06:01 PM CEST zhichang.yuan wrote:
>>>
>>>> +struct lpc_io_ops {
>>>> + unsigned int periosz;
>>>> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr, unsigned char *buf,
>>>> + unsigned long dlen);
>>>> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr,
>>>> + const unsigned char *buf,
>>>> + unsigned long dlen);
>>>> +};
>>>
>>> The operations are not needed unless we also put the earlycon support
>>> in, so maybe leave them out from the first patch and only add them
>>> later (or drop the earlycon support if possible).
>>
>> Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
>> I think we can not do that.
>>
>> These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
>> when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
>> for 8250 serial.
>>
>> I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
>> struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
>> type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
>> meaningful.
>
> My point was that the indirect function calls are not needed at
> until patch four, so you can call the functions directly here,
> and make the logic for indirect function calls part of that
> later patch.
O. I think I got your meaning now.
In patch V2, the lpc_io_ops is not needed even if earlycon is supported. The early_in/early_out operation is
defined in hisi_lpc.c too, we can directly call the hisilpc_target_in/hisilpc_target_out to finish the LPC IO
operations.
At this moment, all the IO functions specific to the child devices of hip06 LPC have their own indirect call
method. This lpc_io_ops will be removed in V3.
>
> What are the other LPC cycle types that could be supported?
O. memory and firmware operations are supported too. But at this moment, we only use IO cycle.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-13 6:31 ` zhichang
@ 2016-09-14 12:34 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:34 UTC (permalink / raw)
To: linux-arm-kernel
Cc: zhichang, zhichang.yuan, lorenzo.pieralisi, gabriele.paoloni,
minyard, benh, john.garry, liviu.dudau, linuxarm, linux-kernel,
zourongrong
On Tuesday, September 13, 2016 2:31:13 PM CEST zhichang wrote:
> >
> > What are the other LPC cycle types that could be supported?
> O. memory and firmware operations are supported too. But at this moment, we only use IO cycle.
What are firmware operations?
Are the memory operations directly mapped or do you also have to go through
an indirect function call?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-14 12:34 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:34 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, September 13, 2016 2:31:13 PM CEST zhichang wrote:
> >
> > What are the other LPC cycle types that could be supported?
> O. memory and firmware operations are supported too. But at this moment, we only use IO cycle.
What are firmware operations?
Are the memory operations directly mapped or do you also have to go through
an indirect function call?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 17:51 ` kbuild test robot
-1 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2016-09-07 17:51 UTC (permalink / raw)
To: Zhichang Yuan
Cc: kbuild-all, linuxarm, linux-arm-kernel, linux-kernel, arnd,
minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
[-- Attachment #1: Type: text/plain, Size: 3286 bytes --]
Hi zhichang.yuan,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.8-rc5 next-20160907]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160907-212837
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All warnings (new ones prefixed by >>):
drivers/staging/xgifb/vb_init.c: In function 'XGIInitNew':
>> drivers/staging/xgifb/vb_init.c:1363:1: warning: the frame size of 8496 bytes is larger than 8192 bytes [-Wframe-larger-than=]
} /* end of init */
^
--
drivers/video/fbdev/s3fb.c: In function 's3fb_set_par':
>> drivers/video/fbdev/s3fb.c:911:1: warning: the frame size of 12320 bytes is larger than 8192 bytes [-Wframe-larger-than=]
}
^
--
drivers/video/fbdev/cirrusfb.c: In function 'cirrusfb_set_par_foo':
>> drivers/video/fbdev/cirrusfb.c:1266:1: warning: the frame size of 8880 bytes is larger than 8192 bytes [-Wframe-larger-than=]
}
^
vim +1363 drivers/staging/xgifb/vb_init.c
d7636e0b apatard@mandriva.com 2010-05-19 1347
bf32fcb9 Kenji Toyama 2011-04-23 1348 XGINew_SetDRAMDefaultRegister340(HwDeviceExtension,
bf32fcb9 Kenji Toyama 2011-04-23 1349 pVBInfo->P3d4,
bf32fcb9 Kenji Toyama 2011-04-23 1350 pVBInfo);
d7636e0b apatard@mandriva.com 2010-05-19 1351
fab04b97 Aaro Koskinen 2011-12-06 1352 XGINew_SetDRAMSize_340(xgifb_info, HwDeviceExtension, pVBInfo);
d7636e0b apatard@mandriva.com 2010-05-19 1353
38c09652 Aaro Koskinen 2012-11-04 1354 xgifb_reg_set(pVBInfo->P3c4, 0x22, 0xfa);
38c09652 Aaro Koskinen 2012-11-04 1355 xgifb_reg_set(pVBInfo->P3c4, 0x21, 0xa3);
d7636e0b apatard@mandriva.com 2010-05-19 1356
b053af16 Aaro Koskinen 2013-07-16 1357 XGINew_ChkSenseStatus(pVBInfo);
b053af16 Aaro Koskinen 2013-07-16 1358 XGINew_SetModeScratch(pVBInfo);
a24d60f4 Prashant P. Shah 2010-09-02 1359
8104e329 Aaro Koskinen 2011-03-13 1360 xgifb_reg_set(pVBInfo->P3d4, 0x8c, 0x87);
d7636e0b apatard@mandriva.com 2010-05-19 1361
b9ebf5e5 Aaro Koskinen 2011-03-13 1362 return 1;
b9ebf5e5 Aaro Koskinen 2011-03-13 @1363 } /* end of init */
:::::: The code at line 1363 was first introduced by commit
:::::: b9ebf5e5913307e67d226e61d953c3c4fd48de99 staging: xgifb: vb_init: move functions to avoid forward declarations
:::::: TO: Aaro Koskinen <aaro.koskinen@iki.fi>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 51358 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
@ 2016-09-07 17:51 ` kbuild test robot
0 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2016-09-07 17:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi zhichang.yuan,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.8-rc5 next-20160907]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160907-212837
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All warnings (new ones prefixed by >>):
drivers/staging/xgifb/vb_init.c: In function 'XGIInitNew':
>> drivers/staging/xgifb/vb_init.c:1363:1: warning: the frame size of 8496 bytes is larger than 8192 bytes [-Wframe-larger-than=]
} /* end of init */
^
--
drivers/video/fbdev/s3fb.c: In function 's3fb_set_par':
>> drivers/video/fbdev/s3fb.c:911:1: warning: the frame size of 12320 bytes is larger than 8192 bytes [-Wframe-larger-than=]
}
^
--
drivers/video/fbdev/cirrusfb.c: In function 'cirrusfb_set_par_foo':
>> drivers/video/fbdev/cirrusfb.c:1266:1: warning: the frame size of 8880 bytes is larger than 8192 bytes [-Wframe-larger-than=]
}
^
vim +1363 drivers/staging/xgifb/vb_init.c
d7636e0b apatard at mandriva.com 2010-05-19 1347
bf32fcb9 Kenji Toyama 2011-04-23 1348 XGINew_SetDRAMDefaultRegister340(HwDeviceExtension,
bf32fcb9 Kenji Toyama 2011-04-23 1349 pVBInfo->P3d4,
bf32fcb9 Kenji Toyama 2011-04-23 1350 pVBInfo);
d7636e0b apatard at mandriva.com 2010-05-19 1351
fab04b97 Aaro Koskinen 2011-12-06 1352 XGINew_SetDRAMSize_340(xgifb_info, HwDeviceExtension, pVBInfo);
d7636e0b apatard at mandriva.com 2010-05-19 1353
38c09652 Aaro Koskinen 2012-11-04 1354 xgifb_reg_set(pVBInfo->P3c4, 0x22, 0xfa);
38c09652 Aaro Koskinen 2012-11-04 1355 xgifb_reg_set(pVBInfo->P3c4, 0x21, 0xa3);
d7636e0b apatard at mandriva.com 2010-05-19 1356
b053af16 Aaro Koskinen 2013-07-16 1357 XGINew_ChkSenseStatus(pVBInfo);
b053af16 Aaro Koskinen 2013-07-16 1358 XGINew_SetModeScratch(pVBInfo);
a24d60f4 Prashant P. Shah 2010-09-02 1359
8104e329 Aaro Koskinen 2011-03-13 1360 xgifb_reg_set(pVBInfo->P3d4, 0x8c, 0x87);
d7636e0b apatard at mandriva.com 2010-05-19 1361
b9ebf5e5 Aaro Koskinen 2011-03-13 1362 return 1;
b9ebf5e5 Aaro Koskinen 2011-03-13 @1363 } /* end of init */
:::::: The code at line 1363 was first introduced by commit
:::::: b9ebf5e5913307e67d226e61d953c3c4fd48de99 staging: xgifb: vb_init: move functions to avoid forward declarations
:::::: TO: Aaro Koskinen <aaro.koskinen@iki.fi>
:::::: CC: Greg Kroah-Hartman <gregkh@suse.de>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 51358 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160908/98f4ec17/attachment-0001.obj>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 13:33 ` Zhichang Yuan
-1 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linuxarm, linux-arm-kernel, linux-kernel
Cc: arnd, minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
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. This patch drives the UART port with
the specific serial in/out function pair based on the indirect-IO mechanism
introduced by Hip06 LPC driver.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
.../devicetree/bindings/serial/hisi-lpc-uart.txt | 50 ++++++++++
drivers/tty/serial/8250/8250_hisi_lpc.c | 104 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 9 ++
drivers/tty/serial/8250/Makefile | 1 +
4 files changed, 164 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..ff391cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
@@ -0,0 +1,50 @@
+* 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.
+- indirect-io : identify this device based on the indirect IO mechanism.
+
+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 = <0x01 0xe4 0x0 0xe4 0x04>,
+ <0x01 0x2f8 0x0 0x2f8 0x08>;
+
+ ipmi0: bt@e4 {
+ compatible = "ipmi-bt";
+ device_type = "ipmi";
+ reg = <0x01 0xe4 0x04>;
+ status = "disabled";
+ };
+
+ uart0: lpc-uart@2f8 {
+ compatible = "hisilicon,lpc-uart";
+ reg = <0x01 0x2f8 0x08>;
+ 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..fbaae89
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_hisi_lpc.c
@@ -0,0 +1,104 @@
+/*
+ * 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>
+
+
+static int hslpc8250_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port uart = {};
+ struct uart_port *port = &uart.port;
+ int err = 0;
+ struct resource *iores;
+
+ if (!pdev->dev.parent)
+ return -ENODEV;
+ dev_info(&pdev->dev, "##probe entering\n");
+
+ iores = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (!iores) {
+ dev_err(&pdev->dev, "can not find the IO0\n");
+ return -ENXIO;
+ }
+ port->iobase = iores->start;
+
+ 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);
+
+ 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 hslpc8250_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 = hslpc8250_probe,
+ .remove = hslpc8250_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
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-07 13:33 ` Zhichang Yuan
0 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linux-arm-kernel
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. This patch drives the UART port with
the specific serial in/out function pair based on the indirect-IO mechanism
introduced by Hip06 LPC driver.
Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
---
.../devicetree/bindings/serial/hisi-lpc-uart.txt | 50 ++++++++++
drivers/tty/serial/8250/8250_hisi_lpc.c | 104 +++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 9 ++
drivers/tty/serial/8250/Makefile | 1 +
4 files changed, 164 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..ff391cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/hisi-lpc-uart.txt
@@ -0,0 +1,50 @@
+* 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.
+- indirect-io : identify this device based on the indirect IO mechanism.
+
+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 at 2f8 {
+ compatible = "hisilicon,lpc-uart";
+ reg = <0x01 0x2f8 0x08>;
+ status = "disabled";
+ };
+
+Example with low-pin-count parent device:
+
+ isa at a01b0000 {
+ compatible = "hisilicon,low-pin-count";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ reg = <0x0 0xa01b0000 0x0 0x1000>;
+ ranges = <0x01 0xe4 0x0 0xe4 0x04>,
+ <0x01 0x2f8 0x0 0x2f8 0x08>;
+
+ ipmi0: bt at e4 {
+ compatible = "ipmi-bt";
+ device_type = "ipmi";
+ reg = <0x01 0xe4 0x04>;
+ status = "disabled";
+ };
+
+ uart0: lpc-uart at 2f8 {
+ compatible = "hisilicon,lpc-uart";
+ reg = <0x01 0x2f8 0x08>;
+ 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..fbaae89
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_hisi_lpc.c
@@ -0,0 +1,104 @@
+/*
+ * 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>
+
+
+static int hslpc8250_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port uart = {};
+ struct uart_port *port = &uart.port;
+ int err = 0;
+ struct resource *iores;
+
+ if (!pdev->dev.parent)
+ return -ENODEV;
+ dev_info(&pdev->dev, "##probe entering\n");
+
+ iores = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (!iores) {
+ dev_err(&pdev->dev, "can not find the IO0\n");
+ return -ENXIO;
+ }
+ port->iobase = iores->start;
+
+ 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);
+
+ 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 hslpc8250_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 = hslpc8250_probe,
+ .remove = hslpc8250_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
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 14:50 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 14:50 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong
On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> the specific serial in/out function pair based on the indirect-IO mechanism
> introduced by Hip06 LPC driver.
>
> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
driver?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-07 14:50 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 14:50 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> the specific serial in/out function pair based on the indirect-IO mechanism
> introduced by Hip06 LPC driver.
>
> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
driver?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-07 14:50 ` Arnd Bergmann
@ 2016-09-08 9:51 ` zhichang
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-08 9:51 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zourongrong
Hi, Arnd
On 2016年09月07日 22:50, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
>> the specific serial in/out function pair based on the indirect-IO mechanism
>> introduced by Hip06 LPC driver.
>>
>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>
> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> driver?
I think two reasons for that:
1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
2. It seems UPIO_PORT is not supported there.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-08 9:51 ` zhichang
0 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-08 9:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Arnd
On 2016?09?07? 22:50, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
>> the specific serial in/out function pair based on the indirect-IO mechanism
>> introduced by Hip06 LPC driver.
>>
>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>
> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> driver?
I think two reasons for that:
1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
2. It seems UPIO_PORT is not supported there.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-08 9:51 ` zhichang
@ 2016-09-08 9:58 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 9:58 UTC (permalink / raw)
To: zhichang
Cc: linux-arm-kernel, Zhichang Yuan, linuxarm, linux-kernel,
lorenzo.pieralisi, minyard, benh, gabriele.paoloni, john.garry,
liviu.dudau, zourongrong
On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
> On 2016年09月07日 22:50, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> >> the specific serial in/out function pair based on the indirect-IO mechanism
> >> introduced by Hip06 LPC driver.
> >>
> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> >
> > Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> > driver?
> I think two reasons for that:
> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
ACPI has its own way of describing serial ports, use that instead.
> 2. It seems UPIO_PORT is not supported there.
Should be easy enough to add.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-08 9:58 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 9:58 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
> On 2016?09?07? 22:50, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> >> the specific serial in/out function pair based on the indirect-IO mechanism
> >> introduced by Hip06 LPC driver.
> >>
> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> >
> > Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> > driver?
> I think two reasons for that:
> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
ACPI has its own way of describing serial ports, use that instead.
> 2. It seems UPIO_PORT is not supported there.
Should be easy enough to add.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-08 9:58 ` Arnd Bergmann
@ 2016-09-14 11:48 ` zhichang.yuan
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-14 11:48 UTC (permalink / raw)
To: Arnd Bergmann, zhichang
Cc: linux-arm-kernel, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zourongrong
On 2016/9/8 17:58, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
>> On 2016年09月07日 22:50, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
>>>> the specific serial in/out function pair based on the indirect-IO mechanism
>>>> introduced by Hip06 LPC driver.
>>>>
>>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>>>
>>> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
>>> driver?
>> I think two reasons for that:
>> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
>
> ACPI has its own way of describing serial ports, use that instead.
Could you give me some info about ACPI serial ports?
I found there is _CRS specific for serial, but it seems no serial driver use that.
Thanks!
Zhichang
>
>> 2. It seems UPIO_PORT is not supported there.
>
> Should be easy enough to add.
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-14 11:48 ` zhichang.yuan
0 siblings, 0 replies; 55+ messages in thread
From: zhichang.yuan @ 2016-09-14 11:48 UTC (permalink / raw)
To: linux-arm-kernel
On 2016/9/8 17:58, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
>> On 2016?09?07? 22:50, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
>>>> the specific serial in/out function pair based on the indirect-IO mechanism
>>>> introduced by Hip06 LPC driver.
>>>>
>>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
>>>
>>> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
>>> driver?
>> I think two reasons for that:
>> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
>
> ACPI has its own way of describing serial ports, use that instead.
Could you give me some info about ACPI serial ports?
I found there is _CRS specific for serial, but it seems no serial driver use that.
Thanks!
Zhichang
>
>> 2. It seems UPIO_PORT is not supported there.
>
> Should be easy enough to add.
>
> Arnd
>
> .
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
2016-09-14 11:48 ` zhichang.yuan
@ 2016-09-14 12:07 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:07 UTC (permalink / raw)
To: linux-arm-kernel
Cc: zhichang.yuan, zhichang, lorenzo.pieralisi, gabriele.paoloni,
minyard, benh, john.garry, liviu.dudau, linuxarm, linux-kernel,
zourongrong
On Wednesday, September 14, 2016 7:48:49 PM CEST zhichang.yuan wrote:
> On 2016/9/8 17:58, Arnd Bergmann wrote:
> > On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
> >> On 2016年09月07日 22:50, Arnd Bergmann wrote:
> >>> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> >>>> the specific serial in/out function pair based on the indirect-IO mechanism
> >>>> introduced by Hip06 LPC driver.
> >>>>
> >>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> >>>
> >>> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> >>> driver?
> >> I think two reasons for that:
> >> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
> >
> > ACPI has its own way of describing serial ports, use that instead.
> Could you give me some info about ACPI serial ports?
> I found there is _CRS specific for serial, but it seems no serial driver use that.
drivers/tty/serial/8250/8250_dw.c is a driver that uses ACPI matching,
and possibly drivers/tty/serial/8250/8250_pnp.c could work as well.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 3/4] ARM64 LPC: support serial based on low-pin-count
@ 2016-09-14 12:07 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 14, 2016 7:48:49 PM CEST zhichang.yuan wrote:
> On 2016/9/8 17:58, Arnd Bergmann wrote:
> > On Thursday, September 8, 2016 5:51:25 PM CEST zhichang wrote:
> >> On 2016?09?07? 22:50, Arnd Bergmann wrote:
> >>> On Wednesday, September 7, 2016 9:33:52 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. This patch drives the UART port with
> >>>> the specific serial in/out function pair based on the indirect-IO mechanism
> >>>> introduced by Hip06 LPC driver.
> >>>>
> >>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com>
> >>>
> >>> Any reason this cannot just use the regular drivers/tty/serial/8250/8250_of.c
> >>> driver?
> >> I think two reasons for that:
> >> 1. 8250_of.c is only for devicetree, but we need to support ACPI device too;
> >
> > ACPI has its own way of describing serial ports, use that instead.
> Could you give me some info about ACPI serial ports?
> I found there is _CRS specific for serial, but it seems no serial driver use that.
drivers/tty/serial/8250/8250_dw.c is a driver that uses ACPI matching,
and possibly drivers/tty/serial/8250/8250_pnp.c could work as well.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 13:33 ` Zhichang Yuan
-1 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linuxarm, linux-arm-kernel, linux-kernel
Cc: arnd, minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
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 | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 7ac0551..1cc5581 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -374,6 +374,135 @@ void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
}
+static struct extio_ops hisilpc_earlyop __initdata;
+
+/**
+ * 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.
+ * @ptaddr: the io address where read operation targets to.
+ * @dlen: data length in byte to be read each IO operation.
+ * @count: how many IO operations expected.
+ * for earlycon, dlen and count should be one.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static u64 __init hisilpc_early_in(void *devobj, unsigned long ptaddr,
+ void *inbuf, size_t dlen,unsigned int count)
+{
+ unsigned int ret = 0;
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+ struct uart_port *port;
+ unsigned int rd_data;
+
+ port = (struct uart_port *)devobj;
+ if (!port->mapbase || !port->membase || inbuf ||
+ count != 1 || dlen !=1)
+ return -EINVAL;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = dlen;
+ lpcdev.membase = port->membase;
+
+ ret = hisilpc_target_in(&lpcdev, ¶, ptaddr,
+ (unsigned char *)&rd_data, count);
+
+ return (ret) ? : rd_data;
+}
+
+/**
+ * hisilpc_early_out - write/output operation specific for hisi LPC earlycon.
+ * @devobj: pointer to device relevant information of the caller.
+ * @outbuf: the buffer where the data to be written is stored.
+ * @ptaddr: the io address where write operation targets to.
+ * @dlen: data length in byte to be written each IO operation.
+ * @count: how many IO operations expected.
+ * for earlycon, dlen and count must be one.
+ *
+ */
+static void __init hisilpc_early_out(void *devobj, unsigned long ptaddr,
+ const void *outbuf, size_t dlen,
+ unsigned int count)
+{
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+ struct uart_port *port;
+
+ port = (struct uart_port *)devobj;
+ if (!port->mapbase || !port->membase || !outbuf ||
+ dlen != 1 || count != 1)
+ return;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = dlen;
+ lpcdev.membase = port->membase;
+
+ (void)hisilpc_target_out(&lpcdev, ¶, ptaddr, outbuf, count);
+}
+
+
+/**
+ * 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;
+ }
+
+ /* must set iotype as UPIO_PORT for Hip06 indirect-io */
+ device->port.iotype = UPIO_PORT;
+
+ hisilpc_earlyop.pfin = hisilpc_early_in;
+ hisilpc_earlyop.pfout = hisilpc_early_out;
+ hisilpc_earlyop.devpara = &device->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();
+
+ arm64_set_simops(&hisilpc_earlyop);
+
+ return early_serial8250_setup(device, options);
+}
+
+
+
+EARLYCON_DECLARE(hisilpcuart, early_hisilpc8250_setup);
+OF_EARLYCON_DECLARE(hisilpcuart, "hisi,rawlpc-uart",
+ early_hisilpc8250_setup);
+
/**
* hisilpc_probe - the probe callback function for hisi lpc device,
* will finish all the intialization.
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-07 13:33 ` Zhichang Yuan
0 siblings, 0 replies; 55+ messages in thread
From: Zhichang Yuan @ 2016-09-07 13:33 UTC (permalink / raw)
To: linux-arm-kernel
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 | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 7ac0551..1cc5581 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -374,6 +374,135 @@ void hisilpc_comm_outb(void *devobj, unsigned long ptaddr,
}
+static struct extio_ops hisilpc_earlyop __initdata;
+
+/**
+ * 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.
+ * @ptaddr: the io address where read operation targets to.
+ * @dlen: data length in byte to be read each IO operation.
+ * @count: how many IO operations expected.
+ * for earlycon, dlen and count should be one.
+ *
+ * Returns 0 on success, non-zero on fail.
+ *
+ */
+static u64 __init hisilpc_early_in(void *devobj, unsigned long ptaddr,
+ void *inbuf, size_t dlen,unsigned int count)
+{
+ unsigned int ret = 0;
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+ struct uart_port *port;
+ unsigned int rd_data;
+
+ port = (struct uart_port *)devobj;
+ if (!port->mapbase || !port->membase || inbuf ||
+ count != 1 || dlen !=1)
+ return -EINVAL;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = dlen;
+ lpcdev.membase = port->membase;
+
+ ret = hisilpc_target_in(&lpcdev, ¶, ptaddr,
+ (unsigned char *)&rd_data, count);
+
+ return (ret) ? : rd_data;
+}
+
+/**
+ * hisilpc_early_out - write/output operation specific for hisi LPC earlycon.
+ * @devobj: pointer to device relevant information of the caller.
+ * @outbuf: the buffer where the data to be written is stored.
+ * @ptaddr: the io address where write operation targets to.
+ * @dlen: data length in byte to be written each IO operation.
+ * @count: how many IO operations expected.
+ * for earlycon, dlen and count must be one.
+ *
+ */
+static void __init hisilpc_early_out(void *devobj, unsigned long ptaddr,
+ const void *outbuf, size_t dlen,
+ unsigned int count)
+{
+ struct lpc_cycle_para para;
+ struct hisilpc_dev lpcdev;
+ struct uart_port *port;
+
+ port = (struct uart_port *)devobj;
+ if (!port->mapbase || !port->membase || !outbuf ||
+ dlen != 1 || count != 1)
+ return;
+
+ para.opflags = FG_EARLYCON_LPC;
+ para.csize = dlen;
+ lpcdev.membase = port->membase;
+
+ (void)hisilpc_target_out(&lpcdev, ¶, ptaddr, outbuf, count);
+}
+
+
+/**
+ * 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;
+ }
+
+ /* must set iotype as UPIO_PORT for Hip06 indirect-io */
+ device->port.iotype = UPIO_PORT;
+
+ hisilpc_earlyop.pfin = hisilpc_early_in;
+ hisilpc_earlyop.pfout = hisilpc_early_out;
+ hisilpc_earlyop.devpara = &device->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();
+
+ arm64_set_simops(&hisilpc_earlyop);
+
+ return early_serial8250_setup(device, options);
+}
+
+
+
+EARLYCON_DECLARE(hisilpcuart, early_hisilpc8250_setup);
+OF_EARLYCON_DECLARE(hisilpcuart, "hisi,rawlpc-uart",
+ early_hisilpc8250_setup);
+
/**
* hisilpc_probe - the probe callback function for hisi lpc device,
* will finish all the intialization.
--
1.9.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-07 14:52 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 14:52 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zhichang.yuan02, zourongrong
On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
> 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>
>
I'm skeptical about this too. Is this just needed because the 8250
earlycon support comes before the lpc bus initialization?
Could we start the LPC driver earlier to work around that?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-07 14:52 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-07 14:52 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
> 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>
>
I'm skeptical about this too. Is this just needed because the 8250
earlycon support comes before the lpc bus initialization?
Could we start the LPC driver earlier to work around that?
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-07 14:52 ` Arnd Bergmann
@ 2016-09-08 10:04 ` zhichang
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-08 10:04 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Zhichang Yuan, linuxarm, linux-kernel, lorenzo.pieralisi,
minyard, benh, gabriele.paoloni, john.garry, liviu.dudau,
zourongrong
Hi, Arnd,
On 2016年09月07日 22:52, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
>> 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>
>>
>
> I'm skeptical about this too. Is this just needed because the 8250
> earlycon support comes before the lpc bus initialization?
I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
"earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
Hip06 LPC uart need two base addresses for earlycon.
2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
IO type is UPIO_PORT. This is spcial...
3. Just as your guess, earlycon should be earlier than lpc initialization.
Best,
Zhichang
>
> Could we start the LPC driver earlier to work around that?
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-08 10:04 ` zhichang
0 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-08 10:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi, Arnd,
On 2016?09?07? 22:52, Arnd Bergmann wrote:
> On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
>> 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>
>>
>
> I'm skeptical about this too. Is this just needed because the 8250
> earlycon support comes before the lpc bus initialization?
I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
"earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
Hip06 LPC uart need two base addresses for earlycon.
2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
IO type is UPIO_PORT. This is spcial...
3. Just as your guess, earlycon should be earlier than lpc initialization.
Best,
Zhichang
>
> Could we start the LPC driver earlier to work around that?
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-08 10:04 ` zhichang
@ 2016-09-08 11:04 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 11:04 UTC (permalink / raw)
To: zhichang
Cc: linux-arm-kernel, Zhichang Yuan, linuxarm, linux-kernel,
lorenzo.pieralisi, minyard, benh, gabriele.paoloni, john.garry,
liviu.dudau, zourongrong
On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote:
> Hi, Arnd,
>
> On 2016年09月07日 22:52, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
> >> 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>
> >>
> >
> > I'm skeptical about this too. Is this just needed because the 8250
> > earlycon support comes before the lpc bus initialization?
> I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
>
> 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
> "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
We should never need to specify the addresses manually like this,
it's actually supposed to work if you just list "earlycon" here.
The first membase is apparently only used during setup:
+ 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))
Why doesn't the firmware do this before handing off control of
the kernel to the console?
> Hip06 LPC uart need two base addresses for earlycon.
> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
> IO type is UPIO_PORT. This is spcial...
This sounds like a deficiency in the of_setup_earlycon() function,
which can only handle MMIO addresses, and won't actually
be able to understand nodes without a "ranges" property like
you have here.
I think we need to add a special case for port ranges here.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-08 11:04 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-08 11:04 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote:
> Hi, Arnd,
>
> On 2016?09?07? 22:52, Arnd Bergmann wrote:
> > On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
> >> 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>
> >>
> >
> > I'm skeptical about this too. Is this just needed because the 8250
> > earlycon support comes before the lpc bus initialization?
> I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
>
> 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
> "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
We should never need to specify the addresses manually like this,
it's actually supposed to work if you just list "earlycon" here.
The first membase is apparently only used during setup:
+ 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))
Why doesn't the firmware do this before handing off control of
the kernel to the console?
> Hip06 LPC uart need two base addresses for earlycon.
> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
> IO type is UPIO_PORT. This is spcial...
This sounds like a deficiency in the of_setup_earlycon() function,
which can only handle MMIO addresses, and won't actually
be able to understand nodes without a "ranges" property like
you have here.
I think we need to add a special case for port ranges here.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-08 11:04 ` Arnd Bergmann
@ 2016-09-14 11:26 ` zhichang
-1 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-14 11:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Zhichang Yuan, linuxarm, linux-kernel,
lorenzo.pieralisi, minyard, benh, gabriele.paoloni, john.garry,
liviu.dudau, zourongrong
On 2016年09月08日 19:04, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote:
>> Hi, Arnd,
>>
>> On 2016年09月07日 22:52, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
>>>> 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>
>>>>
>>>
>>> I'm skeptical about this too. Is this just needed because the 8250
>>> earlycon support comes before the lpc bus initialization?
>> I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
>>
>> 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
>> "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
>
> We should never need to specify the addresses manually like this,
> it's actually supposed to work if you just list "earlycon" here.
Do you mean flat-tree earlycon?
Ok, will support this in V3.
>
> The first membase is apparently only used during setup:
>
> + 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))
>
> Why doesn't the firmware do this before handing off control of
> the kernel to the console?
This is a checking on the LPC controller status.
I think we can keep this here.
>
>> Hip06 LPC uart need two base addresses for earlycon.
>> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
>> IO type is UPIO_PORT. This is spcial...
>
> This sounds like a deficiency in the of_setup_earlycon() function,
> which can only handle MMIO addresses, and won't actually
> be able to understand nodes without a "ranges" property like
> you have here.
>
Yes.
The current of_setup_earlycon only support MMIO and the first reg property must be memory.
We can not support our LPC uart without any new code.
But we can implement a private earlycon setup function and register it to the __earlycon_table, things will be ok.
I will do it in V3.
Best,
Zhichang
> I think we need to add a special case for port ranges here.
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-14 11:26 ` zhichang
0 siblings, 0 replies; 55+ messages in thread
From: zhichang @ 2016-09-14 11:26 UTC (permalink / raw)
To: linux-arm-kernel
On 2016?09?08? 19:04, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote:
>> Hi, Arnd,
>>
>> On 2016?09?07? 22:52, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote:
>>>> 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>
>>>>
>>>
>>> I'm skeptical about this too. Is this just needed because the 8250
>>> earlycon support comes before the lpc bus initialization?
>> I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart.
>>
>> 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that
>> "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate.
>
> We should never need to specify the addresses manually like this,
> it's actually supposed to work if you just list "earlycon" here.
Do you mean flat-tree earlycon?
Ok, will support this in V3.
>
> The first membase is apparently only used during setup:
>
> + 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))
>
> Why doesn't the firmware do this before handing off control of
> the kernel to the console?
This is a checking on the LPC controller status.
I think we can keep this here.
>
>> Hip06 LPC uart need two base addresses for earlycon.
>> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
>> IO type is UPIO_PORT. This is spcial...
>
> This sounds like a deficiency in the of_setup_earlycon() function,
> which can only handle MMIO addresses, and won't actually
> be able to understand nodes without a "ranges" property like
> you have here.
>
Yes.
The current of_setup_earlycon only support MMIO and the first reg property must be memory.
We can not support our LPC uart without any new code.
But we can implement a private earlycon setup function and register it to the __earlycon_table, things will be ok.
I will do it in V3.
Best,
Zhichang
> I think we need to add a special case for port ranges here.
>
> Arnd
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-14 11:26 ` zhichang
@ 2016-09-14 12:36 ` Arnd Bergmann
-1 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:36 UTC (permalink / raw)
To: zhichang
Cc: linux-arm-kernel, Zhichang Yuan, linuxarm, linux-kernel,
lorenzo.pieralisi, minyard, benh, gabriele.paoloni, john.garry,
liviu.dudau, zourongrong
On Wednesday, September 14, 2016 7:26:22 PM CEST zhichang wrote:
>
> >
> >> Hip06 LPC uart need two base addresses for earlycon.
> >> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
> >> IO type is UPIO_PORT. This is spcial...
> >
> > This sounds like a deficiency in the of_setup_earlycon() function,
> > which can only handle MMIO addresses, and won't actually
> > be able to understand nodes without a "ranges" property like
> > you have here.
> >
> Yes.
> The current of_setup_earlycon only support MMIO and the first reg property must be memory.
>
> We can not support our LPC uart without any new code.
> But we can implement a private earlycon setup function and register
> it to the __earlycon_table, things will be ok.
I still think you should adapt of_setup_earlycon instead to handle IORESOURCE_IO
registers.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
@ 2016-09-14 12:36 ` Arnd Bergmann
0 siblings, 0 replies; 55+ messages in thread
From: Arnd Bergmann @ 2016-09-14 12:36 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday, September 14, 2016 7:26:22 PM CEST zhichang wrote:
>
> >
> >> Hip06 LPC uart need two base addresses for earlycon.
> >> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart
> >> IO type is UPIO_PORT. This is spcial...
> >
> > This sounds like a deficiency in the of_setup_earlycon() function,
> > which can only handle MMIO addresses, and won't actually
> > be able to understand nodes without a "ranges" property like
> > you have here.
> >
> Yes.
> The current of_setup_earlycon only support MMIO and the first reg property must be memory.
>
> We can not support our LPC uart without any new code.
> But we can implement a private earlycon setup function and register
> it to the __earlycon_table, things will be ok.
I still think you should adapt of_setup_earlycon instead to handle IORESOURCE_IO
registers.
Arnd
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH V2 4/4] ARM64 LPC: support earlycon for UART connected to LPC
2016-09-07 13:33 ` Zhichang Yuan
@ 2016-09-08 9:26 ` kbuild test robot
-1 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2016-09-08 9:26 UTC (permalink / raw)
To: Zhichang Yuan
Cc: kbuild-all, linuxarm, linux-arm-kernel, linux-kernel, arnd,
minyard, benh, lorenzo.pieralisi, liviu.dudau, zourongrong,
john.garry, gabriele.paoloni, zhichang.yuan02, zhichang.yuan
[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]
Hi zhichang.yuan,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc5 next-20160908]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160907-212837
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `early_hisilpc8250_setup':
>> binder.c:(.init.text+0x479c): undefined reference to `early_serial8250_setup'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 51359 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread