[PATCHv2] Fix CAN on socfpga, for net/master
Marc Kleine-Budde
mkl at pengutronix.de
Fri May 2 05:27:03 PDT 2014
On 05/02/2014 10:48 AM, Pavel Machek wrote:
> Apparently, socfpga CAN needs 32-bit accesses and different raminit
> sequence.
Please split into several patches:
- add 32 bit accessor functions
and replace two 16 bit accesses by a single 32 bit access
- if-DCAN-then-32bit access
please explain in the commit message why this is needed
- add hwinit support for non ti devices
>
> Tested-by: Thor Thayer <tthayer at altera.com>
> Signed-off-by: Thor Thayer <tthayer at altera.com>
> Signed-off-by: Pavel Machek <pavel at denx.de>
>
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index a2ca820..824ec21 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -48,6 +48,7 @@
> #define C_CAN_IFACE(reg, iface) (C_CAN_IF1_##reg + (iface) * IF_ENUM_REG_LEN)
>
> /* control extension register D_CAN specific */
> +#define CONTROL_MIL BIT(17)
> #define CONTROL_EX_PDR BIT(8)
>
> /* control register */
> @@ -239,12 +240,20 @@ static inline void c_can_reset_ram(const struct c_can_priv *priv, bool enable)
>
> static void c_can_irq_control(struct c_can_priv *priv, bool enable)
> {
> - u32 ctrl = priv->read_reg(priv, C_CAN_CTRL_REG) & ~CONTROL_IRQMSK;
> + u32 ctrl;
> +
> + if (priv->type == BOSCH_D_CAN)
> + ctrl = priv->read_reg32(priv, C_CAN_CTRL_REG) & ~CONTROL_IRQMSK;
> + else
> + ctrl = priv->read_reg(priv, C_CAN_CTRL_REG) & ~CONTROL_IRQMSK;
>
> if (enable)
> ctrl |= CONTROL_IRQMSK;
>
> - priv->write_reg(priv, C_CAN_CTRL_REG, ctrl);
> + if (priv->type == BOSCH_D_CAN)
> + priv->write_reg32(priv, C_CAN_CTRL_REG, ctrl);
> + else
> + priv->write_reg(priv, C_CAN_CTRL_REG, ctrl);
> }
>
> static void c_can_obj_update(struct net_device *dev, int iface, u32 cmd, u32 obj)
> @@ -252,8 +261,7 @@ static void c_can_obj_update(struct net_device *dev, int iface, u32 cmd, u32 obj
> struct c_can_priv *priv = netdev_priv(dev);
> int cnt, reg = C_CAN_IFACE(COMREQ_REG, iface);
>
> - priv->write_reg(priv, reg + 1, cmd);
> - priv->write_reg(priv, reg, obj);
> + priv->write_reg32(priv, reg, (cmd << 16) | obj);
>
> for (cnt = MIN_TIMEOUT_VALUE; cnt; cnt--) {
> if (!(priv->read_reg(priv, reg) & IF_COMR_BUSY))
> @@ -328,8 +336,7 @@ static void c_can_setup_tx_object(struct net_device *dev, int iface,
> change_bit(idx, &priv->tx_dir);
> }
>
> - priv->write_reg(priv, C_CAN_IFACE(ARB1_REG, iface), arb);
> - priv->write_reg(priv, C_CAN_IFACE(ARB2_REG, iface), arb >> 16);
> + priv->write_reg32(priv, C_CAN_IFACE(ARB1_REG, iface), arb);
>
> priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), ctrl);
>
> @@ -391,8 +398,7 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, u32 ctrl)
>
> frame->can_dlc = get_can_dlc(ctrl & 0x0F);
>
> - arb = priv->read_reg(priv, C_CAN_IFACE(ARB1_REG, iface));
> - arb |= priv->read_reg(priv, C_CAN_IFACE(ARB2_REG, iface)) << 16;
> + arb = priv->read_reg32(priv, C_CAN_IFACE(ARB1_REG, iface));
>
> if (arb & IF_ARB_MSGXTD)
> frame->can_id = (arb & CAN_EFF_MASK) | CAN_EFF_FLAG;
> @@ -424,12 +430,10 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
> struct c_can_priv *priv = netdev_priv(dev);
>
> mask |= BIT(29);
> - priv->write_reg(priv, C_CAN_IFACE(MASK1_REG, iface), mask);
> - priv->write_reg(priv, C_CAN_IFACE(MASK2_REG, iface), mask >> 16);
> + priv->write_reg32(priv, C_CAN_IFACE(MASK1_REG, iface), mask);
>
> id |= IF_ARB_MSGVAL;
> - priv->write_reg(priv, C_CAN_IFACE(ARB1_REG, iface), id);
> - priv->write_reg(priv, C_CAN_IFACE(ARB2_REG, iface), id >> 16);
> + priv->write_reg32(priv, C_CAN_IFACE(ARB1_REG, iface), id);
>
> priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), mcont);
> c_can_object_put(dev, iface, obj, IF_COMM_RCV_SETUP);
> @@ -510,16 +514,22 @@ static int c_can_set_bittiming(struct net_device *dev)
> netdev_info(dev,
> "setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
>
> - ctrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
> + if (priv->type == BOSCH_D_CAN)
> + ctrl_save = priv->read_reg32(priv, C_CAN_CTRL_REG);
> + else
> + ctrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
> ctrl_save &= ~CONTROL_INIT;
> - priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_CCE | CONTROL_INIT);
> + priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_CCE | CONTROL_INIT); /* FIXME: want to do write32? */
> res = c_can_wait_for_ctrl_init(dev, priv, CONTROL_INIT);
> if (res)
> return res;
>
> priv->write_reg(priv, C_CAN_BTR_REG, reg_btr);
> priv->write_reg(priv, C_CAN_BRPEXT_REG, reg_brpe);
> - priv->write_reg(priv, C_CAN_CTRL_REG, ctrl_save);
> + if (priv->type == BOSCH_D_CAN)
> + priv->write_reg32(priv, C_CAN_CTRL_REG, ctrl_save);
> + else
> + priv->write_reg(priv, C_CAN_CTRL_REG, ctrl_save);
>
> return c_can_wait_for_ctrl_init(dev, priv, 0);
> }
> @@ -1240,7 +1250,7 @@ int c_can_power_up(struct net_device *dev)
>
> /* Clear PDR and INIT bits */
> val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
> - val &= ~CONTROL_EX_PDR;
> + val &= ~(CONTROL_EX_PDR | CONTROL_MIL);
> priv->write_reg(priv, C_CAN_CTRL_EX_REG, val);
> val = priv->read_reg(priv, C_CAN_CTRL_REG);
> val &= ~CONTROL_INIT;
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index c56f1b1..10c3fd2 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -78,6 +78,7 @@ enum reg {
> C_CAN_INTPND2_REG,
> C_CAN_MSGVAL1_REG,
> C_CAN_MSGVAL2_REG,
> + C_CAN_FUNCTION_REG,
> };
>
> static const u16 reg_map_c_can[] = {
> @@ -129,6 +130,7 @@ static const u16 reg_map_d_can[] = {
> [C_CAN_BRPEXT_REG] = 0x0E,
> [C_CAN_INT_REG] = 0x10,
> [C_CAN_TEST_REG] = 0x14,
> + [C_CAN_FUNCTION_REG] = 0x18,
> [C_CAN_TXRQST1_REG] = 0x88,
> [C_CAN_TXRQST2_REG] = 0x8A,
> [C_CAN_NEWDAT1_REG] = 0x9C,
> @@ -188,6 +190,8 @@ struct c_can_priv {
> u32 comm_rcv_high;
> u32 rxmasked;
> u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
> + u32 (*read_reg32) (struct c_can_priv *priv, enum reg index);
> + void (*write_reg32) (struct c_can_priv *priv, enum reg index, u32 val);
> };
>
> struct net_device *alloc_c_can_dev(void);
> 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 change the commenting style.
> * 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);
> +
> + ctrl = readl(priv->raminit_ctrlreg);
As far as I can see raminit_ctrlreg is in this case a non shared
resource, so you don't need any spinlock. Also you don't need to use
raminit_ctrlreg, as it is C_CAN_FUNCTION_REG.
> + ctrl &= ~DCAN_RAM_INIT_BIT;
> + writel(ctrl, priv->raminit_ctrlreg);
> + 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;
> +}
> +
> +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);
^ ^
Please add spaces around the >>
> + 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 |
More information about the linux-arm-kernel
mailing list