Xen project Mailing List

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

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.

Cheers,

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