Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: "Ravi Kumar Bokka (Temp)" <rbokka@codeaurora.org>,
	Rob Herring <robh+dt@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
	dhavalp@codeaurora.org, mturney@codeaurora.org,
	sparate@codeaurora.org, c_rbokka@codeaurora.org,
	mkurumel@codeaurora.org
Subject: Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
Date: Tue, 2 Jun 2020 11:56:03 +0100	[thread overview]
Message-ID: <1d276cdc-247c-b663-0f69-0961cf75134b@linaro.org> (raw)
In-Reply-To: <CAD=FV=VcfYVQMmWvSkBG0VNBcqt5a3Y_b4eNBgDenPB5wUNGaw@mail.gmail.com>

On 01/06/2020 19:08, Doug Anderson wrote:
>> Am not 100% sure if "qcom,fuse-blow-frequency" is something integration
>> specific or SoC Specific, My idea was that this will give more
>> flexibility in future. As adding new SoC Support does not need driver
>> changes.
>>
>> Having said that, Am okay either way!
> Yeah, it's always a balance.  I guess the question is: why do we think
> driver changes are worse than dts changes?  The value still needs to
> be somewhere and having it in the driver isn't a terrible place.
> 

TBH, its an overkill if we are using same IP version across multiple SoCs.

> 
>> Incase we go compatible way, I would like to see compatible strings
>> having proper IP versions to have ip version rather than SoC names.
>>
>> Having SoC names in compatible string means both driver and bindings
>> need update for every new SoC which can be overhead very soon!
> Almost certainly the compatible strings should have SoC names in them.
> Yes it means a binding update every time a new SoC comes up but that
> is just how device tree works.  Presumably there's enough chatter on
> this that Rob H has totally tuned it out at this point in time, but
> there are many other instances of this.
> 
> NOTE: just because we have the SoC name in the compatible string
> _doesn't_  mean that the driver has to change.  You already said that
> the IP version can be detected earlier in this thread, right?  You
> said:
> 
> I found out that there is a version register at offset of 0x6000 which
> can give MAJOR, MINOR and STEP numbers.
> 
> So how about this:
> 
> a) Compatible contains "SoC" version and the generic "qcom,qfrom", so:
> 
> compatible = "qcom,sdm845-qfprom", "qcom,qfrom"
> 
> b) Bindings will need to be updated for every new SoC, but that's
> normal and should be a trivial patch to just add a new SoC to the
> list.
> 
> c) If the driver can be made to make its decisions about frequencies /
> timings completely by MAJOR/MINOR/STEP numbers then it can use those
> in its decision and it will never need to use the SoC-specific
> compatible string.  The SoC-specific compatible string will only be
> present as a fallback "oops we have to workaround a bug that we didn't
> know about".

This makes more sense to me, I would still stay with  MAJOR/MINOR/STEP 
numbers mostly unless we are dealing with some corner cases.

thanks,
srini
> 
> 
>> Rob can help review once we have v2 bindings out!
> Sounds good.  If you're still not convinced by my arguments we can see
> if we can get Rob to clarify once we have a v2.:-)
> 
> 