| Message ID | 1473255233-154297-5-git-send-email-yuanzhichang@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote: > From: "zhichang.yuan" <yuanzhichang@hisilicon.com> > > This patch support the earlycon for UART connected to LPC on Hip06. > This patch is depended on the LPC driver. > > Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> > I'm skeptical about this too. Is this just needed because the 8250 earlycon support comes before the lpc bus initialization? Could we start the LPC driver earlier to work around that? Arnd
Hi zhichang.yuan,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc5 next-20160908]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Zhichang-Yuan/ARM64-LPC-legacy-ISA-I-O-support/20160907-212837
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `early_hisilpc8250_setup':
>> binder.c:(.init.text+0x479c): undefined reference to `early_serial8250_setup'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi, Arnd, On 2016年09月07日 22:52, Arnd Bergmann wrote: > On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote: >> From: "zhichang.yuan" <yuanzhichang@hisilicon.com> >> >> This patch support the earlycon for UART connected to LPC on Hip06. >> This patch is depended on the LPC driver. >> >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >> > > I'm skeptical about this too. Is this just needed because the 8250 > earlycon support comes before the lpc bus initialization? I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart. 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate. Hip06 LPC uart need two base addresses for earlycon. 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart IO type is UPIO_PORT. This is spcial... 3. Just as your guess, earlycon should be earlier than lpc initialization. Best, Zhichang > > Could we start the LPC driver earlier to work around that? > > Arnd >
On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote: > Hi, Arnd, > > On 2016年09月07日 22:52, Arnd Bergmann wrote: > > On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote: > >> From: "zhichang.yuan" <yuanzhichang@hisilicon.com> > >> > >> This patch support the earlycon for UART connected to LPC on Hip06. > >> This patch is depended on the LPC driver. > >> > >> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> > >> > > > > I'm skeptical about this too. Is this just needed because the 8250 > > earlycon support comes before the lpc bus initialization? > I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart. > > 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that > "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate. We should never need to specify the addresses manually like this, it's actually supposed to work if you just list "earlycon" here. The first membase is apparently only used during setup: + writel(LPC_IRQ_CLEAR, device->port.membase + LPC_REG_IRQ_ST); + /* ensure the LPC is available */ + while (!(readl(device->port.membase + LPC_REG_OP_STATUS) & + LPC_STATUS_IDLE)) Why doesn't the firmware do this before handing off control of the kernel to the console? > Hip06 LPC uart need two base addresses for earlycon. > 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart > IO type is UPIO_PORT. This is spcial... This sounds like a deficiency in the of_setup_earlycon() function, which can only handle MMIO addresses, and won't actually be able to understand nodes without a "ranges" property like you have here. I think we need to add a special case for port ranges here. Arnd
On 2016年09月08日 19:04, Arnd Bergmann wrote: > On Thursday, September 8, 2016 6:04:31 PM CEST zhichang wrote: >> Hi, Arnd, >> >> On 2016年09月07日 22:52, Arnd Bergmann wrote: >>> On Wednesday, September 7, 2016 9:33:53 PM CEST Zhichang Yuan wrote: >>>> From: "zhichang.yuan" <yuanzhichang@hisilicon.com> >>>> >>>> This patch support the earlycon for UART connected to LPC on Hip06. >>>> This patch is depended on the LPC driver. >>>> >>>> Signed-off-by: zhichang.yuan <yuanzhichang@hisilicon.com> >>>> >>> >>> I'm skeptical about this too. Is this just needed because the 8250 >>> earlycon support comes before the lpc bus initialization? >> I think you wonder why early_serial8250_setup can not be used direclty for this earlycon of LPC uart. >> >> 1. the earlycon kernel parameter format of LPC uart is different from 8250. something like that >> "earlycon=hisilpcuart,mmio,0xa01b0000,0,0x2f8". You see, there is one more parameter after the baudrate. > > We should never need to specify the addresses manually like this, > it's actually supposed to work if you just list "earlycon" here. Do you mean flat-tree earlycon? Ok, will support this in V3. > > The first membase is apparently only used during setup: > > + writel(LPC_IRQ_CLEAR, device->port.membase + LPC_REG_IRQ_ST); > + /* ensure the LPC is available */ > + while (!(readl(device->port.membase + LPC_REG_OP_STATUS) & > + LPC_STATUS_IDLE)) > > Why doesn't the firmware do this before handing off control of > the kernel to the console? This is a checking on the LPC controller status. I think we can keep this here. > >> Hip06 LPC uart need two base addresses for earlycon. >> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart >> IO type is UPIO_PORT. This is spcial... > > This sounds like a deficiency in the of_setup_earlycon() function, > which can only handle MMIO addresses, and won't actually > be able to understand nodes without a "ranges" property like > you have here. > Yes. The current of_setup_earlycon only support MMIO and the first reg property must be memory. We can not support our LPC uart without any new code. But we can implement a private earlycon setup function and register it to the __earlycon_table, things will be ok. I will do it in V3. Best, Zhichang > I think we need to add a special case for port ranges here. > > Arnd >
On Wednesday, September 14, 2016 7:26:22 PM CEST zhichang wrote: > > > > >> Hip06 LPC uart need two base addresses for earlycon. > >> 2. the IO type is mmio to introduce a memory base address to access LPC register file. But the real uart > >> IO type is UPIO_PORT. This is spcial... > > > > This sounds like a deficiency in the of_setup_earlycon() function, > > which can only handle MMIO addresses, and won't actually > > be able to understand nodes without a "ranges" property like > > you have here. > > > Yes. > The current of_setup_earlycon only support MMIO and the first reg property must be memory. > > We can not support our LPC uart without any new code. > But we can implement a private earlycon setup function and register > it to the __earlycon_table, things will be ok. I still think you should adapt of_setup_earlycon instead to handle IORESOURCE_IO registers. Arnd
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 7ac0551..1cc5581 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -374,6 +374,135 @@ void hisilpc_comm_outb(void *devobj, unsigned long ptaddr, } +static struct extio_ops hisilpc_earlyop __initdata; + +/** + * hisilpc_early_in - read/input operation specific for hisi LPC earlycon. + * @devobj: pointer to device relevant information of the caller. + * @inbuf: the buffer where the read back data is populated. + * @ptaddr: the io address where read operation targets to. + * @dlen: data length in byte to be read each IO operation. + * @count: how many IO operations expected. + * for earlycon, dlen and count should be one. + * + * Returns 0 on success, non-zero on fail. + * + */ +static u64 __init hisilpc_early_in(void *devobj, unsigned long ptaddr, + void *inbuf, size_t dlen,unsigned int count) +{ + unsigned int ret = 0; + struct lpc_cycle_para para; + struct hisilpc_dev lpcdev; + struct uart_port *port; + unsigned int rd_data; + + port = (struct uart_port *)devobj; + if (!port->mapbase || !port->membase || inbuf || + count != 1 || dlen !=1) + return -EINVAL; + + para.opflags = FG_EARLYCON_LPC; + para.csize = dlen; + lpcdev.membase = port->membase; + + ret = hisilpc_target_in(&lpcdev, ¶, ptaddr, + (unsigned char *)&rd_data, count); + + return (ret) ? : rd_data; +} + +/** + * hisilpc_early_out - write/output operation specific for hisi LPC earlycon. + * @devobj: pointer to device relevant information of the caller. + * @outbuf: the buffer where the data to be written is stored. + * @ptaddr: the io address where write operation targets to. + * @dlen: data length in byte to be written each IO operation. + * @count: how many IO operations expected. + * for earlycon, dlen and count must be one. + * + */ +static void __init hisilpc_early_out(void *devobj, unsigned long ptaddr, + const void *outbuf, size_t dlen, + unsigned int count) +{ + struct lpc_cycle_para para; + struct hisilpc_dev lpcdev; + struct uart_port *port; + + port = (struct uart_port *)devobj; + if (!port->mapbase || !port->membase || !outbuf || + dlen != 1 || count != 1) + return; + + para.opflags = FG_EARLYCON_LPC; + para.csize = dlen; + lpcdev.membase = port->membase; + + (void)hisilpc_target_out(&lpcdev, ¶, ptaddr, outbuf, count); +} + + +/** + * early_hisilpc8250_setup - initilize the lpc earlycon + * @device: pointer to the elarycon device + * @options: a option string from earlycon kernel-parameter + * + * Returns 0 on success, non-zero on fail. + * + */ +static int __init early_hisilpc8250_setup(struct earlycon_device *device, + const char *options) +{ + char *p; + int ret; + + if (!device->port.membase) + return -ENODEV; + + if (device->port.iotype != UPIO_MEM) + return -EINVAL; + + if (device->options) { + p = strchr(device->options, ','); + if (p && (p + 1) != '\0') { + ret = kstrtoul(++p, 0, + (unsigned long *)&device->port.iobase); + if (ret || device->port.iobase == 0) + return ret ?: -EFAULT; + } else + device->port.iobase = 0x2f8; + } else { + device->port.iobase = 0x2f8; + device->baud = 0; + } + + /* must set iotype as UPIO_PORT for Hip06 indirect-io */ + device->port.iotype = UPIO_PORT; + + hisilpc_earlyop.pfin = hisilpc_early_in; + hisilpc_earlyop.pfout = hisilpc_early_out; + hisilpc_earlyop.devpara = &device->port; + + + /* disable interrupts from LPC */ + writel(LPC_IRQ_CLEAR, device->port.membase + LPC_REG_IRQ_ST); + /* ensure the LPC is available */ + while (!(readl(device->port.membase + LPC_REG_OP_STATUS) & + LPC_STATUS_IDLE)) + cpu_relax(); + + arm64_set_simops(&hisilpc_earlyop); + + return early_serial8250_setup(device, options); +} + + + +EARLYCON_DECLARE(hisilpcuart, early_hisilpc8250_setup); +OF_EARLYCON_DECLARE(hisilpcuart, "hisi,rawlpc-uart", + early_hisilpc8250_setup); + /** * hisilpc_probe - the probe callback function for hisi lpc device, * will finish all the intialization.