> -----Original Message----- > From: Thierry Reding <thierry.reding@xxxxxxxxx> > Sent: Tuesday, August 13, 2019 3:26 PM > To: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; > mark.rutland@xxxxxxx; Jonathan Hunter <jonathanh@xxxxxxxxxx>; Laxman > Dewangan <ldewangan@xxxxxxxxxx>; jslaby@xxxxxxxx; linux- > serial@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > tegra@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 07/14] serial: tegra: add compatible for new chips > > On Mon, Aug 12, 2019 at 04:58:16PM +0530, Krishna Yarlagadda wrote: > > Add new compatible string for Tegra186. It differs from earlier chips > > as it has fifo mode enable check and 8 byte dma buffer. > > Add new compatible string for Tegra194. Tegra194 has different error > > tolerance levels for baud rate compared to older chips. > > > > Signed-off-by: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx> > > --- > > Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt | > > 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > I think device tree binding patches are supposed to start with > "dt-binding: ...". > > Also: "fifo" -> "FIFO" and "dma" -> "DMA" in the commit message. > > > > > diff --git > > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt > > b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt > > index d7edf73..187ec78 100644 > > --- > > a/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.txt > > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra20-hsuart.t > > +++ xt > > @@ -1,7 +1,8 @@ > > NVIDIA Tegra20/Tegra30 high speed (DMA based) UART controller driver. > > > > Required properties: > > -- compatible : should be "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart". > > +- compatible : should be "nvidia,tegra30-hsuart", > > +"nvidia,tegra20-hsuart", > > + nvidia,tegra186-hsuart, nvidia,tegra194-hsuart. > > Please use quotes around the compatible strings like the existing ones. > Also, I think it might be better to list these explicitly rather than just give a list > of potential values. As it is right now, it's impossible to tell whether this is > meant to say "should be all of these" > or whether it is meant to say "should be one of these". > > Thierry > Will update in next version. KY > > - reg: Should contain UART controller registers location and length. > > - interrupts: Should contain UART controller interrupts. > > - clocks: Must contain one entry, for the module clock. > > -- > > 2.7.4 > >