[PATCH 1/2] hwinit support for non-TI devices

[mkl@pengutronix.de (Marc Kleine-Budde)]

On 05/05/2014 02:08 PM, Pavel Machek wrote:
> Non-TI chips (including socfpga) needs different raminit
> sequence. Implement it.
> 
> Tested-by: Thor Thayer <tthayer@altera.com>
> Signed-off-by: Thor Thayer <tthayer@altera.com>
> Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 1df0b32..476d1e0 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -40,6 +40,7 @@
>  #define CAN_RAMINIT_START_MASK(i)	(0x001 << (i))
>  #define CAN_RAMINIT_DONE_MASK(i)	(0x100 << (i))
>  #define CAN_RAMINIT_ALL_MASK(i)		(0x101 << (i))
> +#define DCAN_RAM_INIT_BIT	(1 << 3)
>  static DEFINE_SPINLOCK(raminit_lock);
>  /*
>   * 16-bit c_can registers can be arranged differently in the memory
> @@ -80,7 +81,7 @@ static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask,
>  		udelay(1);
>  }
>  
> -static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
> +static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
>  {
>  	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
>  	u32 ctrl;
> @@ -88,7 +89,8 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
>  	spin_lock(&raminit_lock);
>  
>  	ctrl = readl(priv->raminit_ctrlreg);
> -	/* We clear the done and start bit first. The start bit is
> +	/*
> +	 * We clear the done and start bit first. The start bit is

Please don't reformat comments.

>  	 * looking at the 0 -> transition, but is not self clearing;
>  	 * And we clear the init done bit as well.
>  	 */
> @@ -108,6 +110,54 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
>  	spin_unlock(&raminit_lock);
>  }
>  
> +static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
> +{
> +	u32 ctrl;
> +
> +	spin_lock(&raminit_lock);

Why do you need this spinlock?

> +
> +	ctrl = readl(priv->raminit_ctrlreg);
> +	ctrl &= ~DCAN_RAM_INIT_BIT;
> +	writel(ctrl, priv->raminit_ctrlreg);

Why don't use use the reg directly? Have you read my previous review?

> +	c_can_hw_raminit_wait(priv, ctrl, 0);
> +
> +	if (enable) {
> +		/* Set start bit. */
> +		ctrl |= DCAN_RAM_INIT_BIT;
> +		writel(ctrl, priv->raminit_ctrlreg);
> +		c_can_hw_raminit_wait(priv, ctrl, 0);
> +	}
> +	spin_unlock(&raminit_lock);
> +}
> +
> +static u32 c_can_plat_read_reg32(struct c_can_priv *priv, enum reg index)
> +{
> +	u32 val;
> +
> +	val = priv->read_reg(priv, index);
> +	val |= ((u32) priv->read_reg(priv, index + 1)) << 16;
> +
> +	return val;
> +}

Why are you adding the read32() support here? Your commit message
doesn't mention it. Please move this into a different patch.

> +
> +static void c_can_plat_write_reg32(struct c_can_priv *priv, enum reg index,
> +		u32 val)
> +{
> +	priv->write_reg(priv, index + 1, val>>16);
> +	priv->write_reg(priv, index, val);
> +}
> +
> +static u32 d_can_plat_read_reg32(struct c_can_priv *priv, enum reg index)
> +{
> +	return readl(priv->base + priv->regs[index]);
> +}
> +
> +static void d_can_plat_write_reg32(struct c_can_priv *priv, enum reg index,
> +		u32 val)
> +{
> +	writel(val, priv->base + priv->regs[index]);
> +}
> +
>  static struct platform_device_id c_can_id_table[] = {
>  	[BOSCH_C_CAN_PLATFORM] = {
>  		.name = KBUILD_MODNAME,
> @@ -201,11 +251,15 @@ static int c_can_plat_probe(struct platform_device *pdev)
>  		case IORESOURCE_MEM_32BIT:
>  			priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
>  			priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> +			priv->read_reg32 = c_can_plat_read_reg32;
> +			priv->write_reg32 = c_can_plat_write_reg32;
>  			break;
>  		case IORESOURCE_MEM_16BIT:
>  		default:
>  			priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
>  			priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> +			priv->read_reg32 = c_can_plat_read_reg32;
> +			priv->write_reg32 = c_can_plat_write_reg32;
>  			break;
>  		}
>  		break;
> @@ -214,6 +268,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
>  		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
>  		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
>  		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> +		priv->read_reg32 = d_can_plat_read_reg32;
> +		priv->write_reg32 = d_can_plat_write_reg32;
>  
>  		if (pdev->dev.of_node)
>  			priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
> @@ -221,11 +277,22 @@ static int c_can_plat_probe(struct platform_device *pdev)
>  			priv->instance = pdev->id;
>  
>  		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +		/*
> +		 * Not all D_CAN module have a separate register for the D_CAN
> +		 * RAM initialization. Use default RAM init bit in D_CAN module
> +		 * if not specified in DT.
> +		 */
> +		if (!res) {
> +			priv->raminit = c_can_hw_raminit;
> +			priv->raminit_ctrlreg = addr + priv->regs[C_CAN_FUNCTION_REG];
> +			break;
> +		}
> +
>  		priv->raminit_ctrlreg = devm_ioremap_resource(&pdev->dev, res);
>  		if (IS_ERR(priv->raminit_ctrlreg) || priv->instance < 0)
>  			dev_info(&pdev->dev, "control memory is not used for raminit\n");
>  		else
> -			priv->raminit = c_can_hw_raminit;
> +			priv->raminit = c_can_hw_raminit_ti;
>  		break;
>  	default:
>  		ret = -EINVAL;
> 

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   |