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

Reply via email to