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