Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06

From: Arnd Bergmann
Date: Fri Sep 23 2016 - 11:56:10 EST


On Friday, September 23, 2016 2:59:55 PM CEST Gabriele Paoloni wrote:
>
> > > From the perspective of the indirect IO function the input parameter
> > > is an unsigned long addr that (now) can be either:
> > > 1) an IO token coming from a legacy pci device
> > > 2) a phys address that lives on the LPC bus
> > >
> > > These are conceptually two separate address spaces (and actually they
> > > both start from 0).
> >
> > Why? Any IORESOURCE_IO address always refers to the logical I/O port
> > range in Linux, not the physical address that is used on a bus.
>
> If I read the code correctly when you get an I/O token you just add it
> to PCI_IOBASE.
> This is enough since pci_remap_iospace set the virtual address to
> PCI_IOBASE + the I/O token offset; so we can read/write to
> vaddr = PCI_IOBASE + token as pci_remap_iospace has mapped it correctly
> to the respective PCI cpu address (that is set in the I/O range property
> of the host controller)
>
> In the patchset accessors LPC operates directly on the cpu addresses
> and the input parameter of the accessors can be either an IO token or
> a cpu address
>
> +static inline void outb(u8 value, unsigned long addr)
> +{
> +#ifdef CONFIG_ARM64_INDIRECT_PIO
> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
> + addr <= arm64_extio_ops->end)
>
> Here below we operate on cpu address
>
> + extio_outb(value, addr);
> + else
> +#endif

I missed this bug earlier, this obviously needs to be

arm64_extio_ops->outb(value, addr - arm64_extio_ops->start);

or possibly

arm64_extio_ops->outb(arm64_extio_ops, value, addr);

as the outb function won't know what the offset is, but
that needed to be fixed regardless.

Arnd