linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Atish Patra <Atish.Patra@wdc.com>
To: Paul Walmsley <paul.walmsley@sifive.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexios Zavras <alexios.zavras@intel.com>,
	Allison Randal <allison@lohutok.net>,
	Anup Patel <Anup.Patel@wdc.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Enrico Weigelt <info@metux.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Johan Hovold <johan@kernel.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	Rob Herring <robh+dt@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2 5/5] dt-bindings: Update the isa string description
Date: Wed, 31 Jul 2019 06:43:21 +0000	[thread overview]
Message-ID: <5E8CC941-E519-4B4F-8F5E-253D0C797431@wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.21.9999.1907302124010.15340@viisi.sifive.com>



On 7/30/19, 9:52 PM, "Paul Walmsley" <paul.walmsley@sifive.com> wrote:

    On Tue, 30 Jul 2019, Atish Patra wrote:
    
    > The yaml documentation description of isa strings section doesn't
    > specify anything about the case sensitiveness of the isa strings.
    > The RISC-V specification clearly specifies it to be case insensitive.
    > However, Linux kernel supports only lower case isa strings.
    
    The DT binding documentation specifies an interface.  As such the binding 
    isn't determined by any particular piece of software.  So justifying the 
    binding update by referring to what the Linux kernel currently supports 
    isn't that relevant.  If you still really believe that software should be 
    required to handle mixed-case DT ISA strings, the right answer would be to 
    change the software, as your original patches proposed.  The way you've 
    written this patch description, it sounds like you still don't agree with 
    the conclusion that a strictly lowercase string is a good approach.
   
    If I've misunderstood your intent here, and you do think that specifying 
    an all lowercase string is sufficient,

I think that specifying an all lowercase string is sufficient. I did not 
realize that current commit text could mean something else (.

 then instead of the patch 
    description above, how about something like:
    
    "Since the RISC-V specification states that ISA description strings are 
    case-insensitive, there's no functional difference between mixed-case, 
    upper-case, and lower-case ISA strings.  Thus, to simplify parsing, 
    specify that the letters present of riscv,isa must be all lowercase."
    
Sounds good to me. I will update the patch. Thanks for updated commit text.

    That way it's clear that, per the RISC-V specification, there's no 
    functional difference associated with case.
    
    However, if what you're saying is that you still don't like this outcome, 
    let me know and I'll write the patch myself.  That way you don't have to 
    have your name associated with a change that you don't believe in.
    
    > Update the yaml documentation accordingly to avoid any confusion.
    > 
    > Signed-off-by: Atish Patra <atish.patra@wdc.com>
    > ---
    >  Documentation/devicetree/bindings/riscv/cpus.yaml | 6 +++++-
    >  1 file changed, 5 insertions(+), 1 deletion(-)
    > 
    > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
    > index c899111aa5e3..e22a2b7ebafa 100644
    > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
    > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
    > @@ -46,10 +46,14 @@ properties:
    >            - rv64imafdc
    >      description:
    >        Identifies the specific RISC-V instruction set architecture
    > -      supported by the hart.  These are documented in the RISC-V
    > +      supported by the hart. These are documented in the RISC-V
    >        User-Level ISA document, available from
    >        https://riscv.org/specifications/
    >  
    > +      Linux kernel only supports lower case isa strings. Thus,
    
    In the past, the DT maintainers have pushed back against explicitly 
    mentioning the Linux kernel in binding documentation, since the DT 
    bindings define an interface that's independent of the underlying software 
    implementation.  How about just stating something like "Letters in the 
    riscv,isa string must be all lowercase" ?
    
Sure.

Regards,
Atish

    > +      isa strings must be specified in lower case in device tree
    > +      as well.
    > +
    
    - Paul
    


      reply	other threads:[~2019-07-31  6:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-31  1:24 [PATCH v2 0/5] Miscellaneous fixes Atish Patra
2019-07-31  1:24 ` [PATCH v2 1/5] RISC-V: Remove per cpu clocksource Atish Patra
2019-08-16 15:09   ` Daniel Lezcano
2019-08-16 18:55     ` Atish Patra
2019-08-26 23:48       ` Palmer Dabbelt
2019-07-31  1:24 ` [PATCH v2 2/5] RISC-V: Add riscv_isa reprensenting ISA features common across CPUs Atish Patra
2019-07-31  4:23   ` Paul Walmsley
2019-07-31  6:27     ` Atish Patra
2019-07-31  1:24 ` [PATCH v2 3/5] RISC-V: Fix unsupported isa string info Atish Patra
2019-07-31  1:24 ` [PATCH v2 4/5] RISC-V: Export few kernel symbols Atish Patra
2019-07-31  1:24 ` [PATCH v2 5/5] dt-bindings: Update the isa string description Atish Patra
2019-07-31  4:52   ` Paul Walmsley
2019-07-31  6:43     ` Atish Patra [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5E8CC941-E519-4B4F-8F5E-253D0C797431@wdc.com \
    --to=atish.patra@wdc.com \
    --cc=Anup.Patel@wdc.com \
    --cc=alexios.zavras@intel.com \
    --cc=allison@lohutok.net \
    --cc=aou@eecs.berkeley.edu \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=info@metux.net \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=palmer@sifive.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).