From: Marek Vasut <marex@denx.de> To: Joel Holdsworth <joel@airwebreathe.org.uk>, atull@opensource.altera.com, moritz.fischer@ettus.com, geert@linux-m68k.org, robh@kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org, clifford@clifford.at Subject: Re: [PATCH v8 3/3] fpga: Add support for Lattice iCE40 FPGAs Date: Mon, 7 Nov 2016 19:01:12 +0100 [thread overview] Message-ID: <2255968d-b97c-2b9c-4e4d-7f3717a748e3@denx.de> (raw) In-Reply-To: <1478486962-26794-3-git-send-email-joel@airwebreathe.org.uk> On 11/07/2016 03:49 AM, Joel Holdsworth wrote: > The Lattice iCE40 is a family of FPGAs with a minimalistic architecture > and very regular structure, designed for low-cost, high-volume consumer > and system applications. [...] > +static int ice40_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > + const char *buf, size_t count) > +{ > + struct ice40_fpga_priv *priv = mgr->priv; > + struct spi_device *dev = priv->dev; > + struct spi_message message; > + struct spi_transfer assert_cs_then_reset_delay = {.cs_change = 1, > + .delay_usecs = ICE40_SPI_FPGAMGR_RESET_DELAY}; Should be this way for the sake of readability, fix globally: struct spi_transfer assert_cs_then_reset_delay = { .cs_change = 1, .delay_usecs = ICE40_SPI_FPGAMGR_RESET_DELAY }; Also I believe this could be const. > + struct spi_transfer housekeeping_delay_then_release_cs = { > + .delay_usecs = ICE40_SPI_FPGAMGR_HOUSEKEEPING_DELAY}; Don't we have some less hacky way of toggling the nCS ? Is this even nCS or is this some control pin of the FPGA ? Maybe it should be treated like a regular GPIO instead ? [...] > +static int ice40_fpga_ops_write_complete(struct fpga_manager *mgr, u32 flags) > +{ > + struct ice40_fpga_priv *priv = mgr->priv; > + struct spi_device *dev = priv->dev; > + const u8 padding[ICE40_SPI_FPGAMGR_NUM_ACTIVATION_BYTES] = {0,}; The comma is not needed. > + > + /* Check CDONE is asserted */ > + if (!gpiod_get_value(priv->cdone)) { > + dev_err(&dev->dev, > + "CDONE was not asserted after firmware transfer\n"); > + return -EIO; > + } > + > + /* Send of zero-padding to activate the firmware */ > + return spi_write(dev, padding, sizeof(padding)); > +} [...] > + /* Check board setup data. */ > + if (spi->max_speed_hz > 25000000) { > + dev_err(dev, "Speed is too high\n"); > + return -EINVAL; > + } > + > + if (spi->max_speed_hz < 1000000) { > + dev_err(dev, "Speed is too low\n"); > + return -EINVAL; > + } Do you have some explanation for this limitation ? [...] -- Best regards, Marek Vasut
WARNING: multiple messages have this Message-ID (diff)
From: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> To: Joel Holdsworth <joel-IJEoVVyKhCJXvIrf17iDB/XRex20P6io@public.gmane.org>, atull-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org, moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org, geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org, robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, clifford-cPpHkPqGOEfk7+2FdBfRIA@public.gmane.org Subject: Re: [PATCH v8 3/3] fpga: Add support for Lattice iCE40 FPGAs Date: Mon, 7 Nov 2016 19:01:12 +0100 [thread overview] Message-ID: <2255968d-b97c-2b9c-4e4d-7f3717a748e3@denx.de> (raw) In-Reply-To: <1478486962-26794-3-git-send-email-joel-IJEoVVyKhCJXvIrf17iDB/XRex20P6io@public.gmane.org> On 11/07/2016 03:49 AM, Joel Holdsworth wrote: > The Lattice iCE40 is a family of FPGAs with a minimalistic architecture > and very regular structure, designed for low-cost, high-volume consumer > and system applications. [...] > +static int ice40_fpga_ops_write_init(struct fpga_manager *mgr, u32 flags, > + const char *buf, size_t count) > +{ > + struct ice40_fpga_priv *priv = mgr->priv; > + struct spi_device *dev = priv->dev; > + struct spi_message message; > + struct spi_transfer assert_cs_then_reset_delay = {.cs_change = 1, > + .delay_usecs = ICE40_SPI_FPGAMGR_RESET_DELAY}; Should be this way for the sake of readability, fix globally: struct spi_transfer assert_cs_then_reset_delay = { .cs_change = 1, .delay_usecs = ICE40_SPI_FPGAMGR_RESET_DELAY }; Also I believe this could be const. > + struct spi_transfer housekeeping_delay_then_release_cs = { > + .delay_usecs = ICE40_SPI_FPGAMGR_HOUSEKEEPING_DELAY}; Don't we have some less hacky way of toggling the nCS ? Is this even nCS or is this some control pin of the FPGA ? Maybe it should be treated like a regular GPIO instead ? [...] > +static int ice40_fpga_ops_write_complete(struct fpga_manager *mgr, u32 flags) > +{ > + struct ice40_fpga_priv *priv = mgr->priv; > + struct spi_device *dev = priv->dev; > + const u8 padding[ICE40_SPI_FPGAMGR_NUM_ACTIVATION_BYTES] = {0,}; The comma is not needed. > + > + /* Check CDONE is asserted */ > + if (!gpiod_get_value(priv->cdone)) { > + dev_err(&dev->dev, > + "CDONE was not asserted after firmware transfer\n"); > + return -EIO; > + } > + > + /* Send of zero-padding to activate the firmware */ > + return spi_write(dev, padding, sizeof(padding)); > +} [...] > + /* Check board setup data. */ > + if (spi->max_speed_hz > 25000000) { > + dev_err(dev, "Speed is too high\n"); > + return -EINVAL; > + } > + > + if (spi->max_speed_hz < 1000000) { > + dev_err(dev, "Speed is too low\n"); > + return -EINVAL; > + } Do you have some explanation for this limitation ? [...] -- Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-11-07 18:21 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-11-07 2:49 [PATCH v8 1/3] of: Add vendor prefix for Lattice Semiconductor Joel Holdsworth 2016-11-07 2:49 ` Joel Holdsworth 2016-11-07 2:49 ` [PATCH v8 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager Joel Holdsworth 2016-11-07 17:53 ` Marek Vasut 2016-11-07 17:53 ` Marek Vasut 2016-11-07 18:57 ` Joel Holdsworth 2016-11-14 16:11 ` Rob Herring 2016-11-18 18:56 ` atull 2016-11-18 18:56 ` atull 2016-11-18 19:17 ` Moritz Fischer 2016-11-18 19:17 ` Moritz Fischer 2016-11-18 19:28 ` Marek Vasut 2016-11-18 19:28 ` Marek Vasut 2016-11-07 2:49 ` [PATCH v8 3/3] fpga: Add support for Lattice iCE40 FPGAs Joel Holdsworth 2016-11-07 2:49 ` Joel Holdsworth 2016-11-07 18:01 ` Marek Vasut [this message] 2016-11-07 18:01 ` Marek Vasut 2016-11-07 18:49 ` Joel Holdsworth 2016-11-07 18:49 ` Joel Holdsworth 2016-11-07 18:53 ` Marek Vasut 2016-11-08 17:06 ` Moritz Fischer 2016-11-08 17:30 ` Joel Holdsworth 2016-11-08 17:30 ` Joel Holdsworth 2016-11-09 12:01 ` Marek Vasut 2016-11-09 18:37 ` Joel Holdsworth 2016-11-09 18:39 ` Marek Vasut 2016-11-09 18:54 ` Joel Holdsworth 2016-11-09 18:54 ` Joel Holdsworth 2016-11-10 12:11 ` Marek Vasut 2016-11-10 12:11 ` Marek Vasut 2016-11-08 17:13 ` Joel Holdsworth 2016-11-08 17:13 ` Joel Holdsworth 2016-11-07 18:26 ` Moritz Fischer 2016-11-07 18:26 ` Moritz Fischer 2016-11-07 19:02 ` Joel Holdsworth 2016-11-07 19:02 ` Joel Holdsworth 2016-11-07 21:41 ` Moritz Fischer 2016-11-07 21:41 ` Moritz Fischer
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=2255968d-b97c-2b9c-4e4d-7f3717a748e3@denx.de \ --to=marex@denx.de \ --cc=atull@opensource.altera.com \ --cc=clifford@clifford.at \ --cc=devicetree@vger.kernel.org \ --cc=geert@linux-m68k.org \ --cc=joel@airwebreathe.org.uk \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-spi@vger.kernel.org \ --cc=moritz.fischer@ettus.com \ --cc=robh@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: linkBe 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.