CAN - hard_xmit

      Re: CAN - hard_xmit

      Hello !

      Unfortunately I can confirm that we sometimes observe the same problem in our application. We're unsure whether or not this is a problem triggered by a CAN bus wiring failure.

      Worse than that is in case this situation arises it seems that the whole system is under such a high workload it is no longer responding. We managed to enter via the debugging console and found out that sometimes the system stabilizes after pulling the CAN connectors.

      I saw that in newer kernel versions the driver mcp251x contains at least one bugfix addressing a silicon bug in MCP2515 Rev. B which is described as "repeated frame problem". Unfortunately I was unable to disclose the silicon version of the MCP2515 used in the NetDCU14 design (The errata document is dated 2007 so I would expect the NetDCU14 uses an already fixed version of the MCP2515).

      UPDATE: According to the errata document (ww1.microchip.com/downloads/en/DeviceDoc/80179g.pdf) Revision B4 is the latest revision and in production since 2005 so I would recommend to update the driver (even though I don't expect that this solves our problem).

      Regards,

      Volker
      The patch you are giving here differs only in four points

      1. It uses spi_get_drvdata() instead of dev_get_drvdata() to get hold of the private data pointer. This does not matter at all.

      2. It tries to call some LED stuff. This does not solve any problems with the communication. To the contrary, as it is additional stuff, it can even slow down the communication and can make things even worse.

      3. In function mcp251x_open(), it initializes the IRQ flags a little bit different. However it is still very important that you have set the irq_flags value in your platform data (in arch/arm/mach-s5pv210/mach-netdcu14.c) correctly to IRQF_TRIGGER_LOW | IRQF_ONESHOT. This is required in both versions, yours and ours. So no semantic difference here.

      4. In function mcp251x_hw_tx(), it uses a slightly different version to trigger the transmission. This is what is meant with "solving the repeated frame problem". We can see if we can include this part in our next version.


      Other than that, the driver is identical to our version.

      Your F&S Support Team
      F&S Elektronik Systeme GmbH
      As this is an international forum, please try to post in English.
      Da dies ein internationales Forum ist, bitten wir darum, Beiträge möglichst in Englisch zu verfassen.
      Sorry but I don't get the point of your post.

      There definitely is a problem with the 'original' driver and you cannot get a better proof for that than observations from two different customers. If you are not able to reproduce these problems I can only assume that your test setup differs from ours (both CAN devices are in use, 16 participants communicating with each other). Perhaps MWeber can sketch his test setup and we can try to find the common ground.

      Regards

      Volker
      There are two points:
      1. Do you use the correct initialization that is required? This is not in the driver file, it is in the mach-netdcu14.c file. That's what I said in point 3. Can you show me the structure that you pass to the CAN initialization call, or even better the whole initialization part of the CAN controller? This part I am still missing and I still assume that there is something different to our release V2.0. This could be the reason why it is working here in our version but not in your version.
      2. Here in this thread you are telling me that the driver *is* working after your modification. So I was wondering what part of your code could fix any problems. And I did not find much. This was what I explained in points 1 to 3 of my last post. The only thing that *could* have an influence on the communication was point 4. So if your code actually solves a problem, than we can add this to our next release. However, as you already have a version of this driver running, and other people can download your version here, too, I don't see any immediate need for action. The solution is already available here in this thread.
      Now you are again talking about a problem with the driver. What is true? Does your version solve the problem or not?

      Your F&S Support Team
      F&S Elektronik Systeme GmbH
      As this is an international forum, please try to post in English.
      Da dies ein internationales Forum ist, bitten wir darum, Beiträge möglichst in Englisch zu verfassen.

      Post was edited 1 time, last by “fs-support_HK” ().

      We didn't change anything in the section of the file mach-netdcu14.c regarding the CAN initialization. The relevant section is as follows

      Brainfuck Source Code

      1. /* -------------------- CAN ----------------------------------------------- */
      2. #if defined(CONFIG_CAN_MCP251X) || defined(CONFIG_CAN_MCP251X_MODULE)
      3. static int netdcu14_mcp251x_setup(struct spi_device *spi)
      4. {
      5. unsigned reset, irq;
      6. switch (spi->chip_select) {
      7. case 0: /* CAN0 */
      8. reset = S5PV210_GPH0(2);
      9. irq = S5PV210_GPH1(1); /* EINT9 */
      10. break;
      11. case 1: /* CAN1 */
      12. reset = S5PV210_GPJ3(0);
      13. irq = S5PV210_GPH0(7); /* EINT7 */
      14. break;
      15. default:
      16. return -EINVAL;
      17. }
      18. printk("Reset MCP2515 CAN controller on SPI%d-%d\n",
      19. spi->master->bus_num, spi->chip_select);
      20. /* Configure reset pin as output and reset CAN controller */
      21. gpio_request_one(reset, GPIOF_OUT_INIT_LOW, "mcp251x_reset");
      22. s3c_gpio_setpull(reset, S3C_GPIO_PULL_UP);
      23. udelay(100);
      24. gpio_set_value(reset, 1);
      25. /* Configure irq pin to use pull-up */
      26. s3c_gpio_setpull(S5PV210_GPH1(1), S3C_GPIO_PULL_UP);
      27. return 0;
      28. }
      29. static struct mcp251x_platform_data mcp251x_info = {
      30. .oscillator_frequency = 20 * 1000 * 1000, /* 20MHz */
      31. .irq_flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT, /* Low, not falling */
      32. .board_specific_setup = netdcu14_mcp251x_setup,
      33. .transceiver_enable = NULL,
      34. .power_enable = NULL,
      35. };
      36. static struct mcp251x_platform_data mcp251x_info1 = {
      37. .oscillator_frequency = 20 * 1000 * 1000, /* 20MHz */
      38. .irq_flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT, /* Low, not falling */
      39. .board_specific_setup = netdcu14_mcp251x_setup,
      40. .transceiver_enable = NULL,
      41. .power_enable = NULL,
      42. };
      43. #endif /* CONFIG_CAN_MCP251X || CONFIG_CAN_MCP251X_MODULE */


      Yes, the backported driver solves the problem we had with the driver included in the multiplatform release V2.0. There are at least two changes you didn't mention but that may address critical flaws :

      threaded irqs must provide a primary handler or set the IRQF_ONESHOT flag (I'm not sure whether the data handling is based upon threaded IRQs) :
      git.kernel.org/cgit/linux/kern…3b3b38429da6f70a913f89b04

      repeated frame problem :
      lkml.org/lkml/2012/9/16/120

      But I definitely don't know exactly what change finally solved our problems with the driver included in the multiplatform release V2.0. But I can say that - after applying the patch mentioned above - our NetDCU14 application works at least stable when attached to the CAN bus. So this solves one problem but this doesn't solve the steady loss of CAN messages I mentioned in some of my other posts. Please don't mix these two problems.

      However, as you already have a version of this driver running, and other people can download your version here, too, I don't see any immediate need for action. The solution is already available here in this thread.


      I would expect that other people aren't forced to collect all those patches floating around in this forum to finally get a working multiplatform release V2.0 +, especially in case these other people spent a lot of money to buy hundreds of your boards.

      Regards

      Volker

      vpruess wrote:


      Source Code

      1. static struct mcp251x_platform_data mcp251x_info = {
      2. .oscillator_frequency = 20 * 1000 * 1000, /* 20MHz */
      3. .irq_flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT, /* Low, not falling */
      4. .board_specific_setup = netdcu14_mcp251x_setup,
      5. .transceiver_enable = NULL,
      6. .power_enable = NULL,
      7. };
      8. static struct mcp251x_platform_data mcp251x_info1 = {
      9. .oscillator_frequency = 20 * 1000 * 1000, /* 20MHz */
      10. .irq_flags = IRQF_TRIGGER_LOW | IRQF_ONESHOT, /* Low, not falling */
      11. .board_specific_setup = netdcu14_mcp251x_setup,
      12. .transceiver_enable = NULL,
      13. .power_enable = NULL,
      14. };

      That's the interesting part. And yes it is set as active low, which is correct.

      Yes, the backported driver solves the problem we had with the driver included in the multiplatform release V2.0. There are at least two changes you didn't mention but that may address critical flaws :

      threaded irqs must provide a primary handler or set the IRQF_ONESHOT flag (I'm not sure whether the data handling is based upon threaded IRQs) :
      git.kernel.org/cgit/linux/kern…3b3b38429da6f70a913f89b04

      That's what I mentioned as point 3, the different initialization of the flags value in function mcp251x_open(). In fact when using the above init code from mach-netdcu14.c, the flags will be set to exactly the same value with the old and new code, i.e. IRQF_TRIGGER_LOW | IRQF_ONESHOT. However if the .flags value would be missing in the init code, it would be set to IRQF_TRIGGER_FALLING | IRQF_ONESHOT, which would be wrong.

      repeated frame problem :
      lkml.org/lkml/2012/9/16/120

      This is what I listed as point 4, the different version to trigger the transmission in function mcp251x_hw_tx().

      But I definitely don't know exactly what change finally solved our problems with the driver included in the multiplatform release V2.0. But I can say that - after applying the patch mentioned above - our NetDCU14 application works at least stable when attached to the CAN bus.

      OK. So the version posted by you does actually solve the problem. As your error message was "mcp251x spi1.1: hard_xmit called while tx busy", i.e. the transmit code was called repeatedly, I would assume that the point 4, that is commented in the source code with "avoid repeated frame problem", may actually be the part that solves this problem. At least it sounds reasonable.

      I would expect that other people aren't forced to collect all those patches floating around in this forum to finally get a working multiplatform release V2.0 +

      Yes, we know that this is unfortunate, and we plan to do some release that collects all the patches so far. But at it has to fit into our schedule of doing other releases for other boards and there are also very important things to do. So it still has to wait a little while.

      Your F&S Support Team
      F&S Elektronik Systeme GmbH
      As this is an international forum, please try to post in English.
      Da dies ein internationales Forum ist, bitten wir darum, Beiträge möglichst in Englisch zu verfassen.