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: 10+ 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-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 22:38 ` Mark Brown [this message] 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=linux-riscv@lists.infradead.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: linkBe 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).