Re: [PATCH 2/5] dt-bindings: arm: ti: Add bindings for AM625 SoC

Krzysztof Kozlowski
2022-02-09T21:29:16Z

On 09/02/2022 20:04, Vignesh Raghavendra wrote:
>
>
> On 08/02/22 10:31 pm, Krzysztof Kozlowski wrote:
>> On 08/02/2022 14:18, Vignesh Raghavendra wrote:
>>> From: Nishanth Menon <nm@ti.com>
>>>
>>> The AM62 SoC family is the follow on AM335x built on K3 Multicore SoC
>>> architecture platform, providing ultra-low-power modes, dual display,
>>> multi-sensor edge compute, security and other BOM-saving integration.
>>> The AM62 SoC targets broad market to enable applications such as
>>> Industrial HMI, PLC/CNC/Robot control, Medical Equipment, Building
>>> Automation, Appliances and more.
>>>
>>> Some highlights of this SoC are:
>>>
>>> * Quad-Cortex-A53s (running up to 1.4GHz) in a single cluster.
>>> Pin-to-pin compatible options for single and quad core are available.
>>> * Cortex-M4F for general-purpose or safety usage.
>>> * Dual display support, providing 24-bit RBG parallel interface and
>>> OLDI/LVDS-4 Lane x2, up to 200MHz pixel clock support for 2K display
>>> resolution.
>>> * Selectable GPUsupport, up to 8GFLOPS, providing better user experience
>>> in 3D graphic display case and Android.
>>> * PRU(Programmable Realtime Unit) support for customized programmable
>>> interfaces/IOs.
>>> * Integrated Giga-bit Ethernet switch supporting up to a total of two
>>> external ports (TSN capable).
>>> * 9xUARTs, 5xSPI, 6xI2C, 2xUSB2, 3xCAN-FD, 3x eMMC and SD, GPMC for
>>> NAND/FPGA connection, OSPI memory controller, 3xMcASP for audio,
>>> 1x CSI-RX-4L for Camera, eCAP/eQEP, ePWM, among other peripherals.
>>> * Dedicated Centralized System Controller for Security, Power, and
>>> Resource Management.
>>> * Multiple low power modes support, ex: Deep sleep,Standby, MCU-only,
>>> enabling battery powered system design.
>>>
>>> AM625 is the first device of the family. Add DT bindings for the same.
>>
>> Don't paste the same huge commit description in several commits.
>
> Sorry, I think this is the first commit with full description. I will
> probably trim 4/5 at bit
>
>>
>>>
>>> More details can be found in the Technical Reference Manual:
>>> https://www.ti.com/lit/pdf/spruiv7
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>> ---
>>> Documentation/devicetree/bindings/arm/ti/k3.yaml | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/ti/k3.yaml b/Documentation/devicetree/bindings/arm/ti/k3.yaml
>>> index b03c10fa2e7a..64f3db3ea9dd 100644
>>> --- a/Documentation/devicetree/bindings/arm/ti/k3.yaml
>>> +++ b/Documentation/devicetree/bindings/arm/ti/k3.yaml
>>> @@ -53,6 +53,12 @@ properties:
>>> - ti,am642-sk
>>> - const: ti,am642
>>>
>>> + - description: K3 AM625 SoC
>>> + items:
>>> + - enum:
>>> + - ti,am625-sk
>>> + - const: ti,am625
>>
>> Why keeping it not alphabetically sorted? What sorting did you choose?
>>
>
> Above list is not sorted alphabetically, I tried to keep similar SoCs
> bunched together. AM625 and AM642 are of same family, hence chose to add
> the new entry here.

Then maybe it should be before AM642?

> One alternative is to add it to end of the list (chronologically)?
> Or I can add a patch to sort the list alphabetically first and then
> introduce new compatible. Please let me know your preference?

It's not that important, just wondering. I propose to avoid putting at
the end, because this causes conflicts in case of concurrent work. If I
had to choose, I would propose to sort SoCs by name.

Either way is fine - with trimmed commit msg in patch 4 or here:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Best regards,
Krzysztof