can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Pavel Machek]

Hi!

> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> I have had similar patch here, except that it only listed 2 interrupts
> for each d_can controller.
> 
> Otherwise it looks identical.
> 
> Reviewed-by: Pavel Machek <pavel@denx.de>

Actually, does it work for you? Here it produces warnings during boot

c_can_platform ffc00000.d_can: invalid resource
c_can_platform ffc00000.d_can: control memory is not used for raminit
c_can_platform ffc00000.d_can: c_can_platform device registered
(regs=9085c000, irq=163)
...
can: controller area network core (rev 20120528 abi 9)
NET: Registered protocol family 29
can: raw protocol (rev 20120528)
can: broadcast manager protocol (rev 20120528 t)
can: netlink gateway (rev 20130117) max_hops=1

. Then can0 is correctly registered, and seems to even receive
packets, but only when enter is pressed on serial console... which is
kind of weird. I suspected irq problems, but same symptomps with
"irqpoll" -- it leads to generate 2000 irqs/second, but still waits
for enter.

[Aha, and we have just two interrupts listed, but I don't think that
explains all the problems. Unfortunately, that particular target uses
human for remote power switch...].

Any ideas?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Dinh Nguyen]

On Fri, 2014-04-25 at 21:53 +0200, ZY - pavel wrote:
> Hi!
> 
> > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > 
> > I have had similar patch here, except that it only listed 2 interrupts
> > for each d_can controller.
> > 
> > Otherwise it looks identical.
> > 
> > Reviewed-by: Pavel Machek <pavel@denx.de>
> 
> Actually, does it work for you? Here it produces warnings during boot
> 
> c_can_platform ffc00000.d_can: invalid resource
> c_can_platform ffc00000.d_can: control memory is not used for raminit
> c_can_platform ffc00000.d_can: c_can_platform device registered
> (regs=9085c000, irq=163)
> ...
> can: controller area network core (rev 20120528 abi 9)
> NET: Registered protocol family 29
> can: raw protocol (rev 20120528)
> can: broadcast manager protocol (rev 20120528 t)
> can: netlink gateway (rev 20130117) max_hops=1
> 
> . Then can0 is correctly registered, and seems to even receive
> packets, but only when enter is pressed on serial console... which is
> kind of weird. I suspected irq problems, but same symptomps with
> "irqpoll" -- it leads to generate 2000 irqs/second, but still waits
> for enter.
> 
> [Aha, and we have just two interrupts listed, but I don't think that
> explains all the problems. Unfortunately, that particular target uses
> human for remote power switch...].
> 
> Any ideas?

CC'ing Thor Thayer. He's done the CAN testing on the SOCFPGA platform.

Dinh
> 
> Thanks,
> 									Pavel

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Thor Thayer]

On Fri, 2014-04-25 at 15:24 -0500, Dinh Nguyen wrote:
> On Fri, 2014-04-25 at 21:53 +0200, ZY - pavel wrote:
> > Hi!
> > 
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > 
> > > I have had similar patch here, except that it only listed 2 interrupts
> > > for each d_can controller.
> > > 
> > > Otherwise it looks identical.
> > > 
> > > Reviewed-by: Pavel Machek <pavel@denx.de>
> > 
> > Actually, does it work for you? Here it produces warnings during boot
> > 
> > c_can_platform ffc00000.d_can: invalid resource
> > c_can_platform ffc00000.d_can: control memory is not used for raminit
> > c_can_platform ffc00000.d_can: c_can_platform device registered
> > (regs=9085c000, irq=163)

Hi! Yes, these warnings are being addressed in a patch I submitted that
is currently being reviewed.
(http://article.gmane.org/gmane.linux.can/5620/match=tthayer)

> > ...
> > can: controller area network core (rev 20120528 abi 9)
> > NET: Registered protocol family 29
> > can: raw protocol (rev 20120528)
> > can: broadcast manager protocol (rev 20120528 t)
> > can: netlink gateway (rev 20130117) max_hops=1
> > 
> > . Then can0 is correctly registered, and seems to even receive
> > packets, but only when enter is pressed on serial console... which is
> > kind of weird. I suspected irq problems, but same symptomps with
> > "irqpoll" -- it leads to generate 2000 irqs/second, but still waits
> > for enter.
> > 
> > [Aha, and we have just two interrupts listed, but I don't think that
> > explains all the problems. Unfortunately, that particular target uses
> > human for remote power switch...].
> > 
> > Any ideas?
> 
> CC'ing Thor Thayer. He's done the CAN testing on the SOCFPGA platform.
> 
> Dinh

To get this working well, I had to install a few of the patches that
Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access for
D_CAN) on 9/9/2013.

I have the patches on our rocketboard branch
(rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)

I planned to upstream these changes but there have been some major
changes to CAN recently that may require some refactoring.

Thor

> > 
> > Thanks,
> > 									Pavel
> 
> 

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Pavel Machek]

Hi!

> > > Actually, does it work for you? Here it produces warnings during boot
> > > 
> > > c_can_platform ffc00000.d_can: invalid resource
> > > c_can_platform ffc00000.d_can: control memory is not used for raminit
> > > c_can_platform ffc00000.d_can: c_can_platform device registered
> > > (regs=9085c000, irq=163)
> 
> Hi! Yes, these warnings are being addressed in a patch I submitted that
> is currently being reviewed.
> (http://article.gmane.org/gmane.linux.can/5620/match=tthayer)

That will still produce the 

c_can_platform ffc00000.d_can: invalid resource

message, no? Anyway, I did something along those lines,
too. (untested). I wonder if raminit_type is a good idea... I believe
it would be nicer to just use separate raminit function...

> To get this working well, I had to install a few of the patches that
> Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access for
> D_CAN) on 9/9/2013.

> I have the patches on our rocketboard branch
> (rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)

Thanks a lot for the pointers. I knew about these, but then forgot.

Best regards,
 									Pavel


