quoted

quoted

quoted

quoted

quoted

Create the new compatibles to identify AST2600 MAC0/1 and MAC3/4.
Add conditional schema constraints for Aspeed AST2600 MAC

controllers:

quoted

quoted

quoted

quoted

quoted

- For "aspeed,ast2600-mac01", require rx/tx-internal-delay-ps properties
  with 45ps step.
- For "aspeed,ast2600-mac23", require rx/tx-internal-delay-ps properties
  with 250ps step.

That difference does not justify different compatibles. Basically
you said they have same programming model, just different hardware
characteristics, so same compatible.

This change was originally based on feedback from a previous review

discussion.

quoted

At that time, another reviewer suggested introducing separate
compatibles for
MAC0/1 and MAC2/3 on AST2600, since the delay characteristics differ
and they might not be fully compatible.

Your commit msg does not provide enough of rationale for that.
Difference in DTS properties is rather a counter argument for having
separate compatibles. That's why you have these properties - to mark the

difference.

quoted

quoted

Actually, on the AST2600 there are two dies, and each die has its own MAC.
The MACs on these two dies indeed have different delay configurations.

Is this the logic like: we have multiple snps,dw-apb-uart UARTs on the device,
so we need snps,dw-apb-uart-1, snps,dw-apb-uart-2 and snps,dw-apb-uart-3?

You are right. That doesn't make sense..

quoted

Previously, the driver did not configure these delays - they were set
earlier during the bootloader stage. Now, I'm planning to use the
properties defined in ethernet-controller.yaml to configure these delays

properly within the driver.

quoted

Since these legacy settings have been used for quite some time, I'd
like to deprecate the old compatible and clearly distinguish that the
AST2600 contains two different MACs. Future platforms based on the
AST2600 will use the new compatibles with the correct PHY and delay

configurations.

Why are you repeating the same? So I will repeat the same. You need to
provide rationale why different compatible is justified. Difference in delay itself
is not the enough. Please write concise answer based on device programming
model differences or other rules expressed in writing bindings or numerous
presentations.

I plan to remove the new compatible entry used to identify these MACs and instead 
add a new property to specify the delay step value.
However, I have one question I'd like to discuss with you.

There are four MACs in the AST2600. In the DT bindings and DTS files, what would be 
the recommended way to identify which MAC is which?
In version 3 of my patches, I used the aliases in the DTSI file to allow the driver to get 
the MAC index.

Do you think this is a good approach? Or would it be better to create a new property 
in the DTSI to explicitly configure the index and identify each MAC?

Thanks for your time and suggestions.

Thanks,
Jacky