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

From: Srinivas Kandagatla
Date: Tue Jun 02 2020 - 06:56:10 EST




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