Re: [PATCH 1/6] arm64: dts: qcom: ipq9574: Add PPE device tree node

[Dmitry Baryshkov]

On 12 January 2024 16:40:02 EET, Jie Luo <quic_luoj@xxxxxxxxxxx> wrote:
>
>
>On 1/12/2024 12:06 AM, Dmitry Baryshkov wrote:
>> On Thu, 11 Jan 2024 at 17:31, Jie Luo <quic_luoj@xxxxxxxxxxx> wrote:
>
>>> 
>>>> 
>>>>> +                    reg = <0x3a000000 0xb00000>;
>>>>> +                    #address-cells = <1>;
>>>>> +                    #size-cells = <1>;
>>>>> +                    ranges;
>>>> 
>>>> Put after reg.
>>> Ok.
>>> 
>>>> 
>>>>> +                    status = "okay";
>>>> 
>>>> Drop
>>> Ok.
>>> 
>>>> 
>>>> All of above comments apply to your entire patchset and all places.
>>>> 
>>>> Looking at code further, it does not look like suitable for mainline,
>>>> but copy of downstream code. That's not what we expect upstream. Please
>>>> go back to your bindings first. Also, I really insist you reaching out
>>>> to other folks to help you in this process.
>>>> 
>>>> Best regards,
>>>> Krzysztof
>>>> 
>>> We will do internal review of the gaps and update the patches as per
>>> your comments.
>>> 
>>> Thanks for the review comments.
>> 
>>  From the first glance, the bindings do not follow upstream principles.
>> You have all the settings (tdm, port config, etc) in the DT, while
>> they should instead go to the driver. Well, unless you expect that the
>> board might need to override them.
>> 
>Hi Dmitry,
>The TDM configuration varies per SoC type, since the ethernet port capabilities of the SoCs vary. So we will have two different TDM configurations for IPQ5332 and IPQ9574 SoC. The driver also will
>need to support future SoC, so we choose to configure this from the DTSI. The same reason applies to the port scheduler config as well.

If it differs from SoC to SoC only, it goes to the driver. Point. No other options. Thank you.

>
>Thanks for review comments.