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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 31 Mar 2018, Julien Grall wrote:
> On Sat, 31 Mar 2018, 00:27 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:
>       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.
> 
> 
> I also had comments on the previous version. So it would be nice to give me 
> sometimes to review it again.

Welcome back. I'll do.

Cheers,

Stefano


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

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