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

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

[Konrad Rzeszutek Wilk]

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.
>
> >  #else 
> > -        uart->remapped_io_base = (char *)ioremap(uart->io_base, uart->io_size); 
> > +        uart->remapped_io_base = (char *)ioremap(uart->io_base+ uart->offset, 
>
> Missing space.

Fixed.
>
> > +                                                 uart->io_size); 
> >  #endif 
> >      } 
> >  
>

> Jan 
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Jan Beulich]

>>> 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 +=.

Jan

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

[Konrad Rzeszutek Wilk]

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
> 

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

[Jan Beulich]

>>> On 04.03.14 at 14:29, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 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.

I don't think that makes sense. Just properly add uart->offset as
indicated.

Jan

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

[Konrad Rzeszutek Wilk]

Because they are PCIe and machine nowadys have those instead of
PCI, and they are inexpensive.

Tested with 1415:c138. Should also work on 0xc11f, 0xc11b models
of that chip.

Also on  OXPCIe200 1 Native UART 1415: 0xc40b, 0xc40f, 0xc41b,
0xc41f, 0xc42b, 0xc42f, 0xc43b, 0xc43f, 0xc44b, 0xc44f, 0xc45b
0xc45f, 0xc46b, 0xc46f, 0xc47b, 0xc47f, 0xc48b, 0xc48f, 0xc49b
0xc49f, 0xc4ab, 0xc4af, 0xc4bb, 0xc4bf, 0xc4cb, 0xc4cf

but since I don't have any of those cards this patch does not
enable it.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/drivers/char/ns16550.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 5ea3f13..69beec9 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -51,6 +51,7 @@ static struct ns16550 {
     int reg_width; /* Size of access to use, the registers
                     * themselves are still bytes */
     char __iomem *remapped_io_base;  /* Remapped virtual address of MMIO. */
+    unsigned int offset; /* By default zero. */
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
     u8 lsr_mask;
@@ -89,6 +90,9 @@ struct ns16550_config_mmio {
     unsigned int fifo_size;
     u8 lsr_mask;
     unsigned int max_bars;
+    unsigned int base_baud;
+    unsigned int uart_offset;
+    unsigned int first_offset;
 };
 
 
@@ -111,6 +115,19 @@ static struct ns16550_config_mmio __initdata uart_config[] =
         .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT),
         .max_bars = 1,
     },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = 0x1415,
+        .dev_id = 0xc138,
+        .base_baud = 4000000,
+        .uart_offset = 0x200,
+        .first_offset = 0x1000,
+        .reg_width = 1,
+        .reg_shift = 0,
+        .fifo_size = 16,
+        .lsr_mask = UART_LSR_THRE,
+        .max_bars = 1, /* It can do more, but we would need more custom code.*/
+    }
 };
 #endif
 
@@ -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;
 #else
-        uart->remapped_io_base = (char *)ioremap(uart->io_base, uart->io_size);
+        uart->remapped_io_base = (char *)ioremap(uart->io_base+ uart->offset,
+                                                 uart->io_size);
 #endif
     }
 
@@ -738,6 +756,10 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
                         uart->lsr_mask = uart_config[i].lsr_mask;
                         uart->io_base = ((u64)bar_64 << 32) |
                                         (bar & PCI_BASE_ADDRESS_MEM_MASK);
+                        uart->offset = uart_config[i].first_offset;
+                        uart->offset += bar_idx * uart_config[i].uart_offset;
+                        if ( uart_config[i].base_baud )
+                            uart->clock_hz = uart_config[i].base_baud * 16;
                         /* Set device and MMIO region read only to Dom0 */
                         uart->enable_ro = 1;
                         break;
@@ -917,6 +939,7 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
     uart->io_size   = 8;
     uart->reg_width = 1;
     uart->reg_shift = 0;
+    uart->offset    = 0;
 
 #ifdef HAS_PCI
     uart->enable_ro = 0;
-- 
1.7.7.6

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

[Julien Grall]

Hello Konrad,

On 04/03/14 10:01, Konrad Rzeszutek Wilk wrote:
> @@ -917,6 +939,7 @@ void __init ns16550_init(int index, struct ns16550_defaults *defaults)
>       uart->io_size   = 8;
>       uart->reg_width = 1;
>       uart->reg_shift = 0;
> +    uart->offset    = 0;

You also need to set uart->offset = 0 in ns16550_dt_init which is used 
by ARM.

Regards,

-- 
Julien Grall

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

[Jan Beulich]

>>> 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?

>  #else
> -        uart->remapped_io_base = (char *)ioremap(uart->io_base, uart->io_size);
> +        uart->remapped_io_base = (char *)ioremap(uart->io_base+ uart->offset,

Missing space.

> +                                                 uart->io_size);
>  #endif
>      }
>  

Jan