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

From: Serge Semin <fancer.lancer@gmail.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
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 16:23:35 +0300	[thread overview]
Message-ID: <20220825132335.f7mxjyomz6a5zybf@mobilestation> (raw)
In-Reply-To: <a04f6b56-4bca-cc86-c51f-3a7c6c7ef02a@linaro.org>

On Thu, Aug 25, 2022 at 09:06:42AM +0300, Krzysztof Kozlowski wrote:
> 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.

Agreed then.

> 
> > 
> >>
> >> 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.

Right. I'll make sure the renaming is detected.

-Sergey

> 
> > 
> >> 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