Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Hi Pavel, On 05/18/2010 01:28 PM, Pavel Cheblakov wrote: > This is a general driver for cards based on PLX90xx PCI-bridges. > It supports following cards: > - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) > - Adlink PCI-7841/cPCI-7841 SE card > - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) > - esd CAN-PCI/PMC/266 > - esd CAN-PCIe/2000 > - Marathon CAN-bus-PCI card (http://www.marathon.ru/) > - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) > > This driver has been ported from socketcan of mainline Linux kernel 2.6.34. > Support of esd cards has been added by Matthias Fuchs in linux-next > > Signed-off-by: Pavel Cheblakov As discussed, could you please label the Kconfig entry for the old esd PCI driver as deprecated. Apart from that the patch looks good and you can add my "Acked-by: Wolfgang Grandegger ". Thanks, Wolfgang. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
On 05/18/2010 03:29 PM, Sebastian Smolorz wrote: > Wolfgang Grandegger wrote: >> On 05/18/2010 02:29 PM, Sebastian Smolorz wrote: >>> Hi Wolfgang, >>> >>> Wolfgang Grandegger wrote: On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: > Pavel Cheblakov wrote: >> This is a general driver for cards based on PLX90xx PCI-bridges. >> It supports following cards: >> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) >> - Adlink PCI-7841/cPCI-7841 SE card >> - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) >> - esd CAN-PCI/PMC/266 >> - esd CAN-PCIe/2000 >> - Marathon CAN-bus-PCI card (http://www.marathon.ru/) >> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) > > The esd cards mentioned above are supported by the RTCAN driver > xeno_can_esd_pci. Why do you propose a new driver instead of > extending the existing one? For Socket-CAN, this driver is supposed to support all PLX PCI based boards including the esd CAN PCI cards and also the IXXAT PCI board (not yet done, though). The RTCAN driver xeno_can_esd_pci is a *dedicated* driver for that card without generic support for the PLX PCI chips. Extending it makes little sense. The question is if we want to drop xeno_can_esd_pci and xeno_can_ixxat_pci. >>> >>> With extending the esd_pci driver I meant to take this driver as a >>> basis to add support for more cards. Of course this would mean to >>> rename the driver in order to reflect that. I'm not against a >>> unification of esd_pci and ixxat_pci but it's unneccessary work to >>> write a new RTCAN driver if there exists another one which is tried and >>> tested in the field. So my question was why Pavel did not take the >>> esd_pci driver as the starting position. >> >> He did *not* write a new RTCAN driver. He ported plx_pci.c from the >> mainline kernel to rtcan, > > So it is a new RTCAN driver - never mind, just nitpicking. ;-) > > As I said, I'm in favour of a unified driver which supports more cards than > esd_pci and ixxat_pci. The question is how we proceed best when we include > rt_sja1000_plx_pci. I suggest not to remove esd_pci immediately but rather > mark it deprecated with a reference to the new driver, e.g. in the Kconfig > help text. That way we would have a fallback driver in case of unforeseen > issues with the new driver. Fine with me. Marking it as deprecated is a good thing, though. Note that the esd part of the plx_pci driver was contributed by Matthias Fuchs, including support for the *new* esd pci cards. Wolfgang. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Wolfgang Grandegger wrote: > On 05/18/2010 02:29 PM, Sebastian Smolorz wrote: > > Hi Wolfgang, > > > > Wolfgang Grandegger wrote: > >> On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: > >>> Pavel Cheblakov wrote: > This is a general driver for cards based on PLX90xx PCI-bridges. > It supports following cards: > - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) > - Adlink PCI-7841/cPCI-7841 SE card > - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) > - esd CAN-PCI/PMC/266 > - esd CAN-PCIe/2000 > - Marathon CAN-bus-PCI card (http://www.marathon.ru/) > - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) > >>> > >>> The esd cards mentioned above are supported by the RTCAN driver > >>> xeno_can_esd_pci. Why do you propose a new driver instead of > >>> extending the existing one? > >> > >> For Socket-CAN, this driver is supposed to support all PLX PCI based > >> boards including the esd CAN PCI cards and also the IXXAT PCI board > >> (not yet done, though). The RTCAN driver xeno_can_esd_pci is a > >> *dedicated* driver for that card without generic support for the PLX > >> PCI chips. Extending it makes little sense. The question is if we want > >> to drop xeno_can_esd_pci and xeno_can_ixxat_pci. > > > > With extending the esd_pci driver I meant to take this driver as a > > basis to add support for more cards. Of course this would mean to > > rename the driver in order to reflect that. I'm not against a > > unification of esd_pci and ixxat_pci but it's unneccessary work to > > write a new RTCAN driver if there exists another one which is tried and > > tested in the field. So my question was why Pavel did not take the > > esd_pci driver as the starting position. > > He did *not* write a new RTCAN driver. He ported plx_pci.c from the > mainline kernel to rtcan, So it is a new RTCAN driver - never mind, just nitpicking. ;-) As I said, I'm in favour of a unified driver which supports more cards than esd_pci and ixxat_pci. The question is how we proceed best when we include rt_sja1000_plx_pci. I suggest not to remove esd_pci immediately but rather mark it deprecated with a reference to the new driver, e.g. in the Kconfig help text. That way we would have a fallback driver in case of unforeseen issues with the new driver. > which is much less work and even less error > prune. > > > So talking about potential issues in Pavel's code (e.g. the function > > plx_pci_check_sja1000()) could be unnecessary if he derived the new > > driver from esd_pci . > > Does it harm? It is necessary to find out the number of channels on some > cards. Yes, you are right. -- Sebastian ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
On 05/18/2010 02:29 PM, Sebastian Smolorz wrote: > Hi Wolfgang, > > Wolfgang Grandegger wrote: >> On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: >>> Pavel Cheblakov wrote: This is a general driver for cards based on PLX90xx PCI-bridges. It supports following cards: - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) - Adlink PCI-7841/cPCI-7841 SE card - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) - esd CAN-PCI/PMC/266 - esd CAN-PCIe/2000 - Marathon CAN-bus-PCI card (http://www.marathon.ru/) - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) >>> >>> The esd cards mentioned above are supported by the RTCAN driver >>> xeno_can_esd_pci. Why do you propose a new driver instead of extending >>> the existing one? >> >> For Socket-CAN, this driver is supposed to support all PLX PCI based >> boards including the esd CAN PCI cards and also the IXXAT PCI board (not >> yet done, though). The RTCAN driver xeno_can_esd_pci is a *dedicated* >> driver for that card without generic support for the PLX PCI chips. >> Extending it makes little sense. The question is if we want to drop >> xeno_can_esd_pci and xeno_can_ixxat_pci. > > With extending the esd_pci driver I meant to take this driver as a basis to > add support for more cards. Of course this would mean to rename the driver > in order to reflect that. I'm not against a unification of esd_pci and > ixxat_pci but it's unneccessary work to write a new RTCAN driver if there > exists another one which is tried and tested in the field. So my question > was why Pavel did not take the esd_pci driver as the starting position. He did *not* write a new RTCAN driver. He ported plx_pci.c from the mainline kernel to rtcan, which is much less work and even less error prune. > So talking about potential issues in Pavel's code (e.g. the function > plx_pci_check_sja1000()) could be unnecessary if he derived the new driver > from esd_pci . Does it harm? It is necessary to find out the number of channels on some cards. Wolfgang. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Hi Wolfgang, Wolfgang Grandegger wrote: > On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: > > Pavel Cheblakov wrote: > >> This is a general driver for cards based on PLX90xx PCI-bridges. > >> It supports following cards: > >> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) > >> - Adlink PCI-7841/cPCI-7841 SE card > >> - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) > >> - esd CAN-PCI/PMC/266 > >> - esd CAN-PCIe/2000 > >> - Marathon CAN-bus-PCI card (http://www.marathon.ru/) > >> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) > > > > The esd cards mentioned above are supported by the RTCAN driver > > xeno_can_esd_pci. Why do you propose a new driver instead of extending > > the existing one? > > For Socket-CAN, this driver is supposed to support all PLX PCI based > boards including the esd CAN PCI cards and also the IXXAT PCI board (not > yet done, though). The RTCAN driver xeno_can_esd_pci is a *dedicated* > driver for that card without generic support for the PLX PCI chips. > Extending it makes little sense. The question is if we want to drop > xeno_can_esd_pci and xeno_can_ixxat_pci. With extending the esd_pci driver I meant to take this driver as a basis to add support for more cards. Of course this would mean to rename the driver in order to reflect that. I'm not against a unification of esd_pci and ixxat_pci but it's unneccessary work to write a new RTCAN driver if there exists another one which is tried and tested in the field. So my question was why Pavel did not take the esd_pci driver as the starting position. So talking about potential issues in Pavel's code (e.g. the function plx_pci_check_sja1000()) could be unnecessary if he derived the new driver from esd_pci . -- Sebastian ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Wolfgang Grandegger wrote: > Hi Sebastian, > > On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: > >> Pavel Cheblakov wrote: >> >>> This is a general driver for cards based on PLX90xx PCI-bridges. >>> It supports following cards: >>> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) >>> - Adlink PCI-7841/cPCI-7841 SE card >>> - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) >>> - esd CAN-PCI/PMC/266 >>> - esd CAN-PCIe/2000 >>> - Marathon CAN-bus-PCI card (http://www.marathon.ru/) >>> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) >>> >> The esd cards mentioned above are supported by the RTCAN driver >> xeno_can_esd_pci. Why do you propose a new driver instead of extending the >> existing one? >> > > For Socket-CAN, this driver is supposed to support all PLX PCI based > boards including the esd CAN PCI cards and also the IXXAT PCI board (not > yet done, though). The RTCAN driver xeno_can_esd_pci is a *dedicated* > driver for that card without generic support for the PLX PCI chips. > Extending it makes little sense. The question is if we want to drop > xeno_can_esd_pci and xeno_can_ixxat_pci. > > Wolfgang. > I subscribe to Wolfgang. Moreover, during an adaptation of Socket-CAN plx_pci driver to Xenomai, I remarked that these drivers are too similar. In general, the differences consist only of slightly changed API (early version of Socket-CAN internal API) and different prefixes for some names. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Hi Sebastian, On 05/18/2010 01:42 PM, Sebastian Smolorz wrote: > Pavel Cheblakov wrote: >> This is a general driver for cards based on PLX90xx PCI-bridges. >> It supports following cards: >> - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) >> - Adlink PCI-7841/cPCI-7841 SE card >> - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) >> - esd CAN-PCI/PMC/266 >> - esd CAN-PCIe/2000 >> - Marathon CAN-bus-PCI card (http://www.marathon.ru/) >> - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) > > The esd cards mentioned above are supported by the RTCAN driver > xeno_can_esd_pci. Why do you propose a new driver instead of extending the > existing one? For Socket-CAN, this driver is supposed to support all PLX PCI based boards including the esd CAN PCI cards and also the IXXAT PCI board (not yet done, though). The RTCAN driver xeno_can_esd_pci is a *dedicated* driver for that card without generic support for the PLX PCI chips. Extending it makes little sense. The question is if we want to drop xeno_can_esd_pci and xeno_can_ixxat_pci. Wolfgang. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] rtcan: add rtcan_plx_pci driver
Pavel Cheblakov wrote: > This is a general driver for cards based on PLX90xx PCI-bridges. > It supports following cards: > - Adlink PCI-7841/cPCI-7841 card (http://www.adlinktech.com/) > - Adlink PCI-7841/cPCI-7841 SE card > - esd CAN-PCI/CPCI/PCI104/200 (http://www.esd.eu/) > - esd CAN-PCI/PMC/266 > - esd CAN-PCIe/2000 > - Marathon CAN-bus-PCI card (http://www.marathon.ru/) > - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/) The esd cards mentioned above are supported by the RTCAN driver xeno_can_esd_pci. Why do you propose a new driver instead of extending the existing one? -- Sebastian ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core