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

[Amit Tomer <amittomer25@xxxxxxxxx>]

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.

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