About peak_pci PATH v6.1

DISCUSSION Stephane Grosjean 2 participants 4 messages Feb 20, 2012 – Feb 23, 2012 CAN network drivers

Share & tools

Stephane Grosjean

author

2012-02-20 11:25 UTC #0 lore raw

Hi Oliver,

I found that the i2c_transfer() is able to sleep, which is generally *not* a good idea when in an interrupt context...
So, I changed the delay management with a delayed work mechanism. I successfully tested the change on my testbed, but as you know, this is not the best place for testing...

I hope you will be able to find some time to test that new patch.

@Marc: the peak_pci and peak_pcmcia patches bring modifications to the can/sja1000 dir Kconfig and Makefile, So the apply sequence order is critical (I mean, the peak_pci patch must be applied before the peak_pcmcia, because of lines numbers). How to fix that? I first proposed to use a serie of patches but this was not approved...

Regards,

Stéphane
--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, [email protected]

Marc Kleine-Budde

maintainer

2012-02-20 12:18 UTC #1 lore raw

Quoted 2 lines · Stephane Grosjean

I found that the i2c_transfer() is able to sleep, which is generally *not* a good idea when in an interrupt context...

Not allowed :)

Quoted 11 lines

So, I changed the delay management with a delayed work mechanism. I successfully tested the change on my testbed, but as you know, this is not the best place for testing...
I hope you will be able to find some time to test that new patch.
@Marc: the peak_pci and peak_pcmcia patches bring modifications to the can/sja1000 dir Kconfig and Makefile, So the apply sequence order is critical (I mean, the peak_pci patch must be applied before the peak_pcmcia, because of lines numbers). How to fix that? I first proposed to use a serie of patches but this was not approved...

I think due to my too short answer there was a misunderstanding. IIRC in one patch you 1. fixed a problem (which is already in mainline) and 2. introduced a new feature. This is why I asked for separate patches. All changes can still go into a series.

I already added your "add support for PEAK-System PCIe/PCIeC/miniPCI cards" patch to can-next/master, but If you now have a better version, I'll force update the branch.

I just updated can-next/master, so that it doesn't include your patch. Now it get complicated :) In order to test the patches you need Olivers unplug fix patch, I send it to David today, and it's not part of can-next, yet. For easy testing, I merged linux-can into linux-can-next, the branch is called "with-can-merge" in the linux-can-next repo. Please make your patch series based on that branch, I'll apply it to linux-can-next/master and ask David to merge net into net-next first, in case he hasn't.

Back to your question. Yes, please give us a series of patches, even if its PCI and USB drivers.

I hope you understand what I mean ;)

Marc

Signature · 4 lines

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   |

Stephane Grosjean

author

2012-02-20 13:25 UTC #2 lore raw

Quoted 4 lines · Marc Kleine-Budde

Quoted 2 lines · Stephane Grosjean
I found that the i2c_transfer() is able to sleep, which is generally *not* a good idea when in an interrupt context...
Not allowed :)

Yes I do agree... I already had to fight in the past against that function in an other context (embedded arm board) so I first had suspicions using it here... But its code now includes some contextual tests before (trying to) acquire some mutex lock... So, I (too) quickly concluded that this function was now allowed in that context... I asked the linux-i2c ml and one confirms that this depends on the underlying I2C adapter driver. So to be portable, I did the change accordingly, but I anxiously wait for any feedback from Oliver! ;-)

Quoted 11 lines

Quoted 7 lines
I hope you will be able to find some time to test that new patch.
@Marc: the peak_pci and peak_pcmcia patches bring modifications to the can/sja1000 dir Kconfig and Makefile, So the apply sequence order is critical (I mean, the peak_pci patch must be applied before the peak_pcmcia, because of lines numbers). How to fix that? I first proposed to use a serie of patches but this was not approved...
I think due to my too short answer there was a misunderstanding. IIRC in one patch you 1. fixed a problem (which is already in mainline) and 2. introduced a new feature. This is why I asked for separate patches. All changes can still go into a series.

Ok, so from now, I'll post a serie of patches for the can/sja1000 dir.

Quoted 13 lines

I already added your "add support for PEAK-System PCIe/PCIeC/miniPCI cards" patch to can-next/master, but If you now have a better version, I'll force update the branch.
I just updated can-next/master, so that it doesn't include your patch. Now it get complicated :) In order to test the patches you need Olivers unplug fix patch, I send it to David today, and it's not part of can-next, yet. For easy testing, I merged linux-can into linux-can-next, the branch is called "with-can-merge" in the linux-can-next repo. Please make your patch series based on that branch, I'll apply it to linux-can-next/master and ask David to merge net into net-next first, in case he hasn't.

Quoted 2 lines

Back to your question. Yes, please give us a series of patches, even if its PCI and USB drivers.

... well both have nothing in common so if nobody disagrees, I'll keep using two series of patches: one for can/sj1000, the other for peak_usb...

FYI: regarding the peak_usb driver, I still wait for any Ack from the linux-usb ml...

Stéphane

--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391 Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, [email protected]

Stephane Grosjean

author

2012-02-23 15:29 UTC #3 lore raw

In reply to Marc Kleine-Budde

Hi Marc,

Quoted 5 lines · Marc Kleine-Budde

Quoted 5 lines
@Marc: the peak_pci and peak_pcmcia patches bring modifications to the can/sja1000 dir Kconfig and Makefile, So the apply sequence order is critical (I mean, the peak_pci patch must be applied before the peak_pcmcia, because of lines numbers). How to fix that? I first proposed to use a serie of patches but this was not approved...

Quoted 9 lines

I think due to my too short answer there was a misunderstanding. IIRC in one patch you 1. fixed a problem (which is already in mainline) and 2. introduced a new feature. This is why I asked for separate patches. All changes can still go into a series.
I already added your "add support for PEAK-System PCIe/PCIeC/miniPCI cards" patch to can-next/master, but If you now have a better version, I'll force update the branch.

So what's your opinion about that new serie?

0001 which contains Kconfig changes (pci+pciec+pcmcia) 0002 contains peak_pci modifications (+pciec,...) 0002 contains Makefile "pcmcia.o" adding + peak_pcmcia

???

Moreover, is that new format compatible with the previous pci-related patch you pushed before?

Regards,