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". > 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". I will put posting v4 of this patch on pause until this is resolved to avoid fragmenting the discussion. -Doug
