Wolfgang Grandegger wrote:
> Oliver Hartkopp wrote:
>> Hi Wolfgang,
>>
>> i discovered a problem in the current mainline linux-2.6 git due to the
>> commit
>> of the EMS USB driver.
>>
>> As the EMS USB driver has been (accidently?) added right after the EMS PCI
>> driver the SJA1000 drivers had been torn in pieces and the indention is
>> broken
>> for CAN_KVASER_PCI.
>
> I already realized this problem. The simplest fix is to move the the
> Kconfig entry for the EMS USB driver below the SJA1000 ones.
>
>> I attached a (trivial) patch to move the SJA1000 and USB Kconfig files into
>> their specific directories, which fixes the problem.
>
> I like that fix even more.
>
>> But before we post a patch like this, the question comes up, if we should
>> create directories for
>>
>> - CAN controllers (SJA1000, AT91, CC770, ...)
>> - Bus systems (USB, PCMCIA, IO, ...)
>> - Vendors (EMS, IXXAT, PEAK, homebrew, ...)
>
> I don't like a separate directory per vendor.
Me too.
>
>> E.g. should the EMS PCMCIA driver appear in a pcmcia directory (and Kconfig)
>> or is sja1000 still the better approach?
>
> It should remain in sja1000 because it uses the sja1000dev interface
> (CONFIG_CAN_SJA1000).
>
Ack.
>> For me it would be ok to sort the directory by CAN controllers, 'usb',
>> 'softing', etc - as we have it right now.
>
> The rule of thumb is: if there are more than *3* files for a driver or
> class of drivers, a sub-directory could/should be created.
This was new to me. Every day i learn something new about the kernel ;-)
> For this
> reason I was also happy with the EMS USB CAN driver in the top-level
> directory.
Indeed what data is transferred in the urbs is very CAN USB adapter specific.
For that reason we won't have been able to map USB drivers to CAN controllers
anyway.
>> But at least we should move the Kconfigs into the subdirs as suggested in my
>> patch.
>
> Yes, this is less error prune. Are you going to provide a patch?
Yes. I can do so.
Does it make sense to you to define a CONFIG_CAN_USB analogue to
CONFIG_CAN_SJA1000 ??
The drivers/net/can/usb/Kconfig could look like this then:
config CAN_USB
bool "CAN USB interfaces"
depends on USB && CAN_DEV
---help---
Prompt for CAN USB interfaces
config CAN_EMS_USB
tristate "EMS CPC-USB/ARM7 CAN/USB interface"
depends on CAN_USB
---help---
This driver is for the one channel CPC-USB/ARM7 CAN/USB interface
from from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
Looks really smooth in menuconfig :-)
Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core