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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 30 Mar 2018, Amit Tomer wrote:
> Hello,
> 
> > I tested this on my board and it works like expected. I would very much
> > like to see this driver still in 4.11.
> 
> Thanks for looking into it and Many Thanks for testing it out.

FYI the deadline is the end of next week. If you submit a patch by then
addressing Andre's comment I would be happy to check it in.


> > Some (minor) comments on the code below.
> >
> > On 16/03/18 17:34, Amit Singh Tomar wrote:
> >> This patch adds driver for UART controller found on Armada 3700 SoC.
> >
> > Can you please mention "Marvell" in the subject?
> 
> Ok.
> 
> > These should be indented by one tab (plus two spaces for the help text).
> > It's not obvious - I got this wrong myself the other day ;-), but it's
> > how the rest of the file works.
> 
> Ok.
> 
> > No need for the brackets.
> 
> Ok.
> 
> > Indentation.
> 
> Ok.
> 
> > So why do we need this include file, in a shared directory?
> > All those bits are private to the UART driver and don't need to be
> > exposed to Xen at all.
> > If it's about the earlyprintk support: that's just two values needed
> > there, nothing worth a new include file, I think.
> > So I would recommend to declare the required constants directly in the
> > driver file.
> 
> Yes, I thought earlyprintk could also use a couple of common defines and other
> drivers do the same way.
> 
> 
> Thanks
> -Amit
> 

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