Re: [PATCH v3 2/4] dt-bindings: nvmem: Add properties needed for blowing fuses
From: Srinivas Kandagatla
Date: Thu Jun 18 2020 - 10:01:41 EST
On 18/06/2020 14:48, Doug Anderson wrote:
Hi,
On Thu, Jun 18, 2020 at 3:10 AM Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
+Adding SBoyd.
On 17/06/2020 18:22, Doug Anderson wrote:
Hi,
On Wed, Jun 17, 2020 at 8:19 AM Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
On 17/06/2020 15:51, Douglas Anderson wrote:
From: Ravi Kumar Bokka <rbokka@xxxxxxxxxxxxxx>
On some systems it's possible to actually blow the fuses in the qfprom
from the kernel. Add properties to support that.
NOTE: Whether this is possible depends on the BIOS settings and
whether the kernel has permissions here, so not all boards will be
able to blow fuses in the kernel.
Signed-off-by: Ravi Kumar Bokka <rbokka@xxxxxxxxxxxxxx>
Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
---
Changes in v3:
- Add an extra reg range (at 0x6000 offset for SoCs checked)
- Define two options for reg: 1 item or 4 items.
- No reg-names.
- Add "clocks" and "clock-names" to list of properties.
- Clock is now "sec", not "secclk".
- Add "vcc-supply" to list of properties.
- Fixed up example.
.../bindings/nvmem/qcom,qfprom.yaml | 45 ++++++++++++++++++-
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
index 5efa5e7c4d81..b195212c6193 100644
--- a/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
+++ b/Documentation/devicetree/bindings/nvmem/qcom,qfprom.yaml
@@ -17,8 +17,27 @@ properties:
const: qcom,qfprom
reg:
- items:
- - description: The corrected region.
+ # If the QFPROM is read-only OS image then only the corrected region
+ # needs to be provided. If the QFPROM is writable then all 4 regions
+ # must be provided.
+ oneOf:
+ - items:
+ - description: The corrected region.
+ - items:
+ - description: The corrected region.
+ - description: The raw region.
+ - description: The config region.
+ - description: The security control region.
+
+ # Clock must be provided if QFPROM is writable from the OS image.
+ clocks:
+ maxItems: 1
+ clock-names:
+ const: sec
Do we need clock-names for just one clock here?
I think technically you can get by without, but convention is that
clock-names are always provided for clocks. It's talked about in the
same link I sent that talked about reg-names:
https://lore.kernel.org/r/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@xxxxxxxxxxxxxx/
TBH, This is total confusion!!!
when to use "*-names" Device tree bindings is totally depended on Linux
Subsystem interfaces!
And what is the starting point to draw this line?
Definitely confusing and mostly because the dts stuff grew organically
for a while there. It does feel like Rob is pretty clear on the
current state of things and the policy in the link I provided, though.
Specifically, Rob said:
That probably is because the clock binding has had clock-names from
the start (it may have been the first one). That was probably partly
due to the clock API also was mainly by name already if we want to
admit Linux influence on bindings
Basically the standard way for getting clocks in Linux is
clk_get(name). With just one clock you can call clk_get(NULL) and I
believe that works, but when you add the 2nd clock then you have to
switch APIs to one of the less-commonly-used variants.
In previous NON-DT life clk_get api name argument comes from the clk
names that clk provider registered the clocks with.
If I remember this correctly, the name that is refereed here for
clk_get() is old clkdev api based on clk_lookups and is not the same as
clk-names that we have in Device tree. Atleast in this case!
clk-names has two objectives in DT:
1> To find the index of the clock in the clocks DT property.
2> If actual clk name is specified then if "1" fails then name could
potentially fallback to use old clkdev based clk_lookups.
In this specific case we have "sec" as clock-names which is totally used
for indexing into clocks property and it can not be used for (2) as
there is no clk named "sec" registered by any of the clk providers.
So this does not justify the reasoning why "clock-names" should be used
while "reg-names" should not be used!. Both of them are going to be
finally used for indexing into there respective properties.
Right, you just have to accept the fact that logic doesn't come into
play here. For clocks, always use "clk-names" but also always use a
consistent order (which is now more enforced by the schema checker).
For "reg" almost never use "reg-names".
On the other note:
clock-names are not mandatory according to
Documentation/devicetree/bindings/clock/clock-bindings.txt
For this particular case where clock-names = "sec" is totally used for
indexing and nothing else!
This also brings in greater confusion for both existing and while adding
bindings with "*-names" for new interfaces.
Rob, can you please provide some clarity and direction on when to
use/not-use *-names properties!
If I had to guess Rob will say that we shouldn't add more places where
the convention is to have "-names".
Confusion is not just about new bindings, but with the existing ones! :-)
--srini
I will put posting v4 of this patch on pause until this is resolved to
avoid fragmenting the discussion.
-Doug