Re: [PATCH v6 3/5] arm64: dts: sdm845: add Inline Crypto Engine registers and clock

From: Eric Biggers <ebiggers@kernel.org>
To: Rob Herring <robh+dt@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	SCSI <linux-scsi@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	linux-fscrypt@vger.kernel.org,
	Alim Akhtar <alim.akhtar@samsung.com>,
	Avri Altman <avri.altman@wdc.com>,
	Barani Muthukumaran <bmuthuku@qti.qualcomm.com>,
	Can Guo <cang@codeaurora.org>,
	Elliot Berman <eberman@codeaurora.org>,
	John Stultz <john.stultz@linaro.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Satya Tangirala <satyat@google.com>,
	Steev Klimaszewski <steev@kali.org>,
	Thara Gopinath <thara.gopinath@linaro.org>
Subject: Re: [PATCH v6 3/5] arm64: dts: sdm845: add Inline Crypto Engine registers and clock
Date: Tue, 14 Jul 2020 10:12:03 -0700	[thread overview]
Message-ID: <20200714171203.GC1064009@gmail.com> (raw)
In-Reply-To: <CAL_JsqK-wUuo6azYseC35R=Q509=h9-v4gFvcvy8wXrDgSw5ZQ@mail.gmail.com>

On Tue, Jul 14, 2020 at 10:59:44AM -0600, Rob Herring wrote:
> On Tue, Jul 14, 2020 at 10:43 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Jul 14, 2020 at 10:35:12AM -0600, Rob Herring wrote:
> > > On Tue, Jul 14, 2020 at 10:15 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > >
> > > > On Tue, Jul 14, 2020 at 10:16:04AM -0400, Martin K. Petersen wrote:
> > > > >
> > > > > Eric,
> > > > >
> > > > > > Add the vendor-specific registers and clock for Qualcomm ICE (Inline
> > > > > > Crypto Engine) to the device tree node for the UFS host controller on
> > > > > > sdm845, so that the ufs-qcom driver will be able to use inline crypto.
> > > > >
> > > > > I would like to see an Acked-by for this patch before I merge it.
> > > > >
> > > >
> > > > Andy, Bjorn, or Rob: can you give Acked-by?
> > >
> > > DTS changes should go in via the QCom tree.
> > >
> >
> > So, the DTS patch can't be applied without the driver patches since then the
> > driver would misinterpret the ICE registers as the dev_ref_clk_ctrl registers.
> 
> That sounds broken, but there's no context here for me to comment
> further. DTS changes should work with old/stable kernels. I'd suggest
> you get a review from Bjorn on the driver first.
> 

The "breaking" change is that the dev_ref_clk_ctrl registers are now identified
by name instead of assumed to be index 1.

A reviewer had complained about the device-mapper bindings of this driver before
(https://lkml.kernel.org/r/158334171487.7173.5606223900174949177@swboyd.mtv.corp.google.com).
Changing to identifying the registers by name seemed like an improvement.

If needed I can add a hole at index 1 to make the DTS changes work with
old/stable kernels too, but I didn't know that is a requirement.  (Normally for
Linux kernel development, kernel-internal refactoring is always allowed
upstream.)  If I do this, would this hack have to be carried forever, or would
we be able to fix it up eventually?  Is there any deprecation period for DTS, or
do the latest DTS have to work with a 20 year old kernel?

- Eric