Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1589307480-27508-1-git-send-email-rbokka@codeaurora.org>
 <CAD=FV=VReEUiJY0yDgMkegMnKJ-Fc_c=4_f7_K7g-PG0UVjH5g@mail.gmail.com> <fb7f601f-388f-8a77-bb22-e1398f90326f@codeaurora.org>
In-Reply-To: <fb7f601f-388f-8a77-bb22-e1398f90326f@codeaurora.org>
From:   Doug Anderson <dianders@chromium.org>
Date:   Thu, 14 May 2020 11:21:30 -0700
Message-ID: <CAD=FV=UTB+i_w-z1sYVOrqSP8TFzHa1fovrSU9RX7dBD5WB-ug@mail.gmail.com>
Subject: Re: [RFC v1 0/3] Add QTI QFPROM-Efuse driver support
To:     "Ravi Kumar Bokka (Temp)" <rbokka@codeaurora.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@linaro.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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Wed, May 13, 2020 at 8:35 PM Ravi Kumar Bokka (Temp)
<rbokka@codeaurora.org> 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@codeaurora.org>=
 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 s=
ensors
> 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 whic=
h 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 p=
rogramming sequence etc. Based on information given to us by QC-kernel team=
, existing driver was created to read calibration/sensor fuses and it is ve=
ry 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 rea=
d/write almost whole fuse block and requires to follow HW programming guide=
. Both are completely separate and has no overlapping in terms of functiona=
lities and capability. Please ignore the similarity of names of drivers, th=
ey 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.
>