From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-arm-msm-owner@vger.kernel.org>
Received: from wolverine02.qualcomm.com ([199.106.114.251]:34135 "EHLO
	wolverine02.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754917Ab0JFNsI (ORCPT
	<rfc822;linux-arm-msm@vger.kernel.org>);
	Wed, 6 Oct 2010 09:48:08 -0400
Subject: Re: [PATCH] serial: DCC(JTAG) serial and console emulation support
From: Daniel Walker <dwalker@codeaurora.org>
In-Reply-To: <alpine.LFD.2.00.1010052246380.3107@xanadu.home>
References: <1286305668-19987-1-git-send-email-dwalker@codeaurora.org>
	 <alpine.LFD.2.00.1010052246380.3107@xanadu.home>
Content-Type: text/plain; charset="UTF-8"
Date: Wed, 06 Oct 2010 06:48:06 -0700
Message-ID: <1286372886.22265.63.camel@m0nster>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-arm-msm-owner@vger.kernel.org
List-ID: <linux-arm-msm.vger.kernel.org>
To: Nicolas Pitre <nico@fluxnic.net>
Cc: linux-arm-kernel@lists.infradead.org, Tony Lindgren <tony@atomide.com>, linux-arm-msm@vger.kernel.org, "Hyok S. Choi" <hyok.choi@samsung.com>, Jeff Ohlstein <johlstei@quicinc.com>

On Tue, 2010-10-05 at 22:55 -0400, Nicolas Pitre wrote:
> On Tue, 5 Oct 2010, Daniel Walker wrote:
> 
> > +#if !defined(CONFIG_CPU_V7)
> > +static inline char
> > +__dcc_getchar(void)
> > +{
> > +	char __c;
> > +
> > +	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
> > +		: "=r" (__c) : : "cc");
> > +
> > +	return __c;
> > +}
> > +#else
> > +static inline char
> > +__dcc_getchar(void)
> > +{
> > +	char __c;
> > +
> > +	asm(
> > +	"get_wait:	mrc p14, 0, pc, c0, c1, 0                   \n\
> > +			bne get_wait                                    \n\
> > +			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
> > +		: "=r" (__c) : : "cc");
> > +
> > +	return __c;
> > +}
> > +#endif
> > +
> > +#if !defined(CONFIG_CPU_V7)
> > +static inline void
> > +__dcc_putchar(char c)
> > +{
> > +	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
> > +		: /* no output register */
> > +		: "r" (c) : "cc");
> > +}
> > +#else
> > +static inline void
> > +__dcc_putchar(char c)
> > +{
> > +	asm(
> > +	"put_wait:	mrc p14, 0, pc, c0, c1, 0                   \n\
> > +			bcs put_wait                                \n\
> > +			mcr p14, 0, %0, c0, c5, 0                   "
> > +	: : "r" (c) : "cc");
> > +}
> > +#endif
> 
> Please move the #ifdef conditionals inside the respective functions so 
> to have only one function pair with the various alternatives embedded 
> into them.

My typical clean up policy is do to the inverse of what your suggesting.
Mainly because that's the method that I see used extensive in generic
parts of the kernel.

>>>From my perspective there are pluses an minuses to both. Your method
reduces lines, and duplication. My method makes the code easier to read.

Daniel

-- 
Sent by an consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum.