From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754639AbbDNAZr (ORCPT ); Mon, 13 Apr 2015 20:25:47 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:36707 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751646AbbDNAZp (ORCPT ); Mon, 13 Apr 2015 20:25:45 -0400 Message-ID: <552C5DD9.2030001@vanguardiasur.com.ar> Date: Mon, 13 Apr 2015 21:22:49 -0300 From: Ezequiel Garcia Organization: VanguardiaSur User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Pelle Nilsson , broonie@kernel.org CC: nios2-dev@lists.rocketboards.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: Re: [Nios2-dev] [PATCH] spi: bitbang: Make setup_transfer callback optional References: <552B9045.4040709@xelmo.com> <1428919685-14190-1-git-send-email-per.nilsson@xelmo.com> In-Reply-To: <1428919685-14190-1-git-send-email-per.nilsson@xelmo.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Pelle, Just some minor nitpicks. On 04/13/2015 07:08 AM, Pelle Nilsson wrote: > spi-altera doesn't register this callback, and doesn't have a need for it, > but currently causes a NULL dereference when callback is called. This commit needs a Signed-off-by tag. Also, maybe you can avoid talking about spi-altera in the commit, since this change is not related to it. Instead you can explain that some SPI controllers don't register the callback (e.g. spi-altera) and so this commit makes it optional to prevent a NULL dereference. You explained in the cover that you are fixing commit 30af9b558a56 so you probably want to add a Fixes tag (Documentation/SubmittingPatches explains this procedure). The stable team will use the Fixes tag to backport the change if needed. > --- > drivers/spi/spi-bitbang.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/spi/spi-bitbang.c b/drivers/spi/spi-bitbang.c > index 5ef6638..d02057d 100644 > --- a/drivers/spi/spi-bitbang.c > +++ b/drivers/spi/spi-bitbang.c > @@ -180,7 +180,7 @@ int spi_bitbang_setup(struct spi_device *spi) > { > struct spi_bitbang_cs *cs = spi->controller_state; > struct spi_bitbang *bitbang; > - int retval; > + int retval = 0; > unsigned long flags; > > bitbang = spi_master_get_devdata(spi->master); > @@ -197,7 +197,8 @@ int spi_bitbang_setup(struct spi_device *spi) > if (!cs->txrx_word) > return -EINVAL; > > - retval = bitbang->setup_transfer(spi, NULL); > + if (bitbang->setup_transfer) retval is only used in this scope, so you can declare it only here: if (bitbang->setup_transfer) { int retval = bitbang->setup_transfer(spi, NULL); if (retval < 0) return retval; } > + retval = bitbang->setup_transfer(spi, NULL); > if (retval < 0) > return retval; > > @@ -295,9 +296,11 @@ static int spi_bitbang_transfer_one(struct spi_master *master, > > /* init (-1) or override (1) transfer params */ > if (do_setup != 0) { > - status = bitbang->setup_transfer(spi, t); > - if (status < 0) > - break; > + if (bitbang->setup_transfer) { > + status = bitbang->setup_transfer(spi, t); > + if (status < 0) > + break; > + } > if (do_setup == -1) > do_setup = 0; > } > Other than that, the commit looks good. -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar