Re: [RFC PATCHv1] serial: Support OXPCIe952 aka Oxford Semiconductor Ltd Device c138 (1415:c138)

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Mar 04, 2014 at 10:57:08AM +0000, Jan Beulich wrote:
> >>> On 04.03.14 at 11:36, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > On Mar 4, 2014 4:43 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>
> >> >>> On 04.03.14 at 03:01, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote: 
> >> > @@ -325,11 +342,12 @@ static void __init ns16550_init_preirq(struct 
> > serial_port *port) 
> >> >  #ifdef CONFIG_X86 
> >> >          enum fixed_addresses idx = FIX_COM_BEGIN + (uart - ns16550_com); 
> >> >  
> >> > -        set_fixmap_nocache(idx, uart->io_base); 
> >> > +        set_fixmap_nocache(idx, uart->io_base + uart->offset); 
> >> >          uart->remapped_io_base = (void __iomem *)fix_to_virt(idx); 
> >> >          uart->remapped_io_base += uart->io_base & ~PAGE_MASK; 
> >>
> >> Don't you need to add in uart->offset here too?
> > 
> > The virtual address at that point already covers then BAR + offset.
> 
> Hardly: fix_to_virt() returns a page-aligned address, and anything
> that may affect the offset-in-page value needs to be taken into
> consideration in the right side expression of the +=.

Oh, this is not a offset-in-page (well, it is, but also past the first
page). In this case the offset ends up being BAR + 0x1000 + 0x200 * idx.
It is well past the first page.

Perhaps splitting it up in two offsets would be better?

	set_fixmap_nocache(idx, uart_io_base + uart->pfn_offset * PAGE_SIZE);
	uart->remapped_io_base = (void __iomem)*fix_to_virt(idx);
	uart->remapped_io_base += uart->io_base + uart->offset & ~PAGE_MASK;

And of course we make sure that offset is within 0->4096.

> 
> Jan
>