Re: [PATCH v2 3/3] MIPS: Loongson64: Enable PCI_IO_VMMAP

[John Garry <john.garry@huawei.com>]

On 28/04/2020 03:14, Jiaxun Yang wrote:
> Finally we are able to elegantly add I/O ports for PCI host bridge
> via devicetree with logic_pio.
> 
> To deal with legacy drivers that have fixed I/O ports range we
> reserved 0x10000 in PCI_IOBASE, should be enough for i8259 i8042
> stuff.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   arch/mips/Kconfig                             |  1 +
>   .../include/asm/mach-loongson64/loongson.h    |  2 ++
>   arch/mips/loongson64/init.c                   | 34 ++++++++++++++++---
>   arch/mips/loongson64/pci.c                    |  2 +-
>   4 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 4e5308178649..1669735dacd8 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -479,6 +479,7 @@ config MACH_LOONGSON64
>   	select I8259
>   	select IRQ_MIPS_CPU
>   	select NR_CPUS_DEFAULT_64
> +	select PCI_IO_VMMAP
>   	select USE_GENERIC_EARLY_PRINTK_8250
>   	select SYS_HAS_CPU_LOONGSON64
>   	select SYS_HAS_EARLY_PRINTK
> diff --git a/arch/mips/include/asm/mach-loongson64/loongson.h b/arch/mips/include/asm/mach-loongson64/loongson.h
> index fde1b75c45ea..94035a47be5b 100644
> --- a/arch/mips/include/asm/mach-loongson64/loongson.h
> +++ b/arch/mips/include/asm/mach-loongson64/loongson.h
> @@ -86,6 +86,8 @@ extern int mach_i8259_irq(void);
>   #define LOONGSON_PCIIO_SIZE	0x00100000	/* 1M */
>   #define LOONGSON_PCIIO_TOP	(LOONGSON_PCIIO_BASE+LOONGSON_PCIIO_SIZE-1)
>   
> +#define MMIO_LOWER_RESERVED	0x10000
> +
>   /* Loongson Register Bases */
>   
>   #define LOONGSON_PCICONFIGBASE	0x00
> diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c
> index da38944471f4..4592eb2f78dd 100644
> --- a/arch/mips/loongson64/init.c
> +++ b/arch/mips/loongson64/init.c
> @@ -5,6 +5,7 @@
>    */
>   
>   #include <linux/irqchip.h>
> +#include <linux/logic_pio.h>
>   #include <linux/memblock.h>
>   #include <asm/bootinfo.h>
>   #include <asm/traps.h>
> @@ -29,10 +30,6 @@ void __init prom_init(void)
>   	fw_init_cmdline();
>   	prom_init_env();
>   
> -	/* init base address of io space */
> -	set_io_port_base((unsigned long)
> -		ioremap(LOONGSON_PCIIO_BASE, LOONGSON_PCIIO_SIZE));
> -
>   	prom_init_numa_memory();
>   
>   	/* Hardcode to CPU UART 0 */
> @@ -46,7 +43,36 @@ void __init prom_free_prom_memory(void)
>   {
>   }
>   
> +static __init void reserve_pio_range(void)
> +{
> +	struct logic_pio_hwaddr *range;
> +
> +	range = kzalloc(sizeof(*range), GFP_ATOMIC);
> +	if (!range)
> +		return;
> +
> +	range->fwnode = &of_root->fwnode;
> +	range->size = MMIO_LOWER_RESERVED;
> +	range->hw_start = LOONGSON_PCIIO_BASE;
> +	range->flags = LOGIC_PIO_CPU_MMIO;
> +
> +	if (logic_pio_register_range(range)) {
> +		pr_err("Failed to reserve PIO range for legacy ISA\n");
> +		kfree(range);
> +		return;
> +	}

I think that you should also check the range->io_start == 0 is returned 
here, as your code relies on that.

And I would also like to reiterate that this is somewhat fragile and 
relies on ordering. But I feel better than modifying common code. And I 
agree that the right thing to do is take care of the logic pio 
translations for your drivers now, as you mention, below.

Thanks,
John

> +
> +	/*
> +	 * i8259 would access I/O space, so mapping must be done here.
> +	 * Please remove it when all drivers can be managed by logic_pio.
> +	 */
> +	ioremap_page_range(PCI_IO_START, PCI_IO_START + MMIO_LOWER_RESERVED,
> +				  LOONGSON_PCIIO_BASE,
> +				  pgprot_device(PAGE_KERNEL));
> +}
> +
>   void __init arch_init_irq(void)
>   {
> +	reserve_pio_range();
>   	irqchip_init();
>   }
> diff --git a/arch/mips/loongson64/pci.c b/arch/mips/loongson64/pci.c
> index a440a2725a20..7aecb88dd377 100644
> --- a/arch/mips/loongson64/pci.c
> +++ b/arch/mips/loongson64/pci.c
> @@ -37,7 +37,7 @@ extern int sbx00_acpi_init(void);
>   static int __init pcibios_init(void)
>   {
>   
> -	loongson_pci_controller.io_map_base = mips_io_port_base;
> +	loongson_pci_controller.io_map_base = IOPORT_RW_BASE;
>   	loongson_pci_mem_resource.start = loongson_sysconf.pci_mem_start_addr;
>   	loongson_pci_mem_resource.end = loongson_sysconf.pci_mem_end_addr;
>   
>