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

[<Claudiu.Beznea@microchip.com>]

On 22.03.2022 23:39, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Am 2022-03-18 13:17, schrieb Claudiu.Beznea@microchip.com:
>> 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@microchip.com:
>>>> 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@microchip.com:
>>>>>> 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@walle.cc>
>>>>>>> ---
>>>>>>>  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

You're right, I haven't checked the driver.

> and are supported on the LAN966X. So if anything, SAMA5D2,
> SAM9X60
> and SAMA7G5 need a new compatible, no?

It seems that's true unless some errors in datasheet. I'll double check on
my side.

Thank you,
Claudiu Beznea

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