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. Wolfgang. _______________________________________________ Socketcan-users mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-users
