On 06/05/2011 03:58 PM, Mauro Carvalho Chehab wrote: > Don't add \n\t\t at the beginning of the param_desc. Even the extra \t and \n > format stuff in the middle of the description is unusual. It is better to avoid, > as it may break scripts. OK, I will remove the tabs. Is the use of \n allowed in the description? > Please put all parameters together. Does this mean that the macro definitions should be moved from between the parameter definitions ? > Don't add a card_type. Just add the features that are needed for > XC4000_CARD_WINFAST_CX88 to work. Yes, this is a change I have already planned. The following parameters will be added to the priv and config structures: - the default enabling of power management - amplitude for DVB-T The other conditionals might not actually be necessary, they were just added to avoid changing the behavior of the driver with the PCTV 340e which I cannot test. A third parameter could be added to enable/disable the use of XREG_SMOOTHEDCVBS, although it is not really needed with the currently supported cards (always enabling it should not be a problem). > Please use a generic parameter. In this case, it seems that it is just > disabling one video mode. I don't think you need this here, as the better > is to disable such video mode in cx88. Hard to tell without seeing the > cx88 code that adds support for the Winfast xc4000-based card. I do not think making it card-specific is really needed. It is probably best to just remove the card_type check here. Also, the code is there only to improve support for old (1.2) firmware versions. For xc4000.c and xc4000.h, is it enough to create the following three patches, or should the changes be broken up into more smaller patches ? - removing the use of card_type - uncommenting the firmware version check - coding style / cleanup -- 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
