Re: [PATCH v3 2/4] dt-bindings: nvmem: Add properties needed for blowing fuses

[Srinivas Kandagatla]

+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?


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

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!


Thanks,
srini
> -Doug