linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
@ 2014-06-23  7:11 Zhao Qiang
  2014-06-23  7:11 ` [PATCH v3 2/2] flexcan: add err interrupt for p1010rdb Zhao Qiang
  2014-06-23  7:17 ` [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Marc Kleine-Budde
  0 siblings, 2 replies; 7+ messages in thread
From: Zhao Qiang @ 2014-06-23  7:11 UTC (permalink / raw)
  To: linuxppc-dev, linux-can, wg, mkl, B07421; +Cc: Zhao Qiang

when flexcan is not physically linked, command 'cantest' will
trigger an err_irq, add err_irq handler for it.

Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
	- use a space instead of tab
	- use flexcan_poll_state instead of print
Changes for v3:
	- return IRQ_HANDLED if err is triggered
	- stop transmitted packets when there is an err_interrupt 

 drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..6802a25 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -208,6 +208,7 @@ struct flexcan_priv {
 	void __iomem *base;
 	u32 reg_esr;
 	u32 reg_ctrl_default;
+	unsigned int err_irq;
 
 	struct clk *clk_ipg;
 	struct clk *clk_per;
@@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
+{
+	struct net_device *dev = dev_id;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_ctrl, reg_esr;
+
+	reg_esr = flexcan_read(&regs->esr);
+	reg_ctrl = flexcan_read(&regs->ctrl);
+	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
+		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
+		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
+		netif_stop_queue(dev);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
 static void flexcan_set_bittiming(struct net_device *dev)
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
@@ -944,6 +963,15 @@ static int flexcan_open(struct net_device *dev)
 	if (err)
 		goto out_close;
 
+	if (priv->err_irq) {
+		err = request_irq(priv->err_irq, flexcan_err_irq, IRQF_SHARED,
+				  dev->name, dev);
+		if (err) {
+			free_irq(priv->err_irq, dev);
+			goto out_free_irq;
+		}
+	}
+
 	/* start chip and queuing */
 	err = flexcan_chip_start(dev);
 	if (err)
@@ -1099,7 +1127,7 @@ static int flexcan_probe(struct platform_device *pdev)
 	struct resource *mem;
 	struct clk *clk_ipg = NULL, *clk_per = NULL;
 	void __iomem *base;
-	int err, irq;
+	int err, irq, err_irq;
 	u32 clock_freq = 0;
 
 	if (pdev->dev.of_node)
@@ -1126,6 +1154,10 @@ static int flexcan_probe(struct platform_device *pdev)
 	if (irq <= 0)
 		return -ENODEV;
 
+	err_irq = platform_get_irq(pdev, 1);
+	if (err_irq <= 0)
+		err_irq = 0;
+
 	base = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
@@ -1149,6 +1181,7 @@ static int flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO;
 
 	priv = netdev_priv(dev);
+	priv->err_irq = err_irq;
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] flexcan: add err interrupt for p1010rdb
  2014-06-23  7:11 [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Zhao Qiang
@ 2014-06-23  7:11 ` Zhao Qiang
  2014-06-23  7:17 ` [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Marc Kleine-Budde
  1 sibling, 0 replies; 7+ messages in thread
From: Zhao Qiang @ 2014-06-23  7:11 UTC (permalink / raw)
  To: linuxppc-dev, linux-can, wg, mkl, B07421; +Cc: Zhao Qiang

add err interrupt for p1010rdb into dts.

Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
	- add binding documentation update
Changes for v3:
	- update binding documentation

 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 15 +++++++++++++--
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi               |  6 ++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 56d6cc3..7bf377c 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -10,7 +10,9 @@ Required properties:
   - fsl,p1010-flexcan
 
 - reg : Offset and length of the register set for this device
-- interrupts : Interrupt tuple for this device
+- interrupts : Interrupt tuple for this device.
+	The first interrupt is for FlexCAN(Message Buffer and Wake Up)
+	The second(optional) is for error
 
 Optional properties:
 
@@ -23,7 +25,16 @@ Example:
 	can@1c000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2>;
+		interrupts = <48 0x2 0 0>;
+		interrupt-parent = <&mpic>;
+		clock-frequency = <200000000>; // filled in by bootloader
+	};
+
+	can@1c000 {
+		compatible = "fsl,p1010-flexcan";
+		reg = <0x1c000 0x1000>;
+		interrupts = <48 0x2 0 0
+			      16 0x2 0 0>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
 	};
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index af12ead..47125a6 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -136,13 +136,15 @@
 	can0: can@1c000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2 0 0>;
+		interrupts = <48 0x2 0 0
+			      16 0x2 0 0>;
 	};
 
 	can1: can@1d000 {
 		compatible = "fsl,p1010-flexcan";
 		reg = <0x1d000 0x1000>;
-		interrupts = <61 0x2 0 0>;
+		interrupts = <61 0x2 0 0
+			      16 0x2 0 0>;
 	};
 
 	L2: l2-cache-controller@20000 {
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
  2014-06-23  7:11 [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Zhao Qiang
  2014-06-23  7:11 ` [PATCH v3 2/2] flexcan: add err interrupt for p1010rdb Zhao Qiang
@ 2014-06-23  7:17 ` Marc Kleine-Budde
  2014-06-23  7:26   ` qiang.zhao
  1 sibling, 1 reply; 7+ messages in thread
From: Marc Kleine-Budde @ 2014-06-23  7:17 UTC (permalink / raw)
  To: Zhao Qiang, linuxppc-dev, linux-can, wg, B07421

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

On 06/23/2014 09:11 AM, Zhao Qiang wrote:
> when flexcan is not physically linked, command 'cantest' will
> trigger an err_irq, add err_irq handler for it.
> 
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> Changes for v2:
> 	- use a space instead of tab
> 	- use flexcan_poll_state instead of print
> Changes for v3:
> 	- return IRQ_HANDLED if err is triggered
> 	- stop transmitted packets when there is an err_interrupt 
> 
>  drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..6802a25 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -208,6 +208,7 @@ struct flexcan_priv {
>  	void __iomem *base;
>  	u32 reg_esr;
>  	u32 reg_ctrl_default;
> +	unsigned int err_irq;
>  
>  	struct clk *clk_ipg;
>  	struct clk *clk_per;
> @@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg_ctrl, reg_esr;
> +
> +	reg_esr = flexcan_read(&regs->esr);
> +	reg_ctrl = flexcan_read(&regs->ctrl);
> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {

When does the hardware trigger the interrupt?

> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
> +		netif_stop_queue(dev);

Why are you stopping the txqueue?

> +		return IRQ_HANDLED;
> +	}
> +	return IRQ_NONE;
> +}
> +

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
  2014-06-23  7:17 ` [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Marc Kleine-Budde
@ 2014-06-23  7:26   ` qiang.zhao
  2014-06-23  7:37     ` Marc Kleine-Budde
  0 siblings, 1 reply; 7+ messages in thread
From: qiang.zhao @ 2014-06-23  7:26 UTC (permalink / raw)
  To: Marc Kleine-Budde, linuxppc-dev, linux-can, wg, Scott Wood

DQpPbiAwNi8yMy8yMDE0IDAzOjE4IFBNLCBNYXJjIEtsZWluZS1CdWRkZSB3cm90ZToNCg0KPiAN
Cj4gT24gMDYvMjMvMjAxNCAwOToxMSBBTSwgWmhhbyBRaWFuZyB3cm90ZToNCj4gPiB3aGVuIGZs
ZXhjYW4gaXMgbm90IHBoeXNpY2FsbHkgbGlua2VkLCBjb21tYW5kICdjYW50ZXN0JyB3aWxsIHRy
aWdnZXINCj4gPiBhbiBlcnJfaXJxLCBhZGQgZXJyX2lycSBoYW5kbGVyIGZvciBpdC4NCj4gPg0K
PiA+IFNpZ25lZC1vZmYtYnk6IFpoYW8gUWlhbmcgPEI0NTQ3NUBmcmVlc2NhbGUuY29tPg0KPiA+
IC0tLQ0KPiA+IENoYW5nZXMgZm9yIHYyOg0KPiA+IAktIHVzZSBhIHNwYWNlIGluc3RlYWQgb2Yg
dGFiDQo+ID4gCS0gdXNlIGZsZXhjYW5fcG9sbF9zdGF0ZSBpbnN0ZWFkIG9mIHByaW50IENoYW5n
ZXMgZm9yIHYzOg0KPiA+IAktIHJldHVybiBJUlFfSEFORExFRCBpZiBlcnIgaXMgdHJpZ2dlcmVk
DQo+ID4gCS0gc3RvcCB0cmFuc21pdHRlZCBwYWNrZXRzIHdoZW4gdGhlcmUgaXMgYW4gZXJyX2lu
dGVycnVwdA0KPiA+DQo+ID4gIGRyaXZlcnMvbmV0L2Nhbi9mbGV4Y2FuLmMgfCAzNSArKysrKysr
KysrKysrKysrKysrKysrKysrKysrKysrKysrLQ0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgMzQgaW5z
ZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv
bmV0L2Nhbi9mbGV4Y2FuLmMgYi9kcml2ZXJzL25ldC9jYW4vZmxleGNhbi5jDQo+ID4gaW5kZXgg
ZjQyNWVjMi4uNjgwMmEyNSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL25ldC9jYW4vZmxleGNh
bi5jDQo+ID4gKysrIGIvZHJpdmVycy9uZXQvY2FuL2ZsZXhjYW4uYw0KPiA+IEBAIC0yMDgsNiAr
MjA4LDcgQEAgc3RydWN0IGZsZXhjYW5fcHJpdiB7DQo+ID4gIAl2b2lkIF9faW9tZW0gKmJhc2U7
DQo+ID4gIAl1MzIgcmVnX2VzcjsNCj4gPiAgCXUzMiByZWdfY3RybF9kZWZhdWx0Ow0KPiA+ICsJ
dW5zaWduZWQgaW50IGVycl9pcnE7DQo+ID4NCj4gPiAgCXN0cnVjdCBjbGsgKmNsa19pcGc7DQo+
ID4gIAlzdHJ1Y3QgY2xrICpjbGtfcGVyOw0KPiA+IEBAIC03NDQsNiArNzQ1LDI0IEBAIHN0YXRp
YyBpcnFyZXR1cm5fdCBmbGV4Y2FuX2lycShpbnQgaXJxLCB2b2lkDQo+ICpkZXZfaWQpDQo+ID4g
IAlyZXR1cm4gSVJRX0hBTkRMRUQ7DQo+ID4gIH0NCj4gPg0KPiA+ICtzdGF0aWMgaXJxcmV0dXJu
X3QgZmxleGNhbl9lcnJfaXJxKGludCBpcnEsIHZvaWQgKmRldl9pZCkgew0KPiA+ICsJc3RydWN0
IG5ldF9kZXZpY2UgKmRldiA9IGRldl9pZDsNCj4gPiArCXN0cnVjdCBmbGV4Y2FuX3ByaXYgKnBy
aXYgPSBuZXRkZXZfcHJpdihkZXYpOw0KPiA+ICsJc3RydWN0IGZsZXhjYW5fcmVncyBfX2lvbWVt
ICpyZWdzID0gcHJpdi0+YmFzZTsNCj4gPiArCXUzMiByZWdfY3RybCwgcmVnX2VzcjsNCj4gPiAr
DQo+ID4gKwlyZWdfZXNyID0gZmxleGNhbl9yZWFkKCZyZWdzLT5lc3IpOw0KPiA+ICsJcmVnX2N0
cmwgPSBmbGV4Y2FuX3JlYWQoJnJlZ3MtPmN0cmwpOw0KPiA+ICsJaWYgKHJlZ19lc3IgJiBGTEVY
Q0FOX0VTUl9UWF9XUk4pIHsNCj4gDQo+IFdoZW4gZG9lcyB0aGUgaGFyZHdhcmUgdHJpZ2dlciB0
aGUgaW50ZXJydXB0Pw0KDQpXaGVuIHRoZXJlIGlzIG5vIHdpcmUgbGluayBiZXR3ZWVuIHR4IGFu
ZCByeCwgdHggc3RhcnQgdHJhbnNmZXIgYW5kIGRvZXNu4oCZdCBnZXQgdGhlIGFjay4NCg0KPiAN
Cj4gPiArCQlmbGV4Y2FuX3dyaXRlKHJlZ19lc3IgJiB+RkxFWENBTl9FU1JfVFhfV1JOLCAmcmVn
cy0+ZXNyKTsNCj4gPiArCQlmbGV4Y2FuX3dyaXRlKHJlZ19jdHJsICYgfkZMRVhDQU5fQ1RSTF9F
UlJfTVNLLCAmcmVncy0+Y3RybCk7DQo+ID4gKwkJbmV0aWZfc3RvcF9xdWV1ZShkZXYpOw0KPiAN
Cj4gV2h5IGFyZSB5b3Ugc3RvcHBpbmcgdGhlIHR4cXVldWU/DQoNClRoZXJlIGlzIG5vIHdpcmUg
bGluaywgdHggY2FuJ3QgdHJhbnNmZXIgc3VjY2Vzc2Z1bGx5LiANCg0KPiANCj4gPiArCQlyZXR1
cm4gSVJRX0hBTkRMRUQ7DQo+ID4gKwl9DQo+ID4gKwlyZXR1cm4gSVJRX05PTkU7DQo+ID4gK30N
Cj4gPiArDQo+IA0KPiBNYXJjDQo+IA0KPiAtLQ0KPiBQZW5ndXRyb25peCBlLksuICAgICAgICAg
ICAgICAgICAgfCBNYXJjIEtsZWluZS1CdWRkZSAgICAgICAgICAgfA0KPiBJbmR1c3RyaWFsIExp
bnV4IFNvbHV0aW9ucyAgICAgICAgfCBQaG9uZTogKzQ5LTIzMS0yODI2LTkyNCAgICAgfA0KPiBW
ZXJ0cmV0dW5nIFdlc3QvRG9ydG11bmQgICAgICAgICAgfCBGYXg6ICAgKzQ5LTUxMjEtMjA2OTE3
LTU1NTUgfA0KPiBBbXRzZ2VyaWNodCBIaWxkZXNoZWltLCBIUkEgMjY4NiAgfCBodHRwOi8vd3d3
LnBlbmd1dHJvbml4LmRlICAgfA0KDQo=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
  2014-06-23  7:26   ` qiang.zhao
@ 2014-06-23  7:37     ` Marc Kleine-Budde
  2014-06-23  8:15       ` qiang.zhao
  0 siblings, 1 reply; 7+ messages in thread
From: Marc Kleine-Budde @ 2014-06-23  7:37 UTC (permalink / raw)
  To: qiang.zhao, linuxppc-dev, linux-can, wg, Scott Wood

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

On 06/23/2014 09:26 AM, qiang.zhao@freescale.com wrote:
> 
> On 06/23/2014 03:18 PM, Marc Kleine-Budde wrote:
> 
>>
>> On 06/23/2014 09:11 AM, Zhao Qiang wrote:
>>> when flexcan is not physically linked, command 'cantest' will trigger
>>> an err_irq, add err_irq handler for it.
>>>
>>> Signed-off-by: Zhao Qiang <B45475@freescale.com>
>>> ---
>>> Changes for v2:
>>> 	- use a space instead of tab
>>> 	- use flexcan_poll_state instead of print Changes for v3:
>>> 	- return IRQ_HANDLED if err is triggered
>>> 	- stop transmitted packets when there is an err_interrupt
>>>
>>>  drivers/net/can/flexcan.c | 35 ++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index f425ec2..6802a25 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>> @@ -208,6 +208,7 @@ struct flexcan_priv {
>>>  	void __iomem *base;
>>>  	u32 reg_esr;
>>>  	u32 reg_ctrl_default;
>>> +	unsigned int err_irq;
>>>
>>>  	struct clk *clk_ipg;
>>>  	struct clk *clk_per;
>>> @@ -744,6 +745,24 @@ static irqreturn_t flexcan_irq(int irq, void
>> *dev_id)
>>>  	return IRQ_HANDLED;
>>>  }
>>>
>>> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id) {
>>> +	struct net_device *dev = dev_id;
>>> +	struct flexcan_priv *priv = netdev_priv(dev);
>>> +	struct flexcan_regs __iomem *regs = priv->base;
>>> +	u32 reg_ctrl, reg_esr;
>>> +
>>> +	reg_esr = flexcan_read(&regs->esr);
>>> +	reg_ctrl = flexcan_read(&regs->ctrl);
>>> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
>>
>> When does the hardware trigger the interrupt?
> 
> When there is no wire link between tx and rx, tx start transfer and doesn’t get the ack.

You are testing for the warning interrupt, not for the
FLEXCAN_ESR_ACK_ERR (which is triggered there isn't any ACK).

>>> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
>>> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
>>> +		netif_stop_queue(dev);
>>
>> Why are you stopping the txqueue?
> 
> There is no wire link, tx can't transfer successfully. 

You are testing for the warning interrupt, which is triggered if the
error counter increases from 95 to 96. And the error counter can
increase due to several reasons. No link is only one of them. If the CAN
core cannot transmit new packages any more the flow control in the
driver will take care.

What about calling the normal interrupt if er err_irq occurs, as this
function will take care of both normal and error interrupts anyway?

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
  2014-06-23  7:37     ` Marc Kleine-Budde
@ 2014-06-23  8:15       ` qiang.zhao
  2014-06-23  8:31         ` Marc Kleine-Budde
  0 siblings, 1 reply; 7+ messages in thread
From: qiang.zhao @ 2014-06-23  8:15 UTC (permalink / raw)
  To: Marc Kleine-Budde, linuxppc-dev, linux-can, wg, Scott Wood

T24gMDYvMjMvMjAxNCAwMzozNyBQTSwgTWFyYyBLbGVpbmUtQnVkZGUgd3JvdGU6DQoNCj4gLS0t
LS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogTWFyYyBLbGVpbmUtQnVkZGUgW21haWx0
bzpta2xAcGVuZ3V0cm9uaXguZGVdDQo+IFNlbnQ6IE1vbmRheSwgSnVuZSAyMywgMjAxNCAzOjM3
IFBNDQo+IFRvOiBaaGFvIFFpYW5nLUI0NTQ3NTsgbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5v
cmc7IGxpbnV4LQ0KPiBjYW5Admdlci5rZXJuZWwub3JnOyB3Z0BncmFuZGVnZ2VyLmNvbTsgV29v
ZCBTY290dC1CMDc0MjENCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2MyAxLzJdIGZsZXhjYW46IGFk
ZCBlcnJfaXJxIGhhbmRsZXIgZm9yIGZsZXhjYW4NCj4gDQo+IE9uIDA2LzIzLzIwMTQgMDk6MjYg
QU0sIHFpYW5nLnpoYW9AZnJlZXNjYWxlLmNvbSB3cm90ZToNCj4gPg0KPiA+IE9uIDA2LzIzLzIw
MTQgMDM6MTggUE0sIE1hcmMgS2xlaW5lLUJ1ZGRlIHdyb3RlOg0KPiA+DQo+ID4+DQo+ID4+IE9u
IDA2LzIzLzIwMTQgMDk6MTEgQU0sIFpoYW8gUWlhbmcgd3JvdGU6DQo+ID4+PiB3aGVuIGZsZXhj
YW4gaXMgbm90IHBoeXNpY2FsbHkgbGlua2VkLCBjb21tYW5kICdjYW50ZXN0JyB3aWxsDQo+ID4+
PiB0cmlnZ2VyIGFuIGVycl9pcnEsIGFkZCBlcnJfaXJxIGhhbmRsZXIgZm9yIGl0Lg0KPiA+Pj4N
Cj4gPj4+IFNpZ25lZC1vZmYtYnk6IFpoYW8gUWlhbmcgPEI0NTQ3NUBmcmVlc2NhbGUuY29tPg0K
PiA+Pj4gLS0tDQo+ID4+PiBDaGFuZ2VzIGZvciB2MjoNCj4gPj4+IAktIHVzZSBhIHNwYWNlIGlu
c3RlYWQgb2YgdGFiDQo+ID4+PiAJLSB1c2UgZmxleGNhbl9wb2xsX3N0YXRlIGluc3RlYWQgb2Yg
cHJpbnQgQ2hhbmdlcyBmb3IgdjM6DQo+ID4+PiAJLSByZXR1cm4gSVJRX0hBTkRMRUQgaWYgZXJy
IGlzIHRyaWdnZXJlZA0KPiA+Pj4gCS0gc3RvcCB0cmFuc21pdHRlZCBwYWNrZXRzIHdoZW4gdGhl
cmUgaXMgYW4gZXJyX2ludGVycnVwdA0KPiA+Pj4NCj4gPj4+ICBkcml2ZXJzL25ldC9jYW4vZmxl
eGNhbi5jIHwgMzUgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0NCj4gPj4+ICAx
IGZpbGUgY2hhbmdlZCwgMzQgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiA+Pj4NCj4g
Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL25ldC9jYW4vZmxleGNhbi5jIGIvZHJpdmVycy9uZXQv
Y2FuL2ZsZXhjYW4uYw0KPiA+Pj4gaW5kZXggZjQyNWVjMi4uNjgwMmEyNSAxMDA2NDQNCj4gPj4+
IC0tLSBhL2RyaXZlcnMvbmV0L2Nhbi9mbGV4Y2FuLmMNCj4gPj4+ICsrKyBiL2RyaXZlcnMvbmV0
L2Nhbi9mbGV4Y2FuLmMNCj4gPj4+IEBAIC0yMDgsNiArMjA4LDcgQEAgc3RydWN0IGZsZXhjYW5f
cHJpdiB7DQo+ID4+PiAgCXZvaWQgX19pb21lbSAqYmFzZTsNCj4gPj4+ICAJdTMyIHJlZ19lc3I7
DQo+ID4+PiAgCXUzMiByZWdfY3RybF9kZWZhdWx0Ow0KPiA+Pj4gKwl1bnNpZ25lZCBpbnQgZXJy
X2lycTsNCj4gPj4+DQo+ID4+PiAgCXN0cnVjdCBjbGsgKmNsa19pcGc7DQo+ID4+PiAgCXN0cnVj
dCBjbGsgKmNsa19wZXI7DQo+ID4+PiBAQCAtNzQ0LDYgKzc0NSwyNCBAQCBzdGF0aWMgaXJxcmV0
dXJuX3QgZmxleGNhbl9pcnEoaW50IGlycSwgdm9pZA0KPiA+PiAqZGV2X2lkKQ0KPiA+Pj4gIAly
ZXR1cm4gSVJRX0hBTkRMRUQ7DQo+ID4+PiAgfQ0KPiA+Pj4NCj4gPj4+ICtzdGF0aWMgaXJxcmV0
dXJuX3QgZmxleGNhbl9lcnJfaXJxKGludCBpcnEsIHZvaWQgKmRldl9pZCkgew0KPiA+Pj4gKwlz
dHJ1Y3QgbmV0X2RldmljZSAqZGV2ID0gZGV2X2lkOw0KPiA+Pj4gKwlzdHJ1Y3QgZmxleGNhbl9w
cml2ICpwcml2ID0gbmV0ZGV2X3ByaXYoZGV2KTsNCj4gPj4+ICsJc3RydWN0IGZsZXhjYW5fcmVn
cyBfX2lvbWVtICpyZWdzID0gcHJpdi0+YmFzZTsNCj4gPj4+ICsJdTMyIHJlZ19jdHJsLCByZWdf
ZXNyOw0KPiA+Pj4gKw0KPiA+Pj4gKwlyZWdfZXNyID0gZmxleGNhbl9yZWFkKCZyZWdzLT5lc3Ip
Ow0KPiA+Pj4gKwlyZWdfY3RybCA9IGZsZXhjYW5fcmVhZCgmcmVncy0+Y3RybCk7DQo+ID4+PiAr
CWlmIChyZWdfZXNyICYgRkxFWENBTl9FU1JfVFhfV1JOKSB7DQo+ID4+DQo+ID4+IFdoZW4gZG9l
cyB0aGUgaGFyZHdhcmUgdHJpZ2dlciB0aGUgaW50ZXJydXB0Pw0KPiA+DQo+ID4gV2hlbiB0aGVy
ZSBpcyBubyB3aXJlIGxpbmsgYmV0d2VlbiB0eCBhbmQgcngsIHR4IHN0YXJ0IHRyYW5zZmVyIGFu
ZA0KPiBkb2VzbuKAmXQgZ2V0IHRoZSBhY2suDQo+IA0KPiBZb3UgYXJlIHRlc3RpbmcgZm9yIHRo
ZSB3YXJuaW5nIGludGVycnVwdCwgbm90IGZvciB0aGUNCj4gRkxFWENBTl9FU1JfQUNLX0VSUiAo
d2hpY2ggaXMgdHJpZ2dlcmVkIHRoZXJlIGlzbid0IGFueSBBQ0spLg0KPiANCj4gPj4+ICsJCWZs
ZXhjYW5fd3JpdGUocmVnX2VzciAmIH5GTEVYQ0FOX0VTUl9UWF9XUk4sICZyZWdzLT5lc3IpOw0K
PiA+Pj4gKwkJZmxleGNhbl93cml0ZShyZWdfY3RybCAmIH5GTEVYQ0FOX0NUUkxfRVJSX01TSywg
JnJlZ3MtPmN0cmwpOw0KPiA+Pj4gKwkJbmV0aWZfc3RvcF9xdWV1ZShkZXYpOw0KPiA+Pg0KPiA+
PiBXaHkgYXJlIHlvdSBzdG9wcGluZyB0aGUgdHhxdWV1ZT8NCj4gPg0KPiA+IFRoZXJlIGlzIG5v
IHdpcmUgbGluaywgdHggY2FuJ3QgdHJhbnNmZXIgc3VjY2Vzc2Z1bGx5Lg0KPiANCj4gWW91IGFy
ZSB0ZXN0aW5nIGZvciB0aGUgd2FybmluZyBpbnRlcnJ1cHQsIHdoaWNoIGlzIHRyaWdnZXJlZCBp
ZiB0aGUNCj4gZXJyb3IgY291bnRlciBpbmNyZWFzZXMgZnJvbSA5NSB0byA5Ni4gQW5kIHRoZSBl
cnJvciBjb3VudGVyIGNhbiBpbmNyZWFzZQ0KPiBkdWUgdG8gc2V2ZXJhbCByZWFzb25zLiBObyBs
aW5rIGlzIG9ubHkgb25lIG9mIHRoZW0uIElmIHRoZSBDQU4gY29yZQ0KPiBjYW5ub3QgdHJhbnNt
aXQgbmV3IHBhY2thZ2VzIGFueSBtb3JlIHRoZSBmbG93IGNvbnRyb2wgaW4gdGhlIGRyaXZlciB3
aWxsDQo+IHRha2UgY2FyZS4NCg0KV2hlbiBUeCBlcnJvciBjb3VudGVyIGluY3JlYXNlcyBmcm9t
IDk1IHRvIDk2LCB0aGVyZSBtdXN0IGJlIGlzc3VlIGZvciB0eCwNClNvIHdoeSBjYW4ndCBJIHN0
b3AgdGhlIHR4cXVldWU/IA0KWW91IHNhaWQgdGhhdCB0aGVyZSBhcmUgc2V2ZXJhbCByZWFzb25z
LCB3b3VsZCB5b3UgbGlrZSB0byB0YWtlIHNvbWUgZXhhbXBsZXM/DQoNCj4gDQo+IFdoYXQgYWJv
dXQgY2FsbGluZyB0aGUgbm9ybWFsIGludGVycnVwdCBpZiBlciBlcnJfaXJxIG9jY3VycywgYXMg
dGhpcw0KPiBmdW5jdGlvbiB3aWxsIHRha2UgY2FyZSBvZiBib3RoIG5vcm1hbCBhbmQgZXJyb3Ig
aW50ZXJydXB0cyBhbnl3YXk/DQoNCkNhbGxpbmcgdGhlIG5vcm1hbCBpbnRlcnJ1cHQgZG9lc24n
dCB3b3JrLg0KDQo+IA0KPiBNYXJjDQo+IA0KPiAtLQ0KPiBQZW5ndXRyb25peCBlLksuICAgICAg
ICAgICAgICAgICAgfCBNYXJjIEtsZWluZS1CdWRkZSAgICAgICAgICAgfA0KPiBJbmR1c3RyaWFs
IExpbnV4IFNvbHV0aW9ucyAgICAgICAgfCBQaG9uZTogKzQ5LTIzMS0yODI2LTkyNCAgICAgfA0K
PiBWZXJ0cmV0dW5nIFdlc3QvRG9ydG11bmQgICAgICAgICAgfCBGYXg6ICAgKzQ5LTUxMjEtMjA2
OTE3LTU1NTUgfA0KPiBBbXRzZ2VyaWNodCBIaWxkZXNoZWltLCBIUkEgMjY4NiAgfCBodHRwOi8v
d3d3LnBlbmd1dHJvbml4LmRlICAgfA0KDQo=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] flexcan: add err_irq handler for flexcan
  2014-06-23  8:15       ` qiang.zhao
@ 2014-06-23  8:31         ` Marc Kleine-Budde
  0 siblings, 0 replies; 7+ messages in thread
From: Marc Kleine-Budde @ 2014-06-23  8:31 UTC (permalink / raw)
  To: qiang.zhao, linuxppc-dev, linux-can, wg, Scott Wood

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

On 06/23/2014 10:15 AM, qiang.zhao@freescale.com wrote:
[...]
>>>>> +	reg_esr = flexcan_read(&regs->esr);
>>>>> +	reg_ctrl = flexcan_read(&regs->ctrl);
>>>>> +	if (reg_esr & FLEXCAN_ESR_TX_WRN) {
>>>>
>>>> When does the hardware trigger the interrupt?
>>>
>>> When there is no wire link between tx and rx, tx start transfer and
>> doesn’t get the ack.
>>
>> You are testing for the warning interrupt, not for the
>> FLEXCAN_ESR_ACK_ERR (which is triggered there isn't any ACK).
>>
>>>>> +		flexcan_write(reg_esr & ~FLEXCAN_ESR_TX_WRN, &regs->esr);
>>>>> +		flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, &regs->ctrl);
>>>>> +		netif_stop_queue(dev);
>>>>
>>>> Why are you stopping the txqueue?
>>>
>>> There is no wire link, tx can't transfer successfully.
>>
>> You are testing for the warning interrupt, which is triggered if the
>> error counter increases from 95 to 96. And the error counter can increase
>> due to several reasons. No link is only one of them. If the CAN core
>> cannot transmit new packages any more the flow control in the driver will
>> take care.
> 
> When Tx error counter increases from 95 to 96, there must be issue for tx,
> So why can't I stop the txqueue? 

Why do you want to stop the queue? It's compliant with the CAN spec to
keep sending CAN frames if the error counter increases to 96. If there
isn't any problem with the CAN bus anymore, the TX error counters will
decrease with every successfully transmitted CAN frame.

> You said that there are several reasons, would you like to take some examples?

See CAN Error Confinement Rules in
http://www.can-wiki.info/doku.php?id=can_faq_erors

>> What about calling the normal interrupt if er err_irq occurs, as this
>> function will take care of both normal and error interrupts anyway?
> 
> Calling the normal interrupt doesn't work.

Why?

flexcan_irq() if FLEXCAN_CTRL_TWRN_MSK is set and will schedule the NAPI
routine:

> 	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
> 	    (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> 	    flexcan_has_and_handle_berr(priv, reg_esr)) {
[...]
> 		napi_schedule(&priv->napi);
> 	}

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-06-23  8:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-23  7:11 [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Zhao Qiang
2014-06-23  7:11 ` [PATCH v3 2/2] flexcan: add err interrupt for p1010rdb Zhao Qiang
2014-06-23  7:17 ` [PATCH v3 1/2] flexcan: add err_irq handler for flexcan Marc Kleine-Budde
2014-06-23  7:26   ` qiang.zhao
2014-06-23  7:37     ` Marc Kleine-Budde
2014-06-23  8:15       ` qiang.zhao
2014-06-23  8:31         ` Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).