Re: WG: PCAN-USB: new socketCAN driver available

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 07.12.2011 11:29, Grosjean Stephane wrote:

> Hi,
> 
> First of all, thank you for your time and all your comments.
> 
> Le 06/12/2011 22:24, Oliver Hartkopp a écrit :
>> On 06.12.2011 21:47, Sam Ravnborg wrote:
>>
>>> Today it is really recommended to do:
>>>
>>> obj-$(CONFIG_CAN_PEAK_USB) += peak_usb.o
>>> peak_usb-y := pcan_usb_core.o pcan_usb.o
>>>
>>> obj-$(CONFIG_CAN_PEAK_USB_PRO) += peak_usb_pro.o
>>> peak_usb_pro-y := pcan_usb_core.o pcan_usb_pro.o
>>>
>>> Both solutions will work - but the latter allows for some nice
>>> "kbuild" assignments to conditional stuff.
>>>
> 
> right...
> 
>> I just tried out the suggestion myself ...
>>
>> As my idea was to link the common functions directly to the module binary this
>> may double the needed size. Maybe your approach is really better 8-)
>>
>> But the fact that the select should be omitted is still true.
>>
>> Better you define a
>>
>> config CAN_PEAK_USB
>>     tristate "PEAK System USB adapters"
>>
>> and then
>>
>> config CAN_PCAN_USB
>>     tristate "PEAK PCAN USB adapter"
>>     depends on CAN_PEAK_USB
>>
>>
>> config CAN_PCAN_USB_PRO
>>     tristate "PEAK PCAN USB Pro adapter"
>>     depends on CAN_PEAK_USB
>>
>> ...
>>
>> Regards,
>> Oliver
> 
> I thought this was better for memory size and also for easier handling of
> (any) further PEAK USB adapter, never know...
> But, what next? According to your below Kconfig and Sam's Kbuild, and if I
> understand well your suggestions, the Kbuild file always builds 2 module
> files, while I wanted to build only a single one, capable of handling one or
> both adapters.


Ah, ok. I formerly thought that you plan to have three separated kernel
modules (core, usb, usbpro) - but i just checked that you only create a
peak_usb.ko which is perfectly fine.

> Please, confirm that a mainline kernel driver should (must?) handle one and

> only one kind of device.


No. If your driver supports "PEAK PCAN USB / USB Pro adapters" you can put
this into a single Kconfig entry and build a single peak_usb.ko if you like.

I don't know whether it's worth the effort to build separate drivers and
kernel modules. The current code has some space for improvements and cleanups :-)

When you're finished your work with the USB Pro it's time to post it on
linux-can ML and the we can discuss together how the sources and the driver
should be packaged best.

Post early & often :-)

> If it should (must), I will modify the Kbuild file as proposed by Sam, in
> order to build a peak_usb.ko and/or a peak_usb_pro.ko file.


This was just a suggestion.

> 
> But in that case, I wonder why the Kernel API offers some data structures like
> "struct usb_driver.id_table" which are supposed to describe more than one
> device id.


Yes you can do that also (as written above).

Regards,
Oliver