* Re: [PATCH v2 1/2] can: mcba_usb: fix typo
From: Marc Kleine-Budde @ 2017-11-28 8:46 UTC (permalink / raw)
To: Martin Kelly, linux-can
Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <20171127234916.25865-1-mkelly@xevo.com>
[-- Attachment #1.1: Type: text/plain, Size: 452 bytes --]
On 11/28/2017 12:49 AM, Martin Kelly wrote:
> Fix typo "analizer" --> "Analyzer".
>
> Signed-off-by: Martin Kelly <mkelly@xevo.com>
Both applied to can.
Tnx,
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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can
Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <2f7df48e-ee2a-f6b8-3251-e839b6bbdc73@pengutronix.de>
On 11/27/2017 03:17 PM, Marc Kleine-Budde wrote:
> On 11/27/2017 09:41 PM, Martin Kelly wrote:
>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
>
> Please add a patch description. Let's have every patch one.
>
> Marc
>
Alright, I sent a v2 series with a description.
^ permalink raw reply
* [PATCH v2 2/2] can: mcba_usb: fix device disconnect bug
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
To: linux-can
Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
Wolfgang Grandegger, Martin Kelly
In-Reply-To: <20171127234916.25865-1-mkelly@xevo.com>
Currently, when you disconnect the device, the driver infinitely
resubmits all URBs, so you see:
Rx URB aborted (-32)
in an infinite loop.
Fix this by catching -EPIPE (what we get in urb->status when the device
disconnects) and not resubmitting.
With this patch, I can plug and unplug many times and the driver
recovers correctly.
Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
drivers/net/can/usb/mcba_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index c4355f0a20d5..ef417dcddbf7 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -592,6 +592,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
break;
case -ENOENT:
+ case -EPIPE:
case -ESHUTDOWN:
return;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
To: linux-can
Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
Wolfgang Grandegger, Martin Kelly
Fix typo "analizer" --> "Analyzer".
Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
drivers/net/can/usb/mcba_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 7f0272558bef..c4355f0a20d5 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -862,7 +862,7 @@ static int mcba_usb_probe(struct usb_interface *intf,
goto cleanup_unregister_candev;
}
- dev_info(&intf->dev, "Microchip CAN BUS analizer connected\n");
+ dev_info(&intf->dev, "Microchip CAN BUS Analyzer connected\n");
return 0;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 1/2] can: mcba_usb: fix typo
From: Marc Kleine-Budde @ 2017-11-27 23:17 UTC (permalink / raw)
To: Martin Kelly, linux-can
Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <20171127204200.32599-1-mkelly@xevo.com>
[-- Attachment #1.1: Type: text/plain, Size: 442 bytes --]
On 11/27/2017 09:41 PM, Martin Kelly wrote:
> Signed-off-by: Martin Kelly <mkelly@xevo.com>
Please add a patch description. Let's have every patch one.
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: 488 bytes --]
^ permalink raw reply
* [PATCH 2/2] can: mcba_usb: fix device disconnect bug
From: Martin Kelly @ 2017-11-27 20:42 UTC (permalink / raw)
To: linux-can
Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
Wolfgang Grandegger, Martin Kelly
In-Reply-To: <20171127204200.32599-1-mkelly@xevo.com>
Currently, when you disconnect the device, the driver infinitely
resubmits all URBs, so you see:
Rx URB aborted (-32)
in an infinite loop.
Fix this by catching -EPIPE (what we get in urb->status when the device
disconnects) and not resubmitting.
With this patch, I can plug and unplug many times and the driver
recovers correctly.
Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
drivers/net/can/usb/mcba_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index c4355f0a20d5..ef417dcddbf7 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -592,6 +592,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
break;
case -ENOENT:
+ case -EPIPE:
case -ESHUTDOWN:
return;
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 20:41 UTC (permalink / raw)
To: linux-can
Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
Wolfgang Grandegger, Martin Kelly
Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
drivers/net/can/usb/mcba_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 7f0272558bef..c4355f0a20d5 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -862,7 +862,7 @@ static int mcba_usb_probe(struct usb_interface *intf,
goto cleanup_unregister_candev;
}
- dev_info(&intf->dev, "Microchip CAN BUS analizer connected\n");
+ dev_info(&intf->dev, "Microchip CAN BUS Analyzer connected\n");
return 0;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Stefan Agner @ 2017-11-27 16:34 UTC (permalink / raw)
To: Mirza Krak, Marc Kleine-Budde
Cc: ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal, Wolfgang Grandegger,
linux-can, Varun Sethi, Poonam Aggrwal
In-Reply-To: <CALw8SCWNXJYuNWDtAxS9iFkKjMhcwQ60Pi7Mq3ne=k0Oynr47w@mail.gmail.com>
On 2017-11-23 21:17, Mirza Krak wrote:
> 2017-11-23 21:12 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
>> On Nov 22, 2017 13:00, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
>>
>
> < snip >
>
>>
>> Cc'ed Stefan Agner - maybe he has access to this SoC.
>>
Without reading the full thread, I did some testing back when we added
vf610 support:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275903.html
>>> @Marc, @Wolfgang,
>>> What do you think? Or shall we fix first and then wait?
>>
>> Let's wait if Stefan can test, otherwise just fix.
>>
>>
>> I have access to an vf610 and can test CAN stuff.
>>
>> What is it more specifically we want to test? I read the thread but not
>> completely clear to me.
>
Thanks Mirza for your testing!
--
Stefan
^ permalink raw reply
* Re: [PATCH] can: ti_hecc: Fix napi poll return value for repoll
From: Marc Kleine-Budde @ 2017-11-27 14:18 UTC (permalink / raw)
To: Oliver Stäbler, Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <20171120134515.2658-1-oliver.staebler@bytesatwork.ch>
[-- Attachment #1.1: Type: text/plain, Size: 622 bytes --]
On 11/20/2017 02:45 PM, Oliver Stäbler wrote:
> After commit d75b1ade567f ("net: less interrupt masking in NAPI") napi
> repoll is done only when work_done == budget.
> So we need to return budget if there are still packets to receive.
>
> Signed-off-by: Oliver Stäbler <oliver.staebler@bytesatwork.ch>
Applied to can.
Tnx,
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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] can: peak/pci: fix potential bug when probe() fails
From: Marc Kleine-Budde @ 2017-11-27 14:16 UTC (permalink / raw)
To: Stephane Grosjean, Oliver Hartkopp; +Cc: linux-can Mailing List
In-Reply-To: <20171123144435.12282-1-s.grosjean@peak-system.com>
[-- Attachment #1.1: Type: text/plain, Size: 855 bytes --]
On 11/23/2017 03:44 PM, Stephane Grosjean wrote:
> PCI/PCIe drivers for PEAK-System CAN/CAN-FD interfaces do some access to the
> PCI config during probing. In case one of these accesses fails, a POSITIVE
> PCIBIOS_xxx error code is returned back. This POSITIVE error code MUST be
> converted into a NEGATIVE errno for the probe() function to indicate it
> failed. Using the pcibios_err_to_errno() function, we make sure that the
> return code will always be negative.
>
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Applied to can.
Tnx,
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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/6] Remodel FlexCAN register r/w APIs for big endian
From: Marc Kleine-Budde @ 2017-11-27 14:07 UTC (permalink / raw)
To: Pankaj Bansal, wg-5Yr1BZd7O62+XT7JhA+gdA,
linux-can-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: V.Sethi-3arQi8VN3Tc, poonam.aggrwal-3arQi8VN3Tc
In-Reply-To: <1511529733-27942-1-git-send-email-pankaj.bansal-3arQi8VN3Tc@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1608 bytes --]
On 11/24/2017 02:22 PM, Pankaj Bansal wrote:
> This patch series remodels the FlexCAN register r/w APIs for big endian.
> The endianness is checked based on optional big-endian property in
> device tree. if this property is not present in device tree node then
> controller is assumed to be little endian. if this property is present then
> controller is assumed to be big endian.
>
> An exception to this rule is powerpc P1010RDB, which is always
> big-endian, even if big-endian is not present in dts. This is
> checked using p1010-flexcan compatible in dts.
>
> Therefore, remove p1010-flexcan compatible from imx series dts,
> as their flexcan core is little endian.
>
> Finally this series adds support for NXP LS1021A SOC in flexcan,
> which is arm based SOC having big-endian FlexCAN controller.
>
> Pankaj Bansal (6):
> can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN
> controllers.
> can: flexcan: adding platform specific details for LS1021A
> Documentation : can : flexcan : Add big-endian property to device tree
> powerpc: dts: P1010: Add endianness property to flexcan node
> arm: dts: Remove p1010-flexcan compatible from imx series dts
> arm/dts: Add nodes for flexcan devices present on LS1021A-Rev2 SoC
Applied to can-next.
Thanks,
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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v4 1/6] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
From: Marc Kleine-Budde @ 2017-11-27 14:06 UTC (permalink / raw)
To: Pankaj Bansal, wg@grandegger.com, linux-can@vger.kernel.org,
robh+dt@kernel.org, mark.rutland@arm.com,
devicetree@vger.kernel.org
Cc: Varun Sethi, Poonam Aggrwal, Bhupesh Sharma, Sakar Arora
In-Reply-To: <AM0PR0402MB39405B9B5E5C04B2492221D6F1240@AM0PR0402MB3940.eurprd04.prod.outlook.com>
[-- Attachment #1.1: Type: text/plain, Size: 1574 bytes --]
On 11/26/2017 03:20 AM, Pankaj Bansal wrote:
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index a13a489..4c873fb 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>
>> [...]
>> I think this will select LE for non DT devices, right?
>>
>
> Yes. As per below code snippet:
>
> static struct property *__of_find_property(const struct device_node *np,
> const char *name, int *lenp)
> {
> struct property *pp;
>
> if (!np)
> return NULL;
> ....
> }
>
> If no device node is present null is returned.
> So we select le as default.
This is what I found out, too.
>>> + if (of_property_read_bool(pdev->dev.of_node, "big-endian")) {
>>> + priv->read = flexcan_read_be;
>>> + priv->write = flexcan_write_be;
>>> + } else {
>>> + if (of_device_is_compatible(pdev->dev.of_node,
>>> + "fsl,p1010-flexcan")) {
>>> + priv->read = flexcan_read_be;
>>> + priv->write = flexcan_write_be;
>>> + } else {
>>> + priv->read = flexcan_read_le;
>>> + priv->write = flexcan_write_le;
>>> + }
>>> + }
>>> +
>>> priv->can.clock.freq = clock_freq;
>>> priv->can.bittiming_const = &flexcan_bittiming_const;
>>> priv->can.do_set_mode = flexcan_set_mode;
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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Marc Kleine-Budde @ 2017-11-27 14:00 UTC (permalink / raw)
To: Mirza Krak, Wolfgang Grandegger
Cc: ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal,
linux-can@vger.kernel.org, Varun Sethi, Poonam Aggrwal,
Stefan Agner
In-Reply-To: <CALw8SCWK55Byqackx3YQ_h8c5Jo5JFLtt3-WJryHwOEtP-Rtgw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 883 bytes --]
On 11/26/2017 10:11 PM, Mirza Krak wrote:
>>> Will try again next week when I am back at the office.
>>>
>>> I ran my tests on 4.14 kernel.
>>
>> This Flexcan core needs the FLEXCAN_QUIRK_BROKEN_PERR_STATE as well, like
>> all other cores. Adding that quirk for the vf610 will cure the problems.
>
> I will probably get some time during the coming week to test adding
> FLEXCAN_QUIRK_BROKEN_PERR_STATE and I can send a patch if it all looks
> good.
I've created a patch, waiting for your Tested-by.
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git/log/?h=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: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: Rob Herring @ 2017-11-26 22:28 UTC (permalink / raw)
To: kernel-TqfNSX0MhmxHKSADF0wUEw
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Mark Rutland,
linux-can-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171124183509.12810-2-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
On Fri, Nov 24, 2017 at 06:35:08PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>
> Add device-tree bindings for Microcip CanFD Controller mcp2517fd
s/Microcip/Microchip/
>
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
> .../bindings/net/can/microchip,mcp2517fd.txt | 47 ++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
>
> diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt b/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
> new file mode 100644
> index 000000000000..96cbf0c96895
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
> @@ -0,0 +1,47 @@
> +* Microchip MCP2517 stand-alone CAN controller device tree bindings
> +
> +Required properties:
> + - compatible: Should be one of the following:
> + - "microchip,mcp2517fd" for MCP2517fd.
> + - reg: SPI chip select.
> + - clocks: The clock feeding the CAN controller.
> + - interrupt-parent: The parent interrupt controller.
> + - interrupts: Should contain IRQ line for the CAN controller.
> +
> +Optional properties:
> + - vdd-supply: Regulator that powers the CAN controller.
> + - xceiver-supply: Regulator that powers the CAN transceiver.
> + - microchip,clock_out_div = <0|1|2|4|10>: Clock output pin divider
s/_/-/
And on the rest. Don't use underscores...
> + 0 = Start of Frame output
> + default: 10
> + - microchip,clock_div = <1|2>: internal clock divider - default 1
> + - microchip,gpio_opendrain: gpio (int0,1) in open drain mode
> + instead of default push/pull
> + - microchip,int_opendrain: int pin in open drain mode
> + instead of default push/pull
IIRC, we have a standard property for this.
> + - microchip,txcan_opendrain: txcan pin in open drain mode
> + instead of default push/pull
> + - microchip,gpio0_mode : gpio mode functionality
> + 0 = input
> + 1 = TX interrupt output - default
> + 2 = output default low
> + 3 = output default high
> + 4 = (tx) transceiver standby
> + - microchip,gpio1_mode : gpio mode functionality
> + 0 = input - default
> + 1 = RX interrupt output - default
> + 2 = output default low
> + 3 = output default high
> +
> +Example:
> + can0: can@1 {
> + compatible = "microchip,mcp2515";
> + reg = <1>;
> + clocks = <&clk24m>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <13 0x8>;
> + vdd-supply = <®5v0>;
> + xceiver-supply = <®5v0>;
> + microchip,gpio0_mode = <4>;
> + microchip,gpio0_mode = <1>;
> + };
> --
> 2.11.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/6] Documentation : can : flexcan : Add big-endian property to device tree
From: Rob Herring @ 2017-11-26 22:18 UTC (permalink / raw)
To: Pankaj Bansal
Cc: wg-5Yr1BZd7O62+XT7JhA+gdA, mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, V.Sethi-3arQi8VN3Tc,
poonam.aggrwal-3arQi8VN3Tc
In-Reply-To: <1511529733-27942-4-git-send-email-pankaj.bansal-3arQi8VN3Tc@public.gmane.org>
On Fri, Nov 24, 2017 at 06:52:10PM +0530, Pankaj Bansal wrote:
> The FlexCAN controller can be modelled as little or big endian depending
> on SOC design. This device tree property identifies the controller
> endianness and the driver reads/writes controller registers based on
> that.
>
> This is optional property. i.e. if this property is not present in
> device tree node then controller is assumed to be little endian. if this
> property is present then controller is assumed to be big endian.
>
> Signed-off-by: Pankaj Bansal <pankaj.bansal-3arQi8VN3Tc@public.gmane.org>
> Reviewed-by: Poonam Aggrwal <poonam.aggrwal-3arQi8VN3Tc@public.gmane.org>
> ---
> Changes in v4:
> - document the default behaviour in the bindings if the big-endian property
> is missing.
> - Merged device tree changes and driver changes in one series
> Changes in v3:
> - No change.
> Changes in v2:
> - No change.
> - Added one more patch in series.
>
> Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Mirza Krak @ 2017-11-26 21:11 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Marc Kleine-Budde, ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal,
linux-can@vger.kernel.org, Varun Sethi, Poonam Aggrwal,
Stefan Agner
In-Reply-To: <d339bdc6-49c4-3184-6fa4-12521f491f20@grandegger.com>
2017-11-24 20:19 GMT+01:00 Wolfgang Grandegger <wg@grandegger.com>:
> Hello Mirza,
>
< snip >
>>>
>>> You can test such state changes as show below. First setup the interface
>>> and
>>> start a session calling "candump" to report the state changes:
>>>
>>> # ip link set can0 up type can bitrate 500000 restart-ms 100
>>> # candump -td -e any,0:0,#FFFFFFFF
>>>
>>> Then
>>>
>>> 1. disconnect the cable and send messages with "cangen"
>>>
>>> # cangen -i can0
>>>
>>> "candump" should then report the error state changes "error-active" ->
>>> "warning" -> "error-passive". If you don't see "error-passive", the
>>> hardware
>>> needs the FLEXCAN_QUIRK_BROKEN_PERR_STATE as well. Then reconnect the
>>> cable
>>> and send messages till the "error active" state is reached (again with
>>> "cangen").
>>
>>
>> This was the result of this test:
>>
>> root@colibri-vf:~# ./candump -td -e can0,0:0,#FFFFFFFF
>> (000.000000) can0 0C8 [8] E7 D5 75 20 1C F8 AA 6D
>> (000.200201) can0 7CD [8] 58 5F B1 1E D7 AB 3C 15
>> (000.200123) can0 7FD [8] EC 43 EF 11 BB 76 50 4F
>> (000.200119) can0 20D [8] 64 E0 A6 74 A2 A4 22 42
>> (000.200115) can0 226 [7] CB 4D 12 46 F3 37 F1
>> (000.200000) can0 42F [0]
>> (000.200245) can0 0F9 [8] AD 61 16 77 0A 64 50 5C
>> (000.199983) can0 76E [0]
>> (000.200206) can0 3CB [6] 53 28 03 34 98 43
>> (000.200152) can0 230 [8] 70 EF 66 38 2D BA 5D 4D
>> (000.200106) can0 568 [8] E8 30 AE 1C 75 77 4E 5E
>> (000.200274) can0 20000004 [8] 00 04 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{rx-error-warning}
>>
>> and the statistics
>>
>> root@colibri-vf:~# ip -d -s link show can0
>> 2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state
>> UNKNOWN mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can state ERROR-WARNING (berr-counter tx 0 rx 128) restart-ms 100
>> bitrate 496240 sample-point 0.857
>> tq 143 prop-seg 5 phase-seg1 6 phase-seg2 2 sjw 1
>> flexcan: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..256 brp-inc 1
>> clock 83368421
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 0 0 1 0 0
>> numtxqueues 1
>> RX: bytes packets errors dropped overrun mcast
>> 8 1 0 0 0 0
>> TX: bytes packets errors dropped carrier collsns
>> 69 11 0 0 0 0
>>
>> I ran out of time as wanted to test a bit more as I was confused that
>> it is showing RX errors on the first test? I was not able to get it
>> back to ERROR-ACTIVE from ERROR-WARNING either. But posted result
>> anyway
>
>
> The received error messages are counted as well.
>
>>> 2. The second test is quite similar. Instead of disconnecting the cable,
>>> short-circuit the CAN low and high lines. The device should then go to
>>> "bus-off".
>>
>>
>> And this one:
>>
>> root@colibri-vf:~# ./candump -td -e any,0:0,#FFFFFFFF
>> (000.000000) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{tx-error-passive}
>> (000.000809) can0 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> bus-off
>> (000.109797) can0 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> restarted-after-bus-off
>> (000.100551) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{tx-error-passive}
>> (000.000823) can0 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> bus-off
>> (000.108644) can0 20000100 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> restarted-after-bus-off
>> (000.090692) can0 20000004 [8] 00 08 00 00 00 00 00 00 ERRORFRAME
>> controller-problem{tx-error-warning}
>> (000.000797) can0 20000040 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
>> bus-off
>>
>> Will try again next week when I am back at the office.
>>
>> I ran my tests on 4.14 kernel.
>
>
> This Flexcan core needs the FLEXCAN_QUIRK_BROKEN_PERR_STATE as well, like
> all other cores. Adding that quirk for the vf610 will cure the problems.
>
I will probably get some time during the coming week to test adding
FLEXCAN_QUIRK_BROKEN_PERR_STATE and I can send a patch if it all looks
good.
--
Med Vänliga Hälsningar / Best Regards
Mirza Krak
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: kernel @ 2017-11-26 19:53 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Oliver Hartkopp, Marc Kleine-Budde, Rob Herring, Mark Rutland,
linux-can, devicetree
In-Reply-To: <3b1c4778-b155-7aaf-e5b4-14061e687161@grandegger.com>
Hi Wolfgang!
> On 26.11.2017, at 20:05, Wolfgang Grandegger <wg@grandegger.com> wrote:
>> Test messages sent and received: 32
>> (I can not go to longer tests because I may hit a bug I am just
>> now trying to fix in the current code level).
>
> This test does not load the bus a lot but uses burst of message to trigger out-of-order issues. Please run the test without "-v" and much longer.
I agree, but as mentioned: I am debugging something else, which makes the
system slightly unstable, so I refrained from running longer tests...
>> If you are interested in any more specific tests that I should
>> be running, then please list those and I can report them
>> when submitting V2 of the patch set.
>
> Other useful test are about bus error reporting and error state changes.
>
> Can bus errors been disabled via interrupt? I do not see that CAN_CTRLMODE_BERR_REPORTING is handled. This means that bus error reporting is always on which may put heavy load on the system. e.g if no cable is connected and a message sent (at 1MB/s).
>
> Concerning the error state changes, have a look to [1].
Yes - it comes with the 9 (32-bit) register read on every interrupt.
This is actually an optimization to avoid kernel latencies that would
occur when reading 3 sets of registers separately.
Also longer reads means a higher likleyhood that DMA will be used
by the spi-controller/driver.
>
>> Just to put everything into perspective:
>> The equipment available to me is:
>> * beagle bone black with c_can
>> * raspberry pi 3 with mcp2515
>> * raspberry pi CM3 with mcp2517fd
>> * raspberry pi 2 with mcp2517fd
>> * saleae logic analyzer
>
> [1] https://marc.info/?l=linux-can&m=151147114323582&w=2
>
I will try to run those and add them to the cover page for V2.
> The driver has more than 3000 lines... a review will take some time.
Please look mostly at the device-tree and the main comments about the
rationale and performance optimizations at first.
Feedback on these I could include with V2 of the patch that will
fix also the above mentioned errors and MAB avoidance code.
I should be able to post it later this week.
Thanks,
Martin
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: Wolfgang Grandegger @ 2017-11-26 19:05 UTC (permalink / raw)
To: kernel
Cc: Oliver Hartkopp, Marc Kleine-Budde, Rob Herring, Mark Rutland,
linux-can, devicetree
In-Reply-To: <CEE8AB99-2B26-44C0-A8D1-DD339DC9FEF7@martin.sperl.org>
Am 26.11.2017 um 19:29 schrieb kernel@martin.sperl.org:
>
>> On 26.11.2017, at 17:18, Wolfgang Grandegger <wg@grandegger.com> wrote:
>>
>> Hello Martin,
>>
>> Am 25.11.2017 um 15:47 schrieb kernel@martin.sperl.org:
>>
>> ...snip...
>>> I see the following counters:
>>> 7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
>>> link/can promiscuity 0
>>> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>>
>> Why do you use "ONE_SHOT" in all your test cases?
> I did test oneshot and non-oneshot - I just did not document them
> here, I am just mentioning the issue of dropped frames when the driver
> is submitting them correctly.
>
> Also - for what it is worse - the RX case would not be impacted by one-shot
> enabled or not.
>
>
>> Did you also check for out-of-order issues, e.g. by using "canfdtest”?
>
> As for out of order: yes I have taken extreme care that we do not
> get out of order packets for all possible cases testing with cangen - there
> may be one error case where there could be an out of order case in the
> tx path (unless we want to drop that frame - it would be a MAB case)
>
> But until now I did not know about canfdtest, but for all practical
> purposes it can not test for out-of-order delivery on a 100% saturated
> bus the way it is working - latencies are too big for that!
>
> Still for completeness a quick test from beagleboneblack to a
> RPI CM3 with a mcp2517fd gives:
> root@beaglebone:~/can-utils# ./canfdtest -g -vvv can0 -l 32
> interface = can0, family = 29, type = 3, proto = 1
> 0078: [8] 01 02 03 04 05 06 07 08
> 0078: [8] 02 03 04 05 06 07 08 09
> 0078: [8] 03 04 05 06 07 08 09 0a
> 0078: [8] 04 05 06 07 08 09 0a 0b
> 0078: [8] 05 06 07 08 09 0a 0b 0c
> 0078: [8] 06 07 08 09 0a 0b 0c 0d
> 0078: [8] 07 08 09 0a 0b 0c 0d 0e
> 0078: [8] 08 09 0a 0b 0c 0d 0e 0f
> 0078: [8] 09 0a 0b 0c 0d 0e 0f 10
> 0078: [8] 0a 0b 0c 0d 0e 0f 10 11
> 0078: [8] 0b 0c 0d 0e 0f 10 11 12
> 0078: [8] 0c 0d 0e 0f 10 11 12 13
> 0078: [8] 0d 0e 0f 10 11 12 13 14
> 0078: [8] 0e 0f 10 11 12 13 14 15
> 0078: [8] 0f 10 11 12 13 14 15 16
> 0078: [8] 10 11 12 13 14 15 16 17
> 0078: [8] 11 12 13 14 15 16 17 18
> 0078: [8] 12 13 14 15 16 17 18 19
> 0078: [8] 13 14 15 16 17 18 19 1a
> 0078: [8] 14 15 16 17 18 19 1a 1b
> 0078: [8] 15 16 17 18 19 1a 1b 1c
> 0078: [8] 16 17 18 19 1a 1b 1c 1d
> 0078: [8] 17 18 19 1a 1b 1c 1d 1e
> 0078: [8] 18 19 1a 1b 1c 1d 1e 1f
> 0078: [8] 19 1a 1b 1c 1d 1e 1f 20
> 0078: [8] 1a 1b 1c 1d 1e 1f 20 21
> 0078: [8] 1b 1c 1d 1e 1f 20 21 22
> 0078: [8] 1c 1d 1e 1f 20 21 22 23
> 0078: [8] 1d 1e 1f 20 21 22 23 24
> 0078: [8] 1e 1f 20 21 22 23 24 25
> 0078: [8] 1f 20 21 22 23 24 25 26
> 0078: [8] 20 21 22 23 24 25 26 27
>
> Test messages sent and received: 32
>
> (I can not go to longer tests because I may hit a bug I am just
> now trying to fix in the current code level).
This test does not load the bus a lot but uses burst of message to
trigger out-of-order issues. Please run the test without "-v" and much
longer.
> If you are interested in any more specific tests that I should
> be running, then please list those and I can report them
> when submitting V2 of the patch set.
Other useful test are about bus error reporting and error state changes.
Can bus errors been disabled via interrupt? I do not see that
CAN_CTRLMODE_BERR_REPORTING is handled. This means that bus error
reporting is always on which may put heavy load on the system. e.g if no
cable is connected and a message sent (at 1MB/s).
Concerning the error state changes, have a look to [1].
> Just to put everything into perspective:
> The equipment available to me is:
> * beagle bone black with c_can
> * raspberry pi 3 with mcp2515
> * raspberry pi CM3 with mcp2517fd
> * raspberry pi 2 with mcp2517fd
> * saleae logic analyzer
[1] https://marc.info/?l=linux-can&m=151147114323582&w=2
The driver has more than 3000 lines... a review will take some time.
Thanks for your contribution.
Wolfgang.
>
> Thanks,
> Martin--
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: kernel @ 2017-11-26 18:29 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Oliver Hartkopp, Marc Kleine-Budde, Rob Herring, Mark Rutland,
linux-can, devicetree
In-Reply-To: <0c2f4a35-3b86-7ae6-ccc6-2751fa92f497@grandegger.com>
> On 26.11.2017, at 17:18, Wolfgang Grandegger <wg@grandegger.com> wrote:
>
> Hello Martin,
>
> Am 25.11.2017 um 15:47 schrieb kernel@martin.sperl.org:
>
> ...snip...
>> I see the following counters:
>> 7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>
> Why do you use "ONE_SHOT" in all your test cases?
I did test oneshot and non-oneshot - I just did not document them
here, I am just mentioning the issue of dropped frames when the driver
is submitting them correctly.
Also - for what it is worse - the RX case would not be impacted by one-shot
enabled or not.
> Did you also check for out-of-order issues, e.g. by using "canfdtest”?
As for out of order: yes I have taken extreme care that we do not
get out of order packets for all possible cases testing with cangen - there
may be one error case where there could be an out of order case in the
tx path (unless we want to drop that frame - it would be a MAB case)
But until now I did not know about canfdtest, but for all practical
purposes it can not test for out-of-order delivery on a 100% saturated
bus the way it is working - latencies are too big for that!
Still for completeness a quick test from beagleboneblack to a
RPI CM3 with a mcp2517fd gives:
root@beaglebone:~/can-utils# ./canfdtest -g -vvv can0 -l 32
interface = can0, family = 29, type = 3, proto = 1
0078: [8] 01 02 03 04 05 06 07 08
0078: [8] 02 03 04 05 06 07 08 09
0078: [8] 03 04 05 06 07 08 09 0a
0078: [8] 04 05 06 07 08 09 0a 0b
0078: [8] 05 06 07 08 09 0a 0b 0c
0078: [8] 06 07 08 09 0a 0b 0c 0d
0078: [8] 07 08 09 0a 0b 0c 0d 0e
0078: [8] 08 09 0a 0b 0c 0d 0e 0f
0078: [8] 09 0a 0b 0c 0d 0e 0f 10
0078: [8] 0a 0b 0c 0d 0e 0f 10 11
0078: [8] 0b 0c 0d 0e 0f 10 11 12
0078: [8] 0c 0d 0e 0f 10 11 12 13
0078: [8] 0d 0e 0f 10 11 12 13 14
0078: [8] 0e 0f 10 11 12 13 14 15
0078: [8] 0f 10 11 12 13 14 15 16
0078: [8] 10 11 12 13 14 15 16 17
0078: [8] 11 12 13 14 15 16 17 18
0078: [8] 12 13 14 15 16 17 18 19
0078: [8] 13 14 15 16 17 18 19 1a
0078: [8] 14 15 16 17 18 19 1a 1b
0078: [8] 15 16 17 18 19 1a 1b 1c
0078: [8] 16 17 18 19 1a 1b 1c 1d
0078: [8] 17 18 19 1a 1b 1c 1d 1e
0078: [8] 18 19 1a 1b 1c 1d 1e 1f
0078: [8] 19 1a 1b 1c 1d 1e 1f 20
0078: [8] 1a 1b 1c 1d 1e 1f 20 21
0078: [8] 1b 1c 1d 1e 1f 20 21 22
0078: [8] 1c 1d 1e 1f 20 21 22 23
0078: [8] 1d 1e 1f 20 21 22 23 24
0078: [8] 1e 1f 20 21 22 23 24 25
0078: [8] 1f 20 21 22 23 24 25 26
0078: [8] 20 21 22 23 24 25 26 27
Test messages sent and received: 32
(I can not go to longer tests because I may hit a bug I am just
now trying to fix in the current code level).
If you are interested in any more specific tests that I should
be running, then please list those and I can report them
when submitting V2 of the patch set.
Just to put everything into perspective:
The equipment available to me is:
* beagle bone black with c_can
* raspberry pi 3 with mcp2515
* raspberry pi CM3 with mcp2517fd
* raspberry pi 2 with mcp2517fd
* saleae logic analyzer
Thanks,
Martin
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: Wolfgang Grandegger @ 2017-11-26 16:18 UTC (permalink / raw)
To: kernel, Oliver Hartkopp
Cc: Marc Kleine-Budde, Rob Herring, Mark Rutland, linux-can,
devicetree
In-Reply-To: <263BAF25-3F9A-4BE8-8E19-757B3C22A037@martin.sperl.org>
Hello Martin,
Am 25.11.2017 um 15:47 schrieb kernel@martin.sperl.org:
...snip...
> I see the following counters:
> 7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
> link/can promiscuity 0
> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
Why do you use "ONE_SHOT" in all your test cases?
Did you also check for out-of-order issues, e.g. by using "canfdtest"?
Wolfgang.
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: kernel @ 2017-11-26 15:43 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: Wolfgang Grandegger, Marc Kleine-Budde, linux-can
In-Reply-To: <0a300896-3980-3b4e-ce5a-a3a97de3df55@hartkopp.net>
> On 26.11.2017, at 13:38, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
> -devicetree
> -Mark Rutland
> -Rob Herring
>
> On 11/25/2017 03:47 PM, kernel@martin.sperl.org wrote:
>>> Unfortunately [PATCH 2/2] only hit the devicetree mailing list but not the linux-can mailing list for the review.
>> I have no idea why you have not received it, but I have sent all
>> of them to the same Recipient list!
>> I even got a relayed confirmation for all 3 of them:
>> 19:35:28.689 2 DEQUEUER [7520159] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
>> 19:35:29.823 2 DEQUEUER [7520154] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
>> 19:35:31.902 2 DEQUEUER [7520160] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
>> So they should all have arrived!
>
> In fact patch 2/2 didn't show up on the mailing list archive and in my inbox ...
>
> https://marc.info/?l=linux-can&r=1&b=201711&w=2
>
> Hm ... obviously no sender issue then :-)
I shall send a V2 anyway - a few more issues/performance improvements came up...
>
>>> Seems you improved the SPI handling here. Would it make sense to separate the SPI-related part of the code to a separate C-file so that the existing mcp251x driver can benefit from these improvements too?
>> The biggest problem is that the Register-design of the mcp2517fd is totally
>> different compared to mcp251x, so most of those “optimizations” for the
>> mcp2517 do not apply to the mcp251x (SPI-API wise IMO the mcp2515 is much
>> more efficient - better still when having dedicated gpios for the RX lines
>> to avoid having to query the status first via spi (reduces latencies
>> and thus reduces packet loss/overflow on a saturated CAN bus).
>> I have a spi_async only version of the mcp251x as well (which has been dormant
>> for a long time, as the mcp251x stopped showing interrupt stopped issues...).
>
> Interesting. I was not aware that the mcp251x is working reliable under heavy loads. I've just seen people providing bcm2835 related hacks/fixes to make it run with the Raspi.
For all practical purposes it is related to edge vs. level interrupts
and the last I have seen seems to indicate that - on a RPI3 - it can
handle the 1MHz Can-bus utilized 100% with 0 length 11 bit can frames
(this is what I call worsted case and target for in all my tests)
>
>>>> (still dropps are observed in the can/network stack).
>>>
>>> Are you sure drops are taking place in the network layer? Can you give me some more details about this statement?
>> Well - when I increase net->stat.rx_dropped there is always a
>> dev_warn_ratelimited that comes along. Also rx_errors is increased at the
>> same time. And I see the drops counters increase while there are no messages
>> in dmesg.
>
> Ok - at least this is not network layer related (the code in linux/net/can) but network driver stuff (linux/drivers/net/can/).
>
>> Also note that the driver is only incrementing rx_packets and rx_bytes only
>> when netif_rx_ni(skb) is called.
>> So the setup is this:
>> * raspberry pi 2 with the mcp2517fd
>> * beagle bone black
>> * 1MHz CAN2.0 bus
>> Beagle bone black:
>> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -p10 -Ii
>> This saturates the CAN BUS to 100%
>> on the RPI2 I have no consumer (candump or similar) and the module has
>> just been reloaded (counters are reset).
>> I see the following counters:
>> 7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>> bitrate 1000000 sample-point 0.750
>> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
>> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
>> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
>> clock 40000000
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>> RX: bytes packets errors dropped overrun mcast
>> 0 998774 703 4588 0 0
>> TX: bytes packets errors dropped carrier collsns
>> 0 0 0 0 0 0
>> So you see that packets + dropped > transmitted (1000000).
>> In a different setup with 4 bytes of data payload:
>> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 10000 -L4 -p10 -Ii
>> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 100000 -L4 -p10 -Ii
>> I get the following statistics:
>> root@rasp2a:~# ip -details -statistics link show can0
>> 8: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>> bitrate 1000000 sample-point 0.750
>> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
>> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
>> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
>> clock 40000000
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>> RX: bytes packets errors dropped overrun mcast
>> 440000 110000 0 4343 0 0
>> TX: bytes packets errors dropped carrier collsns
>> 0 0 0 0 0 0
>> Here 1 get 100% reception rates (bytes and packets are exactly what they
>> are supposed to be) but still 0.1% dropped.
>> When CAN FD is configured on the mcp2517 then things look different
>> even on a RPI2:
>> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -L0 -p10 -Ii
>> root@rasp2a:~# ip -details -statistics link show can0
>> 13: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
>> link/can promiscuity 0
>> can <ONE-SHOT,BERR-REPORTING,FD> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
>> bitrate 1000000 sample-point 0.750
>> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
>> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
>> dbitrate 2000000 dsample-point 0.750
>> dtq 25 dprop-seg 7 dphase-seg1 7 dphase-seg2 5 dsjw 1
>> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
>> clock 40000000
>> re-started bus-errors arbit-lost error-warn error-pass bus-off
>> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
>> RX: bytes packets errors dropped overrun mcast
>> 0 999840 3 752028 0 0
>> TX: bytes packets errors dropped carrier collsns
>> 0 0 0 0 0 0
>> In this case the number of packets actually lost on the controller is 0.01%
>> But the number of dropped packets on the stack is 75%.
>
> Looking into https://marc.info/?l=devicetree&m=151154853410879&w=2 :
>
> +static int mcp2517fd_can_transform_rx_fd(struct spi_device *spi,
> + struct mcp2517fd_obj_rx *rx)
> +{
> + struct mcp2517fd_priv *priv = spi_get_drvdata(spi);
> + struct canfd_frame *frame;
> + struct sk_buff *skb;
> + u32 flags = rx->header.flags;
> +
> + /* allocate the skb buffer */
> + skb = alloc_canfd_skb(priv->net, &frame);
> + if (!skb) {
> + dev_err(&spi->dev, "cannot allocate RX skb\n");
> + priv->net->stats.rx_dropped++;
> + return -ENOMEM;
>
> Here the packet is counted as dropped - and the function is finished.
>
but it would have been logged to dmesg and would not have
been accounted for as Packets and bytes. (as you found out below)
> + }
> +
> + mcp2517fd_mcpid_to_canid(rx->header.id, flags, &frame->can_id);
> + frame->flags |= (flags & CAN_OBJ_FLAGS_BRS) ? CANFD_BRS : 0;
> + frame->flags |= (flags & CAN_OBJ_FLAGS_ESI) ? CANFD_ESI : 0;
> +
> + frame->len = can_dlc2len((flags & CAN_OBJ_FLAGS_DLC_MASK)
> + >> CAN_OBJ_FLAGS_DLC_SHIFT);
> +
> + memcpy(frame->data, rx->data, frame->len);
> +
> + priv->net->stats.rx_packets++;
>
> Here the packet is counted.
>
> + priv->net->stats.rx_bytes += frame->len;
> +
> + can_led_event(priv->net, CAN_LED_EVENT_RX);
> +
> + netif_rx_ni(skb);
> +
> + return 0;
> +}
>
> I wonder how you can send 1.000.000 frames and then count
> 999.840 + 752.028 = 1.751.868 on the reception side ?!?
well - the driver is submitting the packets to the network
stack and if it can not handle it (say because of memory,…)
then it would still drop it. But as this (probably) happens
in a workqueue this is asynchronous and work may “be left”.
So essentially “internal” drops of the network stack.
>
>> The shorter the interval the lower the dropped count.
>> P.s: For the last test-case here the fifo utilization statistics:
>> root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/fifo_usage/* | awk '{ C=C+$1; printf "%8i %s\n",C,$0}’
>> Total Count/fifo
>> 384401 384401
>> 674306 289905
>> 960133 285827
>> 990117 29984
>> 994351 4234
>> 996745 2394
>> 997668 923
>> 998217 549
>> 998593 376
>> 998849 256
>> 999059 210
>> 999240 181
>> 999385 145
>> 999508 123
>> 999604 96
>> 999680 76
>> 999745 65
>> 999797 52
>> 999840 43
>> root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/rx/fifo_count
>> 19
>> root@rasp2a:~# dmesg
>> [77843.229423] mcp2517fd spi0.0: RX MAB overflow
>> [77856.329824] mcp2517fd spi0.0: RX MAB overflow
>> [77883.299221] mcp2517fd spi0.0: RX MAB overflow
>> These dmesg lines correspond with the errors (and would also increase “dropped” by 3)
>
> But doesn't this FIFO behavior just show that the SPI connection is too slow to be able to cope with the CAN FD traffic?
I believe I have understood the source of those MAB errors.
that is what will get fixed in V2.
(It has to do with CS getting deasserted sometimes very late
(> 20us after the last spi clock beat) and that introduces
some locking issues between SPI reads and Controller writes
to the same SRAM word.
So the order of the fifos had to be rearranged, so that
this can not happen…
Martin
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: Oliver Hartkopp @ 2017-11-26 12:38 UTC (permalink / raw)
To: kernel; +Cc: Wolfgang Grandegger, Marc Kleine-Budde, linux-can
In-Reply-To: <263BAF25-3F9A-4BE8-8E19-757B3C22A037@martin.sperl.org>
-devicetree
-Mark Rutland
-Rob Herring
On 11/25/2017 03:47 PM, kernel@martin.sperl.org wrote:
>> Unfortunately [PATCH 2/2] only hit the devicetree mailing list but not the linux-can mailing list for the review.
>
> I have no idea why you have not received it, but I have sent all
> of them to the same Recipient list!
>
> I even got a relayed confirmation for all 3 of them:
> 19:35:28.689 2 DEQUEUER [7520159] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
> 19:35:29.823 2 DEQUEUER [7520154] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
> 19:35:31.902 2 DEQUEUER [7520160] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
>
> So they should all have arrived!
In fact patch 2/2 didn't show up on the mailing list archive and in my
inbox ...
https://marc.info/?l=linux-can&r=1&b=201711&w=2
Hm ... obviously no sender issue then :-)
>> Seems you improved the SPI handling here. Would it make sense to separate the SPI-related part of the code to a separate C-file so that the existing mcp251x driver can benefit from these improvements too?
>
> The biggest problem is that the Register-design of the mcp2517fd is totally
> different compared to mcp251x, so most of those “optimizations” for the
> mcp2517 do not apply to the mcp251x (SPI-API wise IMO the mcp2515 is much
> more efficient - better still when having dedicated gpios for the RX lines
> to avoid having to query the status first via spi (reduces latencies
> and thus reduces packet loss/overflow on a saturated CAN bus).
>
> I have a spi_async only version of the mcp251x as well (which has been dormant
> for a long time, as the mcp251x stopped showing interrupt stopped issues...).
Interesting. I was not aware that the mcp251x is working reliable under
heavy loads. I've just seen people providing bcm2835 related hacks/fixes
to make it run with the Raspi.
>>> (still dropps are observed in the can/network stack).
>>
>> Are you sure drops are taking place in the network layer? Can you give me some more details about this statement?
> Well - when I increase net->stat.rx_dropped there is always a
> dev_warn_ratelimited that comes along. Also rx_errors is increased at the
> same time. And I see the drops counters increase while there are no messages
> in dmesg.
Ok - at least this is not network layer related (the code in
linux/net/can) but network driver stuff (linux/drivers/net/can/).
> Also note that the driver is only incrementing rx_packets and rx_bytes only
> when netif_rx_ni(skb) is called.
>
> So the setup is this:
> * raspberry pi 2 with the mcp2517fd
> * beagle bone black
> * 1MHz CAN2.0 bus
>
> Beagle bone black:
> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -p10 -Ii
>
> This saturates the CAN BUS to 100%
>
> on the RPI2 I have no consumer (candump or similar) and the module has
> just been reloaded (counters are reset).
>
> I see the following counters:
> 7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
> link/can promiscuity 0
> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 1000000 sample-point 0.750
> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
> clock 40000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> RX: bytes packets errors dropped overrun mcast
> 0 998774 703 4588 0 0
> TX: bytes packets errors dropped carrier collsns
> 0 0 0 0 0 0
>
> So you see that packets + dropped > transmitted (1000000).
>
> In a different setup with 4 bytes of data payload:
> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 10000 -L4 -p10 -Ii
> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 100000 -L4 -p10 -Ii
>
> I get the following statistics:
> root@rasp2a:~# ip -details -statistics link show can0
> 8: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
> link/can promiscuity 0
> can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 1000000 sample-point 0.750
> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
> clock 40000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> RX: bytes packets errors dropped overrun mcast
> 440000 110000 0 4343 0 0
> TX: bytes packets errors dropped carrier collsns
> 0 0 0 0 0 0
> Here 1 get 100% reception rates (bytes and packets are exactly what they
> are supposed to be) but still 0.1% dropped.
>
> When CAN FD is configured on the mcp2517 then things look different
> even on a RPI2:
> root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -L0 -p10 -Ii
> root@rasp2a:~# ip -details -statistics link show can0
> 13: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
> link/can promiscuity 0
> can <ONE-SHOT,BERR-REPORTING,FD> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 1000000 sample-point 0.750
> tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
> mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
> dbitrate 2000000 dsample-point 0.750
> dtq 25 dprop-seg 7 dphase-seg1 7 dphase-seg2 5 dsjw 1
> mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
> clock 40000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> RX: bytes packets errors dropped overrun mcast
> 0 999840 3 752028 0 0
> TX: bytes packets errors dropped carrier collsns
> 0 0 0 0 0 0
>
> In this case the number of packets actually lost on the controller is 0.01%
> But the number of dropped packets on the stack is 75%.
Looking into https://marc.info/?l=devicetree&m=151154853410879&w=2 :
+static int mcp2517fd_can_transform_rx_fd(struct spi_device *spi,
+ struct mcp2517fd_obj_rx *rx)
+{
+ struct mcp2517fd_priv *priv = spi_get_drvdata(spi);
+ struct canfd_frame *frame;
+ struct sk_buff *skb;
+ u32 flags = rx->header.flags;
+
+ /* allocate the skb buffer */
+ skb = alloc_canfd_skb(priv->net, &frame);
+ if (!skb) {
+ dev_err(&spi->dev, "cannot allocate RX skb\n");
+ priv->net->stats.rx_dropped++;
+ return -ENOMEM;
Here the packet is counted as dropped - and the function is finished.
+ }
+
+ mcp2517fd_mcpid_to_canid(rx->header.id, flags, &frame->can_id);
+ frame->flags |= (flags & CAN_OBJ_FLAGS_BRS) ? CANFD_BRS : 0;
+ frame->flags |= (flags & CAN_OBJ_FLAGS_ESI) ? CANFD_ESI : 0;
+
+ frame->len = can_dlc2len((flags & CAN_OBJ_FLAGS_DLC_MASK)
+ >> CAN_OBJ_FLAGS_DLC_SHIFT);
+
+ memcpy(frame->data, rx->data, frame->len);
+
+ priv->net->stats.rx_packets++;
Here the packet is counted.
+ priv->net->stats.rx_bytes += frame->len;
+
+ can_led_event(priv->net, CAN_LED_EVENT_RX);
+
+ netif_rx_ni(skb);
+
+ return 0;
+}
I wonder how you can send 1.000.000 frames and then count
999.840 + 752.028 = 1.751.868 on the reception side ?!?
> The shorter the interval the lower the dropped count.
>
> P.s: For the last test-case here the fifo utilization statistics:
> root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/fifo_usage/* | awk '{ C=C+$1; printf "%8i %s\n",C,$0}’
> Total Count/fifo
> 384401 384401
> 674306 289905
> 960133 285827
> 990117 29984
> 994351 4234
> 996745 2394
> 997668 923
> 998217 549
> 998593 376
> 998849 256
> 999059 210
> 999240 181
> 999385 145
> 999508 123
> 999604 96
> 999680 76
> 999745 65
> 999797 52
> 999840 43
> root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/rx/fifo_count
> 19
> root@rasp2a:~# dmesg
> [77843.229423] mcp2517fd spi0.0: RX MAB overflow
> [77856.329824] mcp2517fd spi0.0: RX MAB overflow
> [77883.299221] mcp2517fd spi0.0: RX MAB overflow
>
> These dmesg lines correspond with the errors (and would also increase “dropped” by 3)
>
But doesn't this FIFO behavior just show that the SPI connection is too
slow to be able to cope with the CAN FD traffic?
Best regards,
Oliver
^ permalink raw reply
* RE: [PATCH v4 1/6] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
From: Pankaj Bansal @ 2017-11-26 2:20 UTC (permalink / raw)
To: Marc Kleine-Budde, wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org,
linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Varun Sethi, Poonam Aggrwal, Bhupesh Sharma, Sakar Arora
In-Reply-To: <44f37aef-7f45-a7c7-7320-01a62e23fbde-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Friday, November 24, 2017 8:33 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; wg@grandegger.com; linux-
> can@vger.kernel.org; robh+dt@kernel.org; mark.rutland@arm.com;
> devicetree@vger.kernel.org
> Cc: Varun Sethi <V.Sethi@nxp.com>; Poonam Aggrwal
> <poonam.aggrwal@nxp.com>; Bhupesh Sharma
> <bhupesh.sharma@freescale.com>; Sakar Arora
> <Sakar.Arora@freescale.com>
> Subject: Re: [PATCH v4 1/6] can: flexcan: Remodel FlexCAN register r/w APIs
> for big endian FlexCAN controllers.
>
> On 11/24/2017 02:22 PM, Pankaj Bansal wrote:
> > The FlexCAN driver assumed that FlexCAN controller is big endian for
> > powerpc architecture and little endian for other architectures.
> >
> > But this may not be the case. FlexCAN controller can be little or big
> > endian on any architecture. For e.g. NXP LS1021A ARM based SOC has big
> > endian FlexCAN controller.
> >
> > Therefore, the driver has been modified to add a provision for both
> > types of controllers using an additional device tree property. On a
> > "fsl,p1010-flexcan" device BE is default, on all other devices LE is.
> >
> > Big Endian controllers should have "big-endian" set in the device tree.
> > check "Documentation/devicetree/bindings/net/can/fsl-flexcan.txt" for
> > usage.
> >
> > This is the standard practice followed in linux. for more info check:
> > Documentation/devicetree/bindings/common-properties.txt
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Signed-off-by: Sakar Arora <Sakar.Arora@freescale.com>
> > Reviewed-by: Zhengxiong Jin <Jason.Jin@freescale.com>
> > Reviewed-by: Poonam Aggrwal <poonam.aggrwal@nxp.com>
> > ---
> > Changes in v4:
> > - Merged device tree changes and driver changes in one series
> > Changes in v3:
> > - Added fsl,imx25-flexcan, fsl,imx35-flexcan and fsl,imx53-flexcan
> > support to the driver.
> > - Modified patch deciption to state default endianness followed and
> > to include fsl-flexcan.txt referance.
> > Changes in v2:
> > - Modified patch deciption to include common-properties.txt reference.
> > - Reorder the LE/BE read/write APIs for better readability of code
> > - Added an exception to force BE API selection, for powerpc based
> platform
> > P1010. This ensures that new linux kernel works with old P1010
> > device-tree, while future powerpc platforms that use big endian
> > FlexCAN controller need to specify big-endian in device tree in
> > FlexCAN node.
> > - Tested on P1010 after backporting to freescale sdk 1.4 linux, without
> > any change in device-tree.
> > - Tested on NXP LS1021A arm based platform.
> >
> > drivers/net/can/flexcan.c | 233 ++++++++++++++++++++----------------
> > 1 file changed, 131 insertions(+), 102 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index a13a489..4c873fb 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
>
> [...]
> I think this will select LE for non DT devices, right?
>
Yes. As per below code snippet:
static struct property *__of_find_property(const struct device_node *np,
const char *name, int *lenp)
{
struct property *pp;
if (!np)
return NULL;
....
}
If no device node is present null is returned.
So we select le as default.
> > + if (of_property_read_bool(pdev->dev.of_node, "big-endian")) {
> > + priv->read = flexcan_read_be;
> > + priv->write = flexcan_write_be;
> > + } else {
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "fsl,p1010-flexcan")) {
> > + priv->read = flexcan_read_be;
> > + priv->write = flexcan_write_be;
> > + } else {
> > + priv->read = flexcan_read_le;
> > + priv->write = flexcan_write_le;
> > + }
> > + }
> > +
> > priv->can.clock.freq = clock_freq;
> > priv->can.bittiming_const = &flexcan_bittiming_const;
> > priv->can.do_set_mode = flexcan_set_mode;
> >
>
> 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 |
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: kernel @ 2017-11-25 14:47 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
linux-can, devicetree
In-Reply-To: <3d355c51-09a3-40b8-46bf-8437e26a95f3@hartkopp.net>
Hi Oliver!
> On 25.11.2017, at 13:03, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
> Hello Martin,
>
> thanks for the contribution!
>
> Unfortunately [PATCH 2/2] only hit the devicetree mailing list but not the linux-can mailing list for the review.
I have no idea why you have not received it, but I have sent all
of them to the same Recipient list!
I even got a relayed confirmation for all 3 of them:
19:35:28.689 2 DEQUEUER [7520159] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
19:35:29.823 2 DEQUEUER [7520154] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
19:35:31.902 2 DEQUEUER [7520160] SMTP(vger.kernel.org)linux-can@vger.kernel.org relayed: relayed via vger.kernel.org
So they should all have arrived!
>
> Btw. I already have two questions from the description:
>
> On 11/24/2017 07:35 PM, kernel@martin.sperl.org wrote:
>
> (..)
>
>> The driver has been heavily optimized so that it can handle
>> a 100% utilized 1MHz Can-bus (with 11 bit can frames with DLC=0)
>> even on less powerfull SOCs like the raspberry pi 1 without
>> dropping frames due to driver/spi latencies
>
> (..)
>
> > The driver implements a lock-less design for transmissions
> > making use instead of prepared spi messages submitted via spi_async
> > for transmission in the start_xmit_start code without requireing
> > an extra workqueue and the corresponding latencies.
>
> Seems you improved the SPI handling here. Would it make sense to separate the SPI-related part of the code to a separate C-file so that the existing mcp251x driver can benefit from these improvements too?
The biggest problem is that the Register-design of the mcp2517fd is totally
different compared to mcp251x, so most of those “optimizations” for the
mcp2517 do not apply to the mcp251x (SPI-API wise IMO the mcp2515 is much
more efficient - better still when having dedicated gpios for the RX lines
to avoid having to query the status first via spi (reduces latencies
and thus reduces packet loss/overflow on a saturated CAN bus).
I have a spi_async only version of the mcp251x as well (which has been dormant
for a long time, as the mcp251x stopped showing interrupt stopped issues...).
>
>> (still dropps are observed in the can/network stack).
>
> Are you sure drops are taking place in the network layer? Can you give me some more details about this statement?
Well - when I increase net->stat.rx_dropped there is always a
dev_warn_ratelimited that comes along. Also rx_errors is increased at the
same time. And I see the drops counters increase while there are no messages
in dmesg.
Also note that the driver is only incrementing rx_packets and rx_bytes only
when netif_rx_ni(skb) is called.
So the setup is this:
* raspberry pi 2 with the mcp2517fd
* beagle bone black
* 1MHz CAN2.0 bus
Beagle bone black:
root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -p10 -Ii
This saturates the CAN BUS to 100%
on the RPI2 I have no consumer (candump or similar) and the module has
just been reloaded (counters are reset).
I see the following counters:
7: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
link/can promiscuity 0
can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
bitrate 1000000 sample-point 0.750
tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
clock 40000000
re-started bus-errors arbit-lost error-warn error-pass bus-off
0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
RX: bytes packets errors dropped overrun mcast
0 998774 703 4588 0 0
TX: bytes packets errors dropped carrier collsns
0 0 0 0 0 0
So you see that packets + dropped > transmitted (1000000).
In a different setup with 4 bytes of data payload:
root@beaglebone:~# cangen can0 -g0.0 -L0 -n 10000 -L4 -p10 -Ii
root@beaglebone:~# cangen can0 -g0.0 -L0 -n 100000 -L4 -p10 -Ii
I get the following statistics:
root@rasp2a:~# ip -details -statistics link show can0
8: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
link/can promiscuity 0
can <ONE-SHOT,BERR-REPORTING> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
bitrate 1000000 sample-point 0.750
tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
clock 40000000
re-started bus-errors arbit-lost error-warn error-pass bus-off
0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
RX: bytes packets errors dropped overrun mcast
440000 110000 0 4343 0 0
TX: bytes packets errors dropped carrier collsns
0 0 0 0 0 0
Here 1 get 100% reception rates (bytes and packets are exactly what they
are supposed to be) but still 0.1% dropped.
When CAN FD is configured on the mcp2517 then things look different
even on a RPI2:
root@beaglebone:~# cangen can0 -g0.0 -L0 -n 1000000 -L0 -p10 -Ii
root@rasp2a:~# ip -details -statistics link show can0
13: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 10
link/can promiscuity 0
can <ONE-SHOT,BERR-REPORTING,FD> state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
bitrate 1000000 sample-point 0.750
tq 25 prop-seg 14 phase-seg1 15 phase-seg2 10 sjw 1
mcp2517fd: tseg1 2..256 tseg2 1..128 sjw 1..128 brp 1..256 brp-inc 1
dbitrate 2000000 dsample-point 0.750
dtq 25 dprop-seg 7 dphase-seg1 7 dphase-seg2 5 dsjw 1
mcp2517fd: dtseg1 1..32 dtseg2 1..16 dsjw 1..16 dbrp 1..256 dbrp-inc 1
clock 40000000
re-started bus-errors arbit-lost error-warn error-pass bus-off
0 0 0 0 0 0 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
RX: bytes packets errors dropped overrun mcast
0 999840 3 752028 0 0
TX: bytes packets errors dropped carrier collsns
0 0 0 0 0 0
In this case the number of packets actually lost on the controller is 0.01%
But the number of dropped packets on the stack is 75%.
The shorter the interval the lower the dropped count.
Ciao,
Martin
P.s: For the last test-case here the fifo utilization statistics:
root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/fifo_usage/* | awk '{ C=C+$1; printf "%8i %s\n",C,$0}’
Total Count/fifo
384401 384401
674306 289905
960133 285827
990117 29984
994351 4234
996745 2394
997668 923
998217 549
998593 376
998849 256
999059 210
999240 181
999385 145
999508 123
999604 96
999680 76
999745 65
999797 52
999840 43
root@rasp2a:~# cat /sys/kernel/debug/mcp2517fd-can0/rx/fifo_count
19
root@rasp2a:~# dmesg
[77843.229423] mcp2517fd spi0.0: RX MAB overflow
[77856.329824] mcp2517fd spi0.0: RX MAB overflow
[77883.299221] mcp2517fd spi0.0: RX MAB overflow
These dmesg lines correspond with the errors (and would also increase “dropped” by 3)
^ permalink raw reply
* Re: [PATCH 0/2] Microchip mcp2517fd can controller driver
From: Oliver Hartkopp @ 2017-11-25 12:03 UTC (permalink / raw)
To: kernel
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
linux-can, devicetree
In-Reply-To: <20171124183509.12810-1-kernel@martin.sperl.org>
Hello Martin,
thanks for the contribution!
Unfortunately [PATCH 2/2] only hit the devicetree mailing list but not
the linux-can mailing list for the review.
Btw. I already have two questions from the description:
On 11/24/2017 07:35 PM, kernel@martin.sperl.org wrote:
(..)
> The driver has been heavily optimized so that it can handle
> a 100% utilized 1MHz Can-bus (with 11 bit can frames with DLC=0)
> even on less powerfull SOCs like the raspberry pi 1 without
> dropping frames due to driver/spi latencies
(..)
> The driver implements a lock-less design for transmissions
> making use instead of prepared spi messages submitted via spi_async
> for transmission in the start_xmit_start code without requireing
> an extra workqueue and the corresponding latencies.
Seems you improved the SPI handling here. Would it make sense to
separate the SPI-related part of the code to a separate C-file so that
the existing mcp251x driver can benefit from these improvements too?
> (still dropps are observed in the can/network stack).
Are you sure drops are taking place in the network layer? Can you give
me some more details about this statement?
Best regards,
Oliver
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox