On Tue, Aug 23, 2022 at 11:44:16AM +0300, Krzysztof Kozlowski wrote:

On 23/08/2022 11:32, Serge Semin wrote:

quoted

On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote:

quoted

On 22/08/2022 22:07, Serge Semin wrote:

quoted

The Zynq A05 DDRC controller has nothing in common with DW uMCTL2 DDRC:
the CSRs layout is absolutely different and it doesn't has IRQ unlike DW
uMCTL2 DDR controller of all versions (v1.x, v2.x and v3.x). Thus there is
no any reason to have these controllers described by the same bindings.
Thus let's split them up.

While at it rename the original Synopsys uMCTL2 DT-schema file to a more
descriptive - snps,dw-umctl2-ddrc.yaml and add a more detailed title and
description of the device bindings.

quoted

quoted

Filename should be based on compatible, so if renaming then
snps,ddrc-3.80a.yaml or snps,ddrc.yaml... which leads to original
filename anyway. Therefore nack for rename.

Original name was synopsys,ddrc-ecc.yaml which doesn't match any of
the compatible strings. 

quoted

New requirement? I've submitted not a single patch to the DT-bindings
sources and didn't get any comment from Rob about that. 

This is not a new requirement. It has been since some time and Rob gave
such reviews.

https://lore.kernel.org/linux-devicetree/YlhkwvGdcf4ozTzG@robh.at.kernel.org/ (local)

April 2022. So it's new. It would be nice to have it defined somewhere
in docs (writing-bindings.rst?). So does the compatibles order (this
was surprising to me too).

For devices with multiple compatibles that's a bit tricky, but assuming
the bindings describe both original design from Synopsys and it's
implementations, then something closer to Synopsys makes sense.

The closest name would be snps,dw-umctl2-ddrc.yaml. snps,ddrc is too
generic especially for the IP-cores vendor. It doesn't have a
reference to the actual IP-core the device in subject is based on.

quoted

In addition
There are DT bindings with names different from what is defined in the
compatible name. Moreover there are tons of bindings with various
compatible names. What name to choose then? Finally the current name
is too generic to use for actual DW uMCTL2 DDRC controller.

There are thousands of bugs, inconsistencies, naming differences in
kernel. I don't find these as arguments to repeat the practice...so the
bindings file name should be based on the compatible.

Did I ask for an exception? I justified why the renaming was necessary. You
said it goes against the practice of having the DT-schema named as the
device compatible strings and just nacked. But above in this message you said

"assuming the bindings describe both original design from Synopsys
and it's implementations, then something closer to Synopsys makes sense"

What I suggest makes more sense than some abstract Synopsys DDRC,
which may refer to a Synopsys DDR controller other than the subject
one. So I see two solutions here:
1. Adding a new generic compatible string like "snps,dw-umctl2-ddrc"
and deprecate the "snps,ddrc-3.80a". It gets to be even more justified
seeing the Synopsys IP-core version has been exported in the device
CSRs since IP-core v3.20a. So having the version attached to the
compatible string was absolutely redundant.
2. Just deprecate the generic compatible string, the new compatible
devices will be supposed to use a vendor-specific compatible strings,
but still rename the DT-bindings file. This makes sense since the
current generic name isn't quiet well structured. It' prefix-part is
too generic and at the same time it refers to a device reversion for
no much reason.

What do you think?

* Note I've got it you'd prefer the renaming being performed in a
separate patch.

-Sergey

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel