On Tue, Aug 23, 2022 at 11:44:16AM +0300, Krzysztof Kozlowski wrote: > On 23/08/2022 11:32, Serge Semin wrote: > > On Tue, Aug 23, 2022 at 11:17:23AM +0300, Krzysztof Kozlowski wrote: > >> On 22/08/2022 22:07, Serge Semin wrote: > >>> 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. > >> > > > >> 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. > > > > 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@xxxxxxxxxxxxxxxxxx/ 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. > > > > 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