[This is what I did for debugging.]

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 5f2c8a4..4bc48cc 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -454,7 +454,7 @@
 		dcan0: d_can@ffc00000 {
 			compatible = "bosch,d_can";
 			reg = <0xffc00000 0x1000>;
-			interrupts = <0 131 4>, <0 132 4>;
+			interrupts = <0 131 4>, <0 132 4>, <0 133 4>, <0 134 4>;
 			clocks = <&can0_clk>;
 			status = "disabled";
 		};
@@ -462,7 +462,7 @@
 		dcan1: d_can@ffc01000 {
 			compatible = "bosch,d_can";
 			reg = <0xffc01000 0x1000>;
-			interrupts = <0 135 4>, <0 136 4>;
+			interrupts = <0 135 4>, <0 136 4>, <0 137 4>, <0 138 4>;
 			clocks = <&can1_clk>;
 			status = "disabled";
 		};
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 806d927..6d9514d 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -80,6 +80,15 @@ static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask,
 		udelay(1);
 }
 
+/* socfpga has CFR at offset 0x18, bit 0x08 is "request automatic ram init". Needed?
+   CCTRL.1 bit needs to be set for this.
+
+
+   CRR at offset 0x20 should contain revision information.
+
+   HWS contains information about # of objects
+*/
+
 static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
 {
 	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
@@ -88,7 +97,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
 	 * looking at the 0 -> transition, but is not self clearing;
 	 * And we clear the init done bit as well.
 	 */
@@ -193,6 +203,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
 		goto exit_iounmap;
 	}
 
+	printk("net device\n");
+
 	priv = netdev_priv(dev);
 	switch (id->driver_data) {
 	case BOSCH_C_CAN:
@@ -220,11 +232,26 @@ static int c_can_plat_probe(struct platform_device *pdev)
 		else
 			priv->instance = pdev->id;
 
+		printk("d_can: get resource\n");
 		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		printk("d_can: ioremap_resource\n");
 		priv->raminit_ctrlreg = devm_ioremap_resource(&pdev->dev, res);
-		if (IS_ERR(priv->raminit_ctrlreg) || (int)priv->instance < 0)
+		if (IS_ERR(priv->raminit_ctrlreg) || (int)priv->instance < 0) {
+			u32 version;
 			dev_info(&pdev->dev, "control memory is not used for raminit\n");
-		else
+			
+#if 0
+			version = readl(addr + 0x20); /* CRR */
+			printk("Version, should be 11161128: %lx\n", version);
+			/* CFR */
+			writel(8, addr + 0x18);
+			if (!(readl(addr + 0x18) & 8))
+				printk("Not initializing?\n");
+
+			while (readl(addr + 0x18) & 8)
+				printk("Waiting for init...\n");
+#endif			
+		} else
 			priv->raminit = c_can_hw_raminit;
 		break;
 	default:
@@ -242,6 +269,8 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
+	printk("..register\n");
+
 	ret = register_c_can_dev(dev);
 	if (ret) {
 		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Pavel Machek]

Hi!

> Hi! Yes, these warnings are being addressed in a patch I submitted that
> is currently being reviewed.
> (http://article.gmane.org/gmane.linux.can/5620/match=tthayer)

What about this patch, instead? It should provide same functionality,
but does not do hard-to-understand operations with 0s and ~0s, it more
obviously does not affect the TI version, and it still manages to add
less lines of code. (But I was not yet able to test it.)

Thanks,
								Pavel

---

    Add support to standard D_CAN RAM Init, as opposed to TI.
    
    Partly based on patch by Thor Thayer <tthayer@altera.com>
    
    Signed-off-by: Pavel Machek <pavel@denx.de>

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index faa8404..d436734 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -88,6 +88,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[] = {
@@ -139,6 +140,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,
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 806d927..fba168f 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
 	 * looking at the 0 -> transition, but is not self clearing;
 	 * And we clear the init done bit as well.
 	 */
@@ -108,6 +110,26 @@ 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);
+	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 struct platform_device_id c_can_id_table[] = {
 	[BOSCH_C_CAN_PLATFORM] = {
 		.name = KBUILD_MODNAME,
@@ -221,11 +243,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) || (int)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;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Pavel Machek]

Hi!

> To get this working well, I had to install a few of the patches that
> Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access for
> D_CAN) on 9/9/2013.
> 
> I have the patches on our rocketboard branch
> (rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)
> 
> I planned to upstream these changes but there have been some major
> changes to CAN recently that may require some refactoring.

I ported those changes to 3.15-rc2 (but can't test them at the
moment).

Signed-off-by: Pavel Machek <pavel@denx.de>

Best regards,
								Pavel

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index a5c8dcf..3ad0bd4 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 */
@@ -237,25 +238,25 @@ static inline int get_tx_echo_msg_obj(int txecho)
 	return (txecho & C_CAN_NEXT_MSG_OBJ_MASK) + C_CAN_MSG_OBJ_TX_FIRST;
 }
 
-static u32 c_can_read_reg32(struct c_can_priv *priv, enum reg index)
-{
-	u32 val = priv->read_reg(priv, index);
-	val |= ((u32) priv->read_reg(priv, index + 1)) << 16;
-	return val;
-}
-
 static void c_can_enable_all_interrupts(struct c_can_priv *priv,
 						int enable)
 {
-	unsigned int cntrl_save = priv->read_reg(priv,
-						C_CAN_CTRL_REG);
+	u32 cntrl_save;
+
+	if (priv->type == BOSCH_D_CAN)
+		cntrl_save = priv->read_reg32(priv, C_CAN_CTRL_REG);
+	else
+		cntrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
 
 	if (enable)
 		cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
 	else
 		cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
 
-	priv->write_reg(priv, C_CAN_CTRL_REG, cntrl_save);
+	if (priv->type == BOSCH_D_CAN)
+		priv->write_reg32(priv, C_CAN_CTRL_REG, cntrl_save);
+	else
+		priv->write_reg(priv, C_CAN_CTRL_REG, cntrl_save);
 }
 
 static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
