Re: Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
On Sun, 2015-02-08 at 20:47 +0100, Tilman Schmidt wrote: > Am 07.02.2015 um 21:43 schrieb Paul Bolle: > > On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: > >> Does anyone still use these cards? > > 0) Good question. > I very much doubt it. It was an ISA card, not even PnP. I'd imagine > any systems of that kind would be retired by now. [] > I guess > somebody could just submit a patch series starting with: > +++ b/drivers/isdn/Kconfig > @@ -20,25 +20,6 @@ menuconfig ISDN > > if ISDN > > -menuconfig ISDN_I4L > - tristate "Old ISDN4Linux (deprecated)" > - depends on TTY [] > Shall I? I think removing code for completely unused, no longer sold or supported devices could be a good thing. If someone pipes up saying "I use that" within some shortish time-frame, it's easy to undo. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 07.02.2015 um 21:43 schrieb Paul Bolle: > On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: >> Does anyone still use these cards? > > 0) Good question. I very much doubt it. It was an ISA card, not even PnP. I'd imagine any systems of that kind would be retired by now. > 2) Broader picture: if I remember correctly there are now four > different flavors of ISDN in the kernel: - really old: pre-i4l I don't think any traces of that are still present in current kernel releases. It should all have been left behind on the switch to 2.6. > - very old: i4l - just old: CAPI - not so old: mISDN Those are the in-tree ones. To make matters worse, the Asterisk people invented three more (Zaptel, DAHDI and vISDN) which are maintained out-of-tree. Apparently they weren't satisfied with any the in-tree ones and didn't feel like helping to improve one of them to match their needs. > [snip] So the current ISDN situation is a bit messy. That's putting it mildly. > Tilman might be able to provide a clearer, and maybe less grumpy, > summary of the current situation. Don't know about either. ;-) > [M]aybe we should consider, say, removing i4l and pre i4l and see > who complains. That might be a rude thing to do. So perhaps the > various ISDN flavors should be left alone until ... what exactly? I'd support that step. I don't think it'll hurt anyone because the cards supported by i4l are mostly ISA cards anyway. The only exceptions are the HiSax family which is now supported by mISDN, and the Hypercope family which is supported by CAPI. In the past we had Documentation/feature-removal-schedule.txt for announcing removals like that but Linus shot that down on 2012-10-01 (commit 9c0ece069b32e8e122aea71aa47181c10eb85ba7) so I guess somebody could just submit a patch series starting with - --- a/drivers/isdn/Kconfig +++ b/drivers/isdn/Kconfig @@ -20,25 +20,6 @@ menuconfig ISDN if ISDN - -menuconfig ISDN_I4L - - tristate "Old ISDN4Linux (deprecated)" - - depends on TTY - - ---help--- - - This driver allows you to use an ISDN adapter for networking - - connections and as dialin/out device. The isdn-tty's have a built - - in AT-compatible modem emulator. Network devices support autodial, - - channel-bundling, callback and caller-authentication without having - - a daemon running. A reduced T.70 protocol is supported with tty's - - suitable for German BTX. On D-Channel, the protocols EDSS1 - - (Euro-ISDN) and 1TR6 (German style) are supported. See - - for more information. - - - - ISDN support in the linux kernel is moving towards a new API, - - called CAPI (Common ISDN Application Programming Interface). - - Therefore the old ISDN4Linux layer will eventually become obsolete. - - It is still available, though, for use with adapters that are not - - supported by the new CAPI subsystem yet. - - source "drivers/isdn/i4l/Kconfig" menuconfig ISDN_CAPI and working its way from that to remove anything that's become unreachable. Shall I? - -- Tilman Schmidt E-Mail: til...@imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite) -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQEcBAEBAgAGBQJU171KAAoJEFPuqx0v+F+qnqgH/3KCC6wnXWKOxfuWZ13lwqa2 GuFy+DMYZbXi5e5IHIYnTO9LvA7uzg4ryokP4iMWazhIpx5Cx0Riil59LJExE59p jsLizpDeZ+z+wwrEZHimc8lYEFO70abvgKL5NdkF8luc+QPIJAmDyLA+8c6J0rKo VcIb+vq3hW06L7FeKD3Y03NU1xZDtG6HxhnCQRV5b8Ve6sfeu4Oz/83yLyvvfNbK mnRWbUisO2TmBWcTbZ0QB889iPBRrR1HD4TLA9WZ38fQs7YbuKNbz4jxMKDDSh2i JHDK8tzn5P+73CPHkvRfELODn+V5uzUWh6QHJFD+4sMOKvNZfbxlEmt1GANmto0= =PtE1 -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 07.02.2015 um 21:43 schrieb Paul Bolle: On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: Does anyone still use these cards? 0) Good question. I very much doubt it. It was an ISA card, not even PnP. I'd imagine any systems of that kind would be retired by now. 2) Broader picture: if I remember correctly there are now four different flavors of ISDN in the kernel: - really old: pre-i4l I don't think any traces of that are still present in current kernel releases. It should all have been left behind on the switch to 2.6. - very old: i4l - just old: CAPI - not so old: mISDN Those are the in-tree ones. To make matters worse, the Asterisk people invented three more (Zaptel, DAHDI and vISDN) which are maintained out-of-tree. Apparently they weren't satisfied with any the in-tree ones and didn't feel like helping to improve one of them to match their needs. [snip] So the current ISDN situation is a bit messy. That's putting it mildly. Tilman might be able to provide a clearer, and maybe less grumpy, summary of the current situation. Don't know about either. ;-) [M]aybe we should consider, say, removing i4l and pre i4l and see who complains. That might be a rude thing to do. So perhaps the various ISDN flavors should be left alone until ... what exactly? I'd support that step. I don't think it'll hurt anyone because the cards supported by i4l are mostly ISA cards anyway. The only exceptions are the HiSax family which is now supported by mISDN, and the Hypercope family which is supported by CAPI. In the past we had Documentation/feature-removal-schedule.txt for announcing removals like that but Linus shot that down on 2012-10-01 (commit 9c0ece069b32e8e122aea71aa47181c10eb85ba7) so I guess somebody could just submit a patch series starting with - --- a/drivers/isdn/Kconfig +++ b/drivers/isdn/Kconfig @@ -20,25 +20,6 @@ menuconfig ISDN if ISDN - -menuconfig ISDN_I4L - - tristate Old ISDN4Linux (deprecated) - - depends on TTY - - ---help--- - - This driver allows you to use an ISDN adapter for networking - - connections and as dialin/out device. The isdn-tty's have a built - - in AT-compatible modem emulator. Network devices support autodial, - - channel-bundling, callback and caller-authentication without having - - a daemon running. A reduced T.70 protocol is supported with tty's - - suitable for German BTX. On D-Channel, the protocols EDSS1 - - (Euro-ISDN) and 1TR6 (German style) are supported. See - - file:Documentation/isdn/README for more information. - - - - ISDN support in the linux kernel is moving towards a new API, - - called CAPI (Common ISDN Application Programming Interface). - - Therefore the old ISDN4Linux layer will eventually become obsolete. - - It is still available, though, for use with adapters that are not - - supported by the new CAPI subsystem yet. - - source drivers/isdn/i4l/Kconfig menuconfig ISDN_CAPI and working its way from that to remove anything that's become unreachable. Shall I? - -- Tilman Schmidt E-Mail: til...@imap.cc Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite) -BEGIN PGP SIGNATURE- Version: GnuPG v2 iQEcBAEBAgAGBQJU171KAAoJEFPuqx0v+F+qnqgH/3KCC6wnXWKOxfuWZ13lwqa2 GuFy+DMYZbXi5e5IHIYnTO9LvA7uzg4ryokP4iMWazhIpx5Cx0Riil59LJExE59p jsLizpDeZ+z+wwrEZHimc8lYEFO70abvgKL5NdkF8luc+QPIJAmDyLA+8c6J0rKo VcIb+vq3hW06L7FeKD3Y03NU1xZDtG6HxhnCQRV5b8Ve6sfeu4Oz/83yLyvvfNbK mnRWbUisO2TmBWcTbZ0QB889iPBRrR1HD4TLA9WZ38fQs7YbuKNbz4jxMKDDSh2i JHDK8tzn5P+73CPHkvRfELODn+V5uzUWh6QHJFD+4sMOKvNZfbxlEmt1GANmto0= =PtE1 -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Kill I4L? (was: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors)
On Sun, 2015-02-08 at 20:47 +0100, Tilman Schmidt wrote: Am 07.02.2015 um 21:43 schrieb Paul Bolle: On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: Does anyone still use these cards? 0) Good question. I very much doubt it. It was an ISA card, not even PnP. I'd imagine any systems of that kind would be retired by now. [] I guess somebody could just submit a patch series starting with: +++ b/drivers/isdn/Kconfig @@ -20,25 +20,6 @@ menuconfig ISDN if ISDN -menuconfig ISDN_I4L - tristate Old ISDN4Linux (deprecated) - depends on TTY [] Shall I? I think removing code for completely unused, no longer sold or supported devices could be a good thing. If someone pipes up saying I use that within some shortish time-frame, it's easy to undo. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 2015-02-07 at 21:55 +0100, Bas Peters wrote: > I thought it might have been > useful and a good way to learn Maybe pick a device you have (or maybe buy something in the staging directory like a realtek wireless device) and play with adding support for something in it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
2015-02-07 21:43 GMT+01:00 Paul Bolle : > [Adding Tilman.] > > On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: >> Does anyone still use these cards? > > 0) Good question. > > 1) None of the (two dozen) commits in drivers/isdn/act2000/ added since > v2.6.12 appear to be triggered complaints, suggestions, etc. of actual > users. > > 2) Broader picture: if I remember correctly there are now four different > flavors of ISDN in the kernel: > - really old: pre-i4l > - very old: i4l > - just old: CAPI > - not so old: mISDN > > I could spend another 24 hours refining and relabeling these categories, > and matching the various drivers to these categories. But I'm pretty > sure none of the current categories contain all the drivers. And I'm > certain all current categories support one or more drivers that none of > the other categories do. So the current ISDN situation is a bit messy. > > Tilman might be able to provide a clearer, and maybe less grumpy, > summary of the current situation. > > 3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it > was obsolete before that! > > 4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my > ISP will disconnect my, let's call it, ADSL over ISDN connection in less > that two months because they can't be bothered anymore to support that > niche. > > 5) So we could let the various flavors of ISDN limp along for another > few years. But maybe we should consider, say, removing i4l and pre i4l > and see who complains. That might be a rude thing to do. So perhaps the > various ISDN flavors should be left alone until ... what exactly? > > > Paul Bolle > In that case I will drop this patchset. I thought it might have been useful and a good way to learn but I'm just wasting everyone's time with it if it's that deprecated. Regards, Bas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
[Adding Tilman.] On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: > Does anyone still use these cards? 0) Good question. 1) None of the (two dozen) commits in drivers/isdn/act2000/ added since v2.6.12 appear to be triggered complaints, suggestions, etc. of actual users. 2) Broader picture: if I remember correctly there are now four different flavors of ISDN in the kernel: - really old: pre-i4l - very old: i4l - just old: CAPI - not so old: mISDN I could spend another 24 hours refining and relabeling these categories, and matching the various drivers to these categories. But I'm pretty sure none of the current categories contain all the drivers. And I'm certain all current categories support one or more drivers that none of the other categories do. So the current ISDN situation is a bit messy. Tilman might be able to provide a clearer, and maybe less grumpy, summary of the current situation. 3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it was obsolete before that! 4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my ISP will disconnect my, let's call it, ADSL over ISDN connection in less that two months because they can't be bothered anymore to support that niche. 5) So we could let the various flavors of ISDN limp along for another few years. But maybe we should consider, say, removing i4l and pre i4l and see who complains. That might be a rude thing to do. So perhaps the various ISDN flavors should be left alone until ... what exactly? Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 2015-02-07 at 20:51 +0300, Sergei Shtylyov wrote:
> On 02/07/2015 08:06 PM, Bas Peters wrote:
>
> > This patch fixes the following checkpatch errors:
> > 1. trailing statement
> > 1. assignment of variable in if condition
> > 1. incorrectly placed brace after function definition
I wonder about the usefulness of these changes.
Does anyone still use these cards?
> > diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
[]
> > @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
> > m->hdr.cmd.cmd = c; \
> > m->hdr.cmd.subcmd = s; \
> > m->hdr.msgnum = actcapi_nextsmsg(card); \
> > - } else m = NULL;\
> > + } else
> > + m = NULL; \
>
> Documentation/CodingStyle has an extra rule for such case: *else* branch
> should also have {} since *if* branch has {}.
The macro itself is already poor form.
-
#define ACTCAPI_MKHDR(l, c, s) {\
skb = alloc_skb(l + 8, GFP_ATOMIC); \
if (skb) { \
m = (actcapi_msg *)skb_put(skb, l + 8); \
m->hdr.len = l + 8; \
m->hdr.applicationID = 1; \
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
} else m = NULL;\
}
-
o hidden arguments skb, m and card
o missing do {} while around multi-statement macro
It'd probably be nicer to change the macro to a function
and pass m and card.
This would reduce the object size too.
But maybe any change here is not necessary.
If anything is done, I suggest something like:
---
drivers/isdn/act2000/capi.c | 204 +---
1 file changed, 115 insertions(+), 89 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..a7ecf51 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -104,29 +104,36 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
return 0;
}
-#define ACTCAPI_MKHDR(l, c, s) { \
- skb = alloc_skb(l + 8, GFP_ATOMIC); \
- if (skb) { \
- m = (actcapi_msg *)skb_put(skb, l + 8); \
- m->hdr.len = l + 8; \
- m->hdr.applicationID = 1; \
- m->hdr.cmd.cmd = c; \
- m->hdr.cmd.subcmd = s; \
- m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
- }
+static struct sk_buff *actcapi_mkhdr(act2000_card *card, actcapi_msg **m,
+int len, int cmd, int subcmd)
+{
+ struct sk_buff *skb;
-#define ACTCAPI_CHKSKB if (!skb) { \
- printk(KERN_WARNING "actcapi: alloc_skb failed\n"); \
- return; \
- }
+ len += 8;
-#define ACTCAPI_QUEUE_TX { \
- actcapi_debug_msg(skb, 1); \
- skb_queue_tail(>sndq, skb); \
- act2000_schedule_tx(card); \
+ skb = alloc_skb(len, GFP_ATOMIC);
+ if (!skb) {
+ *m = NULL;
+ return NULL;
}
+ *m = (actcapi_msg *)skb_put(skb, len);
+ (*m)->hdr.len = len;
+ (*m)->hdr.applicationID = 1;
+ (*m)->hdr.cmd.cmd = cmd;
+ (*m)->hdr.cmd.subcmd = subcmd;
+ (*m)->hdr.msgnum = actcapi_nextsmsg(card);
+
+ return skb;
+}
+
+static void actcapi_queue_tx(act2000_card *card, struct sk_buff *skb)
+{
+ actcapi_debug_msg(skb, 1);
+ skb_queue_tail(>sndq, skb);
+ act2000_schedule_tx(card);
+}
+
int
actcapi_listen_req(act2000_card *card)
{
@@ -137,16 +144,15 @@ actcapi_listen_req(act2000_card *card)
for (i = 0; i < ACT2000_BCH; i++)
eazmask |= card->bch[i].eazmask;
- ACTCAPI_MKHDR(9, 0x05, 0x00);
- if (!skb) {
- printk(KERN_WARNING "actcapi: alloc_skb failed\n");
+ skb = actcapi_mkhdr(card, , 9, 0x05, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m->msg.listen_req.controller = 0;
m->msg.listen_req.infomask = 0x3f; /* All information */
m->msg.listen_req.eazmask = eazmask;
m->msg.listen_req.simask = (eazmask)
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On 02/07/2015 09:08 PM, Bas Peters wrote: Please don't top-post. I must have missed something. I fixed it on my end. Do I now resend the entire patchset or just this particular patch? The entire patch set, at least DaveM wants it this way. regards, Bas WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
I must have missed something. I fixed it on my end. Do I now resend
the entire patchset or just this particular patch?
regards,
Bas
2015-02-07 19:02 GMT+01:00 Julia Lawall :
> On Sat, 7 Feb 2015, Sergei Shtylyov wrote:
>
>> Hello.
>>
>> On 02/07/2015 08:06 PM, Bas Peters wrote:
>>
>> > This patch fixes the following checkpatch errors:
>> > 1. trailing statement
>> > 1. assignment of variable in if condition
>> > 1. incorrectly placed brace after function definition
>>
>> > Signed-off-by: Bas Peters
>> > ---
>> > drivers/isdn/act2000/capi.c | 9 ++---
>> > 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> > diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
>> > index 3f66ca2..5d677e6 100644
>> > --- a/drivers/isdn/act2000/capi.c
>> > +++ b/drivers/isdn/act2000/capi.c
>> > @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
>> > m->hdr.cmd.cmd = c; \
>> > m->hdr.cmd.subcmd = s; \
>> > m->hdr.msgnum = actcapi_nextsmsg(card); \
>> > - } else m = NULL;\
>> > + } else
>
> It looks like a \ was lost on this line. It should have caused a compiler
> error.
>
> julia
>
>> > + m = NULL; \
>>
>>Documentation/CodingStyle has an extra rule for such case: *else* branch
>> should also have {} since *if* branch has {}.
>>
>> [...]
>>
>> WNR, Sergei
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 7 Feb 2015, Sergei Shtylyov wrote:
> Hello.
>
> On 02/07/2015 08:06 PM, Bas Peters wrote:
>
> > This patch fixes the following checkpatch errors:
> > 1. trailing statement
> > 1. assignment of variable in if condition
> > 1. incorrectly placed brace after function definition
>
> > Signed-off-by: Bas Peters
> > ---
> > drivers/isdn/act2000/capi.c | 9 ++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
>
> > diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
> > index 3f66ca2..5d677e6 100644
> > --- a/drivers/isdn/act2000/capi.c
> > +++ b/drivers/isdn/act2000/capi.c
> > @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
> > m->hdr.cmd.cmd = c; \
> > m->hdr.cmd.subcmd = s; \
> > m->hdr.msgnum = actcapi_nextsmsg(card); \
> > - } else m = NULL;\
> > + } else
It looks like a \ was lost on this line. It should have caused a compiler
error.
julia
> > + m = NULL; \
>
>Documentation/CodingStyle has an extra rule for such case: *else* branch
> should also have {} since *if* branch has {}.
>
> [...]
>
> WNR, Sergei
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
Thanks, will take that into account in future patches.
With kind regards,
Bas
2015-02-07 18:51 GMT+01:00 Sergei Shtylyov :
> Hello.
>
> On 02/07/2015 08:06 PM, Bas Peters wrote:
>
>> This patch fixes the following checkpatch errors:
>> 1. trailing statement
>> 1. assignment of variable in if condition
>> 1. incorrectly placed brace after function definition
>
>
>> Signed-off-by: Bas Peters
>> ---
>> drivers/isdn/act2000/capi.c | 9 ++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>
>> diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
>> index 3f66ca2..5d677e6 100644
>> --- a/drivers/isdn/act2000/capi.c
>> +++ b/drivers/isdn/act2000/capi.c
>> @@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr
>> *hdr)
>> m->hdr.cmd.cmd = c; \
>> m->hdr.cmd.subcmd = s; \
>> m->hdr.msgnum = actcapi_nextsmsg(card); \
>> - } else m = NULL;\
>> + } else
>> + m = NULL; \
>
>
>Documentation/CodingStyle has an extra rule for such case: *else* branch
> should also have {} since *if* branch has {}.
>
> [...]
>
> WNR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
Hello.
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
[...]
WNR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m->hdr.cmd.cmd = c; \
m->hdr.cmd.subcmd = s; \
m->hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
}
#define ACTCAPI_CHKSKB if (!skb) { \
@@ -563,7 +564,8 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff
*skb) {
blocknr = msg->msg.data_b3_ind.blocknr;
skb_pull(skb, 19);
card->interface.rcvcallb_skb(card->myid, chan, skb);
- if (!(skb = alloc_skb(11, GFP_ATOMIC))) {
+ skb = alloc_skb(11, GFP_ATOMIC);
+ if (!skb) {
printk(KERN_WARNING "actcapi: alloc_skb failed\n");
return 1;
}
@@ -990,7 +992,8 @@ actcapi_debug_dlpd(actcapi_dlpd *dlpd)
}
#ifdef DEBUG_DUMP_SKB
-static void dump_skb(struct sk_buff *skb) {
+static void dump_skb(struct sk_buff *skb)
+{
char tmp[80];
char *p = skb->data;
char *t = tmp;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
Hello.
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters baspeter...@gmail.com
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
[...]
WNR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 7 Feb 2015, Sergei Shtylyov wrote:
Hello.
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters baspeter...@gmail.com
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
It looks like a \ was lost on this line. It should have caused a compiler
error.
julia
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
[...]
WNR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
Thanks, will take that into account in future patches.
With kind regards,
Bas
2015-02-07 18:51 GMT+01:00 Sergei Shtylyov sergei.shtyl...@cogentembedded.com:
Hello.
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters baspeter...@gmail.com
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr
*hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
[...]
WNR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
I must have missed something. I fixed it on my end. Do I now resend
the entire patchset or just this particular patch?
regards,
Bas
2015-02-07 19:02 GMT+01:00 Julia Lawall julia.law...@lip6.fr:
On Sat, 7 Feb 2015, Sergei Shtylyov wrote:
Hello.
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters baspeter...@gmail.com
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
It looks like a \ was lost on this line. It should have caused a compiler
error.
julia
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
[...]
WNR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On 02/07/2015 09:08 PM, Bas Peters wrote: Please don't top-post. I must have missed something. I fixed it on my end. Do I now resend the entire patchset or just this particular patch? The entire patch set, at least DaveM wants it this way. regards, Bas WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
Signed-off-by: Bas Peters baspeter...@gmail.com
---
drivers/isdn/act2000/capi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..5d677e6 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
}
#define ACTCAPI_CHKSKB if (!skb) { \
@@ -563,7 +564,8 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff
*skb) {
blocknr = msg-msg.data_b3_ind.blocknr;
skb_pull(skb, 19);
card-interface.rcvcallb_skb(card-myid, chan, skb);
- if (!(skb = alloc_skb(11, GFP_ATOMIC))) {
+ skb = alloc_skb(11, GFP_ATOMIC);
+ if (!skb) {
printk(KERN_WARNING actcapi: alloc_skb failed\n);
return 1;
}
@@ -990,7 +992,8 @@ actcapi_debug_dlpd(actcapi_dlpd *dlpd)
}
#ifdef DEBUG_DUMP_SKB
-static void dump_skb(struct sk_buff *skb) {
+static void dump_skb(struct sk_buff *skb)
+{
char tmp[80];
char *p = skb-data;
char *t = tmp;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 2015-02-07 at 20:51 +0300, Sergei Shtylyov wrote:
On 02/07/2015 08:06 PM, Bas Peters wrote:
This patch fixes the following checkpatch errors:
1. trailing statement
1. assignment of variable in if condition
1. incorrectly placed brace after function definition
I wonder about the usefulness of these changes.
Does anyone still use these cards?
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
[]
@@ -113,7 +113,8 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
+ } else
+ m = NULL; \
Documentation/CodingStyle has an extra rule for such case: *else* branch
should also have {} since *if* branch has {}.
The macro itself is already poor form.
-
#define ACTCAPI_MKHDR(l, c, s) {\
skb = alloc_skb(l + 8, GFP_ATOMIC); \
if (skb) { \
m = (actcapi_msg *)skb_put(skb, l + 8); \
m-hdr.len = l + 8; \
m-hdr.applicationID = 1; \
m-hdr.cmd.cmd = c; \
m-hdr.cmd.subcmd = s; \
m-hdr.msgnum = actcapi_nextsmsg(card); \
} else m = NULL;\
}
-
o hidden arguments skb, m and card
o missing do {} while around multi-statement macro
It'd probably be nicer to change the macro to a function
and pass m and card.
This would reduce the object size too.
But maybe any change here is not necessary.
If anything is done, I suggest something like:
---
drivers/isdn/act2000/capi.c | 204 +---
1 file changed, 115 insertions(+), 89 deletions(-)
diff --git a/drivers/isdn/act2000/capi.c b/drivers/isdn/act2000/capi.c
index 3f66ca2..a7ecf51 100644
--- a/drivers/isdn/act2000/capi.c
+++ b/drivers/isdn/act2000/capi.c
@@ -104,29 +104,36 @@ actcapi_chkhdr(act2000_card *card, actcapi_msghdr *hdr)
return 0;
}
-#define ACTCAPI_MKHDR(l, c, s) { \
- skb = alloc_skb(l + 8, GFP_ATOMIC); \
- if (skb) { \
- m = (actcapi_msg *)skb_put(skb, l + 8); \
- m-hdr.len = l + 8; \
- m-hdr.applicationID = 1; \
- m-hdr.cmd.cmd = c; \
- m-hdr.cmd.subcmd = s; \
- m-hdr.msgnum = actcapi_nextsmsg(card); \
- } else m = NULL;\
- }
+static struct sk_buff *actcapi_mkhdr(act2000_card *card, actcapi_msg **m,
+int len, int cmd, int subcmd)
+{
+ struct sk_buff *skb;
-#define ACTCAPI_CHKSKB if (!skb) { \
- printk(KERN_WARNING actcapi: alloc_skb failed\n); \
- return; \
- }
+ len += 8;
-#define ACTCAPI_QUEUE_TX { \
- actcapi_debug_msg(skb, 1); \
- skb_queue_tail(card-sndq, skb); \
- act2000_schedule_tx(card); \
+ skb = alloc_skb(len, GFP_ATOMIC);
+ if (!skb) {
+ *m = NULL;
+ return NULL;
}
+ *m = (actcapi_msg *)skb_put(skb, len);
+ (*m)-hdr.len = len;
+ (*m)-hdr.applicationID = 1;
+ (*m)-hdr.cmd.cmd = cmd;
+ (*m)-hdr.cmd.subcmd = subcmd;
+ (*m)-hdr.msgnum = actcapi_nextsmsg(card);
+
+ return skb;
+}
+
+static void actcapi_queue_tx(act2000_card *card, struct sk_buff *skb)
+{
+ actcapi_debug_msg(skb, 1);
+ skb_queue_tail(card-sndq, skb);
+ act2000_schedule_tx(card);
+}
+
int
actcapi_listen_req(act2000_card *card)
{
@@ -137,16 +144,15 @@ actcapi_listen_req(act2000_card *card)
for (i = 0; i ACT2000_BCH; i++)
eazmask |= card-bch[i].eazmask;
- ACTCAPI_MKHDR(9, 0x05, 0x00);
- if (!skb) {
- printk(KERN_WARNING actcapi: alloc_skb failed\n);
+ skb = actcapi_mkhdr(card, m, 9, 0x05, 0x00);
+ if (!skb)
return -ENOMEM;
- }
+
m-msg.listen_req.controller = 0;
m-msg.listen_req.infomask = 0x3f; /* All information */
m-msg.listen_req.eazmask = eazmask;
m-msg.listen_req.simask = (eazmask) ? 0x86 : 0; /* All SI's */
-
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
[Adding Tilman.] On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: Does anyone still use these cards? 0) Good question. 1) None of the (two dozen) commits in drivers/isdn/act2000/ added since v2.6.12 appear to be triggered complaints, suggestions, etc. of actual users. 2) Broader picture: if I remember correctly there are now four different flavors of ISDN in the kernel: - really old: pre-i4l - very old: i4l - just old: CAPI - not so old: mISDN I could spend another 24 hours refining and relabeling these categories, and matching the various drivers to these categories. But I'm pretty sure none of the current categories contain all the drivers. And I'm certain all current categories support one or more drivers that none of the other categories do. So the current ISDN situation is a bit messy. Tilman might be able to provide a clearer, and maybe less grumpy, summary of the current situation. 3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it was obsolete before that! 4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my ISP will disconnect my, let's call it, ADSL over ISDN connection in less that two months because they can't be bothered anymore to support that niche. 5) So we could let the various flavors of ISDN limp along for another few years. But maybe we should consider, say, removing i4l and pre i4l and see who complains. That might be a rude thing to do. So perhaps the various ISDN flavors should be left alone until ... what exactly? Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
2015-02-07 21:43 GMT+01:00 Paul Bolle pebo...@tiscali.nl: [Adding Tilman.] On Sat, 2015-02-07 at 11:19 -0800, Joe Perches wrote: Does anyone still use these cards? 0) Good question. 1) None of the (two dozen) commits in drivers/isdn/act2000/ added since v2.6.12 appear to be triggered complaints, suggestions, etc. of actual users. 2) Broader picture: if I remember correctly there are now four different flavors of ISDN in the kernel: - really old: pre-i4l - very old: i4l - just old: CAPI - not so old: mISDN I could spend another 24 hours refining and relabeling these categories, and matching the various drivers to these categories. But I'm pretty sure none of the current categories contain all the drivers. And I'm certain all current categories support one or more drivers that none of the other categories do. So the current ISDN situation is a bit messy. Tilman might be able to provide a clearer, and maybe less grumpy, summary of the current situation. 3) Anyhow, this is an i4l card. i4l is deprecated since v2.6.22. And it was obsolete before that! 4) Furthermore, it seems consumer grade ISDN is on the way out. Eg, my ISP will disconnect my, let's call it, ADSL over ISDN connection in less that two months because they can't be bothered anymore to support that niche. 5) So we could let the various flavors of ISDN limp along for another few years. But maybe we should consider, say, removing i4l and pre i4l and see who complains. That might be a rude thing to do. So perhaps the various ISDN flavors should be left alone until ... what exactly? Paul Bolle In that case I will drop this patchset. I thought it might have been useful and a good way to learn but I'm just wasting everyone's time with it if it's that deprecated. Regards, Bas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] drivers: isdn: act2000: capi.c: fix checkpatch errors
On Sat, 2015-02-07 at 21:55 +0100, Bas Peters wrote: I thought it might have been useful and a good way to learn Maybe pick a device you have (or maybe buy something in the staging directory like a realtek wireless device) and play with adding support for something in it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
