Re: [PATCH 1/4] can: mcp251x: Check return value of spi_setup()

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]

On 03/28/2014 11:14 AM, Alexander Shiyan wrote:
> This patch moves setup of SPI bus a bit earlier and adds check
> for spi_setup() result to be sure SPI bus is communicated with the
> device properly.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/net/can/mcp251x.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
> index 28c11f8..26879c7 100644
> --- a/drivers/net/can/mcp251x.c
> +++ b/drivers/net/can/mcp251x.c
> @@ -1032,8 +1032,8 @@ static int mcp251x_can_probe(struct spi_device *spi)
>  	struct mcp251x_platform_data *pdata = dev_get_platdata(&spi->dev);
>  	struct net_device *net;
>  	struct mcp251x_priv *priv;
> -	int freq, ret = -ENODEV;
>  	struct clk *clk;
> +	int freq, ret;
>  
>  	clk = devm_clk_get(&spi->dev, NULL);
>  	if (IS_ERR(clk)) {
> @@ -1076,6 +1076,18 @@ static int mcp251x_can_probe(struct spi_device *spi)
>  	priv->net = net;
>  	priv->clk = clk;
>  
> +	spi_set_drvdata(spi, priv);
> +
> +	/* Configure the SPI bus */
> +	spi->bits_per_word = 8;
> +	if (mcp251x_is_2510(spi))
> +		spi->max_speed_hz = spi->max_speed_hz ? : 5 * 1000 * 1000;
> +	else
> +		spi->max_speed_hz = spi->max_speed_hz ? : 10 * 1000 * 1000;
> +	ret = spi_setup(spi);
> +	if (ret)
> +		return ret;

You're omitting the error handling here. I'll send a v2.

> +
>  	priv->power = devm_regulator_get(&spi->dev, "vdd");
>  	priv->transceiver = devm_regulator_get(&spi->dev, "xceiver");
>  	if ((PTR_ERR(priv->power) == -EPROBE_DEFER) ||
> @@ -1088,8 +1100,6 @@ static int mcp251x_can_probe(struct spi_device *spi)
>  	if (ret)
>  		goto out_clk;
>  
> -	spi_set_drvdata(spi, priv);
> -
>  	priv->spi = spi;
>  	mutex_init(&priv->mcp_lock);
>  
> @@ -1134,15 +1144,6 @@ static int mcp251x_can_probe(struct spi_device *spi)
>  
>  	SET_NETDEV_DEV(net, &spi->dev);
>  
> -	/* Configure the SPI bus */
> -	spi->mode = spi->mode ? : SPI_MODE_0;
> -	if (mcp251x_is_2510(spi))
> -		spi->max_speed_hz = spi->max_speed_hz ? : 5 * 1000 * 1000;
> -	else
> -		spi->max_speed_hz = spi->max_speed_hz ? : 10 * 1000 * 1000;
> -	spi->bits_per_word = 8;
> -	spi_setup(spi);
> -
>  	/* Here is OK to not lock the MCP, no one knows about it yet */
>  	if (!mcp251x_hw_probe(spi)) {
>  		ret = -ENODEV;
> 

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]