Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Rob Herring <robh@kernel.org>,
	Michal Simek <michal.simek@xilinx.com>,
	Borislav Petkov <bp@alien8.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>, Rob Herring <robh+dt@kernel.org>,
	Manish Narani <manish.narani@xilinx.com>,
	Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>,
	Michail Ivanov <Michail.Ivanov@baikalelectronics.ru>,
	Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>,
	Punnaiah Choudary Kalluri  <punnaiah.choudary.kalluri@xilinx.com>,
	Dinh Nguyen <dinguyen@kernel.org>,
	James Morse <james.morse@arm.com>,
	Robert Richter <rric@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 16/20] dt-bindings: memory: snps: Detach Zynq DDRC controller support
Date: Thu, 25 Aug 2022 09:06:42 +0300	[thread overview]
Message-ID: <a04f6b56-4bca-cc86-c51f-3a7c6c7ef02a@linaro.org> (raw)
In-Reply-To: <20220824172724.ny2xpryn76h6ftv6@mobilestation>

On 24/08/2022 20:27, Serge Semin wrote:

> 
> Note what Rob said concerned the generic compatible "fallback" case,
> not the generic compatible string in general. It's ok to have a
> generic device name defined irrespective to the platform vendor.
> Moreover it's applicable in case of the DW uMCTL2 DDRC IP-core since
> first IP-core version is auto-detectable starting from v3.20a and
> second I managed to implement auto-detection solutions for almost
> all the DDR/ECC-specific parameters. So I am more inclined to the
> solution 1) suggested by me in the previous email message:
> - deprecate "snps,ddrc-3.80a" string.
> - add new generic "snps,dw-umctl2-ddrc" compatible string.
> - rename the DT-bindings file.

Sounds ok.

> 
>>
>> Here the Linux driver also binds to generic synopsys compatible, so I
>> would assume it has a meaning and use case on its own.
> 
> Please see my messages above regarding the current Synopsys DW uMCTL2
> EDAC driver implementation.
> 
>>
>>>
>>> What do you think?
>>>
>>> * Note I've got it you'd prefer the renaming being performed in a
>>> separate patch.
>>
>> The rename could be in the split patch as here, but then I assume the
>> rename part to be detected by git and be a pure rename. However:
>> 1. The git did not mark it as rename (you might need to use custom
>> arguments to -M/-B/-C),
> 
> Of course git hasn't detected it as rename, because aside with renaming
> I've split the bindings up. Splitting these two updates up into two
> patches will give us what you said. So to speak I suggest the next
> updates for v2:
> PATCH X. Detach the Zynq A05 DDRC DT-bindings to a separate schema.
> PATCH X + 1. Rename the Synopsys DW uMCTL2 DDRC bindings file and add a more
> descriptive generic compatible string name.
> 
> What do you think?

Regardless of the split the rename can be and should be detected by Git.
That's why we have these options. If it is not detected, you changed too
much during rename, so it is not a rename anymore. Relatively small
amount of changes would still be detected.

> 
>> 2. There were also changes in the process (allOf:if:then).
> 
> Right. But this is in another patchset. I'll address your notes in there.

Best regards,
Krzysztof