[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1400794689.16407.57.camel@x220>
Date: Thu, 22 May 2014 23:38:09 +0200
From: Paul Bolle <pebolle@...cali.nl>
To: Karsten Keil <kkeil@...ux-pingi.de>
Cc: Tilman Schmidt <tilman@...p.cc>, netdev@...r.kernel.org,
isdn4linux@...tserv.isdn4linux.de,
"Keil, Karsten" <isdn@...ux-pingi.de>
Subject: Re: [PATCH 1/4] isdn/capi: move capi_info2str to capidrv.c
Karsten,
On Thu, 2014-05-22 at 08:32 +0200, Karsten Keil wrote:
> Am 21.05.2014 23:39, schrieb Tilman Schmidt:
> > capi_info2str() is apparently meant to be of general utility. It is
> > actually only used in capidrv.c. So move it from capiutil.c to
> > capidrv.c and (obviously) stop exporting it.
> >
> > And, since we're touching this, merge the two versions of this
> > function.
>
> I disagree here, since this is a general helper function and should be
> not in a special driver, but stay in capiutils.c which is the place for
> the driver independent stuff. I used this function from time to time to
> instrument other places for debugging as well.
Thanks for commenting. It would be nice if you could also comment on
patch 4/4. You might be able to tell whether the things I say there
about, well, the history of CAPI (middleware) device nodes are correct,
as you were already maintainer during that period, weren't you?
This patch, 1/4, and patch 2/4, simplify the Kconfig file and the code a
bit. It makes it bit easier to understand how the CAPI code fits
together. Same thing with my commit d1958f8c2f0d ("isdn/capi: Make
Middleware depend on CAPI2.0"). It is also nice to drop the
ISDN_DRV_AVMB1_VERBOSE_REASON name, which only makes sense to people
that know the ancient history of this code.
Anyhow, this patch might complicate your local debugging practices. That
might be inconvenient for you. But in mainline we see a function that's
used in one place only. And I think cleaning up mainline code is what
counts.
Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists