Hi Oliver, On 08/04/2011 07:54 PM, Oliver Hartkopp wrote: > 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?!?
Hm, we have in the Makefiles: ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG which enables the dev_dbg() messages. Therefore it's needed. Wolfgang. _______________________________________________ Socketcan-users mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-users
