RE: [PULL] http://linuxtv.org/hg/~mkrufky/sms1xxx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






-----Original Message-----
From: mkrufky@xxxxxxxxx on behalf of Michael Krufky
Sent: Thu 2/26/2009 9:23 PM
To: Mauro Carvalho Chehab
Cc: linux-media@xxxxxxxxxxxxxxx; Uri Shkolnik
Subject: Re: [PULL] http://linuxtv.org/hg/~mkrufky/sms1xxx

On Thu, Feb 26, 2009 at 2:13 PM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxxxxx> wrote:
> On Fri, 20 Feb 2009 11:53:51 -0500
> Michael Krufky <mkrufky@xxxxxxxxxxx> wrote:
>
>> Mauro,
>>
>> As you know, there is a large amount of pending changesets from Siano
>> that are waiting for merge.
>>
>> Given the large amount of changes and the impact it has on the
>> driver's current functionality, I am going to make the merge requests
>> in phases.
>>
>> This is the first phase, which merges the first round of changesets
>> from Uri, in addition to codingstyle cleanups and some fixes for some
>> current devices.
>>
>> I am still processing through the remaining changesets, but I think
>> it's important to try to merge some of this now, to reduce the delta
>> of the changes that will need review when all is done and ready.
>>
>> This tree has been tested with the currently supported devices, and it
>> is a step towards merge of the pending Siano changesets.
>>
>> I am sure you will have some comments about these changes -- Let's try
>> to merge what we can now, and all comments will be addressed in future
>> cleanups.  Please post your comments to this thread so that we may
>> hold any review discussions in this public forum.
>>
>> The main objective of these changesets is to break down the sms1xxx
>> module into sub-modules.  Siano doesn't want to load the usb part of
>> the driver into memory unless the usb interface is to be used (which
>> won't always be the case).
>>
>> After these changes are merged, I will send additional merge requests
>> for the later changesets and the addition of the SPI interface.
>>
>> Please pull from:
>>
>> http://linuxtv.org/hg/~mkrufky/sms1xxx
>>
>> for the following changesets:
>>
>> - sms1xxx: enable rf switch on Hauppauge Tiger devices
>> - sms1xxx: move definition of struct smsdvb_client_t into smsdvb.c
>> - sms1xxx: restore smsusb_driver.name to smsusb
>> - sms1xxx: move smsusb_id_table into smsusb.c
>> - import changes from Siano
>> - sms1xxx: fix checkpatch.pl violations introduced by previous changeset
>> - sms1xxx: load smsdvb module automatically based on device id
>>
>>  Makefile     |    4
>>  sms-cards.c  |   92 ++++++++++------------
>>  sms-cards.h  |    7 -
>>  smscoreapi.c |  162 +++++++++++++++++++++++++++++----------
>>  smscoreapi.h |   46 ++---------
>>  smsdvb.c     |   66 +++++++++++++--
>>  smsusb.c     |   73 +++++++++++++++--
>>  7 files changed, 304 insertions(+), 146 deletions(-)
>
> Hmm... Why are you adding all those symbols to be exported?
>
> +EXPORT_SYMBOL(smscore_onresponse);
> +EXPORT_SYMBOL(sms_get_board);
> +EXPORT_SYMBOL(sms_debug);
> +EXPORT_SYMBOL(smscore_putbuffer);
> +EXPORT_SYMBOL(smscore_registry_getmode);
> +EXPORT_SYMBOL(smscore_register_device);
> +EXPORT_SYMBOL(smscore_set_board_id);
> +EXPORT_SYMBOL(smscore_start_device);
> +EXPORT_SYMBOL(smscore_unregister_device);
> +EXPORT_SYMBOL(smscore_getbuffer);
> +EXPORT_SYMBOL(smscore_get_device_mode);
> +EXPORT_SYMBOL(smscore_register_client);
> +EXPORT_SYMBOL(smscore_unregister_hotplug);
> +EXPORT_SYMBOL(smsclient_sendrequest);
> +EXPORT_SYMBOL(smscore_unregister_client);
> +EXPORT_SYMBOL(smscore_get_board_id);
> +EXPORT_SYMBOL(smscore_register_hotplug);
>
> Shouldn't all of they be instead EXPORT_SYMBOL_GPL?
>
> Cheers,
> Mauro
>

cc added to Uri @ Siano.

Uri,

Would it be okay with you if we make the above change?  You don't have
to send in a patch for this right now, since I am already in the midst
of merging your patch series.  If you give permission to change these
exports to EXPORT_SYMBOL_GPL, then please provide your sign-off and I
will merge that change for you in the next series merge.

Thanks,

Mike

----------------------------------------------------------

Mike, Mauro,


Please feel free to modify **anything** in the code that you think will make it more compatible with Linux kernel's methods, guidelines, coding style and/or any other relevant directives. Each of you have much more knowledge about it than I have.

There is one thing that is significant to me, which is the the flexibility of the package (meaning how easy it to add more bus drivers, more interfaces and other customers' code / product related adjustments etc. ).

Please remember that this **very** modular software module should continue to function with multiple bus-drivers (TS, SPI/SPP, HIF, I2C, ....) foreign boards ("sms-cards"), and interfaces (DVB v3, DVB v5 and Siano's proprietary) from various sources which some of them are not (at the moment) available as open source (or sometimes violate Linux kernel's rules).
The idea is the the module's binary build content is selected by 'make menuconfig' and *any* variant which includes at least one bus driver (but maybe more) and at least one interface (but maybe more) is legit, and that a add/merge/patch of foreign bus-driver/interface will be kept an effortlessly task.

Mike, you have all relevant code segments. May I suggest that you will build up a full source tree with all modifications you would like to apply (and I also can give you my repository trunk's head so you'll always have a reference). After you have such a tree, give me a link and I'll back-merge it into Siano's repository.
"I hereby promise" not to bark/nudge/cry/etc., about anything that is not truly functionality related.

Any indentations, system call replacements, other required modifications are mostly welcome. That applies of course also to the 'EXPORT_SYMBOL_GPL'. This module is anyway under GPL (v2) license, so if you think it's better to change that, go ahead.


Thanks and Regards,

Uri



      
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux