Re: [PATCH 2/4] can: mcp251x: Improve mcp251x_hw_reset()

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

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

On 03/28/2014 11:14 AM, Alexander Shiyan wrote:
> The MCP251x utilizes an oscillator startup timer (OST), which holds the
> chip in reset, to insure that the oscillator has stabilized before the
> internal state machine begins to operate. The OST maintains reset for
> the first 128 OSC clock cycles after power up or reset.
> So, this patch removes unnecessary loops and reduce delay for power on
> and reset to the safe value.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  drivers/net/can/mcp251x.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
> index 26879c7..fea41b4 100644
> --- a/drivers/net/can/mcp251x.c
> +++ b/drivers/net/can/mcp251x.c
> @@ -214,6 +214,8 @@
>  
>  #define TX_ECHO_SKB_MAX	1
>  
> +#define MCP251X_OST_DELAY_MS	(5)
> +
>  #define DEVICE_NAME "mcp251x"
>  
>  static int mcp251x_enable_dma; /* Enable SPI DMA. Default: 0 (Off) */
> @@ -625,28 +627,20 @@ static int mcp251x_hw_reset(struct spi_device *spi)
>  {
>  	struct mcp251x_priv *priv = spi_get_drvdata(spi);
>  	int ret;
> -	unsigned long timeout;
> +
> +	/* Wait for oscillator startup timer after power up */
> +	mdelay(MCP251X_OST_DELAY_MS);
>  
>  	priv->spi_tx_buf[0] = INSTRUCTION_RESET;
> -	ret = spi_write(spi, priv->spi_tx_buf, 1);
> -	if (ret) {
> -		dev_err(&spi->dev, "reset failed: ret = %d\n", ret);
> -		return -EIO;
> -	}
> +	ret = mcp251x_spi_trans(spi, 1);
> +	if (ret)
> +		return ret;
>  
> -	/* Wait for reset to finish */
> -	timeout = jiffies + HZ;
> -	mdelay(10);
> -	while ((mcp251x_read_reg(spi, CANSTAT) & CANCTRL_REQOP_MASK)
> -	       != CANCTRL_REQOP_CONF) {
> -		schedule();
> -		if (time_after(jiffies, timeout)) {
> -			dev_err(&spi->dev, "MCP251x didn't"
> -				" enter in conf mode after reset\n");
> -			return -EBUSY;
> -		}
> -	}
> -	return 0;
> +	/* Wait for oscillator startup timer after reset */
> +	mdelay(MCP251X_OST_DELAY_MS);
> +	
> +	return ((mcp251x_read_reg(spi, CANSTAT) & CANCTRL_REQOP_MASK) ==
> +		CANCTRL_REQOP_CONF) ? 0 : -ENODEV;

I've converted this into a more standard/readable:

+       reg = mcp251x_read_reg(spi, CANSTAT);
+       if ((reg & CANCTRL_REQOP_MASK) != CANCTRL_REQOP_CONF)
+               return -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 --]