Re: [PATCH v1 3/6] ARM: dts: lan966x: add all flexcom usart nodes

[Michael Walle]

Am 2022-03-18 13:17, schrieb Claudiu.Beznea@xxxxxxxxxxxxx:
> On 07.03.2022 14:04, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> > the
> > content is safe
> >
> > Am 2022-03-07 12:53, schrieb Claudiu.Beznea@xxxxxxxxxxxxx:
> > > On 04.03.2022 13:01, Michael Walle wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you 
> > > > know
> > > > the
> > > > content is safe
> > > >
> > > > Hi,
> > > >
> > > > thanks for the quick review.
> > > >
> > > > Am 2022-03-04 09:30, schrieb Claudiu.Beznea@xxxxxxxxxxxxx:
> > > > > On 03.03.2022 18:03, Michael Walle wrote:
> > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > > > > know
> > > > > > the content is safe
> > > > > >
> > > > > > Add all the usart nodes for the flexcom block. There was already
> > > > > > an usart node for the flexcom3 block. But it missed the DMA
> > > > > > channels.
> > > > >
> > > > > And it would be good to go though a different patch.
> > > >
> > > > sure
> > > >
> > > > > > Although the DMA channels are specified, DMA is not
> > > > > > enabled by default because break detection doesn't work with DMA.
> > > > > >
> > > > > > Keep the nodes disabled by default.
> > > > > >
> > > > > > Signed-off-by: Michael Walle <michael@xxxxxxxx>
> > > > > > ---
> > > > > >  arch/arm/boot/dts/lan966x.dtsi | 55
> > > > > > ++++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 55 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm/boot/dts/lan966x.dtsi
> > > > > > b/arch/arm/boot/dts/lan966x.dtsi
> > > > > > index a7d46a2ca058..bea69b6d2749 100644
> > > > > > --- a/arch/arm/boot/dts/lan966x.dtsi
> > > > > > +++ b/arch/arm/boot/dts/lan966x.dtsi
> > > > > > @@ -92,6 +92,19 @@ flx0: flexcom@e0040000 {
> > > > > >                         #size-cells = <1>;
> > > > > >                         ranges = <0x0 0xe0040000 0x800>;
> > > > > >                         status = "disabled";
> > > > > > +
> > > > > > +                       usart0: serial@200 {
> > > > > > +                               compatible =
> > > > > > "atmel,at91sam9260-usart";
> > > > >
> > > > > Are the usart blocks in lan966x 1:1 compatible with what is is
> > > > > sam9260?
> > > > > In
> > > > > case not it may worth to have a new compatible here, for lan966x,
> > > > > such
> > > > > that
> > > > > when new features will be implemented in usart driver for lan966x 
> > > > > the
> > > > > old
> > > > > DT (this one) will work with the new kernel implementation.
> > > >
> > > > During my review of the inital dtsi patch, I've asked the same
> > > > question
> > > > [1]
> > > > and I was told they are the same.
> > > >
> > > > At least this exact usart compatible is already in this file. I was
> > > > under
> > > > the impression, that was the least controversial compatible :)
> > >
> > > OK.
> > >
> > > > But you'll need to tell me if they are the same or not, I don't have
> > > > any clue what microchip has reused.
> > >
> > > From software point of view comparing registers should be good, as 
> > > far
> > > as I
> > > can tell. All AT91 datasheet should be available. I though you have
> > > checked
> > > one against LAN966. At the moment I don't have a DS for LAN966. I'll
> > > find
> > > one and have a look.
> >
> > So my train of thought was like: even if the registers are the same I
> > cannot be sure that it is the exact same IP and will behave the same.
> > Therefore, it is something only microchip can answer.
> >
> > You can find the registers at
> > https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
> >
> > I'm not aware of any "classic" datasheet.
>
> You can find all AT91 datasheet on Microchip web site [1].
>
> Simple register comparison b/w register mapping at [2] and SAMA5D2
> datasheet [3] (which uses the same compatible),  SAM9X60 datasheet [3] 
> and
> SAMA7G5 datasheet (not public at the moment) brings up a difference at
> register FLEX_US_CR (bits 16, 17) which are not available on SAMA5D2,
> SAM9X60 or SAMA7G5. Unless this is a mistake on documentation at [2] I 
> say
> it needs a new compatible.

I can't follow you here. These bits are already used in the current UART
driver and are supported on the LAN966X. So if anything, SAMA5D2, 
SAM9X60
and SAMA7G5 need a new compatible, no?

-michael

> Kavya, could you confirm this?
>
> Thank you,
> Claudiu Beznea
>
> [1] https://www.microchip.com/
> [2] 
> https://microchip-ung.github.io/lan9668_reginfo/reginfo_LAN9668.html
> [3] 
> http://ww1.microchip.com/downloads/en/devicedoc/ds60001476b.pdf#G22.2193277
> [4]
> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductDocuments/DataSheets/SAM9X60-Data-Sheet-DS60001579E.pdf
>
> > -michael