From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api Date: Tue, 10 Nov 2009 14:07:17 -0700 Message-ID: References: <1257884778.14374.5@antares> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1257884778.14374.5@antares> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+glppd-linuxppc64-dev=m.gmane.org@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+glppd-linuxppc64-dev=m.gmane.org@lists.ozlabs.org To: =?ISO-8859-1?Q?Albrecht_Dre=DF?= Cc: Linux PPC Development , devicetree-discuss@lists.ozlabs.org, Wim Van Sebroeck List-Id: devicetree@vger.kernel.org On Tue, Nov 10, 2009 at 1:26 PM, Albrecht Dre=DF = wrote: > Hi Grant: > > Am 10.11.09 20:59 schrieb(en) Grant Likely: >> >> On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dre=DF >> wrote: >> > Use the MPC5200 GPT api for the WDT which drastically simplifies this >> > file. >> > >> > Signed-off-by: Albrecht Dre=DF >> > --- >> > >> > =A0drivers/watchdog/mpc5200_wdt.c | =A0246 >> > +++++++++++----------------------------- >> > =A01 files changed, 65 insertions(+), 181 deletions(-) >> >> >> Can the WDT functionality just be merged entirely into >> arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for >> this file entirely? =A0I think I'd rather have all the GPT "built in" >> behaviour handled by a single driver. > > I also thought about it, as it has IMHO the cleaner code, and it would ha= ve > the extra benefit that the gpt-wdt api doesn't need to be public. > > However, the reasons I hesitated to do so are: > - I don't want to remove a file someone else wrote (even it doesn't work); Shouldn't be a problem, and I'll handle the fallout if it is. > - WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52xx > which might not be the "logical" place from the directory layout's pov; There is precedence of this in the past, particularly on arch or platform specific hardware drivers and multifunction devices. (Heck, that's almost entirely what arch/powerpc/sysdev is). I'm not concerned. > - a file living in arch/powerpc/platforms/52xx depends upon config options > set from drivers/watchdog/Kconfig which may be confusing. I'm not concerned about this either. > You see these are more political/cosmetical questions, so I would prefer = to > leave the decision to the maintainers (i.e. you and Wim). =A0Preparing a = fully > merged driver is actually a matter of minutes! Do it. I'll champion getting it in. Wim, do you have any issues with this? g. -- = Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.