@@ -286,9 +287,8 @@ static inline void c_can_object_get(struct net_device *dev,
 	 * register and message RAM must be complete in 6 CAN-CLK
 	 * period.
 	 */
-	priv->write_reg(priv, C_CAN_IFACE(COMMSK_REG, iface),
-			IFX_WRITE_LOW_16BIT(mask));
-	priv->write_reg(priv, C_CAN_IFACE(COMREQ_REG, iface),
+	priv->write_reg32(priv, C_CAN_IFACE(COMREQ_REG, iface),
+			(IFX_WRITE_LOW_16BIT(mask) << 16) |
 			IFX_WRITE_LOW_16BIT(objno));
 
 	if (c_can_msg_obj_is_busy(priv, iface))
@@ -306,9 +306,8 @@ static inline void c_can_object_put(struct net_device *dev,
 	 * register and message RAM must be complete in 6 CAN-CLK
 	 * period.
 	 */
-	priv->write_reg(priv, C_CAN_IFACE(COMMSK_REG, iface),
-			(IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
-	priv->write_reg(priv, C_CAN_IFACE(COMREQ_REG, iface),
+	priv->write_reg32(priv, C_CAN_IFACE(COMREQ_REG, iface),
+			((IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)) << 16) |
 			IFX_WRITE_LOW_16BIT(objno));
 
 	if (c_can_msg_obj_is_busy(priv, iface))
@@ -334,10 +333,8 @@ static void c_can_write_msg_object(struct net_device *dev,
 
 	flags |= IF_ARB_MSGVAL;
 
-	priv->write_reg(priv, C_CAN_IFACE(ARB1_REG, iface),
-				IFX_WRITE_LOW_16BIT(id));
-	priv->write_reg(priv, C_CAN_IFACE(ARB2_REG, iface), flags |
-				IFX_WRITE_HIGH_16BIT(id));
+	priv->write_reg32(priv, C_CAN_IFACE(ARB1_REG, iface),
+				id | (flags << 16));
 
 	for (i = 0; i < frame->can_dlc; i += 2) {
 		priv->write_reg(priv, C_CAN_IFACE(DATA1_REG, iface) + i / 2,
@@ -442,25 +439,24 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->write_reg(priv, C_CAN_IFACE(MASK1_REG, iface),
-			IFX_WRITE_LOW_16BIT(mask));
-
 	/* According to C_CAN documentation, the reserved bit
 	 * in IFx_MASK2 register is fixed 1
 	 */
-	priv->write_reg(priv, C_CAN_IFACE(MASK2_REG, iface),
-			IFX_WRITE_HIGH_16BIT(mask) | BIT(13));
+	priv->write_reg32(priv, C_CAN_IFACE(MASK1_REG, iface),
+			((IFX_WRITE_HIGH_16BIT(mask) | BIT(13)) << 16) |
+			IFX_WRITE_LOW_16BIT(mask));
 
-	priv->write_reg(priv, C_CAN_IFACE(ARB1_REG, iface),
+	id |= IF_ARB_MSGVAL;
+
+	priv->write_reg32(priv, C_CAN_IFACE(ARB1_REG, iface),
+			((IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)) << 16) |
 			IFX_WRITE_LOW_16BIT(id));
-	priv->write_reg(priv, C_CAN_IFACE(ARB2_REG, iface),
-			(IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
+	priv->write_reg32(priv, C_CAN_IFACE(MSGCTRL_REG, iface), mcont);
 
-	priv->write_reg(priv, C_CAN_IFACE(MSGCTRL_REG, iface), mcont);
 	c_can_object_put(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
 
 	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
-			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
+			priv->read_reg32(priv, C_CAN_MSGVAL1_REG));
 }
 
 static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
@@ -474,12 +470,12 @@ static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
 	c_can_object_put(dev, iface, objno, IF_COMM_ARB | IF_COMM_CONTROL);
 
 	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
-			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
+			priv->read_reg32(priv, C_CAN_MSGVAL1_REG));
 }
 
 static inline int c_can_is_next_tx_obj_busy(struct c_can_priv *priv, int objno)
 {
-	int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
+	int val = priv->read_reg32(priv, C_CAN_TXRQST1_REG);
 
 	/*
 	 * as transmission request register's bit n-1 corresponds to
@@ -561,16 +557,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);
 }
@@ -743,7 +745,7 @@ static void c_can_do_tx(struct net_device *dev)
 
 	for (; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
 		obj = get_tx_echo_msg_obj(priv->tx_echo);
-		val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
+		val = priv->read_reg32(priv, C_CAN_TXRQST1_REG);
 
 		if (val & (1 << (obj - 1)))
 			break;
@@ -1292,7 +1294,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 d436734..21e0795 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -203,6 +203,8 @@ struct c_can_priv {
 	unsigned int instance;
 	void (*raminit) (const struct c_can_priv *priv, bool enable);
 	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 fba168f..ff844da 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -110,6 +110,34 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
 	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);
+	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 void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
 {
 	u32 ctrl;
@@ -223,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;
@@ -236,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");


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Pavel Machek]

On Sat 2014-04-26 11:36:46, Pavel Machek wrote:
> Hi!
> 
> > To get this working well, I had to install a few of the patches that
> > Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access for
> > D_CAN) on 9/9/2013.
> > 
> > I have the patches on our rocketboard branch
> > (rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)
> > 
> > I planned to upstream these changes but there have been some major
> > changes to CAN recently that may require some refactoring.
> 
> I ported those changes to 3.15-rc2 (but can't test them at the
> moment).

Ok, it seems it is possible to test CAN without actual can hardware
using loopback:

ip link set can0 up type can bitrate 125000 loopback on
ifconfig can0 up
candump can0 &
cansend can0 "123#DEADBEEF"

Moreover, it seems that previous two patches do the trick. Packets are
now echoed as expected. (Ok, small question is "why twice", but I
guess that's just CAN...?)

root@sockit:~# ip link set can0 up type can bitrate 125000 loopback on
c_can_platform ffc00000.d_can can0: setting BTR=1c31 BRPE=0000
root@sockit:~# ifconfig can0 up
root@sockit:~# candump can0 &
root@sockit:~# cansend can0 "123#DEADBEEF"
  can0  123   [4]  DE AD BE EF
  can0  123   [4]  DE AD BE EF
root@sockit:~# 

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Oliver Hartkopp]

On 26.04.2014 22:31, Pavel Machek wrote:
> On Sat 2014-04-26 11:36:46, Pavel Machek wrote:
>> Hi!
>>
>>> To get this working well, I had to install a few of the patches that
>>> Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access for
>>> D_CAN) on 9/9/2013.
>>>
>>> I have the patches on our rocketboard branch
>>> (rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)
>>>
>>> I planned to upstream these changes but there have been some major
>>> changes to CAN recently that may require some refactoring.
>>
>> I ported those changes to 3.15-rc2 (but can't test them at the
>> moment).
> 
> Ok, it seems it is possible to test CAN without actual can hardware
> using loopback:
> 
> ip link set can0 up type can bitrate 125000 loopback on
> ifconfig can0 up
> candump can0 &
> cansend can0 "123#DEADBEEF"
> 
> Moreover, it seems that previous two patches do the trick. Packets are
> now echoed as expected. (Ok, small question is "why twice", but I
> guess that's just CAN...?)

Usually the CAN frame is echo'ed back when the CAN frame is successfully sent
on the medium (tx ok interrupt, see: echo skb functionality in
drivers/net/can/dev.c and in can.txt for multiuser capabilites).

The second CAN frame is obviouly created by some rx interrupt which occurs due
to the loopback functionality inside the C_CAN controller - which is usually
not enabled.

So it's a correct behaviour.

Best regards,
Oliver

> 
> root@sockit:~# ip link set can0 up type can bitrate 125000 loopback on
> c_can_platform ffc00000.d_can can0: setting BTR=1c31 BRPE=0000
> root@sockit:~# ifconfig can0 up
> root@sockit:~# candump can0 &
> root@sockit:~# cansend can0 "123#DEADBEEF"
>   can0  123   [4]  DE AD BE EF
>   can0  123   [4]  DE AD BE EF
> root@sockit:~# 
> 
> Best regards,
> 								Pavel
> 

Re: can problems on socfpga [was Re: [PATCH v2 4/6] ARM: socfpga: dts: add can0+1]

[Marc Kleine-Budde]

Hello,

I'm not in the office (and fighting with my phone's email client) until 5th of May, however please rebase all your patches against the latest net/master from David Miller, as this tree includes all c_can fixes by Thomas.

Marc

(This probability end as a tofu)

On 26 April 2014 21:31:31 BST, Pavel Machek <pavel@denx.de> wrote:
>On Sat 2014-04-26 11:36:46, Pavel Machek wrote:
>> Hi!
>> 
>> > To get this working well, I had to install a few of the patches
>that
>> > Benedict Spranger submitted ([PATCH 05/16] c_can: use 32 bit access
>for
>> > D_CAN) on 9/9/2013.
>> > 
>> > I have the patches on our rocketboard branch
>> > (rocketboards.org/gitweb/?p=linux-socfpga-git;a=summary)
>> > 
>> > I planned to upstream these changes but there have been some major
>> > changes to CAN recently that may require some refactoring.
>> 
>> I ported those changes to 3.15-rc2 (but can't test them at the
>> moment).
>
>Ok, it seems it is possible to test CAN without actual can hardware
>using loopback:
>
>ip link set can0 up type can bitrate 125000 loopback on
>ifconfig can0 up
>candump can0 &
>cansend can0 "123#DEADBEEF"
>
>Moreover, it seems that previous two patches do the trick. Packets are
>now echoed as expected. (Ok, small question is "why twice", but I
>guess that's just CAN...?)
>
>root@sockit:~# ip link set can0 up type can bitrate 125000 loopback on
>c_can_platform ffc00000.d_can can0: setting BTR=1c31 BRPE=0000
>root@sockit:~# ifconfig can0 up
>root@sockit:~# candump can0 &
>root@sockit:~# cansend can0 "123#DEADBEEF"
>  can0  123   [4]  DE AD BE EF
>  can0  123   [4]  DE AD BE EF
>root@sockit:~# 
>
>Best regards,
>								Pavel

[patch] Fix CAN on socfpga, for net/master

[Pavel Machek]

Apparently, socfpga CAN needs 32-bit accesses and different raminit
sequence.

Signed-off-by: Pavel Machek <pavel@denx.de>

---

> 
> I'm not in the office (and fighting with my phone's email client) until 5th of May, however please rebase all your patches against the latest net/master from David Miller, as this tree includes all c_can fixes by Thomas.
> 

Ok, combined patch is here. Lightly tested using loopback on Arrow
SocKit.

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
 	 * 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);
+	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);
+	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;


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Fix CAN on socfpga, for net/master

[Thor Thayer]

On Sun, 2014-04-27 at 14:25 +0200, ZY - pavel wrote:
> Apparently, socfpga CAN needs 32-bit accesses and different raminit
> sequence.
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> ---
> 
> > 
> > I'm not in the office (and fighting with my phone's email client) until 5th of May, however please rebase all your patches against the latest net/master from David Miller, as this tree includes all c_can fixes by Thomas.
> > 
> 
> Ok, combined patch is here. Lightly tested using loopback on Arrow
> SocKit.

Hi Pavel. 

Thanks for your help on the SOCFPGA support! I can test this on the
Altera Development Kit but I'm not able to cleanly apply to the latest
David Miller net-next branch. Is there a different branch I should use? 

I also like your updates to my patch as shown below - much cleaner.

Thanks,

Thor

> 
> 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
>  	 * 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);
> +	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);
> +	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;
> 
> 

Re: [patch] Fix CAN on socfpga, for net/master

[Pavel Machek]

Hi!

On Mon 2014-04-28 15:20:49, Thor Thayer wrote:
> On Sun, 2014-04-27 at 14:25 +0200, ZY - pavel wrote:
> > Apparently, socfpga CAN needs 32-bit accesses and different raminit
> > sequence.
> > 
> > Signed-off-by: Pavel Machek <pavel@denx.de>
> > 
> > ---
> > 
> > > 
> > > I'm not in the office (and fighting with my phone's email client) until 5th of May, however please rebase all your patches against the latest net/master from David Miller, as this tree includes all c_can fixes by Thomas.
> > > 
> > 
> > Ok, combined patch is here. Lightly tested using loopback on Arrow
> > SocKit.
> 
> Hi Pavel. 
> 
> Thanks for your help on the SOCFPGA support! I can test this on the
> Altera Development Kit but I'm not able to cleanly apply to the latest
> David Miller net-next branch. Is there a different branch I should
> use?

It should have been against net-next branch... but perhaps I made a
mistake somewhere. I should get some sleep now.

Or ... let me upload my tree so that you can generate the diff.

You should be able to get it using:

git pull git://git.denx.de/linux-denx-pavel.git my_net_master

> I also like your updates to my patch as shown below - much cleaner.

Thanks :-).

Best regards,

								Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Fix CAN on socfpga, for net/master

[T Thayer]

On Mon, Apr 28, 2014 at 4:15 PM, Pavel Machek <pavel@denx.de> wrote:
> Hi!
>
> On Mon 2014-04-28 15:20:49, Thor Thayer wrote:
>> On Sun, 2014-04-27 at 14:25 +0200, ZY - pavel wrote:
>> > Apparently, socfpga CAN needs 32-bit accesses and different raminit
>> > sequence.
>> >
>> > Signed-off-by: Pavel Machek <pavel@denx.de>
>> >
>> > ---
>> >
>> > >
>> > > I'm not in the office (and fighting with my phone's email client) until 5th of May, however please rebase all your patches against the latest net/master from David Miller, as this tree includes all c_can fixes by Thomas.
>> > >
>> >
>> > Ok, combined patch is here. Lightly tested using loopback on Arrow
>> > SocKit.
>>
>> Hi Pavel.
>>
>> Thanks for your help on the SOCFPGA support! I can test this on the
>> Altera Development Kit but I'm not able to cleanly apply to the latest
>> David Miller net-next branch. Is there a different branch I should
>> use?
>
> It should have been against net-next branch... but perhaps I made a
> mistake somewhere. I should get some sleep now.
>
> Or ... let me upload my tree so that you can generate the diff.
>
> You should be able to get it using:
>
> git pull git://git.denx.de/linux-denx-pavel.git my_net_master
>
>> I also like your updates to my patch as shown below - much cleaner.
>
> Thanks :-).
>
> Best regards,
>
>                                                                 Pavel

Hi Pavel,

I didn't get a chance to pull and test your patch but a visual
inspection looks good.

Since you based this patch on my original code, feel free to add my signed-off.

Thor

>
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Fix CAN on socfpga, for net/master

[Pavel Machek]

Hi!

> >>> > Apparently, socfpga CAN needs 32-bit accesses and different raminit
> >>> > sequence.
> >>> >
> >>> > Signed-off-by: Pavel Machek <pavel@denx.de>

...

> >> Or ... let me upload my tree so that you can generate the diff.
> >>
> >> You should be able to get it using:
> >>
> >> git pull git://git.denx.de/linux-denx-pavel.git my_net_master
> >>
> >>> I also like your updates to my patch as shown below - much cleaner.

> > Since you based this patch on my original code, feel free to add my
> signed-off.

Will do.

> I pulled in your branch
> (git://git.denx.de/linux-denx-pavel.gitmy_net_master) and tested it on
> the Altera Development Kit. It works well -
> I don't see any issues at a number of different bitrates (80K, 200K, 800K,
> 1Mb).

Thanks for testing. I'll also add your Tested-by: header, if you don't
mind.

> I didn't investigate what the issues were with applying the patch though. I
> just used your branch.

The way I generated that branch... you picked up some extra code in
other areas of the kernel, but it should not affect the testing.

Thanks!
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Fix CAN on socfpga, for net/master

[Thor Thayer]

On Wed, Apr 30, 2014 at 4:53 PM, Pavel Machek <pavel@denx.de> wrote:
> Hi!
>
>> >>> > Apparently, socfpga CAN needs 32-bit accesses and different raminit
>> >>> > sequence.
>> >>> >
>> >>> > Signed-off-by: Pavel Machek <pavel@denx.de>
>
> ...
>
>> >> Or ... let me upload my tree so that you can generate the diff.
>> >>
>> >> You should be able to get it using:
>> >>
>> >> git pull git://git.denx.de/linux-denx-pavel.git my_net_master
>> >>
>> >>> I also like your updates to my patch as shown below - much cleaner.
>
>> > Since you based this patch on my original code, feel free to add my
>> signed-off.
>
> Will do.
>
>> I pulled in your branch
>> (git://git.denx.de/linux-denx-pavel.gitmy_net_master) and tested it on
>> the Altera Development Kit. It works well -
>> I don't see any issues at a number of different bitrates (80K, 200K, 800K,
>> 1Mb).
>
> Thanks for testing. I'll also add your Tested-by: header, if you don't
> mind.

Hi Pavel. That would be fine.

Thor

>
>> I didn't investigate what the issues were with applying the patch though. I
>> just used your branch.
>
> The way I generated that branch... you picked up some extra code in
> other areas of the kernel, but it should not affect the testing.
>
> Thanks!
>                                                                 Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCHv2] Fix CAN on socfpga, for net/master

[Pavel Machek]

Apparently, socfpga CAN needs 32-bit accesses and different raminit
sequence.

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.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
 	 * 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);
+	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);
+	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;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv2] Fix CAN on socfpga, for net/master

[Marc Kleine-Budde]

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@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.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   |

Re: [PATCHv2] Fix CAN on socfpga, for net/master

[Pavel Machek]

Hi!

> 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

Can do. 2/2.

> - if-DCAN-then-32bit access
>   please explain in the commit message why this is needed

That patch is below... but I guess I'll need help from Altera
here. Can you explain why it is needed?

> - add hwinit support for non ti devices

Can do. 1/2.

Thanks,
									Pavel

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.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)
@@ -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,
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv2] Fix CAN on socfpga, for net/master

[Pavel Machek]

Hi!

> 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

> > @@ -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.

Ok.

> > +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.

I was adapting existing code which had the raminit paths shared. Will
remove the spinlock.

And yes, I can replace it with priv->read_... , but it will result in
some churn of consts...

> > +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 >>

Sorry about that.

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 2/2] Add 32-bit accesses

[Pavel Machek]

Add helpers for 32-bit accesses and replace open-coded 32-bit access
with calls to helpers.

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.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
@@ -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.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
@@ -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);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 2/2] Add 32-bit accesses

[Marc Kleine-Budde]

On 05/05/2014 02:08 PM, Pavel Machek wrote:
> 
> Add helpers for 32-bit accesses and replace open-coded 32-bit access
> with calls to helpers.

In general looks good, but please add the modifications from the
platform file, where you add the 32 callbacks to this patch. Please
don't forget to modify the pci driver, too.

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   |

[PATCHv3] C_CAN: Add 32-bit accesses

[Pavel Machek]

Add helpers for 32-bit accesses and replace open-coded 32-bit access
with calls to helpers. Minimum changes are done to the pci case, as I
don't have access to that hardware.

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.c b/drivers/net/can/c_can/c_can.c
index a2ca820..e154b4c 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -252,8 +252,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 +327,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 +389,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 +421,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);
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index c56f1b1..ce0e642 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -188,6 +188,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_pci.c b/drivers/net/can/c_can/c_can_pci.c
index fe5f630..419bb7c 100644
--- a/drivers/net/can/c_can/c_can_pci.c
+++ b/drivers/net/can/c_can/c_can_pci.c
@@ -63,6 +63,23 @@ static void c_can_pci_write_reg_aligned_to_32bit(struct c_can_priv *priv,
 	writew(val, priv->base + 2 * priv->regs[index]);
 }
 
+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);
+	priv->write_reg(priv, index, val);
+}
+
 static int c_can_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *ent)
 {
@@ -151,6 +168,8 @@ static int c_can_pci_probe(struct pci_dev *pdev,
 		ret = -EINVAL;
 		goto out_free_c_can;
 	}
+	priv->read_reg32 = c_can_plat_read_reg32;
+	priv->write_reg32 = c_can_plat_write_reg32;
 
 	ret = register_c_can_dev(dev);
 	if (ret) {
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 1df0b32..002bf9b 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -108,6 +108,34 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
 	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);
+	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 +229,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 +246,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");


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv3] C_CAN: Add 32-bit accesses

[Marc Kleine-Budde]

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

On 05/06/2014 03:57 PM, Pavel Machek wrote:
> Add helpers for 32-bit accesses and replace open-coded 32-bit access
> with calls to helpers. Minimum changes are done to the pci case, as I
> don't have access to that hardware.

I'll ask David Miller to merge net/master into net-next/master so that I
can apply this patch.

For now, I've applied the patch to can-next testing-c_can
(git://gitorious.org/linux-can/linux-can-next.git testing-c_can)

Can you make other patches based on that tree, please.

> --- a/drivers/net/can/c_can/c_can_pci.c
> +++ b/drivers/net/can/c_can/c_can_pci.c
> @@ -63,6 +63,23 @@ static void c_can_pci_write_reg_aligned_to_32bit(struct c_can_priv *priv,
>  	writew(val, priv->base + 2 * priv->regs[index]);
>  }
>  
> +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);

spaces around >>, I'll fix this while applying.

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 --]

Re: [PATCHv3] C_CAN: Add 32-bit accesses

[Pavel Machek]

Hi!

> > Add helpers for 32-bit accesses and replace open-coded 32-bit access
> > with calls to helpers. Minimum changes are done to the pci case, as I
> > don't have access to that hardware.
> 
> I'll ask David Miller to merge net/master into net-next/master so that I
> can apply this patch.
> 
> For now, I've applied the patch to can-next testing-c_can
> (git://gitorious.org/linux-can/linux-can-next.git testing-c_can)
> 
> Can you make other patches based on that tree, please.

Yes, give me few minutes.

> > --- a/drivers/net/can/c_can/c_can_pci.c
> > +++ b/drivers/net/can/c_can/c_can_pci.c
> > +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);
> 
> spaces around >>, I'll fix this while applying.

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCHv3] C_CAN: hwinit support for non-TI devices

[Pavel Machek]

Non-TI chips (including socfpga) needs different raminit
sequence. Implement it.

const's in the hwinit prototype forced me to add const to more
prototypes. This makes changes bigger but the result is cleaner.

Unfortunately, testing-c_can does not boot on sockit, but previous
version of patch was tested by me and Thor Thayer.

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.h b/drivers/net/can/c_can/c_can.h
index ce0e642..99ad1aa 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,
@@ -176,8 +178,10 @@ struct c_can_priv {
 	atomic_t tx_active;
 	unsigned long tx_dir;
 	int last_status;
-	u16 (*read_reg) (struct c_can_priv *priv, enum reg index);
-	void (*write_reg) (struct c_can_priv *priv, enum reg index, u16 val);
+	u16 (*read_reg) (const struct c_can_priv *priv, enum reg index);
+	void (*write_reg) (const struct c_can_priv *priv, enum reg index, u16 val);
+	u32 (*read_reg32) (const struct c_can_priv *priv, enum reg index);
+	void (*write_reg32) (const struct c_can_priv *priv, enum reg index, u32 val);
 	void __iomem *base;
 	const u16 *regs;
 	void *priv;		/* for board-specific data */
@@ -188,8 +192,6 @@ 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 002bf9b..b27dd8f 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
@@ -47,31 +48,31 @@ static DEFINE_SPINLOCK(raminit_lock);
  * registers can be aligned to a 16-bit boundary or 32-bit boundary etc.
  * Handle the same by providing a common read/write interface.
  */
-static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv *priv,
+static u16 c_can_plat_read_reg_aligned_to_16bit(const struct c_can_priv *priv,
 						enum reg index)
 {
 	return readw(priv->base + priv->regs[index]);
 }
 
-static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv *priv,
+static void c_can_plat_write_reg_aligned_to_16bit(const struct c_can_priv *priv,
 						enum reg index, u16 val)
 {
 	writew(val, priv->base + priv->regs[index]);
 }
 
-static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv *priv,
+static u16 c_can_plat_read_reg_aligned_to_32bit(const struct c_can_priv *priv,
 						enum reg index)
 {
 	return readw(priv->base + 2 * priv->regs[index]);
 }
 
-static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv *priv,
+static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
 						enum reg index, u16 val)
 {
 	writew(val, priv->base + 2 * priv->regs[index]);
 }
 
-static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask,
+static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
 				  u32 val)
 {
 	/* We look only at the bits of our instance. */
@@ -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;
@@ -96,19 +97,19 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
 	ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
 	writel(ctrl, priv->raminit_ctrlreg);
 	ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
-	c_can_hw_raminit_wait(priv, ctrl, mask);
+	c_can_hw_raminit_wait_ti(priv, ctrl, mask);
 
 	if (enable) {
 		/* Set start bit and wait for the done bit. */
 		ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
 		writel(ctrl, priv->raminit_ctrlreg);
 		ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
-		c_can_hw_raminit_wait(priv, ctrl, mask);
+		c_can_hw_raminit_wait_ti(priv, ctrl, mask);
 	}
 	spin_unlock(&raminit_lock);
 }
 
-static u32 c_can_plat_read_reg32(struct c_can_priv *priv, enum reg index)
+static u32 c_can_plat_read_reg32(const struct c_can_priv *priv, enum reg index)
 {
 	u32 val;
 
@@ -118,24 +119,46 @@ static u32 c_can_plat_read_reg32(struct c_can_priv *priv, enum reg index)
 	return val;
 }
 
-static void c_can_plat_write_reg32(struct c_can_priv *priv, enum reg index,
+static void c_can_plat_write_reg32(const 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)
+static u32 d_can_plat_read_reg32(const 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,
+static void d_can_plat_write_reg32(const struct c_can_priv *priv, enum reg index,
 		u32 val)
 {
 	writel(val, priv->base + priv->regs[index]);
 }
 
+static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask)
+{
+	while (priv->read_reg32(priv, C_CAN_FUNCTION_REG) & mask)
+		udelay(1);
+}
+
+static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
+{
+	u32 ctrl;
+
+	ctrl = priv->read_reg32(priv, C_CAN_FUNCTION_REG);
+	ctrl &= ~DCAN_RAM_INIT_BIT;
+	priv->write_reg32(priv, C_CAN_FUNCTION_REG, ctrl);
+	c_can_hw_raminit_wait(priv, ctrl);
+
+	if (enable) {
+		ctrl |= DCAN_RAM_INIT_BIT;
+		priv->write_reg32(priv, C_CAN_FUNCTION_REG, ctrl);
+		c_can_hw_raminit_wait(priv, ctrl);
+	}
+}
+
 static struct platform_device_id c_can_id_table[] = {
 	[BOSCH_C_CAN_PLATFORM] = {
 		.name = KBUILD_MODNAME,
@@ -255,11 +278,20 @@ 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 modules 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;
+			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;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv3] C_CAN: hwinit support for non-TI devices

[Marc Kleine-Budde]

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

On 05/13/2014 03:09 PM, Pavel Machek wrote:
> Non-TI chips (including socfpga) needs different raminit
> sequence. Implement it.
> 
> const's in the hwinit prototype forced me to add const to more
> prototypes. This makes changes bigger but the result is cleaner.

Why not split into several patches?

> Unfortunately, testing-c_can does not boot on sockit, but previous
> version of patch was tested by me and Thor Thayer.

Have a look at:

    git@gitorious.org:linux-can/linux-can-next.git
testing-c_can-based-on-net

This branch uses net/master as it's base, I've split your patch into 3,
and squashed the 32bit-const changes into the 32-bit access patch. The
remaining patch looks quite clean.

Can you please test the branch, I hope it boots on sockit.

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 --]

Re: [PATCHv3] C_CAN: hwinit support for non-TI devices

[Pavel Machek]

Hi!


On Tue 2014-05-13 15:36:21, Marc Kleine-Budde wrote:
> On 05/13/2014 03:09 PM, Pavel Machek wrote:
> > Non-TI chips (including socfpga) needs different raminit
> > sequence. Implement it.
> > 
> > const's in the hwinit prototype forced me to add const to more
> > prototypes. This makes changes bigger but the result is cleaner.
> 
> Why not split into several patches?
> 
> > Unfortunately, testing-c_can does not boot on sockit, but previous
> > version of patch was tested by me and Thor Thayer.
> 
> Have a look at:
> 
>     git@gitorious.org:linux-can/linux-can-next.git
> testing-c_can-based-on-net

git fetch git@gitorious.org:linux-can/linux-can-next.git
testing-c_can-based-on-net:testing-c_can-based-on-net
The authenticity of host 'gitorious.org (87.238.52.168)' can't be
established.
RSA key fingerprint is
7e:af:8d:ec:f0:39:5e:ba:52:16:ce:19:fa:d4:b8:7d.

But I was able to

git fetch git://gitorious.org/linux-can/linux-can-next.git
testing-c_can-based-on-net:testing-c_can-based-on-net 

.

> This branch uses net/master as it's base, I've split your patch into 3,
> and squashed the 32bit-const changes into the 32-bit access patch. The
> remaining patch looks quite clean.

Thanks a lot!

> Can you please test the branch, I hope it boots on sockit.

That one still dies early in boot, but I was able to apply some
changes from my main tree (not CAN related) and get it to boot.

And yes, it seems to work:

[It still does not contain some "interesting" changes from altera
tree, but as I could not get explanation why they are needed -- and
they don't seem to be needed -- maybe it is better that way.]

Thanks,
								Pavel

root@sockit:~# uname -a
Linux sockit 3.15.0-rc4-00470-g4477be8-dirty #99 SMP Tue May 13
17:02:02 CEST 2014 armv7l GNU/Linux
root@sockit:~# ip link set can0 up type can bitrate 125000 loopback on
c_can_platform ffc00000.d_can can0: setting BTR=1c31 BRPE=0000
root@sockit:~# ifconfig can0 up
root@sockit:~# candump can0 &
root@sockit:~# cansend can0 "123#DEADBEEF"
  can0  123   [4]  DE AD BE EF
  can0  123   [4]  DE AD BE EF
root@sockit:~# cansend can0 "123#DEADBEEF"
  can0  123   [4]  DE AD BE EF
  can0  123   [4]  DE AD BE EF

commit 4477be8c9c6070d94b37758a8b35ce60882c2617
Merge: 37e25b8 a64b6a7
Author: Pavel Machek <pavel@denx.de>
Date:   Tue May 13 17:01:30 2014 +0200

    Merge commit 'a64b6a7de2e89f0468ad00066940cfa2d48f886a' into
    testing-c_can-based-on-n
    
    testing-c_can-based-on-net does not boot, this one could.

commit 37e25b8a3432a66027fd3252ec682bd627a929c5


									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCHv3] C_CAN: hwinit support for non-TI devices

[Marc Kleine-Budde]

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

On 05/13/2014 05:08 PM, Pavel Machek wrote:
> Hi!
> 
> 
> On Tue 2014-05-13 15:36:21, Marc Kleine-Budde wrote:
>> On 05/13/2014 03:09 PM, Pavel Machek wrote:
>>> Non-TI chips (including socfpga) needs different raminit
>>> sequence. Implement it.
>>>
>>> const's in the hwinit prototype forced me to add const to more
>>> prototypes. This makes changes bigger but the result is cleaner.
>>
>> Why not split into several patches?
>>
>>> Unfortunately, testing-c_can does not boot on sockit, but previous
>>> version of patch was tested by me and Thor Thayer.
>>
>> Have a look at:
>>
>>     git@gitorious.org:linux-can/linux-can-next.git
>> testing-c_can-based-on-net
> 
> git fetch git@gitorious.org:linux-can/linux-can-next.git
> testing-c_can-based-on-net:testing-c_can-based-on-net
> The authenticity of host 'gitorious.org (87.238.52.168)' can't be
> established.
> RSA key fingerprint is
> 7e:af:8d:ec:f0:39:5e:ba:52:16:ce:19:fa:d4:b8:7d.
> 
> But I was able to
> 
> git fetch git://gitorious.org/linux-can/linux-can-next.git
> testing-c_can-based-on-net:testing-c_can-based-on-net 

Sorry, I copied the wrong URL.

>> This branch uses net/master as it's base, I've split your patch into 3,
>> and squashed the 32bit-const changes into the 32-bit access patch. The
>> remaining patch looks quite clean.
> 
> Thanks a lot!
> 
>> Can you please test the branch, I hope it boots on sockit.
> 
> That one still dies early in boot, but I was able to apply some
> changes from my main tree (not CAN related) and get it to boot.

\o/

> And yes, it seems to work:
> 
> [It still does not contain some "interesting" changes from altera
> tree, but as I could not get explanation why they are needed -- and
> they don't seem to be needed -- maybe it is better that way.]

Are there changes to the CAN driver or the SoC infrastructure?

> root@sockit:~# uname -a
> Linux sockit 3.15.0-rc4-00470-g4477be8-dirty #99 SMP Tue May 13
> 17:02:02 CEST 2014 armv7l GNU/Linux
> root@sockit:~# ip link set can0 up type can bitrate 125000 loopback on
> c_can_platform ffc00000.d_can can0: setting BTR=1c31 BRPE=0000
> root@sockit:~# ifconfig can0 up
> root@sockit:~# candump can0 &
> root@sockit:~# cansend can0 "123#DEADBEEF"
>   can0  123   [4]  DE AD BE EF
>   can0  123   [4]  DE AD BE EF
> root@sockit:~# cansend can0 "123#DEADBEEF"
>   can0  123   [4]  DE AD BE EF
>   can0  123   [4]  DE AD BE EF

\o/

Thanks for testing.

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 --]

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

[Pavel Machek]

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
 	 * 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);
+	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);
+	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;

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

[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   |

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

[Marc Kleine-Budde]

On 05/05/2014 02:21 PM, Marc Kleine-Budde wrote:
> 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;

This will not compile.

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   |

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

[Pavel Machek]

On Mon 2014-05-05 14:21:33, Marc Kleine-Budde wrote:
> 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>
> > 
> > @@ -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.

Previous one is not correct coding style. I'd like to get it fixed.

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?

_ti() used spinlock, so I assume I need it, too.

> > +	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?

Can you repost it? I don't think I seen it.

> > +	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.

Should be different patch, yes.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Marc Kleine-Budde]

On 05/05/2014 02:58 PM, Pavel Machek wrote:
> On Mon 2014-05-05 14:21:33, Marc Kleine-Budde wrote:
>> 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>
>>>
>>> @@ -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.
> 
> Previous one is not correct coding style. I'd like to get it fixed.

net/ and drivers/net use a different multiline commenting style.

> 
> 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?
> 
> _ti() used spinlock, so I assume I need it, too.

It's not a shared ressource you're working on. TI does.

> 
>>> +	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?
> 
> Can you repost it? I don't think I seen it.

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   |

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

[Pavel Machek]

> > +
> > +	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?

Not really. I did not notice that there are more comments below all
that quoted text. Standby.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html