On 04.08.2011 19:36, Wolfgang Grandegger wrote: > On 08/04/2011 07:12 PM, Robin Holt wrote: >> On Thu, Aug 04, 2011 at 07:01:40PM +0200, Wolfgang Grandegger wrote: >>> On 08/04/2011 06:41 PM, Robin Holt wrote: >>>> On Thu, Aug 04, 2011 at 06:35:06PM +0200, Wolfgang Grandegger wrote: >>>>> Hi Robin, >>>>> >>>>> On 08/04/2011 06:23 PM, Robin Holt wrote: >>>>>> Add a wrapper function for a register dump when CONFIG_CAN_DEBUG_DEVICES >>>>>> is set. >>>>>> >>>>>> Signed-off-by: Robin Holt <[email protected]> >>>>>> To: Marc Kleine-Budde <[email protected]> >>>>>> Cc: [email protected] >>>>> >>>>> This patch is useful for development but should be dropped for mainline. >>>> >>>> I am not sure why? There is already a CONFIG_CAN_DEBUG_DEVICES Kconfig >>>> setting. This just makes it of use for flexcan.c. The code is not >>>> made less readable by the patch and when CONFIG_CAN_DEBUG_DEVICES=n, >>>> there is no code difference with and without the patch. It seems like >>>> we would minimize bit rot if it were maintained in the mainline tree. >>>> What are the arguments against having it included? >>> >>> Debugging output should be reduced to a few useful messages. A normal >>> user will not understand the register content of that controller and >>> may users use CONFIG_CAN_DEBUG_DEVICES=y by default. Though there should >>> be no debug output in the TX and RX patch. >> >> I did not know that about CONFIG_CAN_DEBUG_DEVICES. What if I change >> the conditional to something more like: >> >> #if defined(CONFIG_CAN_DEBUG_DEVICES) && defined(FLEXCAN_DEBUG). >> Then a developer would have to get FLEXCAN_DEBUG defined, but normal >> users with CONFIG_CAN_DEBUG_DEVICES=y would not be affected. Would that >> be acceptable? > > Fine for development but not for mainline. None of the other drivers > does have advanced debugging capabilities.
AFAIK currently no driver is really using CONFIG_CAN_DEBUG_DEVICES. It's only redirected via all Makefiles ... If we do not populate CONFIG_CAN_DEBUG_DEVICES there's always the option to remove it entirely?!? Btw. dumping controller registers is really too much IMHO. This is something for development as you already stated. Regards, Oliver _______________________________________________ Socketcan-users mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-users
