On 18/05/2020 19:31, Doug Anderson wrote:
Hi,
On Mon, May 18, 2020 at 3:45 AM Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
On 18/05/2020 11:39, Ravi Kumar Bokka (Temp) wrote:
Based on the compatible, do i need to separate probe function for
qfprom-efuse and maintain separate nvmem object to register nvmem
framework. Is this what you are suggesting to implementing this in to
one existing driver?
Yes for same driver we should add new compatible string and add support
to this in existing qfprom driver.
Ideally we should allocate nvmem_config object at probe with different
parameters based on compatible string.
I wish I had better documentation for exactly what was in the SoC
instead of the heavily redacted stuff Qualcomm provides. Really the
answer here is: how do you best describe the hardware? OK, so I just
spent the past hour or so trying to patch together all the bits and
fragments that Qualcomm provided me. Just like a scavenger hunt!
Fun! The best I can patch together is that there is a single QFPROM
with these ranges:
0x00780000 - 0x007800FF
QFPROM HW registers, range 1/2
0x00780120 - 0x007808FF
QFPROM "raw" space
so this is the only region is the QFPROM fuses can be programmed!
0x00782000 - 0x007820FF
QFPROM HW registers, range 2/2
0x00784120 - 0x007848FF
QFPROM "corrected" space
Is this some kind of FEC corrected regions?
0x00786000 - 0x00786FFF
QFPROM memory range that I don't really understand and maybe we don't
worry about right now?
Did I get that right? If so, is there a prize for winning the scavenger hunt?
---
If so then, IMO, it wouldn't be insane to actually keep it as two
drivers and two device tree nodes, as you've done. I'd defer to
Srinivas and Rob Herring, though. The existing driver would be a
read-only driver and provide access to the "corrected" versions of all
the registers. Its node would have "#address-cells = <1>" and
"#size-cells = <1>" because it's expected that other drivers might
need to refer to data stored here.
Your new driver would be read-write and provide access to the "raw"
values. A read from your new driver would not necessarily equal a
read from the old driver if the FEC (forward error correction) kicked
Is this only applicable for corrected address space?
in. Other drivers should never refer to the non-corrected values so
you wouldn't have "#address-cells" and "#size-cells". The only way to
really read or write it would be through sysfs.
It would be super important to document what's happening, of course.
...and ideally name them to make it clearer too.
---
Another alternative (if Srinivas and/or Rob H prefer it) would be to
deprecate the old driver and/or bindings and say that there really
should just be one node and one driver. In that case you'd replace
the old node with:
qfprom@780000 {
compatible = "qcom,sc7180-qfprom-efuse";
May be "qcom,sc7180-qfprom"
reg = <0 0x00780000 0 0x6fff>;
#address-cells = <1>;
#size-cells = <1>;
clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
clock-names = "sec";
qusb2p_hstx_trim: hstx-trim-primary@25b {
reg = <0x25b 0x1>;
bits = <1 3>;
};
};
You'd use the of_match_table solution to figure out the relevant
offsets (0x120, 0x2000, 0x4120, 0x6000) for sc7180 and this new driver
would be responsible for being able to read the corrected values and
Encompassing these offsets in driver as part of the register defines
itself should be a good start!
It will also be nice to understand how similar this thing is with w.rt
other Qcom SoCs?
also for programming. In this case I'm not sure how (assuming it's
valuable) you'd provide read access to the uncorrected data.
I will leave this question to the author of the driver.
--srini
-Doug
