From: Mark Brown <broonie@kernel.org>
To: Emil Renner Berthing <kernel@esmil.dk>
Cc: linux-spi@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Palmer Dabbelt <palmer@sifive.com>,
devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] spi: add driver for the SiFive SPI controller
Date: Tue, 13 Nov 2018 14:38:22 -0800 [thread overview]
Message-ID: <20181113223822.GN2089@sirena.org.uk> (raw)
In-Reply-To: <CANBLGcxF-WgsBVFHf4jGRR+Ox2vz=ewzhwGP1CZX-R5HpDAVBg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]
On Tue, Nov 13, 2018 at 08:48:43PM +0100, Emil Renner Berthing wrote:
> On Tue, 13 Nov 2018 at 19:35, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Nov 12, 2018 at 03:27:36PM +0100, Emil Renner Berthing wrote:
> > > I know the discussions about the sifive devicetree compatible
> > > strings haven't come to a conclusion, so I'm sending this as
> > > an RFC to get some feedback on the rest of the code.
> > I've not seen any of these discussions or earlier versions of this
> > driver so I've no idea what's going on here :(
> No, sorry. This has been discussed on linux-riscv for other drivers
> like the uart. See my last answer.
> > > +Optional properties:
> > > +- sifive,fifo-depth : Depth of hardware queues; defaults to 8
> > > +- sifive,max-bits-per-word : Maximum bits per word; defaults to 8
> > If the hardware isn't fixed yet making these enumerable from the
> > hardware would be good...
> Agreed, but unfortunately this is already in the FU540-C000 chip on
> the HiFive Unleashed board sold by SiFive.
Pick an unused register you can read safely and define that value to the
default :)
> > > +/* for consistency we need this symbol */
> > > +#ifdef REG_FMT
> > > +#undef REG_FMT
> > > +#endif
> > We do? For consistency with what?
> Below all the register offsets are defined as
> REG_<register name>. This is is a pattern I
> copied from other drivers, but here we have a
> register called "fmt" - hence REG_FMT.
> If you have a better pattern that doesn't clash
> with REG_FMT please let me know.
You shouldn't be using such generic names for your internal identifiers,
add a prefix to everything.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: broonie@kernel.org (Mark Brown)
To: linux-riscv@lists.infradead.org
Subject: [RFC PATCH] spi: add driver for the SiFive SPI controller
Date: Tue, 13 Nov 2018 14:38:22 -0800 [thread overview]
Message-ID: <20181113223822.GN2089@sirena.org.uk> (raw)
In-Reply-To: <CANBLGcxF-WgsBVFHf4jGRR+Ox2vz=ewzhwGP1CZX-R5HpDAVBg@mail.gmail.com>
On Tue, Nov 13, 2018 at 08:48:43PM +0100, Emil Renner Berthing wrote:
> On Tue, 13 Nov 2018 at 19:35, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Nov 12, 2018 at 03:27:36PM +0100, Emil Renner Berthing wrote:
> > > I know the discussions about the sifive devicetree compatible
> > > strings haven't come to a conclusion, so I'm sending this as
> > > an RFC to get some feedback on the rest of the code.
> > I've not seen any of these discussions or earlier versions of this
> > driver so I've no idea what's going on here :(
> No, sorry. This has been discussed on linux-riscv for other drivers
> like the uart. See my last answer.
> > > +Optional properties:
> > > +- sifive,fifo-depth : Depth of hardware queues; defaults to 8
> > > +- sifive,max-bits-per-word : Maximum bits per word; defaults to 8
> > If the hardware isn't fixed yet making these enumerable from the
> > hardware would be good...
> Agreed, but unfortunately this is already in the FU540-C000 chip on
> the HiFive Unleashed board sold by SiFive.
Pick an unused register you can read safely and define that value to the
default :)
> > > +/* for consistency we need this symbol */
> > > +#ifdef REG_FMT
> > > +#undef REG_FMT
> > > +#endif
> > We do? For consistency with what?
> Below all the register offsets are defined as
> REG_<register name>. This is is a pattern I
> copied from other drivers, but here we have a
> register called "fmt" - hence REG_FMT.
> If you have a better pattern that doesn't clash
> with REG_FMT please let me know.
You shouldn't be using such generic names for your internal identifiers,
add a prefix to everything.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20181113/0987ee5a/attachment.sig>
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Emil Renner Berthing <kernel@esmil.dk>
Cc: Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org, Palmer Dabbelt <palmer@sifive.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-spi@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
linux-riscv@lists.infradead.org
Subject: Re: [RFC PATCH] spi: add driver for the SiFive SPI controller
Date: Tue, 13 Nov 2018 14:38:22 -0800 [thread overview]
Message-ID: <20181113223822.GN2089@sirena.org.uk> (raw)
Message-ID: <20181113223822.iHZe-1CyR3V-RTDk1k_1oOMtX8ZxRYdmroQBi-OPrUE@z> (raw)
In-Reply-To: <CANBLGcxF-WgsBVFHf4jGRR+Ox2vz=ewzhwGP1CZX-R5HpDAVBg@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1643 bytes --]
On Tue, Nov 13, 2018 at 08:48:43PM +0100, Emil Renner Berthing wrote:
> On Tue, 13 Nov 2018 at 19:35, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Nov 12, 2018 at 03:27:36PM +0100, Emil Renner Berthing wrote:
> > > I know the discussions about the sifive devicetree compatible
> > > strings haven't come to a conclusion, so I'm sending this as
> > > an RFC to get some feedback on the rest of the code.
> > I've not seen any of these discussions or earlier versions of this
> > driver so I've no idea what's going on here :(
> No, sorry. This has been discussed on linux-riscv for other drivers
> like the uart. See my last answer.
> > > +Optional properties:
> > > +- sifive,fifo-depth : Depth of hardware queues; defaults to 8
> > > +- sifive,max-bits-per-word : Maximum bits per word; defaults to 8
> > If the hardware isn't fixed yet making these enumerable from the
> > hardware would be good...
> Agreed, but unfortunately this is already in the FU540-C000 chip on
> the HiFive Unleashed board sold by SiFive.
Pick an unused register you can read safely and define that value to the
default :)
> > > +/* for consistency we need this symbol */
> > > +#ifdef REG_FMT
> > > +#undef REG_FMT
> > > +#endif
> > We do? For consistency with what?
> Below all the register offsets are defined as
> REG_<register name>. This is is a pattern I
> copied from other drivers, but here we have a
> register called "fmt" - hence REG_FMT.
> If you have a better pattern that doesn't clash
> with REG_FMT please let me know.
You shouldn't be using such generic names for your internal identifiers,
add a prefix to everything.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2018-11-13 22:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-12 14:27 [RFC PATCH] spi: add driver for the SiFive SPI controller Emil Renner Berthing
2018-11-12 14:27 ` Emil Renner Berthing
2018-11-12 14:27 ` Emil Renner Berthing
2018-11-13 18:35 ` Mark Brown
2018-11-13 18:35 ` Mark Brown
2018-11-13 18:35 ` Mark Brown
2018-11-13 19:48 ` Emil Renner Berthing
2018-11-13 19:48 ` Emil Renner Berthing
2018-11-13 19:48 ` Emil Renner Berthing
2018-11-13 22:38 ` Mark Brown [this message]
2018-11-13 22:38 ` Mark Brown
2018-11-13 22:38 ` Mark Brown
2019-02-13 10:03 ` Yash Shah
2019-02-13 13:10 ` Emil Renner Berthing
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=20181113223822.GN2089@sirena.org.uk \
--to=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kernel@esmil.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=palmer@sifive.com \
--cc=robh+dt@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.