Re: [Xen-devel] [RFC PATCH] xen/arm: Add MVEBU UART driver for Armada 3700 SoC

[Julien Grall <julien.grall@xxxxxxx>]

On 12/03/18 14:36, Amit Tomer wrote:
> Hi
Hi Amit,

> Thanks for the comments.
>
> > OOI, do you have any plan for adding earlyprintk support for that platform?
> I didn't think about it but I would look into it.
This is quite useful to get output without any serial driver. I am quite 
impressed you managed to debug your serial driver without it :).
> > Please give a link to the Linux driver. This would help me for reviewing and
> > also for future reference.
> Ok.
>
> > This is part of xen/drivers/char/* so even if the driver if only for ARM
> > hardware, you likely want to CC "THE REST" maintainers as this is under
> > drivers/char. scripts/get_maintainers.pl can help you to find relevant
> > maintainers to CC on each patch.
> Ok.
>
>   <xen/*> include should be first, then <asm/*>.
>
> Ok, I was under the impression that it should be sorted in alphabetical order.
They should be sorted alphabetical, but all <asm/*> should be after 
<xen/*> so common headers gets included first, then the arch specific ones.
> > > +
> > > +#define TX_FIFO_SIZE            32
> > > +#define RX_FIFO_SIZE            64
> > > +
> > > +static struct mvebu3700_uart {
> > > +    unsigned int baud, data_bits, parity, stop_bits;
> >
> > Are all those fields necessary? For instance, you always set baud but never
> > read it.
> Not sure about this as I don't know if these fields are used by XEN
> serial infrastructure later on.
This is an internal structure. I can't see how the serial code would 
know the layout and access the fields.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel