spi: bitbang: Make setup_transfer callback optional

Commit Message

Pelle Nilsson April 13, 2015, 10:08 a.m. UTC

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.
---
 drivers/spi/spi-bitbang.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Ezequiel Garcia April 14, 2015, 12:22 a.m. UTC | #1

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.

Patch

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 = 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;
 		}