| Message ID | 1485241525-201782-5-git-send-email-yuanzhichang@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Tue, Jan 24, 2017 at 03:05:24PM +0800, zhichang.yuan wrote: > The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in > I/O port addresses. This patch implements the LPC host controller driver which > perform the I/O operations on the underlying hardware. > We don't want to touch those existing peripherals' driver, such as ipmi-bt. So > this driver applies the indirect-IO introduced in the previous patch after > registering an indirect-IO node to the indirect-IO devices list which will be > searched in the I/O accessors. > As the I/O translations for LPC children depend on the host I/O registration, > we should ensure the host I/O registration is finished before all the LPC > children scanning. That is why an arch_init() hook was added in this patch. > > Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ > MAINTAINERS | 9 + > arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + Acked-by: Rob Herring <robh@kernel.org> > drivers/bus/Kconfig | 8 + > drivers/bus/Makefile | 1 + > drivers/bus/hisi_lpc.c | 599 +++++++++++++++++++++ > 7 files changed, 668 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt > create mode 100644 drivers/bus/hisi_lpc.c -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24/01/2017 08:05, zhichang.yuan wrote: > The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in > I/O port addresses. This patch implements the LPC host controller driver which > perform the I/O operations on the underlying hardware. > We don't want to touch those existing peripherals' driver, such as ipmi-bt. So > this driver applies the indirect-IO introduced in the previous patch after > registering an indirect-IO node to the indirect-IO devices list which will be > searched in the I/O accessors. > As the I/O translations for LPC children depend on the host I/O registration, > we should ensure the host I/O registration is finished before all the LPC > children scanning. That is why an arch_init() hook was added in this patch. > > Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ > MAINTAINERS | 9 + > arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + > arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + > drivers/bus/Kconfig | 8 + > drivers/bus/Makefile | 1 + > drivers/bus/hisi_lpc.c | 599 +++++++++++++++++++++ > 7 files changed, 668 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..213181f > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt > @@ -0,0 +1,33 @@ > +Hisilicon Hip06 low-pin-count device > + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which > + provides I/O access to some legacy ISA devices. > + Hip06 is based on arm64 architecture where there is no I/O space. So, the > + I/O ports here are not cpu addresses, and there is no 'ranges' property in > + LPC device node. > + > +Required properties: > +- compatible: value should be as follows: > + (a) "hisilicon,hip06-lpc" > + (b) "hisilicon,hip07-lpc" > +- #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 memory range where the LPC register set is mapped. > + > +Note: > + The node name before '@' must be "isa" to represent the binding stick to the > + ISA/EISA binding specification. > + > +Example: > + > +isa@a01b0000 { > + compatible = "hisilicon,hip06-lpc"; > + #address-cells = <2>; > + #size-cells = <1>; > + reg = <0x0 0xa01b0000 0x0 0x1000>; > + > + ipmi0: bt@e4 { > + compatible = "ipmi-bt"; > + device_type = "ipmi"; > + reg = <0x01 0xe4 0x04>; > + }; > +}; > diff --git a/MAINTAINERS b/MAINTAINERS > index 26edd83..0153707 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h > F: net/802/hippi.c > F: drivers/net/hippi/ > > +HISILICON LPC BUS DRIVER > +M: Zhichang Yuan <yuanzhichang@hisilicon.com> > +L: linux-arm-kernel@lists.infradead.org > +W: http://www.hisilicon.com > +S: Maintained > +F: drivers/bus/hisi_lpc.c > +F: lib/extio.c > +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt > + > HISILICON NETWORK SUBSYSTEM DRIVER > M: Yisen Zhuang <yisen.zhuang@huawei.com> > M: Salil Mehta <salil.mehta@huawei.com> > diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts > index 7c4114a..75b2b5c 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts > +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts > @@ -52,3 +52,7 @@ > &usb_ehci { > status = "ok"; > }; > + > +&ipmi0 { > + status = "ok"; > +}; > diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > index a049b64..c450f8d 100644 > --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi > @@ -318,6 +318,20 @@ > #size-cells = <2>; > ranges; > > + isa@a01b0000 { > + compatible = "hisilicon,hip06-lpc"; > + #size-cells = <1>; > + #address-cells = <2>; > + reg = <0x0 0xa01b0000 0x0 0x1000>; > + > + ipmi0: bt@e4 { > + compatible = "ipmi-bt"; > + device_type = "ipmi"; > + reg = <0x01 0xe4 0x04>; > + status = "disabled"; > + }; > + }; > + > refclk: refclk { > compatible = "fixed-clock"; > clock-frequency = <50000000>; > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig > index b9e8cfc..58cee84 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" It's not a workaround, it's support. Better word it like "Support for ISA I/O space on Hisilicon HIP0X" > + depends on (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST > + select 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 cc6364b..28e3862 100644 > --- a/drivers/bus/Makefile > +++ b/drivers/bus/Makefile > @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o > obj-$(CONFIG_ARM_CCN) += arm-ccn.o > > obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o > +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o > obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 > --- /dev/null > +++ b/drivers/bus/hisi_lpc.c > @@ -0,0 +1,599 @@ > +/* > + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. > + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> > + * Author: Zou Rongrong <zourongrong@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/pci.h> > +#include <linux/serial_8250.h> > +#include <linux/slab.h> > + > +/* > + * Setting this bit means each IO operation will target to a > + * different port address: > + * 0 means repeatedly IO operations will stick on the same port, > + * such as BT; > + */ > +#define FG_INCRADDR_LPC 0x02 > + > +struct lpc_cycle_para { > + unsigned int opflags; > + unsigned int csize; /* the data length of each operation */ > +}; > + > +struct hisilpc_dev { > + spinlock_t cycle_lock; > + void __iomem *membase; > + struct extio_node *extio; > +}; > + > +/* bounds of the LPC bus address range */ > +#define LPC_MIN_BUS_RANGE 0x0 > + > +/* > + * The maximal IO size for each leagcy bus. legacy? I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. > + * The port size of legacy I/O devices is normally less than 0x400. > + * Defining the I/O range size as 0x400 here should be sufficient for > + * all peripherals under one bus. > + */ This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? > +#define LPC_BUS_IO_SIZE 0x400 > + > +/* The maximum continuous operations */ > +#define LPC_MAX_OPCNT 16 > +/* only support IO data unit length is four at maximum */ > +#define LPC_MAX_DULEN 4 > +#if LPC_MAX_DULEN > LPC_MAX_OPCNT > +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" > +#endif > + > +#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 0x08 > +#define LPC_CMD_TYPE_IO 0x00 > +#define LPC_CMD_WRITE 0x01 > +#define LPC_CMD_READ 0x00 > +/* the bit attribute is W1C. 1 represents OK. */ > +#define LPC_STAT_BYIRQ 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. Are you sure you want this comment to be upstream? :) > + */ > +#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 specific for single IO operation, such as inb. */ > +#define LPC_PEROP_WAITCNT 100 > + > + > +static inline int wait_lpc_idle(unsigned char *mbase, No need to specify inline. > + unsigned int waitcnt) { > + u32 opstatus; > + > + 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); It's a shame we have to busy loop, but I guess no calling code outside is prepared for rescheduling at this point. > + } > + return -ETIME; > +} > + > +/* > + * hisilpc_target_in - trigger a series of lpc cycles to read required data > + * from target peripheral. > + * @pdev: pointer to hisi lpc device > + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, > + unsigned long ptaddr, unsigned char *buf, > + unsigned long opcnt) > +{ > + unsigned long cnt_per_trans; > + unsigned int cmd_word; > + unsigned int waitcnt; > + int ret; > + > + if (!buf || !opcnt || !para || !para->csize || !lpcdev) > + return -EINVAL; > + > + if (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; > + waitcnt = LPC_MAX_WAITCNT; > + } > + > + ret = 0; > + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; > + for (; opcnt && !ret; cnt_per_trans = para->csize) { > + unsigned long flags; > + > + /* whole operation must be atomic */ > + spin_lock_irqsave(&lpcdev->cycle_lock, flags); Ouch. This is going to kill your RT jitter. Is there no better way? > + > + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); > + > + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); > + > + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); > + > + writel(START_WORK, lpcdev->membase + LPC_REG_START); > + > + /* whether the operation is finished */ > + ret = wait_lpc_idle(lpcdev->membase, waitcnt); > + if (!ret) { > + opcnt -= cnt_per_trans; > + for (; cnt_per_trans--; buf++) > + *buf = readl(lpcdev->membase + LPC_REG_RDATA); > + } > + > + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); > + } > + > + return ret; > +} > + > +/** > + * hisilpc_target_out - trigger a series of lpc cycles to write required > + * data to target peripheral. > + * @pdev: pointer to hisi lpc device > + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, > + unsigned long ptaddr, const unsigned char *buf, > + unsigned long opcnt) > +{ > + unsigned long cnt_per_trans; > + unsigned int cmd_word; > + unsigned int waitcnt; > + int ret; > + > + if (!buf || !opcnt || !para || !lpcdev) > + return -EINVAL; > + > + if (opcnt > LPC_MAX_OPCNT) > + return -EINVAL; > + /* default is increasing address */ > + cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE; > + waitcnt = LPC_PEROP_WAITCNT; > + if (!(para->opflags & FG_INCRADDR_LPC)) { > + cmd_word |= LPC_CMD_SAMEADDR; > + waitcnt = LPC_MAX_WAITCNT; > + } > + > + ret = 0; > + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; > + for (; opcnt && !ret; cnt_per_trans = para->csize) { > + unsigned long flags; > + > + spin_lock_irqsave(&lpcdev->cycle_lock, flags); Same thing here > + > + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); > + opcnt -= cnt_per_trans; > + for (; cnt_per_trans--; buf++) > + writel(*buf, lpcdev->membase + LPC_REG_WDATA); > + > + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); > + > + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); > + > + writel(START_WORK, lpcdev->membase + LPC_REG_START); > + > + /* whether the operation is finished */ > + ret = wait_lpc_idle(lpcdev->membase, waitcnt); > + > + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); > + } > + > + return ret; > +} > + > +static inline unsigned long Don't explicitly mention inline, the compiler will figure that out for you. > +hisi_lpc_pio_to_addr(struct hisilpc_dev *lpcdev, unsigned long pio) > +{ > + return pio - lpcdev->extio->io_start + lpcdev->extio->bus_start; > +} > + > + > +/** > + * hisilpc_comm_in - read/input the data from the I/O peripheral > + * through LPC. > + * @devobj: pointer to the device information relevant to LPC controller. > + * @pio: the target I/O port address. > + * @dlen: the data length required to read from the target I/O port. > + * > + * 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. > + */ > +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t dlen) > +{ > + struct hisilpc_dev *lpcdev = devobj; > + struct lpc_cycle_para iopara; > + u32 rd_data; rd_data needs to be initialized to 0. Otherwise it may contain stale stack contents and corrupt non-32bit dlen returns. > + unsigned char *newbuf; > + int ret = 0; > + unsigned long ptaddr; > + > + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen - 1))) > + return -1; Isn't this -EINVAL? > + > + /* the local buffer must be enough for one data unit */ > + if (sizeof(rd_data) < dlen) > + return -1; Same here. Also, the above seems a very convoluted way of saying switch (dlen) { case 1: case 2: case 4: break; default: return -EINVAL; } But I guess the way you write it doesn't hurt ;) Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/01/2017 20:08, Alexander Graf wrote: > Alex, Thanks for checking. > > On 24/01/2017 08:05, zhichang.yuan wrote: >> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the >> peripherals in >> I/O port addresses. This patch implements the LPC host controller >> driver which >> perform the I/O operations on the underlying hardware. >> We don't want to touch those existing peripherals' driver, such as >> ipmi-bt. So >> this driver applies the indirect-IO introduced in the previous patch >> after >> registering an indirect-IO node to the indirect-IO devices list which >> will be >> searched in the I/O accessors. >> As the I/O translations for LPC children depend on the host I/O >> registration, >> we should ensure the host I/O registration is finished before all the LPC >> children scanning. That is why an arch_init() hook was added in this >> patch. >> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >> --- >> .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ >> MAINTAINERS | 9 + >> arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + >> arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + >> drivers/bus/Kconfig | 8 + >> drivers/bus/Makefile | 1 + >> drivers/bus/hisi_lpc.c | 599 >> +++++++++++++++++++++ >> 7 files changed, 668 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..213181f >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >> >> @@ -0,0 +1,33 @@ >> +Hisilicon Hip06 low-pin-count device >> + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which >> + provides I/O access to some legacy ISA devices. >> + Hip06 is based on arm64 architecture where there is no I/O space. >> So, the >> + I/O ports here are not cpu addresses, and there is no 'ranges' >> property in >> + LPC device node. >> + >> +Required properties: >> +- compatible: value should be as follows: >> + (a) "hisilicon,hip06-lpc" >> + (b) "hisilicon,hip07-lpc" >> +- #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 memory range where the LPC register set is mapped. >> + >> +Note: >> + The node name before '@' must be "isa" to represent the binding >> stick to the >> + ISA/EISA binding specification. >> + >> +Example: >> + >> +isa@a01b0000 { >> + compatible = "hisilicon,hip06-lpc"; >> + #address-cells = <2>; >> + #size-cells = <1>; >> + reg = <0x0 0xa01b0000 0x0 0x1000>; >> + >> + ipmi0: bt@e4 { >> + compatible = "ipmi-bt"; >> + device_type = "ipmi"; >> + reg = <0x01 0xe4 0x04>; >> + }; >> +}; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 26edd83..0153707 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h >> F: net/802/hippi.c >> F: drivers/net/hippi/ >> >> +HISILICON LPC BUS DRIVER >> +M: Zhichang Yuan <yuanzhichang@hisilicon.com> >> +L: linux-arm-kernel@lists.infradead.org >> +W: http://www.hisilicon.com >> +S: Maintained >> +F: drivers/bus/hisi_lpc.c >> +F: lib/extio.c >> +F: >> Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >> >> + >> HISILICON NETWORK SUBSYSTEM DRIVER >> M: Yisen Zhuang <yisen.zhuang@huawei.com> >> M: Salil Mehta <salil.mehta@huawei.com> >> diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> index 7c4114a..75b2b5c 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> @@ -52,3 +52,7 @@ >> &usb_ehci { >> status = "ok"; >> }; >> + >> +&ipmi0 { >> + status = "ok"; >> +}; >> diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> index a049b64..c450f8d 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> @@ -318,6 +318,20 @@ >> #size-cells = <2>; >> ranges; >> >> + isa@a01b0000 { >> + compatible = "hisilicon,hip06-lpc"; >> + #size-cells = <1>; >> + #address-cells = <2>; >> + reg = <0x0 0xa01b0000 0x0 0x1000>; >> + >> + ipmi0: bt@e4 { >> + compatible = "ipmi-bt"; >> + device_type = "ipmi"; >> + reg = <0x01 0xe4 0x04>; >> + status = "disabled"; >> + }; >> + }; >> + >> refclk: refclk { >> compatible = "fixed-clock"; >> clock-frequency = <50000000>; >> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig >> index b9e8cfc..58cee84 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" > > It's not a workaround, it's support. Better word it like > > "Support for ISA I/O space on Hisilicon HIP0X" > Agreed >> + depends on (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST >> + select 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 cc6364b..28e3862 100644 >> --- a/drivers/bus/Makefile >> +++ b/drivers/bus/Makefile >> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o >> obj-$(CONFIG_ARM_CCN) += arm-ccn.o >> >> obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o >> +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o >> obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 >> --- /dev/null >> +++ b/drivers/bus/hisi_lpc.c >> @@ -0,0 +1,599 @@ >> +/* >> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. >> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> >> + * Author: Zou Rongrong <zourongrong@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/pci.h> >> +#include <linux/serial_8250.h> >> +#include <linux/slab.h> >> + >> +/* >> + * Setting this bit means each IO operation will target to a >> + * different port address: >> + * 0 means repeatedly IO operations will stick on the same port, >> + * such as BT; >> + */ >> +#define FG_INCRADDR_LPC 0x02 >> + >> +struct lpc_cycle_para { >> + unsigned int opflags; >> + unsigned int csize; /* the data length of each operation */ >> +}; >> + >> +struct hisilpc_dev { >> + spinlock_t cycle_lock; >> + void __iomem *membase; >> + struct extio_node *extio; >> +}; >> + >> +/* bounds of the LPC bus address range */ >> +#define LPC_MIN_BUS_RANGE 0x0 >> + >> +/* >> + * The maximal IO size for each leagcy bus. > > legacy? > > I don't really understand why this bus is legacy though. It looks like a > simple MMIO-to-LPC bridge to me. > I think that he means legacy ISA. >> + * The port size of legacy I/O devices is normally less than 0x400. >> + * Defining the I/O range size as 0x400 here should be sufficient for >> + * all peripherals under one bus. >> + */ > > This comment doesn't make a lot of sense. What is the limit? Is there a > hardware limit? > > We don't dynamically allocate devices on the lpc bus, so why imply a > limit at all? > IIRC from previously asking Zhichang this before, this is the upper range we can address devices on the LPC bus. But the value was 0x1000 then. >> +#define LPC_BUS_IO_SIZE 0x400 >> + >> +/* The maximum continuous operations */ >> +#define LPC_MAX_OPCNT 16 >> +/* only support IO data unit length is four at maximum */ >> +#define LPC_MAX_DULEN 4 >> +#if LPC_MAX_DULEN > LPC_MAX_OPCNT >> +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" >> +#endif >> + >> +#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 0x08 >> +#define LPC_CMD_TYPE_IO 0x00 >> +#define LPC_CMD_WRITE 0x01 >> +#define LPC_CMD_READ 0x00 >> +/* the bit attribute is W1C. 1 represents OK. */ >> +#define LPC_STAT_BYIRQ 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. > > Are you sure you want this comment to be upstream? :) > We will remove or refine >> + */ >> +#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 specific for single IO operation, such as inb. */ >> +#define LPC_PEROP_WAITCNT 100 >> + >> + >> +static inline int wait_lpc_idle(unsigned char *mbase, > > No need to specify inline. > Agreed >> + unsigned int waitcnt) { >> + u32 opstatus; >> + >> + 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); > > It's a shame we have to busy loop, but I guess no calling code outside > is prepared for rescheduling at this point. > >> + } >> + return -ETIME; >> +} >> + >> +/* >> + * hisilpc_target_in - trigger a series of lpc cycles to read >> required data >> + * from target peripheral. >> + * @pdev: pointer to hisi lpc device >> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para >> *para, >> + unsigned long ptaddr, unsigned char *buf, >> + unsigned long opcnt) >> +{ >> + unsigned long cnt_per_trans; >> + unsigned int cmd_word; >> + unsigned int waitcnt; >> + int ret; >> + >> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >> + return -EINVAL; >> + >> + if (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; >> + waitcnt = LPC_MAX_WAITCNT; >> + } >> + >> + ret = 0; >> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >> + unsigned long flags; >> + >> + /* whole operation must be atomic */ >> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); > > Ouch. This is going to kill your RT jitter. Is there no better way? > Obviously the bus register driving is non-atomic, so we need some way to lock out. I think that it is not so critical for low-speed/infrequent-access bus. If we were going to use virtual UART in the BMC on the LPC bus then we could consider more. >> + >> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >> + >> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >> + >> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >> + >> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >> + >> + /* whether the operation is finished */ >> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >> + if (!ret) { >> + opcnt -= cnt_per_trans; >> + for (; cnt_per_trans--; buf++) >> + *buf = readl(lpcdev->membase + LPC_REG_RDATA); >> + } >> + >> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >> + } >> + >> + return ret; >> +} >> + >> +/** >> + * hisilpc_target_out - trigger a series of lpc cycles to write required >> + * data to target peripheral. >> + * @pdev: pointer to hisi lpc device >> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para >> *para, >> + unsigned long ptaddr, const unsigned char *buf, >> + unsigned long opcnt) >> +{ >> + unsigned long cnt_per_trans; >> + unsigned int cmd_word; >> + unsigned int waitcnt; >> + int ret; >> + >> + if (!buf || !opcnt || !para || !lpcdev) >> + return -EINVAL; >> + >> + if (opcnt > LPC_MAX_OPCNT) >> + return -EINVAL; >> + /* default is increasing address */ >> + cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE; >> + waitcnt = LPC_PEROP_WAITCNT; >> + if (!(para->opflags & FG_INCRADDR_LPC)) { >> + cmd_word |= LPC_CMD_SAMEADDR; >> + waitcnt = LPC_MAX_WAITCNT; >> + } >> + >> + ret = 0; >> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >> + unsigned long flags; >> + >> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); > > Same thing here > As above >> + >> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >> + opcnt -= cnt_per_trans; >> + for (; cnt_per_trans--; buf++) >> + writel(*buf, lpcdev->membase + LPC_REG_WDATA); >> + >> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >> + >> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >> + >> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >> + >> + /* whether the operation is finished */ >> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >> + >> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >> + } >> + >> + return ret; >> +} >> + >> +static inline unsigned long > > Don't explicitly mention inline, the compiler will figure that out for you. > Sure >> +hisi_lpc_pio_to_addr(struct hisilpc_dev *lpcdev, unsigned long pio) >> +{ >> + return pio - lpcdev->extio->io_start + lpcdev->extio->bus_start; >> +} >> + >> + >> +/** >> + * hisilpc_comm_in - read/input the data from the I/O peripheral >> + * through LPC. >> + * @devobj: pointer to the device information relevant to LPC >> controller. >> + * @pio: the target I/O port address. >> + * @dlen: the data length required to read from the target I/O port. >> + * >> + * 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. >> + */ >> +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t dlen) >> +{ >> + struct hisilpc_dev *lpcdev = devobj; >> + struct lpc_cycle_para iopara; >> + u32 rd_data; > > rd_data needs to be initialized to 0. Otherwise it may contain stale > stack contents and corrupt non-32bit dlen returns. > I think so, since we read into this value byte-by-byte. We also seem to return a 32b value but should return 64b value according to the prototype. >> + unsigned char *newbuf; >> + int ret = 0; >> + unsigned long ptaddr; >> + >> + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen >> - 1))) >> + return -1; > > Isn't this -EINVAL? Not sure. This value is returned directly to the inb/outb caller, which would not check this value for error. It could be argued that the checking is paranoia. If not, we should treat the failure as a more severe event. > >> + >> + /* the local buffer must be enough for one data unit */ >> + if (sizeof(rd_data) < dlen) >> + return -1; > > Same here. > > Also, the above seems a very convoluted way of saying > > switch (dlen) { > case 1: > case 2: > case 4: It's better > break; > default: > return -EINVAL; > } > > But I guess the way you write it doesn't hurt ;) > > > Alex > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31/01/2017 11:07, John Garry wrote: > On 30/01/2017 20:08, Alexander Graf wrote: >> > > Alex, > > Thanks for checking. > >> >> On 24/01/2017 08:05, zhichang.yuan wrote: >>> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the >>> peripherals in >>> I/O port addresses. This patch implements the LPC host controller >>> driver which >>> perform the I/O operations on the underlying hardware. >>> We don't want to touch those existing peripherals' driver, such as >>> ipmi-bt. So >>> this driver applies the indirect-IO introduced in the previous patch >>> after >>> registering an indirect-IO node to the indirect-IO devices list which >>> will be >>> searched in the I/O accessors. >>> As the I/O translations for LPC children depend on the host I/O >>> registration, >>> we should ensure the host I/O registration is finished before all the >>> LPC >>> children scanning. That is why an arch_init() hook was added in this >>> patch. >>> >>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >>> --- >>> .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ >>> MAINTAINERS | 9 + >>> arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + >>> arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + >>> drivers/bus/Kconfig | 8 + >>> drivers/bus/Makefile | 1 + >>> drivers/bus/hisi_lpc.c | 599 >>> +++++++++++++++++++++ >>> 7 files changed, 668 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..213181f >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >>> >>> >>> @@ -0,0 +1,33 @@ >>> +Hisilicon Hip06 low-pin-count device >>> + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, >>> which >>> + provides I/O access to some legacy ISA devices. >>> + Hip06 is based on arm64 architecture where there is no I/O space. >>> So, the >>> + I/O ports here are not cpu addresses, and there is no 'ranges' >>> property in >>> + LPC device node. >>> + >>> +Required properties: >>> +- compatible: value should be as follows: >>> + (a) "hisilicon,hip06-lpc" >>> + (b) "hisilicon,hip07-lpc" >>> +- #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 memory range where the LPC register set is mapped. >>> + >>> +Note: >>> + The node name before '@' must be "isa" to represent the binding >>> stick to the >>> + ISA/EISA binding specification. >>> + >>> +Example: >>> + >>> +isa@a01b0000 { >>> + compatible = "hisilicon,hip06-lpc"; >>> + #address-cells = <2>; >>> + #size-cells = <1>; >>> + reg = <0x0 0xa01b0000 0x0 0x1000>; >>> + >>> + ipmi0: bt@e4 { >>> + compatible = "ipmi-bt"; >>> + device_type = "ipmi"; >>> + reg = <0x01 0xe4 0x04>; >>> + }; >>> +}; >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 26edd83..0153707 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h >>> F: net/802/hippi.c >>> F: drivers/net/hippi/ >>> >>> +HISILICON LPC BUS DRIVER >>> +M: Zhichang Yuan <yuanzhichang@hisilicon.com> >>> +L: linux-arm-kernel@lists.infradead.org >>> +W: http://www.hisilicon.com >>> +S: Maintained >>> +F: drivers/bus/hisi_lpc.c >>> +F: lib/extio.c >>> +F: >>> Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >>> >>> >>> + >>> HISILICON NETWORK SUBSYSTEM DRIVER >>> M: Yisen Zhuang <yisen.zhuang@huawei.com> >>> M: Salil Mehta <salil.mehta@huawei.com> >>> diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> index 7c4114a..75b2b5c 100644 >>> --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> @@ -52,3 +52,7 @@ >>> &usb_ehci { >>> status = "ok"; >>> }; >>> + >>> +&ipmi0 { >>> + status = "ok"; >>> +}; >>> diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> index a049b64..c450f8d 100644 >>> --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> @@ -318,6 +318,20 @@ >>> #size-cells = <2>; >>> ranges; >>> >>> + isa@a01b0000 { >>> + compatible = "hisilicon,hip06-lpc"; >>> + #size-cells = <1>; >>> + #address-cells = <2>; >>> + reg = <0x0 0xa01b0000 0x0 0x1000>; >>> + >>> + ipmi0: bt@e4 { >>> + compatible = "ipmi-bt"; >>> + device_type = "ipmi"; >>> + reg = <0x01 0xe4 0x04>; >>> + status = "disabled"; >>> + }; >>> + }; >>> + >>> refclk: refclk { >>> compatible = "fixed-clock"; >>> clock-frequency = <50000000>; >>> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig >>> index b9e8cfc..58cee84 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" >> >> It's not a workaround, it's support. Better word it like >> >> "Support for ISA I/O space on Hisilicon HIP0X" >> > > Agreed > >>> + depends on (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST >>> + select 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 cc6364b..28e3862 100644 >>> --- a/drivers/bus/Makefile >>> +++ b/drivers/bus/Makefile >>> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o >>> obj-$(CONFIG_ARM_CCN) += arm-ccn.o >>> >>> obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o >>> +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o >>> obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 >>> --- /dev/null >>> +++ b/drivers/bus/hisi_lpc.c >>> @@ -0,0 +1,599 @@ >>> +/* >>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. >>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> >>> + * Author: Zou Rongrong <zourongrong@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/pci.h> >>> +#include <linux/serial_8250.h> >>> +#include <linux/slab.h> >>> + >>> +/* >>> + * Setting this bit means each IO operation will target to a >>> + * different port address: >>> + * 0 means repeatedly IO operations will stick on the same port, >>> + * such as BT; >>> + */ >>> +#define FG_INCRADDR_LPC 0x02 >>> + >>> +struct lpc_cycle_para { >>> + unsigned int opflags; >>> + unsigned int csize; /* the data length of each operation */ >>> +}; >>> + >>> +struct hisilpc_dev { >>> + spinlock_t cycle_lock; >>> + void __iomem *membase; >>> + struct extio_node *extio; >>> +}; >>> + >>> +/* bounds of the LPC bus address range */ >>> +#define LPC_MIN_BUS_RANGE 0x0 >>> + >>> +/* >>> + * The maximal IO size for each leagcy bus. >> >> legacy? >> >> I don't really understand why this bus is legacy though. It looks like a >> simple MMIO-to-LPC bridge to me. >> > > I think that he means legacy ISA. > >>> + * The port size of legacy I/O devices is normally less than 0x400. >>> + * Defining the I/O range size as 0x400 here should be sufficient for >>> + * all peripherals under one bus. >>> + */ >> >> This comment doesn't make a lot of sense. What is the limit? Is there a >> hardware limit? >> >> We don't dynamically allocate devices on the lpc bus, so why imply a >> limit at all? >> > > IIRC from previously asking Zhichang this before, this is the upper > range we can address devices on the LPC bus. But the value was 0x1000 then. Well, all devices that we want to address are defined by firmware (via device tree or dsdt). So I'm not quite sure what this arbitrary limit buys us. > >>> +#define LPC_BUS_IO_SIZE 0x400 >>> + >>> +/* The maximum continuous operations */ >>> +#define LPC_MAX_OPCNT 16 >>> +/* only support IO data unit length is four at maximum */ >>> +#define LPC_MAX_DULEN 4 >>> +#if LPC_MAX_DULEN > LPC_MAX_OPCNT >>> +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" >>> +#endif >>> + >>> +#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 0x08 >>> +#define LPC_CMD_TYPE_IO 0x00 >>> +#define LPC_CMD_WRITE 0x01 >>> +#define LPC_CMD_READ 0x00 >>> +/* the bit attribute is W1C. 1 represents OK. */ >>> +#define LPC_STAT_BYIRQ 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. >> >> Are you sure you want this comment to be upstream? :) >> > > We will remove or refine > >>> + */ >>> +#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 specific for single IO operation, such as >>> inb. */ >>> +#define LPC_PEROP_WAITCNT 100 >>> + >>> + >>> +static inline int wait_lpc_idle(unsigned char *mbase, >> >> No need to specify inline. >> > > Agreed > >>> + unsigned int waitcnt) { >>> + u32 opstatus; >>> + >>> + 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); >> >> It's a shame we have to busy loop, but I guess no calling code outside >> is prepared for rescheduling at this point. >> >>> + } >>> + return -ETIME; >>> +} >>> + >>> +/* >>> + * hisilpc_target_in - trigger a series of lpc cycles to read >>> required data >>> + * from target peripheral. >>> + * @pdev: pointer to hisi lpc device >>> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para >>> *para, >>> + unsigned long ptaddr, unsigned char *buf, >>> + unsigned long opcnt) >>> +{ >>> + unsigned long cnt_per_trans; >>> + unsigned int cmd_word; >>> + unsigned int waitcnt; >>> + int ret; >>> + >>> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >>> + return -EINVAL; >>> + >>> + if (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; >>> + waitcnt = LPC_MAX_WAITCNT; >>> + } >>> + >>> + ret = 0; >>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>> + unsigned long flags; >>> + >>> + /* whole operation must be atomic */ >>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >> >> Ouch. This is going to kill your RT jitter. Is there no better way? >> > > Obviously the bus register driving is non-atomic, so we need some way to > lock out. > > I think that it is not so critical for low-speed/infrequent-access bus. > > If we were going to use virtual UART in the BMC on the LPC bus then we > could consider more. Well, it basically means that an arbitrary daemon running in user space that checks your temperature readings via the ipmi interface could create a lot of jitter. That could be very critical if you want to use this hardware for real time critical applications, such as telecom. I bet that if you leave it like that and postpone the decision to fix it to "later", in 1 or 2 years you will cause someone weeks of debugging to track down why their voip gateway loses packets from time to time. > >>> + >>> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >>> + >>> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >>> + >>> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >>> + >>> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >>> + >>> + /* whether the operation is finished */ >>> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >>> + if (!ret) { >>> + opcnt -= cnt_per_trans; >>> + for (; cnt_per_trans--; buf++) >>> + *buf = readl(lpcdev->membase + LPC_REG_RDATA); >>> + } >>> + >>> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +/** >>> + * hisilpc_target_out - trigger a series of lpc cycles to write >>> required >>> + * data to target peripheral. >>> + * @pdev: pointer to hisi lpc device >>> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para >>> *para, >>> + unsigned long ptaddr, const unsigned char *buf, >>> + unsigned long opcnt) >>> +{ >>> + unsigned long cnt_per_trans; >>> + unsigned int cmd_word; >>> + unsigned int waitcnt; >>> + int ret; >>> + >>> + if (!buf || !opcnt || !para || !lpcdev) >>> + return -EINVAL; >>> + >>> + if (opcnt > LPC_MAX_OPCNT) >>> + return -EINVAL; >>> + /* default is increasing address */ >>> + cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE; >>> + waitcnt = LPC_PEROP_WAITCNT; >>> + if (!(para->opflags & FG_INCRADDR_LPC)) { >>> + cmd_word |= LPC_CMD_SAMEADDR; >>> + waitcnt = LPC_MAX_WAITCNT; >>> + } >>> + >>> + ret = 0; >>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>> + unsigned long flags; >>> + >>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >> >> Same thing here >> > > As above > >>> + >>> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >>> + opcnt -= cnt_per_trans; >>> + for (; cnt_per_trans--; buf++) >>> + writel(*buf, lpcdev->membase + LPC_REG_WDATA); >>> + >>> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >>> + >>> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >>> + >>> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >>> + >>> + /* whether the operation is finished */ >>> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >>> + >>> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +static inline unsigned long >> >> Don't explicitly mention inline, the compiler will figure that out for >> you. >> > > Sure > >>> +hisi_lpc_pio_to_addr(struct hisilpc_dev *lpcdev, unsigned long pio) >>> +{ >>> + return pio - lpcdev->extio->io_start + lpcdev->extio->bus_start; >>> +} >>> + >>> + >>> +/** >>> + * hisilpc_comm_in - read/input the data from the I/O peripheral >>> + * through LPC. >>> + * @devobj: pointer to the device information relevant to LPC >>> controller. >>> + * @pio: the target I/O port address. >>> + * @dlen: the data length required to read from the target I/O port. >>> + * >>> + * 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. >>> + */ >>> +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t >>> dlen) >>> +{ >>> + struct hisilpc_dev *lpcdev = devobj; >>> + struct lpc_cycle_para iopara; >>> + u32 rd_data; >> >> rd_data needs to be initialized to 0. Otherwise it may contain stale >> stack contents and corrupt non-32bit dlen returns. >> > > I think so, since we read into this value byte-by-byte. We also seem to > return a 32b value but should return 64b value according to the prototype. IIRC LPC (well, PIO) doesn't support bigger requests than 32bit. At least I can't think of an x86 instruction that would allow bigger transactions. So there's no need to make it 64bit. However, the question is why the prototype is 64bit then. Hm. :) Maybe the prototype should be only 32bit. > >>> + unsigned char *newbuf; >>> + int ret = 0; >>> + unsigned long ptaddr; >>> + >>> + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen >>> - 1))) >>> + return -1; >> >> Isn't this -EINVAL? > > Not sure. This value is returned directly to the inb/outb caller, which > would not check this value for error. > > It could be argued that the checking is paranoia. If not, we should > treat the failure as a more severe event. Oh, I see. In that case -1 makes a lot of sense since it's the default read value on x86 for unallocated space. This probably deserves a comment (and/or maybe a #define) Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> + * The port size of legacy I/O devices is normally less than 0x400. >>>> + * Defining the I/O range size as 0x400 here should be sufficient for >>>> + * all peripherals under one bus. >>>> + */ >>> >>> This comment doesn't make a lot of sense. What is the limit? Is there a >>> hardware limit? >>> >>> We don't dynamically allocate devices on the lpc bus, so why imply a >>> limit at all? >>> >> >> IIRC from previously asking Zhichang this before, this is the upper >> range we can address devices on the LPC bus. But the value was 0x1000 >> then. > > Well, all devices that we want to address are defined by firmware (via > device tree or dsdt). So I'm not quite sure what this arbitrary limit > buys us. > Will check with Zhichang. >> >>>> +#define LPC_BUS_IO_SIZE 0x400 >>>> + <snip> >>>> + ret = 0; >>>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>>> + unsigned long flags; >>>> + >>>> + /* whole operation must be atomic */ >>>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >>> >>> Ouch. This is going to kill your RT jitter. Is there no better way? >>> >> >> Obviously the bus register driving is non-atomic, so we need some way to >> lock out. >> >> I think that it is not so critical for low-speed/infrequent-access bus. >> >> If we were going to use virtual UART in the BMC on the LPC bus then we >> could consider more. > > Well, it basically means that an arbitrary daemon running in user space > that checks your temperature readings via the ipmi interface could > create a lot of jitter. That could be very critical if you want to use > this hardware for real time critical applications, such as telecom. > > I bet that if you leave it like that and postpone the decision to fix it > to "later", in 1 or 2 years you will cause someone weeks of debugging to > track down why their voip gateway loses packets from time to time. > >> We need to consider this more. There may a way to access the registers and drive the bus without requiring a lock, but I doubt it. Note: I think that we could make some readl/writel relaxed, which would help. >>>> + >>>> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >>>> + >>>> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >>>> + >>>> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >>>> + >>>> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >>>> + >>>> + /* whether the operation is finished */ >>>> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >>>> + if (!ret) { >>>> + opcnt -= cnt_per_trans; >>>> + for (; cnt_per_trans--; buf++) >>>> + *buf = readl(lpcdev->membase + LPC_REG_RDATA); >>>> + } >>>> + >>>> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >>>> + } >>>> + >>>> + return ret; >>>> +} <snip> >>>> + * hisilpc_comm_in - read/input the data from the I/O peripheral >>>> + * through LPC. >>>> + * @devobj: pointer to the device information relevant to LPC >>>> controller. >>>> + * @pio: the target I/O port address. >>>> + * @dlen: the data length required to read from the target I/O port. >>>> + * >>>> + * 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. >>>> + */ >>>> +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t >>>> dlen) >>>> +{ >>>> + struct hisilpc_dev *lpcdev = devobj; >>>> + struct lpc_cycle_para iopara; >>>> + u32 rd_data; >>> >>> rd_data needs to be initialized to 0. Otherwise it may contain stale >>> stack contents and corrupt non-32bit dlen returns. >>> >> >> I think so, since we read into this value byte-by-byte. We also seem to >> return a 32b value but should return 64b value according to the >> prototype. > > IIRC LPC (well, PIO) doesn't support bigger requests than 32bit. At > least I can't think of an x86 instruction that would allow bigger > transactions. So there's no need to make it 64bit. However, the question > is why the prototype is 64bit then. Hm. :) > > Maybe the prototype should be only 32bit. > Will check with Zhichang. >> >>>> + unsigned char *newbuf; >>>> + int ret = 0; >>>> + unsigned long ptaddr; >>>> + >>>> + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen >>>> - 1))) >>>> + return -1; >>> >>> Isn't this -EINVAL? >> >> Not sure. This value is returned directly to the inb/outb caller, which >> would not check this value for error. >> >> It could be argued that the checking is paranoia. If not, we should >> treat the failure as a more severe event. > > Oh, I see. In that case -1 makes a lot of sense since it's the default > read value on x86 for unallocated space. > > This probably deserves a comment (and/or maybe a #define) > We can add a comment > > Alex > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alex thanks for reviewing [...] > > > >>> + * The port size of legacy I/O devices is normally less than > 0x400. > >>> + * Defining the I/O range size as 0x400 here should be sufficient > for > >>> + * all peripherals under one bus. > >>> + */ > >> > >> This comment doesn't make a lot of sense. What is the limit? Is > there a > >> hardware limit? > >> > >> We don't dynamically allocate devices on the lpc bus, so why imply a > >> limit at all? > >> > > > > IIRC from previously asking Zhichang this before, this is the upper > > range we can address devices on the LPC bus. But the value was 0x1000 > then. > > Well, all devices that we want to address are defined by firmware (via > device tree or dsdt). So I'm not quite sure what this arbitrary limit > buys us. Following a previous discussion with Arnd: << We know that the ISA/LPC bus can only have up to 65536 ports, so you can register all of those, or possibly limit it further to 1024 or 4096 ports, whichever matches the bus implementation. >> (https://lkml.org/lkml/2016/11/10/95) We decided to register a fixed IO range for our LPC. About the specific reason for choosing 0x400 I think Zhichang can clarify once he's back (he's OOO now) > [...] > >>> + > >>> + /* whole operation must be atomic */ > >>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); > >> > >> Ouch. This is going to kill your RT jitter. Is there no better way? > >> > > > > Obviously the bus register driving is non-atomic, so we need some way > to > > lock out. > > > > I think that it is not so critical for low-speed/infrequent-access > bus. > > > > If we were going to use virtual UART in the BMC on the LPC bus then > we > > could consider more. > > Well, it basically means that an arbitrary daemon running in user space > that checks your temperature readings via the ipmi interface could > create a lot of jitter. That could be very critical if you want to use > this hardware for real time critical applications, such as telecom. > > I bet that if you leave it like that and postpone the decision to fix > it > to "later", in 1 or 2 years you will cause someone weeks of debugging > to > track down why their voip gateway loses packets from time to time. Thanks for the heads-up. We'll discuss internally after Chinese holidays to understand if this implementation is compatible with the intended deployment scenarions > > > [...] > >>> +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t > >>> dlen) > >>> +{ > >>> + struct hisilpc_dev *lpcdev = devobj; > >>> + struct lpc_cycle_para iopara; > >>> + u32 rd_data; > >> > >> rd_data needs to be initialized to 0. Otherwise it may contain stale > >> stack contents and corrupt non-32bit dlen returns. > >> > > > > I think so, since we read into this value byte-by-byte. We also seem > to > > return a 32b value but should return 64b value according to the > prototype. > > IIRC LPC (well, PIO) doesn't support bigger requests than 32bit. At > least I can't think of an x86 instruction that would allow bigger > transactions. So there's no need to make it 64bit. However, the > question > is why the prototype is 64bit then. Hm. :) > > Maybe the prototype should be only 32bit. Maybe you're right :) Looking at extio.c we never return a 64b value. I'll double check with Zhichang once he's back... > > > > >>> + unsigned char *newbuf; > >>> + int ret = 0; > >>> + unsigned long ptaddr; > >>> + > >>> + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & > (dlen > >>> - 1))) > >>> + return -1; > >> > >> Isn't this -EINVAL? > > > > Not sure. This value is returned directly to the inb/outb caller, > which > > would not check this value for error. > > > > It could be argued that the checking is paranoia. If not, we should > > treat the failure as a more severe event. > > Oh, I see. In that case -1 makes a lot of sense since it's the default > read value on x86 for unallocated space. > > This probably deserves a comment (and/or maybe a #define) > > > Alex -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Alex, Thanks for your review! John had replied most of your comments, I only mentioned those which haven't made clear. On 2017/1/31 4:08, Alexander Graf wrote: > > > On 24/01/2017 08:05, zhichang.yuan wrote: >> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in >> I/O port addresses. This patch implements the LPC host controller driver which >> perform the I/O operations on the underlying hardware. >> We don't want to touch those existing peripherals' driver, such as ipmi-bt. So >> this driver applies the indirect-IO introduced in the previous patch after >> registering an indirect-IO node to the indirect-IO devices list which will be >> searched in the I/O accessors. >> As the I/O translations for LPC children depend on the host I/O registration, >> we should ensure the host I/O registration is finished before all the LPC >> children scanning. That is why an arch_init() hook was added in this patch. >> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >> --- >> .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ >> MAINTAINERS | 9 + >> arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + >> arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + >> drivers/bus/Kconfig | 8 + >> drivers/bus/Makefile | 1 + >> drivers/bus/hisi_lpc.c | 599 +++++++++++++++++++++ >> 7 files changed, 668 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..213181f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >> @@ -0,0 +1,33 @@ >> +Hisilicon Hip06 low-pin-count device >> + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which >> + provides I/O access to some legacy ISA devices. >> + Hip06 is based on arm64 architecture where there is no I/O space. So, the >> + I/O ports here are not cpu addresses, and there is no 'ranges' property in >> + LPC device node. >> + >> +Required properties: >> +- compatible: value should be as follows: >> + (a) "hisilicon,hip06-lpc" >> + (b) "hisilicon,hip07-lpc" >> +- #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 memory range where the LPC register set is mapped. >> + >> +Note: >> + The node name before '@' must be "isa" to represent the binding stick to the >> + ISA/EISA binding specification. >> + >> +Example: >> + >> +isa@a01b0000 { >> + compatible = "hisilicon,hip06-lpc"; >> + #address-cells = <2>; >> + #size-cells = <1>; >> + reg = <0x0 0xa01b0000 0x0 0x1000>; >> + >> + ipmi0: bt@e4 { >> + compatible = "ipmi-bt"; >> + device_type = "ipmi"; >> + reg = <0x01 0xe4 0x04>; >> + }; >> +}; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 26edd83..0153707 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h >> F: net/802/hippi.c >> F: drivers/net/hippi/ >> >> +HISILICON LPC BUS DRIVER >> +M: Zhichang Yuan <yuanzhichang@hisilicon.com> >> +L: linux-arm-kernel@lists.infradead.org >> +W: http://www.hisilicon.com >> +S: Maintained >> +F: drivers/bus/hisi_lpc.c >> +F: lib/extio.c >> +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >> + >> HISILICON NETWORK SUBSYSTEM DRIVER >> M: Yisen Zhuang <yisen.zhuang@huawei.com> >> M: Salil Mehta <salil.mehta@huawei.com> >> diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> index 7c4114a..75b2b5c 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >> @@ -52,3 +52,7 @@ >> &usb_ehci { >> status = "ok"; >> }; >> + >> +&ipmi0 { >> + status = "ok"; >> +}; >> diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> index a049b64..c450f8d 100644 >> --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >> @@ -318,6 +318,20 @@ >> #size-cells = <2>; >> ranges; >> >> + isa@a01b0000 { >> + compatible = "hisilicon,hip06-lpc"; >> + #size-cells = <1>; >> + #address-cells = <2>; >> + reg = <0x0 0xa01b0000 0x0 0x1000>; >> + >> + ipmi0: bt@e4 { >> + compatible = "ipmi-bt"; >> + device_type = "ipmi"; >> + reg = <0x01 0xe4 0x04>; >> + status = "disabled"; >> + }; >> + }; >> + >> refclk: refclk { >> compatible = "fixed-clock"; >> clock-frequency = <50000000>; >> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig >> index b9e8cfc..58cee84 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" > > It's not a workaround, it's support. Better word it like > > "Support for ISA I/O space on Hisilicon HIP0X" > >> + depends on (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST >> + select 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 cc6364b..28e3862 100644 >> --- a/drivers/bus/Makefile >> +++ b/drivers/bus/Makefile >> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o >> obj-$(CONFIG_ARM_CCN) += arm-ccn.o >> >> obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o >> +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o >> obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 >> --- /dev/null >> +++ b/drivers/bus/hisi_lpc.c >> @@ -0,0 +1,599 @@ >> +/* >> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. >> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> >> + * Author: Zou Rongrong <zourongrong@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/pci.h> >> +#include <linux/serial_8250.h> >> +#include <linux/slab.h> >> + >> +/* >> + * Setting this bit means each IO operation will target to a >> + * different port address: >> + * 0 means repeatedly IO operations will stick on the same port, >> + * such as BT; >> + */ >> +#define FG_INCRADDR_LPC 0x02 >> + >> +struct lpc_cycle_para { >> + unsigned int opflags; >> + unsigned int csize; /* the data length of each operation */ >> +}; >> + >> +struct hisilpc_dev { >> + spinlock_t cycle_lock; >> + void __iomem *membase; >> + struct extio_node *extio; >> +}; >> + >> +/* bounds of the LPC bus address range */ >> +#define LPC_MIN_BUS_RANGE 0x0 >> + >> +/* >> + * The maximal IO size for each leagcy bus. > > legacy? > > I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. yes. The LPC bus is MMIO-to-LPC bridge. My comment is not clear. How about "The maximal IO size of LPC bus"? > >> + * The port size of legacy I/O devices is normally less than 0x400. >> + * Defining the I/O range size as 0x400 here should be sufficient for >> + * all peripherals under one bus. >> + */ > > This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? > > We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? The limit here is to define an IO window for LPC. It is not acceptable to describe the IO window with 'ranges' property in LPC host node, so we choose a fixable upper IO limit for the static LPC IO space. Without this, LPC can't register its IO space through extio helpers. Why 0x400 is chosen? For our LPC, 0x400 cover all the peripherals under the LPC. > >> +#define LPC_BUS_IO_SIZE 0x400 >> + >> +/* The maximum continuous operations */ >> +#define LPC_MAX_OPCNT 16 >> +/* only support IO data unit length is four at maximum */ >> +#define LPC_MAX_DULEN 4 >> +#if LPC_MAX_DULEN > LPC_MAX_OPCNT >> +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" >> +#endif >> + >> +#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 0x08 >> +#define LPC_CMD_TYPE_IO 0x00 >> +#define LPC_CMD_WRITE 0x01 >> +#define LPC_CMD_READ 0x00 >> +/* the bit attribute is W1C. 1 represents OK. */ >> +#define LPC_STAT_BYIRQ 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. > > Are you sure you want this comment to be upstream? :) > >> + */ >> +#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 specific for single IO operation, such as inb. */ >> +#define LPC_PEROP_WAITCNT 100 >> + >> + >> +static inline int wait_lpc_idle(unsigned char *mbase, > > No need to specify inline. > >> + unsigned int waitcnt) { >> + u32 opstatus; >> + >> + 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); > > It's a shame we have to busy loop, but I guess no calling code outside is prepared for rescheduling at this point. > >> + } >> + return -ETIME; >> +} >> + >> +/* >> + * hisilpc_target_in - trigger a series of lpc cycles to read required data >> + * from target peripheral. >> + * @pdev: pointer to hisi lpc device >> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, >> + unsigned long ptaddr, unsigned char *buf, >> + unsigned long opcnt) >> +{ >> + unsigned long cnt_per_trans; >> + unsigned int cmd_word; >> + unsigned int waitcnt; >> + int ret; >> + >> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >> + return -EINVAL; >> + >> + if (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; >> + waitcnt = LPC_MAX_WAITCNT; >> + } >> + >> + ret = 0; >> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >> + unsigned long flags; >> + >> + /* whole operation must be atomic */ >> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); > > Ouch. This is going to kill your RT jitter. Is there no better way? I think there is no other choices. We have to ensure the I/O cycles triggered in serial mode.... Best, Zhichang > >> + >> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >> + >> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >> + >> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >> + >> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >> + >> + /* whether the operation is finished */ >> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >> + if (!ret) { >> + opcnt -= cnt_per_trans; >> + for (; cnt_per_trans--; buf++) >> + *buf = readl(lpcdev->membase + LPC_REG_RDATA); >> + } >> + >> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >> + } >> + >> + return ret; >> +} >> + >> +/** >> + * hisilpc_target_out - trigger a series of lpc cycles to write required >> + * data to target peripheral. >> + * @pdev: pointer to hisi lpc device >> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, >> + unsigned long ptaddr, const unsigned char *buf, >> + unsigned long opcnt) >> +{ >> + unsigned long cnt_per_trans; >> + unsigned int cmd_word; >> + unsigned int waitcnt; >> + int ret; >> + >> + if (!buf || !opcnt || !para || !lpcdev) >> + return -EINVAL; >> + >> + if (opcnt > LPC_MAX_OPCNT) >> + return -EINVAL; >> + /* default is increasing address */ >> + cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE; >> + waitcnt = LPC_PEROP_WAITCNT; >> + if (!(para->opflags & FG_INCRADDR_LPC)) { >> + cmd_word |= LPC_CMD_SAMEADDR; >> + waitcnt = LPC_MAX_WAITCNT; >> + } >> + >> + ret = 0; >> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >> + unsigned long flags; >> + >> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); > > Same thing here > >> + >> + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); >> + opcnt -= cnt_per_trans; >> + for (; cnt_per_trans--; buf++) >> + writel(*buf, lpcdev->membase + LPC_REG_WDATA); >> + >> + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); >> + >> + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); >> + >> + writel(START_WORK, lpcdev->membase + LPC_REG_START); >> + >> + /* whether the operation is finished */ >> + ret = wait_lpc_idle(lpcdev->membase, waitcnt); >> + >> + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); >> + } >> + >> + return ret; >> +} >> + >> +static inline unsigned long > > Don't explicitly mention inline, the compiler will figure that out for you. > >> +hisi_lpc_pio_to_addr(struct hisilpc_dev *lpcdev, unsigned long pio) >> +{ >> + return pio - lpcdev->extio->io_start + lpcdev->extio->bus_start; >> +} >> + >> + >> +/** >> + * hisilpc_comm_in - read/input the data from the I/O peripheral >> + * through LPC. >> + * @devobj: pointer to the device information relevant to LPC controller. >> + * @pio: the target I/O port address. >> + * @dlen: the data length required to read from the target I/O port. >> + * >> + * 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. >> + */ >> +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t dlen) >> +{ >> + struct hisilpc_dev *lpcdev = devobj; >> + struct lpc_cycle_para iopara; >> + u32 rd_data; > > rd_data needs to be initialized to 0. Otherwise it may contain stale stack contents and corrupt non-32bit dlen returns. > >> + unsigned char *newbuf; >> + int ret = 0; >> + unsigned long ptaddr; >> + >> + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen - 1))) >> + return -1; > > Isn't this -EINVAL? > >> + >> + /* the local buffer must be enough for one data unit */ >> + if (sizeof(rd_data) < dlen) >> + return -1; > > Same here. > > Also, the above seems a very convoluted way of saying > > switch (dlen) { > case 1: > case 2: > case 4: > break; > default: > return -EINVAL; > } > > But I guess the way you write it doesn't hurt ;) > > > Alex > > . >
On 13/02/2017 15:39, zhichang.yuan wrote: > Hi, Alex, > > Thanks for your review! > > John had replied most of your comments, I only mentioned those which haven't made clear. > > > On 2017/1/31 4:08, Alexander Graf wrote: >> >> >> On 24/01/2017 08:05, zhichang.yuan wrote: >>> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in >>> I/O port addresses. This patch implements the LPC host controller driver which >>> perform the I/O operations on the underlying hardware. >>> We don't want to touch those existing peripherals' driver, such as ipmi-bt. So >>> this driver applies the indirect-IO introduced in the previous patch after >>> registering an indirect-IO node to the indirect-IO devices list which will be >>> searched in the I/O accessors. >>> As the I/O translations for LPC children depend on the host I/O registration, >>> we should ensure the host I/O registration is finished before all the LPC >>> children scanning. That is why an arch_init() hook was added in this patch. >>> >>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >>> --- >>> .../arm/hisilicon/hisilicon-low-pin-count.txt | 33 ++ >>> MAINTAINERS | 9 + >>> arch/arm64/boot/dts/hisilicon/hip06-d03.dts | 4 + >>> arch/arm64/boot/dts/hisilicon/hip06.dtsi | 14 + >>> drivers/bus/Kconfig | 8 + >>> drivers/bus/Makefile | 1 + >>> drivers/bus/hisi_lpc.c | 599 +++++++++++++++++++++ >>> 7 files changed, 668 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..213181f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >>> @@ -0,0 +1,33 @@ >>> +Hisilicon Hip06 low-pin-count device >>> + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which >>> + provides I/O access to some legacy ISA devices. >>> + Hip06 is based on arm64 architecture where there is no I/O space. So, the >>> + I/O ports here are not cpu addresses, and there is no 'ranges' property in >>> + LPC device node. >>> + >>> +Required properties: >>> +- compatible: value should be as follows: >>> + (a) "hisilicon,hip06-lpc" >>> + (b) "hisilicon,hip07-lpc" >>> +- #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 memory range where the LPC register set is mapped. >>> + >>> +Note: >>> + The node name before '@' must be "isa" to represent the binding stick to the >>> + ISA/EISA binding specification. >>> + >>> +Example: >>> + >>> +isa@a01b0000 { >>> + compatible = "hisilicon,hip06-lpc"; >>> + #address-cells = <2>; >>> + #size-cells = <1>; >>> + reg = <0x0 0xa01b0000 0x0 0x1000>; >>> + >>> + ipmi0: bt@e4 { >>> + compatible = "ipmi-bt"; >>> + device_type = "ipmi"; >>> + reg = <0x01 0xe4 0x04>; >>> + }; >>> +}; >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 26edd83..0153707 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h >>> F: net/802/hippi.c >>> F: drivers/net/hippi/ >>> >>> +HISILICON LPC BUS DRIVER >>> +M: Zhichang Yuan <yuanzhichang@hisilicon.com> >>> +L: linux-arm-kernel@lists.infradead.org >>> +W: http://www.hisilicon.com >>> +S: Maintained >>> +F: drivers/bus/hisi_lpc.c >>> +F: lib/extio.c >>> +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt >>> + >>> HISILICON NETWORK SUBSYSTEM DRIVER >>> M: Yisen Zhuang <yisen.zhuang@huawei.com> >>> M: Salil Mehta <salil.mehta@huawei.com> >>> diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> index 7c4114a..75b2b5c 100644 >>> --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts >>> @@ -52,3 +52,7 @@ >>> &usb_ehci { >>> status = "ok"; >>> }; >>> + >>> +&ipmi0 { >>> + status = "ok"; >>> +}; >>> diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> index a049b64..c450f8d 100644 >>> --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi >>> @@ -318,6 +318,20 @@ >>> #size-cells = <2>; >>> ranges; >>> >>> + isa@a01b0000 { >>> + compatible = "hisilicon,hip06-lpc"; >>> + #size-cells = <1>; >>> + #address-cells = <2>; >>> + reg = <0x0 0xa01b0000 0x0 0x1000>; >>> + >>> + ipmi0: bt@e4 { >>> + compatible = "ipmi-bt"; >>> + device_type = "ipmi"; >>> + reg = <0x01 0xe4 0x04>; >>> + status = "disabled"; >>> + }; >>> + }; >>> + >>> refclk: refclk { >>> compatible = "fixed-clock"; >>> clock-frequency = <50000000>; >>> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig >>> index b9e8cfc..58cee84 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" >> >> It's not a workaround, it's support. Better word it like >> >> "Support for ISA I/O space on Hisilicon HIP0X" >> >>> + depends on (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST >>> + select 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 cc6364b..28e3862 100644 >>> --- a/drivers/bus/Makefile >>> +++ b/drivers/bus/Makefile >>> @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o >>> obj-$(CONFIG_ARM_CCN) += arm-ccn.o >>> >>> obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o >>> +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o >>> obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 >>> --- /dev/null >>> +++ b/drivers/bus/hisi_lpc.c >>> @@ -0,0 +1,599 @@ >>> +/* >>> + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. >>> + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> >>> + * Author: Zou Rongrong <zourongrong@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/pci.h> >>> +#include <linux/serial_8250.h> >>> +#include <linux/slab.h> >>> + >>> +/* >>> + * Setting this bit means each IO operation will target to a >>> + * different port address: >>> + * 0 means repeatedly IO operations will stick on the same port, >>> + * such as BT; >>> + */ >>> +#define FG_INCRADDR_LPC 0x02 >>> + >>> +struct lpc_cycle_para { >>> + unsigned int opflags; >>> + unsigned int csize; /* the data length of each operation */ >>> +}; >>> + >>> +struct hisilpc_dev { >>> + spinlock_t cycle_lock; >>> + void __iomem *membase; >>> + struct extio_node *extio; >>> +}; >>> + >>> +/* bounds of the LPC bus address range */ >>> +#define LPC_MIN_BUS_RANGE 0x0 >>> + >>> +/* >>> + * The maximal IO size for each leagcy bus. >> >> legacy? >> >> I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. > > yes. The LPC bus is MMIO-to-LPC bridge. > My comment is not clear. > > How about "The maximal IO size of LPC bus"? That sounds better, but doesn't really tell me what it's about yet ;). > >> >>> + * The port size of legacy I/O devices is normally less than 0x400. >>> + * Defining the I/O range size as 0x400 here should be sufficient for >>> + * all peripherals under one bus. >>> + */ >> >> This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? >> >> We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? > > The limit here is to define an IO window for LPC. > It is not acceptable to describe the IO window with 'ranges' property in LPC host node, so we choose a fixable upper IO limit > for the static LPC IO space. Without this, LPC can't register its IO space through extio helpers. > > Why 0x400 is chosen? For our LPC, 0x400 cover all the peripherals under the LPC. This is information that should come from firmware then. Whether by coincidence addresses 0x0 - 0x400 include all devices is an implementation detail that the driver shouldn't have to know about. That said, as mentioned elsewhere, we don't need to define single windows at all. Instead, can't we just do it like PCI BARs and declare all PIO regions we get from firmware as readily allocated? Then we only need to explicitly assign ports 0x80, 0x81, 0x99, etc to the LPC bridge and leave the rest for grabs (by PCI hotplug for example) > > > > >> >>> +#define LPC_BUS_IO_SIZE 0x400 >>> + >>> +/* The maximum continuous operations */ >>> +#define LPC_MAX_OPCNT 16 >>> +/* only support IO data unit length is four at maximum */ >>> +#define LPC_MAX_DULEN 4 >>> +#if LPC_MAX_DULEN > LPC_MAX_OPCNT >>> +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" >>> +#endif >>> + >>> +#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 0x08 >>> +#define LPC_CMD_TYPE_IO 0x00 >>> +#define LPC_CMD_WRITE 0x01 >>> +#define LPC_CMD_READ 0x00 >>> +/* the bit attribute is W1C. 1 represents OK. */ >>> +#define LPC_STAT_BYIRQ 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. >> >> Are you sure you want this comment to be upstream? :) >> >>> + */ >>> +#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 specific for single IO operation, such as inb. */ >>> +#define LPC_PEROP_WAITCNT 100 >>> + >>> + >>> +static inline int wait_lpc_idle(unsigned char *mbase, >> >> No need to specify inline. >> >>> + unsigned int waitcnt) { >>> + u32 opstatus; >>> + >>> + 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); >> >> It's a shame we have to busy loop, but I guess no calling code outside is prepared for rescheduling at this point. >> >>> + } >>> + return -ETIME; >>> +} >>> + >>> +/* >>> + * hisilpc_target_in - trigger a series of lpc cycles to read required data >>> + * from target peripheral. >>> + * @pdev: pointer to hisi lpc device >>> + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, >>> + unsigned long ptaddr, unsigned char *buf, >>> + unsigned long opcnt) >>> +{ >>> + unsigned long cnt_per_trans; >>> + unsigned int cmd_word; >>> + unsigned int waitcnt; >>> + int ret; >>> + >>> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >>> + return -EINVAL; >>> + >>> + if (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; >>> + waitcnt = LPC_MAX_WAITCNT; >>> + } >>> + >>> + ret = 0; >>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>> + unsigned long flags; >>> + >>> + /* whole operation must be atomic */ >>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >> >> Ouch. This is going to kill your RT jitter. Is there no better way? > > I think there is no other choices. We have to ensure the I/O cycles triggered in serial mode.... As nobody else accesses the LPC device in the meantime thanks to the lock, you don't need to disable interrupts, right? IIRC in PREEMPT_RT, a normal spinlock becomes a mutex and allows for resched during the LPC transaction. Alex
Hi, Alex, On 2017/2/14 21:29, Alexander Graf wrote: > > > On 13/02/2017 15:39, zhichang.yuan wrote: >> Hi, Alex, >> >> Thanks for your review! >> >> John had replied most of your comments, I only mentioned those which haven't made clear. >> >> >> On 2017/1/31 4:08, Alexander Graf wrote: >>> >>> >>> On 24/01/2017 08:05, zhichang.yuan wrote: >>>> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in >>>> I/O port addresses. This patch implements the LPC host controller driver which >>>> perform the I/O operations on the underlying hardware. >>>> We don't want to touch those existing peripherals' driver, such as ipmi-bt. So >>>> this driver applies the indirect-IO introduced in the previous patch after >>>> registering an indirect-IO node to the indirect-IO devices list which will be >>>> searched in the I/O accessors. >>>> As the I/O translations for LPC children depend on the host I/O registration, >>>> we should ensure the host I/O registration is finished before all the LPC >>>> children scanning. That is why an arch_init() hook was added in this patch. >>>> >>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >>>> --- snip >>>> + >>>> +struct lpc_cycle_para { >>>> + unsigned int opflags; >>>> + unsigned int csize; /* the data length of each operation */ >>>> +}; >>>> + >>>> +struct hisilpc_dev { >>>> + spinlock_t cycle_lock; >>>> + void __iomem *membase; >>>> + struct extio_node *extio; >>>> +}; >>>> + >>>> +/* bounds of the LPC bus address range */ >>>> +#define LPC_MIN_BUS_RANGE 0x0 >>>> + >>>> +/* >>>> + * The maximal IO size for each leagcy bus. >>> >>> legacy? >>> >>> I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. >> >> yes. The LPC bus is MMIO-to-LPC bridge. >> My comment is not clear. >> >> How about "The maximal IO size of LPC bus"? > > That sounds better, but doesn't really tell me what it's about yet ;). > >> >>> >>>> + * The port size of legacy I/O devices is normally less than 0x400. >>>> + * Defining the I/O range size as 0x400 here should be sufficient for >>>> + * all peripherals under one bus. >>>> + */ >>> >>> This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? >>> >>> We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? >> >> The limit here is to define an IO window for LPC. >> It is not acceptable to describe the IO window with 'ranges' property in LPC host node, so we choose a fixable upper IO limit >> for the static LPC IO space. Without this, LPC can't register its IO space through extio helpers. >> >> Why 0x400 is chosen? For our LPC, 0x400 cover all the peripherals under the LPC. > > This is information that should come from firmware then. Whether by coincidence addresses 0x0 - 0x400 include all devices is an implementation detail that the driver shouldn't have to know about. > > That said, as mentioned elsewhere, we don't need to define single windows at all. Instead, can't we just do it like PCI BARs and declare all PIO regions we get from firmware as readily allocated? Then we only need to explicitly assign ports 0x80, 0x81, 0x99, etc to the LPC bridge and leave the rest for grabs (by PCI hotplug for example) > I worry I don't completely catch your idea. From my understanding, each PCI host can parse its resource windows from firmware(such as ACPI, dts). But for our LPC, firmware configurations only tell kernel what is the IO resource of the peripherals under LPC. But to register the io range in the similar way of pci_register_io_range(), at least, we need to know the IO range size. So, how to know this? There is no similar BAR design in LPC, do we traverse all the peripherals to calculate that? >> >> >> >> >>> >>>> +#define LPC_BUS_IO_SIZE 0x400 >>>> + >>>> +/* The maximum continuous operations */ >>>> +#define LPC_MAX_OPCNT 16 >>>> +/* only support IO data unit length is four at maximum */ >>>> +#define LPC_MAX_DULEN 4 snip >>>> + */ >>>> +static int >>>> +hisilpc_target_in(struct hisilpc_dev *lpcdev, struct lpc_cycle_para *para, >>>> + unsigned long ptaddr, unsigned char *buf, >>>> + unsigned long opcnt) >>>> +{ >>>> + unsigned long cnt_per_trans; >>>> + unsigned int cmd_word; >>>> + unsigned int waitcnt; >>>> + int ret; >>>> + >>>> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >>>> + return -EINVAL; >>>> + >>>> + if (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; >>>> + waitcnt = LPC_MAX_WAITCNT; >>>> + } >>>> + >>>> + ret = 0; >>>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>>> + unsigned long flags; >>>> + >>>> + /* whole operation must be atomic */ >>>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >>> >>> Ouch. This is going to kill your RT jitter. Is there no better way? >> >> I think there is no other choices. We have to ensure the I/O cycles triggered in serial mode.... > > As nobody else accesses the LPC device in the meantime thanks to the lock, you don't need to disable interrupts, right? O. Do you mean that we can replace spin_lock_irqsave with spin_lock? As we can connect UART to LPC, I worry the LPC can be accessed in the interrupt context. Thanks, Zhichang > > IIRC in PREEMPT_RT, a normal spinlock becomes a mutex and allows for resched during the LPC transaction. > > > Alex > > . >
On 15/02/2017 12:35, zhichang.yuan wrote: > Hi, Alex, > > > On 2017/2/14 21:29, Alexander Graf wrote: >> >> >> On 13/02/2017 15:39, zhichang.yuan wrote: >>> Hi, Alex, >>> >>> Thanks for your review! >>> >>> John had replied most of your comments, I only mentioned those which haven't made clear. >>> >>> >>> On 2017/1/31 4:08, Alexander Graf wrote: >>>> >>>> >>>> On 24/01/2017 08:05, zhichang.yuan wrote: >>>>> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in >>>>> I/O port addresses. This patch implements the LPC host controller driver which >>>>> perform the I/O operations on the underlying hardware. >>>>> We don't want to touch those existing peripherals' driver, such as ipmi-bt. So >>>>> this driver applies the indirect-IO introduced in the previous patch after >>>>> registering an indirect-IO node to the indirect-IO devices list which will be >>>>> searched in the I/O accessors. >>>>> As the I/O translations for LPC children depend on the host I/O registration, >>>>> we should ensure the host I/O registration is finished before all the LPC >>>>> children scanning. That is why an arch_init() hook was added in this patch. >>>>> >>>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >>>>> --- > snip >>>>> + >>>>> +struct lpc_cycle_para { >>>>> + unsigned int opflags; >>>>> + unsigned int csize; /* the data length of each operation */ >>>>> +}; >>>>> + >>>>> +struct hisilpc_dev { >>>>> + spinlock_t cycle_lock; >>>>> + void __iomem *membase; >>>>> + struct extio_node *extio; >>>>> +}; >>>>> + >>>>> +/* bounds of the LPC bus address range */ >>>>> +#define LPC_MIN_BUS_RANGE 0x0 >>>>> + >>>>> +/* >>>>> + * The maximal IO size for each leagcy bus. >>>> >>>> legacy? >>>> >>>> I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. >>> >>> yes. The LPC bus is MMIO-to-LPC bridge. >>> My comment is not clear. >>> >>> How about "The maximal IO size of LPC bus"? >> >> That sounds better, but doesn't really tell me what it's about yet ;). >> >>> >>>> >>>>> + * The port size of legacy I/O devices is normally less than 0x400. >>>>> + * Defining the I/O range size as 0x400 here should be sufficient for >>>>> + * all peripherals under one bus. >>>>> + */ >>>> >>>> This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? >>>> >>>> We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? >>> >>> The limit here is to define an IO window for LPC. >>> It is not acceptable to describe the IO window with 'ranges' property in LPC host node, so we choose a fixable upper IO limit >>> for the static LPC IO space. Without this, LPC can't register its IO space through extio helpers. >>> >>> Why 0x400 is chosen? For our LPC, 0x400 cover all the peripherals under the LPC. >> >> This is information that should come from firmware then. Whether by coincidence addresses 0x0 - 0x400 include all devices is an implementation detail that the driver shouldn't have to know about. > > >> >> That said, as mentioned elsewhere, we don't need to define single windows at all. Instead, can't we just do it like PCI BARs and declare all PIO regions we get from firmware as readily allocated? Then we only need to explicitly assign ports 0x80, 0x81, 0x99, etc to the LPC bridge and leave the rest for grabs (by PCI hotplug for example) >> > > I worry I don't completely catch your idea. > From my understanding, each PCI host can parse its resource windows from firmware(such as ACPI, dts). But for our LPC, firmware configurations only tell kernel what is the IO resource of the peripherals under LPC. But to register the io range in the similar way of pci_register_io_range(), at least, we need to know the IO range size. > So, how to know this? > There is no similar BAR design in LPC, do we traverse all the peripherals to calculate that? Yes, that's probably the most flexible approach. In fact, I would just traverse all peripherals (or have peripherals call register functions for PIO regions) and then dynamically allocate the PIO space for only those regions. I don't see why you should only have 1 window for your LPC bridge. Alternatively you could just have firmware indicate the region size, similar to how it's done for PCI. Whatever you do, don't hard code it in the driver :). > > >>> >>> >>> >>> >>>> >>>>> +#define LPC_BUS_IO_SIZE 0x400 >>>>> + >>>>> +/* The maximum continuous operations */ >>>>> +#define LPC_MAX_OPCNT 16 >>>>> +/* only support IO data unit length is four at maximum */ >>>>> +#define LPC_MAX_DULEN 4 > snip >>>>> + */ >>>>> +static int >>>>> +hisilpc_target_in(struct hisilpc_dev *lpcdev, struct lpc_cycle_para *para, >>>>> + unsigned long ptaddr, unsigned char *buf, >>>>> + unsigned long opcnt) >>>>> +{ >>>>> + unsigned long cnt_per_trans; >>>>> + unsigned int cmd_word; >>>>> + unsigned int waitcnt; >>>>> + int ret; >>>>> + >>>>> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >>>>> + return -EINVAL; >>>>> + >>>>> + if (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; >>>>> + waitcnt = LPC_MAX_WAITCNT; >>>>> + } >>>>> + >>>>> + ret = 0; >>>>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>>>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>>>> + unsigned long flags; >>>>> + >>>>> + /* whole operation must be atomic */ >>>>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >>>> >>>> Ouch. This is going to kill your RT jitter. Is there no better way? >>> >>> I think there is no other choices. We have to ensure the I/O cycles triggered in serial mode.... >> >> As nobody else accesses the LPC device in the meantime thanks to the lock, you don't need to disable interrupts, right? > > O. Do you mean that we can replace spin_lock_irqsave with spin_lock? That would be a good step into the right direction I think, yes. > As we can connect UART to LPC, I worry the LPC can be accessed in the interrupt context. In that case IRQs are already disabled for the non-preempt-rt case and in preempt-rt, interrupt context should mostly live in threads which are preemptible again. Alex
Hi, Alex, On 2017/2/15 19:53, Alexander Graf wrote: > > > On 15/02/2017 12:35, zhichang.yuan wrote: >> Hi, Alex, >> >> >> On 2017/2/14 21:29, Alexander Graf wrote: >>> >>> >>> On 13/02/2017 15:39, zhichang.yuan wrote: >>>> Hi, Alex, >>>> >>>> Thanks for your review! >>>> >>>> John had replied most of your comments, I only mentioned those which haven't made clear. >>>> >>>> >>>> On 2017/1/31 4:08, Alexander Graf wrote: >>>>> >>>>> >>>>> On 24/01/2017 08:05, zhichang.yuan wrote: >>>>>> The low-pin-count(LPC) interface of Hip06/Hip07 accesses the peripherals in >>>>>> I/O port addresses. This patch implements the LPC host controller driver which >>>>>> perform the I/O operations on the underlying hardware. >>>>>> We don't want to touch those existing peripherals' driver, such as ipmi-bt. So >>>>>> this driver applies the indirect-IO introduced in the previous patch after >>>>>> registering an indirect-IO node to the indirect-IO devices list which will be >>>>>> searched in the I/O accessors. >>>>>> As the I/O translations for LPC children depend on the host I/O registration, >>>>>> we should ensure the host I/O registration is finished before all the LPC >>>>>> children scanning. That is why an arch_init() hook was added in this patch. >>>>>> >>>>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> >>>>>> --- >> snip >>>>>> + >>>>>> +struct lpc_cycle_para { >>>>>> + unsigned int opflags; >>>>>> + unsigned int csize; /* the data length of each operation */ >>>>>> +}; >>>>>> + >>>>>> +struct hisilpc_dev { >>>>>> + spinlock_t cycle_lock; >>>>>> + void __iomem *membase; >>>>>> + struct extio_node *extio; >>>>>> +}; >>>>>> + >>>>>> +/* bounds of the LPC bus address range */ >>>>>> +#define LPC_MIN_BUS_RANGE 0x0 >>>>>> + >>>>>> +/* >>>>>> + * The maximal IO size for each leagcy bus. >>>>> >>>>> legacy? >>>>> >>>>> I don't really understand why this bus is legacy though. It looks like a simple MMIO-to-LPC bridge to me. >>>> >>>> yes. The LPC bus is MMIO-to-LPC bridge. >>>> My comment is not clear. >>>> >>>> How about "The maximal IO size of LPC bus"? >>> >>> That sounds better, but doesn't really tell me what it's about yet ;). >>> >>>> >>>>> >>>>>> + * The port size of legacy I/O devices is normally less than 0x400. >>>>>> + * Defining the I/O range size as 0x400 here should be sufficient for >>>>>> + * all peripherals under one bus. >>>>>> + */ >>>>> >>>>> This comment doesn't make a lot of sense. What is the limit? Is there a hardware limit? >>>>> >>>>> We don't dynamically allocate devices on the lpc bus, so why imply a limit at all? >>>> >>>> The limit here is to define an IO window for LPC. >>>> It is not acceptable to describe the IO window with 'ranges' property in LPC host node, so we choose a fixable upper IO limit >>>> for the static LPC IO space. Without this, LPC can't register its IO space through extio helpers. >>>> >>>> Why 0x400 is chosen? For our LPC, 0x400 cover all the peripherals under the LPC. >>> >>> This is information that should come from firmware then. Whether by coincidence addresses 0x0 - 0x400 include all devices is an implementation detail that the driver shouldn't have to know about. >> >> >>> >>> That said, as mentioned elsewhere, we don't need to define single windows at all. Instead, can't we just do it like PCI BARs and declare all PIO regions we get from firmware as readily allocated? Then we only need to explicitly assign ports 0x80, 0x81, 0x99, etc to the LPC bridge and leave the rest for grabs (by PCI hotplug for example) >>> >> >> I worry I don't completely catch your idea. >> From my understanding, each PCI host can parse its resource windows from firmware(such as ACPI, dts). But for our LPC, firmware configurations only tell kernel what is the IO resource of the peripherals under LPC. But to register the io range in the similar way of pci_register_io_range(), at least, we need to know the IO range size. >> So, how to know this? >> There is no similar BAR design in LPC, do we traverse all the peripherals to calculate that? > > Yes, that's probably the most flexible approach. In fact, I would just traverse all peripherals (or have peripherals call register functions for PIO regions) and then dynamically allocate the PIO space for only those regions. I don't see why you should only have 1 window for your LPC bridge. > > Alternatively you could just have firmware indicate the region size, similar to how it's done for PCI. Whatever you do, don't hard code it in the driver :). > >> >> >>>> >>>> >>>> >>>> >>>>> >>>>>> +#define LPC_BUS_IO_SIZE 0x400 >>>>>> + >>>>>> +/* The maximum continuous operations */ >>>>>> +#define LPC_MAX_OPCNT 16 >>>>>> +/* only support IO data unit length is four at maximum */ >>>>>> +#define LPC_MAX_DULEN 4 >> snip >>>>>> + */ >>>>>> +static int >>>>>> +hisilpc_target_in(struct hisilpc_dev *lpcdev, struct lpc_cycle_para *para, >>>>>> + unsigned long ptaddr, unsigned char *buf, >>>>>> + unsigned long opcnt) >>>>>> +{ >>>>>> + unsigned long cnt_per_trans; >>>>>> + unsigned int cmd_word; >>>>>> + unsigned int waitcnt; >>>>>> + int ret; >>>>>> + >>>>>> + if (!buf || !opcnt || !para || !para->csize || !lpcdev) >>>>>> + return -EINVAL; >>>>>> + >>>>>> + if (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; >>>>>> + waitcnt = LPC_MAX_WAITCNT; >>>>>> + } >>>>>> + >>>>>> + ret = 0; >>>>>> + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; >>>>>> + for (; opcnt && !ret; cnt_per_trans = para->csize) { >>>>>> + unsigned long flags; >>>>>> + >>>>>> + /* whole operation must be atomic */ >>>>>> + spin_lock_irqsave(&lpcdev->cycle_lock, flags); >>>>> >>>>> Ouch. This is going to kill your RT jitter. Is there no better way? >>>> >>>> I think there is no other choices. We have to ensure the I/O cycles triggered in serial mode.... >>> >>> As nobody else accesses the LPC device in the meantime thanks to the lock, you don't need to disable interrupts, right? >> >> O. Do you mean that we can replace spin_lock_irqsave with spin_lock? > > That would be a good step into the right direction I think, yes. > >> As we can connect UART to LPC, I worry the LPC can be accessed in the interrupt context. > > In that case IRQs are already disabled for the non-preempt-rt case and in preempt-rt, interrupt context should mostly live in threads which are preemptible again. > When the IRQs are disabled? Do you mean the interrupt disable just before kernel enters the interrupt routine? It seems this processing can not protect a thread which had owned the LPC spin lock from being preempted. Zhichang > > Alex > > . >
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..213181f --- /dev/null +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt @@ -0,0 +1,33 @@ +Hisilicon Hip06 low-pin-count device + Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which + provides I/O access to some legacy ISA devices. + Hip06 is based on arm64 architecture where there is no I/O space. So, the + I/O ports here are not cpu addresses, and there is no 'ranges' property in + LPC device node. + +Required properties: +- compatible: value should be as follows: + (a) "hisilicon,hip06-lpc" + (b) "hisilicon,hip07-lpc" +- #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 memory range where the LPC register set is mapped. + +Note: + The node name before '@' must be "isa" to represent the binding stick to the + ISA/EISA binding specification. + +Example: + +isa@a01b0000 { + compatible = "hisilicon,hip06-lpc"; + #address-cells = <2>; + #size-cells = <1>; + reg = <0x0 0xa01b0000 0x0 0x1000>; + + ipmi0: bt@e4 { + compatible = "ipmi-bt"; + device_type = "ipmi"; + reg = <0x01 0xe4 0x04>; + }; +}; diff --git a/MAINTAINERS b/MAINTAINERS index 26edd83..0153707 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5855,6 +5855,15 @@ F: include/uapi/linux/if_hippi.h F: net/802/hippi.c F: drivers/net/hippi/ +HISILICON LPC BUS DRIVER +M: Zhichang Yuan <yuanzhichang@hisilicon.com> +L: linux-arm-kernel@lists.infradead.org +W: http://www.hisilicon.com +S: Maintained +F: drivers/bus/hisi_lpc.c +F: lib/extio.c +F: Documentation/devicetree/bindings/arm/hisilicon/hisilicon-low-pin-count.txt + HISILICON NETWORK SUBSYSTEM DRIVER M: Yisen Zhuang <yisen.zhuang@huawei.com> M: Salil Mehta <salil.mehta@huawei.com> diff --git a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts index 7c4114a..75b2b5c 100644 --- a/arch/arm64/boot/dts/hisilicon/hip06-d03.dts +++ b/arch/arm64/boot/dts/hisilicon/hip06-d03.dts @@ -52,3 +52,7 @@ &usb_ehci { status = "ok"; }; + +&ipmi0 { + status = "ok"; +}; diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi index a049b64..c450f8d 100644 --- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi +++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi @@ -318,6 +318,20 @@ #size-cells = <2>; ranges; + isa@a01b0000 { + compatible = "hisilicon,hip06-lpc"; + #size-cells = <1>; + #address-cells = <2>; + reg = <0x0 0xa01b0000 0x0 0x1000>; + + ipmi0: bt@e4 { + compatible = "ipmi-bt"; + device_type = "ipmi"; + reg = <0x01 0xe4 0x04>; + status = "disabled"; + }; + }; + refclk: refclk { compatible = "fixed-clock"; clock-frequency = <50000000>; diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig index b9e8cfc..58cee84 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 (ARM64 && ARCH_HISI && PCI) || COMPILE_TEST + select 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 cc6364b..28e3862 100644 --- a/drivers/bus/Makefile +++ b/drivers/bus/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o obj-$(CONFIG_ARM_CCN) += arm-ccn.o obj-$(CONFIG_BRCMSTB_GISB_ARB) += brcmstb_gisb.o +obj-$(CONFIG_HISILICON_LPC) += hisi_lpc.o obj-$(CONFIG_IMX_WEIM) += imx-weim.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..a96e384 --- /dev/null +++ b/drivers/bus/hisi_lpc.c @@ -0,0 +1,599 @@ +/* + * Copyright (C) 2016 Hisilicon Limited, All Rights Reserved. + * Author: Zhichang Yuan <yuanzhichang@hisilicon.com> + * Author: Zou Rongrong <zourongrong@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/pci.h> +#include <linux/serial_8250.h> +#include <linux/slab.h> + +/* + * Setting this bit means each IO operation will target to a + * different port address: + * 0 means repeatedly IO operations will stick on the same port, + * such as BT; + */ +#define FG_INCRADDR_LPC 0x02 + +struct lpc_cycle_para { + unsigned int opflags; + unsigned int csize; /* the data length of each operation */ +}; + +struct hisilpc_dev { + spinlock_t cycle_lock; + void __iomem *membase; + struct extio_node *extio; +}; + +/* bounds of the LPC bus address range */ +#define LPC_MIN_BUS_RANGE 0x0 + +/* + * The maximal IO size for each leagcy bus. + * The port size of legacy I/O devices is normally less than 0x400. + * Defining the I/O range size as 0x400 here should be sufficient for + * all peripherals under one bus. + */ +#define LPC_BUS_IO_SIZE 0x400 + +/* The maximum continuous operations */ +#define LPC_MAX_OPCNT 16 +/* only support IO data unit length is four at maximum */ +#define LPC_MAX_DULEN 4 +#if LPC_MAX_DULEN > LPC_MAX_OPCNT +#error "LPC.. MAX_DULEN must be not bigger than MAX_OPCNT!" +#endif + +#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 0x08 +#define LPC_CMD_TYPE_IO 0x00 +#define LPC_CMD_WRITE 0x01 +#define LPC_CMD_READ 0x00 +/* the bit attribute is W1C. 1 represents OK. */ +#define LPC_STAT_BYIRQ 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 specific for single IO operation, such as inb. */ +#define LPC_PEROP_WAITCNT 100 + + +static inline int wait_lpc_idle(unsigned char *mbase, + unsigned int waitcnt) { + u32 opstatus; + + 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 peripheral. + * @pdev: pointer to hisi lpc device + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, + unsigned long ptaddr, unsigned char *buf, + unsigned long opcnt) +{ + unsigned long cnt_per_trans; + unsigned int cmd_word; + unsigned int waitcnt; + int ret; + + if (!buf || !opcnt || !para || !para->csize || !lpcdev) + return -EINVAL; + + if (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; + waitcnt = LPC_MAX_WAITCNT; + } + + ret = 0; + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; + for (; opcnt && !ret; cnt_per_trans = para->csize) { + unsigned long flags; + + /* whole operation must be atomic */ + spin_lock_irqsave(&lpcdev->cycle_lock, flags); + + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); + + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); + + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); + + writel(START_WORK, lpcdev->membase + LPC_REG_START); + + /* whether the operation is finished */ + ret = wait_lpc_idle(lpcdev->membase, waitcnt); + if (!ret) { + opcnt -= cnt_per_trans; + for (; cnt_per_trans--; buf++) + *buf = readl(lpcdev->membase + LPC_REG_RDATA); + } + + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); + } + + return ret; +} + +/** + * hisilpc_target_out - trigger a series of lpc cycles to write required + * data to target peripheral. + * @pdev: pointer to hisi lpc device + * @para: some parameters 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 *lpcdev, struct lpc_cycle_para *para, + unsigned long ptaddr, const unsigned char *buf, + unsigned long opcnt) +{ + unsigned long cnt_per_trans; + unsigned int cmd_word; + unsigned int waitcnt; + int ret; + + if (!buf || !opcnt || !para || !lpcdev) + return -EINVAL; + + if (opcnt > LPC_MAX_OPCNT) + return -EINVAL; + /* default is increasing address */ + cmd_word = LPC_CMD_TYPE_IO | LPC_CMD_WRITE; + waitcnt = LPC_PEROP_WAITCNT; + if (!(para->opflags & FG_INCRADDR_LPC)) { + cmd_word |= LPC_CMD_SAMEADDR; + waitcnt = LPC_MAX_WAITCNT; + } + + ret = 0; + cnt_per_trans = (para->csize == 1) ? opcnt : para->csize; + for (; opcnt && !ret; cnt_per_trans = para->csize) { + unsigned long flags; + + spin_lock_irqsave(&lpcdev->cycle_lock, flags); + + writel(cnt_per_trans, lpcdev->membase + LPC_REG_OP_LEN); + opcnt -= cnt_per_trans; + for (; cnt_per_trans--; buf++) + writel(*buf, lpcdev->membase + LPC_REG_WDATA); + + writel(cmd_word, lpcdev->membase + LPC_REG_CMD); + + writel(ptaddr, lpcdev->membase + LPC_REG_ADDR); + + writel(START_WORK, lpcdev->membase + LPC_REG_START); + + /* whether the operation is finished */ + ret = wait_lpc_idle(lpcdev->membase, waitcnt); + + spin_unlock_irqrestore(&lpcdev->cycle_lock, flags); + } + + return ret; +} + +static inline unsigned long +hisi_lpc_pio_to_addr(struct hisilpc_dev *lpcdev, unsigned long pio) +{ + return pio - lpcdev->extio->io_start + lpcdev->extio->bus_start; +} + + +/** + * hisilpc_comm_in - read/input the data from the I/O peripheral + * through LPC. + * @devobj: pointer to the device information relevant to LPC controller. + * @pio: the target I/O port address. + * @dlen: the data length required to read from the target I/O port. + * + * 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. + */ +static u64 hisilpc_comm_in(void *devobj, unsigned long pio, size_t dlen) +{ + struct hisilpc_dev *lpcdev = devobj; + struct lpc_cycle_para iopara; + u32 rd_data; + unsigned char *newbuf; + int ret = 0; + unsigned long ptaddr; + + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN || (dlen & (dlen - 1))) + return -1; + + /* the local buffer must be enough for one data unit */ + if (sizeof(rd_data) < dlen) + return -1; + + newbuf = (unsigned char *)&rd_data; + + ptaddr = hisi_lpc_pio_to_addr(lpcdev, pio); + + iopara.opflags = FG_INCRADDR_LPC; + iopara.csize = dlen; + + ret = hisilpc_target_in(lpcdev, &iopara, ptaddr, newbuf, dlen); + if (ret) + return -1; + + return le32_to_cpu(rd_data); +} + +/** + * hisilpc_comm_out - output the data whose maximum length is four bytes + to the I/O peripheral through the LPC host. + * @devobj: pointer to the device information relevant to LPC controller. + * @outval: a value to be outputted from caller, maximum is four bytes. + * @pio: the target I/O port address. + * @dlen: the data length required writing to the target I/O port. + * + * This function is corresponding to out(b,w,l) only + * + */ +static void hisilpc_comm_out(void *devobj, unsigned long pio, + u32 outval, size_t dlen) +{ + struct hisilpc_dev *lpcdev = devobj; + struct lpc_cycle_para iopara; + const unsigned char *newbuf; + unsigned long ptaddr; + + if (!lpcdev || !dlen || dlen > LPC_MAX_DULEN) + return; + + if (sizeof(outval) < dlen) + return; + + outval = cpu_to_le32(outval); + + newbuf = (const unsigned char *)&outval; + + ptaddr = hisi_lpc_pio_to_addr(lpcdev, pio); + + iopara.opflags = FG_INCRADDR_LPC; + iopara.csize = dlen; + + hisilpc_target_out(lpcdev, &iopara, ptaddr, newbuf, dlen); +} + +/* + * hisilpc_comm_ins - read/input the data in buffer to the I/O + * peripheral through LPC, it corresponds to ins(b,w,l) + * @devobj: pointer to the device information relevant to LPC controller. + * @pio: the target I/O port address. + * @inbuf: a buffer where read/input data bytes are stored. + * @dlen: the data length required writing to the target I/O port. + * @count: how many data units whose length is dlen will be read. + * + */ +static u64 +hisilpc_comm_ins(void *devobj, unsigned long pio, void *inbuf, + size_t dlen, unsigned int count) +{ + struct hisilpc_dev *lpcdev = devobj; + struct lpc_cycle_para iopara; + unsigned char *newbuf; + unsigned int loopcnt, cntleft; + unsigned int max_perburst; + unsigned long ptaddr; + + if (!lpcdev || !inbuf || !count || !dlen || + dlen > LPC_MAX_DULEN || (dlen & (dlen - 1))) + return -1; + + iopara.opflags = 0; + if (dlen > 1) + iopara.opflags |= FG_INCRADDR_LPC; + iopara.csize = dlen; + + ptaddr = hisi_lpc_pio_to_addr(lpcdev, pio); + newbuf = (unsigned char *)inbuf; + /* + * ensure data stream whose length is multiple of dlen to be processed + * each IO input + */ + max_perburst = LPC_MAX_OPCNT & (~(dlen - 1)); + cntleft = count * dlen; + do { + int ret; + + loopcnt = (cntleft >= max_perburst) ? max_perburst : cntleft; + ret = hisilpc_target_in(lpcdev, &iopara, ptaddr, + newbuf, loopcnt); + if (ret) + return ret; + newbuf += loopcnt; + cntleft -= loopcnt; + } while (cntleft); + + return 0; +} + +/* + * hisilpc_comm_outs - write/output the data in buffer to the I/O + * peripheral through LPC, it corresponds to outs(b,w,l) + * @devobj: pointer to the device information relevant to LPC controller. + * @pio: the target I/O port address. + * @outbuf: a buffer where write/output data bytes are stored. + * @dlen: the data length required writing to the target I/O port . + * @count: how many data units whose length is dlen will be written. + * + */ +static void +hisilpc_comm_outs(void *devobj, unsigned long pio, const void *outbuf, + size_t dlen, unsigned int count) +{ + struct hisilpc_dev *lpcdev = devobj; + struct lpc_cycle_para iopara; + const unsigned char *newbuf; + unsigned int loopcnt, cntleft; + unsigned int max_perburst; + unsigned long ptaddr; + + if (!lpcdev || !outbuf || !count || !dlen || + dlen > LPC_MAX_DULEN || (dlen & (dlen - 1))) + return; + + iopara.opflags = 0; + if (dlen > 1) + iopara.opflags |= FG_INCRADDR_LPC; + iopara.csize = dlen; + + ptaddr = hisi_lpc_pio_to_addr(lpcdev, pio); + newbuf = (unsigned char *)outbuf; + /* + * ensure data stream whose length is multiple of dlen to be processed + * each IO input + */ + max_perburst = LPC_MAX_OPCNT & (~(dlen - 1)); + cntleft = count * dlen; + do { + loopcnt = (cntleft >= max_perburst) ? max_perburst : cntleft; + if (hisilpc_target_out(lpcdev, &iopara, ptaddr, newbuf, + loopcnt)) + break; + newbuf += loopcnt; + cntleft -= loopcnt; + } while (cntleft); +} + +static struct extio_ops hisi_lpc_ops = { + .pfin = hisilpc_comm_in, + .pfout = hisilpc_comm_out, + .pfins = hisilpc_comm_ins, + .pfouts = hisilpc_comm_outs, +}; + +/** + * hisilpc_probe - the probe callback function for hisi lpc device, + * will finish all the initialization. + * @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 device *dev = &pdev->dev; + struct resource *res; + struct hisilpc_dev *lpcdev; + int ret = 0; + + dev_info(dev, "probing...\n"); + + lpcdev = devm_kzalloc(dev, sizeof(struct hisilpc_dev), GFP_KERNEL); + if (!lpcdev) + return -ENOMEM; + + spin_lock_init(&lpcdev->cycle_lock); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(dev, "no MEM resource\n"); + return -ENODEV; + } + + lpcdev->membase = devm_ioremap_resource(dev, res); + if (IS_ERR(lpcdev->membase)) { + dev_err(dev, "remap failed\n"); + return PTR_ERR(lpcdev->membase); + } + + /* get the linux virtual IO node registered before. */ + lpcdev->extio = extio_find_node(dev->fwnode); + if (!lpcdev->extio) { + dev_err(dev, "No extio node registered!\n"); + return -EFAULT; + } + + lpcdev->extio->devpara = lpcdev; + lpcdev->extio->ops = &hisi_lpc_ops; + + platform_set_drvdata(pdev, lpcdev); + + /* + * The children scanning is only for dts. For ACPI children, the + * corresponding devices had be created during ACPI scanning. + */ + if (!has_acpi_companion(dev)) { + ret = of_platform_populate(dev->of_node, NULL, NULL, dev); + if (ret) + dev_err(dev, "OF: enumerate LPC bus fail(%d)\n", ret); + } + + if (!ret) + dev_info(dev, "hslpc end probing. range[0x%lx - %lx]\n", + lpcdev->extio->io_start, + lpcdev->extio->io_start + lpcdev->extio->range_size); + else + dev_info(dev, "hslpc probing is fail(%d)\n", ret); + + return ret; +} + +static const struct of_device_id hisilpc_of_match[] = { + { .compatible = "hisilicon,hip06-lpc", }, + { .compatible = "hisilicon,hip07-lpc", }, + {}, +}; + +static const struct acpi_device_id hisilpc_acpi_match[] = { + {"HISI0191", }, + {}, +}; + +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, +}; + + +builtin_platform_driver(hisilpc_driver); + +/* + * hisilpc_bus_platform_notify -- notify callback function specific for + * hisi-lpc bus. Here, will register linux virtual + * PIO for the bus detected, then the bus children + * can translate their bus-local IO to linux PIO. + */ +static int hisilpc_bus_platform_notify(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct extio_node *io_node; + struct device *dev = data; + int ret; + + if (!is_of_node(dev->fwnode) && !is_acpi_node(dev->fwnode)) + return NOTIFY_DONE; + + if (action != BUS_NOTIFY_ADD_DEVICE) + return NOTIFY_DONE; + + /* whether the device notified is hisi-lpc? */ + if (has_acpi_companion(dev)) { + if (!acpi_match_device(hisilpc_acpi_match, dev)) + return NOTIFY_DONE; + } else { + if (!of_match_node(hisilpc_of_match, dev->of_node)) + return NOTIFY_DONE; + } + + /* + * indirectIO bus was detected, time to request the linux virtual + * IO. + */ + io_node = kzalloc(sizeof(*io_node), GFP_KERNEL); + if (!io_node) + return NOTIFY_DONE; + + io_node->bus_start = LPC_MIN_BUS_RANGE; + io_node->range_size = LPC_BUS_IO_SIZE; + + ret = pci_register_io_range(dev->fwnode, IO_RANGE_IOEXT, + io_node->range_size, &io_node->io_start); + if (ret) { + dev_err(dev, "register indirectIO range FAIL!\n"); + kfree(io_node); + return NOTIFY_DONE; + } + io_node->fwnode = dev->fwnode; + /* register the linux virtual IO range node to list. */ + register_extio(io_node); + + return NOTIFY_OK; +} + +static struct notifier_block hisilpc_preinit_nb = { + .notifier_call = hisilpc_bus_platform_notify, +}; + +static int __init hisilpc_init(void) +{ + return bus_register_notifier(&platform_bus_type, &hisilpc_preinit_nb); +} + +/* This initial funtion must be called before the platform bus scanning. */ +arch_initcall(hisilpc_init);