Aw: Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api

["Albrecht Dreß" <albrecht.dress@arcor.de>]

Hi Grant:

O.k., thanks for your comments.  If Wim doesn't have any objections to it, I will provide a merged patch.  One consequence I forgot to mention is that we loose the ability to build the wdt support as module, but I don't think it's a real problem.

I think we still should keep the kernel config option enable/disable the wdt support, which would mask out the wdt code if disabled.  Is that ok for you?

Thanks, Albrecht.


----- Original Nachricht ----
Von:     Grant Likely <grant.likely@secretlab.ca>
An:      Albrecht Dreß <albrecht.dress@arcor.de>
Datum:   10.11.2009 22:07
Betreff: Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api

> On Tue, Nov 10, 2009 at 1:26 PM, Albrecht Dreß <albrecht.dress@arcor.de>
> wrote:
> > Hi Grant:
> >
> > Am 10.11.09 20:59 schrieb(en) Grant Likely:
> >>
> >> On Tue, Nov 10, 2009 at 12:43 PM, Albrecht Dreß
> <albrecht.dress@arcor.de>
> >> wrote:
> >> > Use the MPC5200 GPT api for the WDT which drastically simplifies this
> >> > file.
> >> >
> >> > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de>
> >> > ---
> >> >
> >> >  drivers/watchdog/mpc5200_wdt.c |  246
> >> > +++++++++++-----------------------------
> >> >  1 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?  I 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
> have
> > 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).  Preparing 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.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

Jetzt NEU: Do it youself E-Cards bei Arcor.de!
Stellen Sie Ihr eigenes Unikat zusammen und machen Sie dem Empfänger eine ganz persönliche Freude!
E-Card Marke Eigenbau: HIER KLICKEN: http://www.arcor.de/rd/footer.ecard