Re: [RFC v1 0/3] Add QTI QFPROM-Efuse driver support

[Doug Anderson]

Hi,

On Wed, May 13, 2020 at 8:35 PM Ravi Kumar Bokka (Temp)
<rbokka@xxxxxxxxxxxxxx> wrote:
>
> Hi doug,
>
> Thanks for your feedback. Please find below inline comments.

Probably the mailing list didn't see them.  You responded with HTML
mail.  Please be careful to only respond in plain text.


> Regards,
>
> Ravi Kumar.B
>
>
>
> On 5/13/2020 4:33 AM, Doug Anderson wrote:
>
> Hi,
>
> On Tue, May 12, 2020 at 11:18 AM Ravi Kumar Bokka <rbokka@xxxxxxxxxxxxxx> wrote:
>
> This patch series adds qfprom-efuse controller driver support.
>
> This driver can access the raw qfprom regions for fuse blowing.
>
> The current existed qfprom driver is only supports for cpufreq, thermal sensors
> drivers by read out calibration data, speed bins..etc which is stored by
> qfprom efuses.
>
> I don't understand the interaction between this driver and the
> existing "qcom,qfprom" driver.  Can you please explain?  Are they both
> acting on the same values and this one has write access?  Are there
> two instances of the same hardware block and you're managing one of
> them with this new driver and one with thue old driver?  Something
> else?
>
> [Ravi] Existing QFPROM driver in upstream kernel has limited support which is some hard coded mapping of id vs set of fuses and user can read those fuse with those id-bucket.
> That is simply reading Hw-registers and it doesn't involve any hardware programming sequence etc. Based on information given to us by QC-kernel team, existing driver was created to read calibration/sensor fuses and it is very basic/limited/fixed in functionalities and orthogonal to what we need to on Trogdor.
>
> Requirement for Trogdor fuse blow driver is different which allows to read/write almost whole fuse block and requires to follow HW programming guide. Both are completely separate and has no overlapping in terms of functionalities and capability. Please ignore the similarity of names of drivers, they are different in terms of functionalities and driver internals etc.

If they are targeting the same type of hardware IP block then, in the
very least, the bindings need to be the same.  The bindings are
supposed to be describing the hardware.

Presumably if the underlying hardware is the same you should be able
to write one driver and it can just operate in some sort of
"read-only" mode if it's running somewhere it doesn't have access
permissions to actually change the fuses.




> Ravi Kumar Bokka (3):
>   dt-bindings: nvmem: Add devicetree bindings for qfprom-efuse
>   drivers: nvmem: Add driver for QTI qfprom-efuse support
>   arm64: dts: qcom: sc7180: Add qfprom-efuse
>
>  .../devicetree/bindings/nvmem/qfprom-efuse.yaml    |  40 ++
>  arch/arm64/boot/dts/qcom/sc7180-idp.dts            |   4 +
>  arch/arm64/boot/dts/qcom/sc7180.dtsi               |   9 +
>  drivers/nvmem/Kconfig                              |  10 +
>  drivers/nvmem/Makefile                             |   2 +
>  drivers/nvmem/qfprom-efuse.c                       | 476 +++++++++++++++++++++
>  6 files changed, 541 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom-efuse.yaml
>  create mode 100644 drivers/nvmem/qfprom-efuse.c
>
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by the Linux Foundation.
>