On Tue 14 Jul 10:31 PDT 2020, Bjorn Andersson wrote: > On Tue 14 Jul 10:12 PDT 2020, Eric Biggers wrote: > > > 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxxx). > > 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? > > > > The problem here is that DT binding refactoring is not kernel-internal. > It's two different projects living in the same git. > > There's a wish from various people that we make sure that new DTS > continues to work with existing kernels. This is a nice in theory > there's a lot of examples where we simply couldn't anticipate how future > bindings would look. A particular example is that this prohibits most > advancement in power management. > > > But afaict what you describe above would make a new kernel failing to > operate with the old DTS and that we have agreed to avoid. > So I think the appropriate way to deal with this is to request the reg > byname to detect the new binding and if that fails then assume that > index 1 is dev_ref_clk_ctrl. > I took another look at the git history and I can't find a single dts - either upstream or in any downstream tree - that specifies that second reg. So per my argument below, if you could include a patch that just removes the "dev_ref_clk_ctrl_mem" reference from the binding and driver I would be happy to r-b that and ack this patch. Regards, Bjorn > > There are cases where we just decide not to be backwards compatible, but > it's pretty rare. As for deprecation, I think 1-2 LTS releases is > sufficient, at that time scale it doesn't make sense to sit with an old > DTB anyways (given the current pace of advancements in the kernel). > > Regards, > Bjorn