From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61FE4C433F5 for ; Tue, 11 Oct 2022 07:30:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229996AbiJKHaY (ORCPT ); Tue, 11 Oct 2022 03:30:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39100 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229770AbiJKHaX (ORCPT ); Tue, 11 Oct 2022 03:30:23 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE3843B97A for ; Tue, 11 Oct 2022 00:30:19 -0700 (PDT) Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=[127.0.0.1]) by metis.ext.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1oi9iB-0005L5-Ui; Tue, 11 Oct 2022 09:30:00 +0200 Message-ID: Date: Tue, 11 Oct 2022 09:29:55 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 From: Ahmad Fatoum Subject: Re: [PATCH v16 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager To: Ivan Bornyakov , mdf@kernel.org, hao.wu@intel.com, yilun.xu@intel.com, trix@redhat.com, dg@emlix.com, j.zink@pengutronix.de, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org Cc: devicetree@vger.kernel.org, system@metrotek.ru, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@pengutronix.de References: <20221010172734.26037-1-i.bornyakov@metrotek.ru> <20221010172734.26037-2-i.bornyakov@metrotek.ru> Content-Language: en-US In-Reply-To: <20221010172734.26037-2-i.bornyakov@metrotek.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: a.fatoum@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: devicetree@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hello Ivan, On 10.10.22 19:27, Ivan Bornyakov wrote: > Add support to the FPGA manager for programming Lattice ECP5 FPGA over > slave SPI sysCONFIG interface. > > sysCONFIG interface core functionality is separate from both ECP5 and > SPI specifics, so support for other FPGAs with different port types can > be added in the future. > > Signed-off-by: Ivan Bornyakov I found a small issue with the probe function, see below. While at it, I noted two nitpicks you could address. > +static int sysconfig_spi_bitstream_burst_init(struct sysconfig_priv *priv) > +{ > + const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST; > + struct spi_device *spi = to_spi_device(priv->dev); > + struct spi_transfer xfer = { 0 }; Nitpick: You want to zero all members. Using {} makes your intention clearer even if they are functionally equivalent. > +static int sysconfig_poll_busy(struct sysconfig_priv *priv) > +{ > + unsigned long timeout; > + int ret; > + > + timeout = jiffies + msecs_to_jiffies(SYSCONFIG_POLL_BUSY_TIMEOUT_MS); > + > + while (time_before(jiffies, timeout)) { > + ret = sysconfig_read_busy(priv); > + if (ret <= 0) > + return ret; > + > + usleep_range(SYSCONFIG_POLL_INTERVAL_US, > + SYSCONFIG_POLL_INTERVAL_US * 2); > + } Nitpick: I believe you could rewrite that using read_poll_timeout(). > +int sysconfig_probe(struct sysconfig_priv *priv) [snip] > + program = devm_gpiod_get_optional(dev, "program", GPIOD_OUT_LOW); > + if (IS_ERR(program)) { > + ret = PTR_ERR(program); > + dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret); > + return ret; This would print an error message for -EPROBE_DEFER, which just confuses users. Please use dev_err_probe instead here and elsewhere in the probe function to avoid this. Cheers, Ahmad -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |