lkml.org 
[lkml]   [2015]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [Nios2-dev] [PATCH] spi: bitbang: Make setup_transfer callback optional
    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


    \
     
     \ /
      Last update: 2015-04-14 03:01    [W:2.459 / U:0.344 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site