[PATCH 0/4] isdn: patches for 2.6.31 (v2)

[PATCH 0/4] isdn: patches for 2.6.31 (v2)

[Tilman Schmidt]

Dave, Karsten,

this is the revised version of my patches to the Kernel CAPI subsystem
I'd like to propose for inclusion in kernel release 2.6.31.

The first patch of the series, renaming capi_ctr_reseted() to
capi_ctr_down(), is unchanged.

The patch to drivers/isdn/capi/capiutil.c has been split in two, one
adding the kerneldoc comments and the other one, the NULL pointer
check.

The last patch to Documentation/isdn/INTERFACE.CAPI has grown another
capiutil function description, capi_cmd2str().

Thanks,
Tilman

[PATCH 2/4] isdn: kerneldoc for capiutil.c

[Tilman Schmidt]

Add kerneldoc comments for the exported funtions in capiutil.c.

Impact: documentation
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/capi/capiutil.c |   65 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index 29419a8..c7c2902 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -490,7 +490,14 @@ static void pars_2_message(_cmsg * cmsg)
 	}
 }
 
-/*-------------------------------------------------------*/
+/**
+ * capi_cmsg2message() - assemble CAPI 2.0 message from _cmsg structure
+ * @cmsg:	_cmsg structure
+ * @msg:	buffer for assembled message
+ *
+ * Return value: 0 for success
+ */
+
 unsigned capi_cmsg2message(_cmsg * cmsg, u8 * msg)
 {
 	cmsg->m = msg;
@@ -553,7 +560,14 @@ static void message_2_pars(_cmsg * cmsg)
 	}
 }
 
-/*-------------------------------------------------------*/
+/**
+ * capi_message2cmsg() - disassemble CAPI 2.0 message into _cmsg structure
+ * @cmsg:	_cmsg structure
+ * @msg:	buffer for assembled message
+ *
+ * Return value: 0 for success
+ */
+
 unsigned capi_message2cmsg(_cmsg * cmsg, u8 * msg)
 {
 	memset(cmsg, 0, sizeof(_cmsg));
@@ -573,7 +587,18 @@ unsigned capi_message2cmsg(_cmsg * cmsg, u8 * msg)
 	return 0;
 }
 
-/*-------------------------------------------------------*/
+/**
+ * capi_cmsg_header() - initialize header part of _cmsg structure
+ * @cmsg:	_cmsg structure
+ * @_ApplId:	ApplID field value
+ * @_Command:	Command field value
+ * @_Subcommand:	Subcommand field value
+ * @_Messagenumber:	Message Number field value
+ * @_Controller:	Controller/PLCI/NCCI field value
+ *
+ * Return value: 0 for success
+ */
+
 unsigned capi_cmsg_header(_cmsg * cmsg, u16 _ApplId,
 			  u8 _Command, u8 _Subcommand,
 			  u16 _Messagenumber, u32 _Controller)
@@ -641,6 +666,14 @@ static char *mnames[] =
 	[0x4e] = "MANUFACTURER_RESP"
 };
 
+/**
+ * capi_cmd2str() - convert CAPI 2.0 command/subcommand number to name
+ * @cmd:	command number
+ * @subcmd:	subcommand number
+ *
+ * Return value: static string, NULL if command/subcommand unknown
+ */
+
 char *capi_cmd2str(u8 cmd, u8 subcmd)
 {
 	return mnames[command_2_index(cmd, subcmd)];
@@ -879,6 +912,11 @@ init:
 	return cdb;
 }
 
+/**
+ * cdebbuf_free() - free CAPI debug buffer
+ * @cdb:	buffer to free
+ */
+
 void cdebbuf_free(_cdebbuf *cdb)
 {
 	if (likely(cdb == g_debbuf)) {
@@ -891,6 +929,16 @@ void cdebbuf_free(_cdebbuf *cdb)
 }
 
 
+/**
+ * capi_message2str() - format CAPI 2.0 message for printing
+ * @msg:	CAPI 2.0 message
+ *
+ * Allocates a CAPI debug buffer and fills it with a printable representation
+ * of the CAPI 2.0 message in @msg.
+ * Return value: allocated debug buffer, NULL on error
+ * The returned buffer should be freed by a call to cdebbuf_free() after use.
+ */
+
 _cdebbuf *capi_message2str(u8 * msg)
 {
 	_cdebbuf *cdb;
@@ -926,6 +974,17 @@ _cdebbuf *capi_message2str(u8 * msg)
 	return cdb;
 }
 
+/**
+ * capi_cmsg2str() - format _cmsg structure for printing
+ * @cmsg:	_cmsg structure
+ *
+ * Allocates a CAPI debug buffer and fills it with a printable representation
+ * of the CAPI 2.0 message stored in @cmsg by a previous call to
+ * capi_cmsg2message() or capi_message2cmsg().
+ * Return value: allocated debug buffer, NULL on error
+ * The returned buffer should be freed by a call to cdebbuf_free() after use.
+ */
+
 _cdebbuf *capi_cmsg2str(_cmsg * cmsg)
 {
 	_cdebbuf *cdb;
-- 
1.6.2.1.214.ge986c

[PATCH 1/4] isdn: rename capi_ctr_reseted() to capi_ctr_down()

[Tilman Schmidt]

Change the name of the Kernel CAPI exported function capi_ctr_reseted()
to something representing its purpose better.

Impact: renaming, no functional change
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 Documentation/isdn/INTERFACE.CAPI |    4 ++--
 drivers/isdn/capi/kcapi.c         |    8 ++++----
 drivers/isdn/hardware/avm/b1.c    |    2 +-
 drivers/isdn/hardware/avm/b1dma.c |    2 +-
 drivers/isdn/hardware/avm/c4.c    |    4 ++--
 drivers/isdn/hardware/avm/t1isa.c |    2 +-
 drivers/isdn/hysdn/hycapi.c       |    4 ++--
 include/linux/isdn/capilli.h      |    2 +-
 net/bluetooth/cmtp/capi.c         |    2 +-
 9 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/isdn/INTERFACE.CAPI b/Documentation/isdn/INTERFACE.CAPI
index 786d619..4631538 100644
--- a/Documentation/isdn/INTERFACE.CAPI
+++ b/Documentation/isdn/INTERFACE.CAPI
@@ -45,7 +45,7 @@ From then on, Kernel CAPI may call the registered callback functions for the
 device.
 
 If the device becomes unusable for any reason (shutdown, disconnect ...), the
-driver has to call capi_ctr_reseted(). This will prevent further calls to the
+driver has to call capi_ctr_down(). This will prevent further calls to the
 callback functions by Kernel CAPI.
 
 
@@ -166,7 +166,7 @@ int detach_capi_ctr(struct capi_ctr *ctrlr)
 	register/unregister a device (controller) with Kernel CAPI
 
 void capi_ctr_ready(struct capi_ctr *ctrlr)
-void capi_ctr_reseted(struct capi_ctr *ctrlr)
+void capi_ctr_down(struct capi_ctr *ctrlr)
 	signal controller ready/not ready
 
 void capi_ctr_suspend_output(struct capi_ctr *ctrlr)
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index f331703..57d2636 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -377,14 +377,14 @@ void capi_ctr_ready(struct capi_ctr * card)
 EXPORT_SYMBOL(capi_ctr_ready);
 
 /**
- * capi_ctr_reseted() - signal CAPI controller reset
+ * capi_ctr_down() - signal CAPI controller not ready
  * @card:	controller descriptor structure.
  *
  * Called by hardware driver to signal that the controller is down and
  * unavailable for use.
  */
 
-void capi_ctr_reseted(struct capi_ctr * card)
+void capi_ctr_down(struct capi_ctr * card)
 {
 	u16 appl;
 
@@ -413,7 +413,7 @@ void capi_ctr_reseted(struct capi_ctr * card)
 	notify_push(KCI_CONTRDOWN, card->cnr, 0, 0);
 }
 
-EXPORT_SYMBOL(capi_ctr_reseted);
+EXPORT_SYMBOL(capi_ctr_down);
 
 /**
  * capi_ctr_suspend_output() - suspend controller
@@ -517,7 +517,7 @@ EXPORT_SYMBOL(attach_capi_ctr);
 int detach_capi_ctr(struct capi_ctr *card)
 {
         if (card->cardstate != CARD_DETECTED)
-		capi_ctr_reseted(card);
+		capi_ctr_down(card);
 
 	ncards--;
 
diff --git a/drivers/isdn/hardware/avm/b1.c b/drivers/isdn/hardware/avm/b1.c
index abf05ec..a7c0083 100644
--- a/drivers/isdn/hardware/avm/b1.c
+++ b/drivers/isdn/hardware/avm/b1.c
@@ -330,7 +330,7 @@ void b1_reset_ctr(struct capi_ctr *ctrl)
 	spin_lock_irqsave(&card->lock, flags);
 	capilib_release(&cinfo->ncci_head);
 	spin_unlock_irqrestore(&card->lock, flags);
-	capi_ctr_reseted(ctrl);
+	capi_ctr_down(ctrl);
 }
 
 void b1_register_appl(struct capi_ctr *ctrl,
diff --git a/drivers/isdn/hardware/avm/b1dma.c b/drivers/isdn/hardware/avm/b1dma.c
index da34b98..0e84aaa 100644
--- a/drivers/isdn/hardware/avm/b1dma.c
+++ b/drivers/isdn/hardware/avm/b1dma.c
@@ -759,7 +759,7 @@ void b1dma_reset_ctr(struct capi_ctr *ctrl)
 	memset(cinfo->version, 0, sizeof(cinfo->version));
 	capilib_release(&cinfo->ncci_head);
 	spin_unlock_irqrestore(&card->lock, flags);
-	capi_ctr_reseted(ctrl);
+	capi_ctr_down(ctrl);
 }
 
 /* ------------------------------------------------------------- */
diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
index 9df1d3f..6833301 100644
--- a/drivers/isdn/hardware/avm/c4.c
+++ b/drivers/isdn/hardware/avm/c4.c
@@ -681,7 +681,7 @@ static irqreturn_t c4_handle_interrupt(avmcard *card)
 			spin_lock_irqsave(&card->lock, flags);
 			capilib_release(&cinfo->ncci_head);
 			spin_unlock_irqrestore(&card->lock, flags);
-			capi_ctr_reseted(&cinfo->capi_ctrl);
+			capi_ctr_down(&cinfo->capi_ctrl);
 		}
 		card->nlogcontr = 0;
 		return IRQ_HANDLED;
@@ -909,7 +909,7 @@ static void c4_reset_ctr(struct capi_ctr *ctrl)
         for (i=0; i < card->nr_controllers; i++) {
 		cinfo = &card->ctrlinfo[i];
 		memset(cinfo->version, 0, sizeof(cinfo->version));
-		capi_ctr_reseted(&cinfo->capi_ctrl);
+		capi_ctr_down(&cinfo->capi_ctrl);
 	}
 	card->nlogcontr = 0;
 }
diff --git a/drivers/isdn/hardware/avm/t1isa.c b/drivers/isdn/hardware/avm/t1isa.c
index e772449..1c53fd4 100644
--- a/drivers/isdn/hardware/avm/t1isa.c
+++ b/drivers/isdn/hardware/avm/t1isa.c
@@ -339,7 +339,7 @@ static void t1isa_reset_ctr(struct capi_ctr *ctrl)
 	spin_lock_irqsave(&card->lock, flags);
 	capilib_release(&cinfo->ncci_head);
 	spin_unlock_irqrestore(&card->lock, flags);
-	capi_ctr_reseted(ctrl);
+	capi_ctr_down(ctrl);
 }
 
 static void t1isa_remove(struct pci_dev *pdev)
diff --git a/drivers/isdn/hysdn/hycapi.c b/drivers/isdn/hysdn/hycapi.c
index 53f6ad1..4ffaa14 100644
--- a/drivers/isdn/hysdn/hycapi.c
+++ b/drivers/isdn/hysdn/hycapi.c
@@ -67,7 +67,7 @@ hycapi_reset_ctr(struct capi_ctr *ctrl)
 	printk(KERN_NOTICE "HYCAPI hycapi_reset_ctr\n");
 #endif
 	capilib_release(&cinfo->ncci_head);
-	capi_ctr_reseted(ctrl);
+	capi_ctr_down(ctrl);
 }
 
 /******************************
@@ -347,7 +347,7 @@ int hycapi_capi_stop(hysdn_card *card)
 	if(cinfo) {
 		ctrl = &cinfo->capi_ctrl;
 /*		ctrl->suspend_output(ctrl); */
-		capi_ctr_reseted(ctrl);
+		capi_ctr_down(ctrl);
 	}
 	return 0;
 }
diff --git a/include/linux/isdn/capilli.h b/include/linux/isdn/capilli.h
index 35e9b0f..7acb87a 100644
--- a/include/linux/isdn/capilli.h
+++ b/include/linux/isdn/capilli.h
@@ -79,7 +79,7 @@ int attach_capi_ctr(struct capi_ctr *);
 int detach_capi_ctr(struct capi_ctr *);
 
 void capi_ctr_ready(struct capi_ctr * card);
-void capi_ctr_reseted(struct capi_ctr * card);
+void capi_ctr_down(struct capi_ctr * card);
 void capi_ctr_suspend_output(struct capi_ctr * card);
 void capi_ctr_resume_output(struct capi_ctr * card);
 void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *skb);
diff --git a/net/bluetooth/cmtp/capi.c b/net/bluetooth/cmtp/capi.c
index 78958c0..97f8d68 100644
--- a/net/bluetooth/cmtp/capi.c
+++ b/net/bluetooth/cmtp/capi.c
@@ -382,7 +382,7 @@ static void cmtp_reset_ctr(struct capi_ctr *ctrl)
 
 	BT_DBG("ctrl %p", ctrl);
 
-	capi_ctr_reseted(ctrl);
+	capi_ctr_down(ctrl);
 
 	atomic_inc(&session->terminate);
 	cmtp_schedule(session);
-- 
1.6.2.1.214.ge986c

[PATCH 4/4] isdn: extend INTERFACE.CAPI document

[Tilman Schmidt]

Clarify calling context and return codes of callback methods, and
add a description of the _cmsg structure and helper functions.

Impact: documentation
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 Documentation/isdn/INTERFACE.CAPI |   90 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/Documentation/isdn/INTERFACE.CAPI b/Documentation/isdn/INTERFACE.CAPI
index 4631538..686e107 100644
--- a/Documentation/isdn/INTERFACE.CAPI
+++ b/Documentation/isdn/INTERFACE.CAPI
@@ -114,20 +114,36 @@ char *driver_name
 int (*load_firmware)(struct capi_ctr *ctrlr, capiloaddata *ldata)
 	(optional) pointer to a callback function for sending firmware and
 	configuration data to the device
+	Return value: 0 on success, error code on error
+	Called in process context.
 
 void (*reset_ctr)(struct capi_ctr *ctrlr)
-	pointer to a callback function for performing a reset on the device,
-	releasing all registered applications
+	(optional) pointer to a callback function for performing a reset on
+	the device, releasing all registered applications
+	Called in process context.
 
 void (*register_appl)(struct capi_ctr *ctrlr, u16 applid,
 			capi_register_params *rparam)
 void (*release_appl)(struct capi_ctr *ctrlr, u16 applid)
 	pointers to callback functions for registration and deregistration of
 	applications with the device
+	Calls to these functions are serialized by Kernel CAPI so that only
+	one call to any of them is active at any time.
 
 u16  (*send_message)(struct capi_ctr *ctrlr, struct sk_buff *skb)
 	pointer to a callback function for sending a CAPI message to the
 	device
+	Return value: CAPI error code
+	If the method returns 0 (CAPI_NOERROR) the driver has taken ownership
+	of the skb and the caller may no longer access it. If it returns a
+	non-zero (error) value then ownership of the skb returns to the caller
+	who may reuse or free it.
+	The return value should only be used to signal problems with respect
+	to accepting or queueing the message. Errors occurring during the
+	actual processing of the message should be signaled with an
+	appropriate reply message.
+	Calls to this function are not serialized by Kernel CAPI, ie. it must
+	be prepared to be re-entered.
 
 char *(*procinfo)(struct capi_ctr *ctrlr)
 	pointer to a callback function returning the entry for the device in
@@ -138,6 +154,8 @@ read_proc_t *ctr_read_proc
 	system entry, /proc/capi/controllers/<n>; will be called with a
 	pointer to the device's capi_ctr structure as the last (data) argument
 
+Note: Callback functions are never called in interrupt context.
+
 - to be filled in before calling capi_ctr_ready():
 
 u8 manu[CAPI_MANUFACTURER_LEN]
@@ -153,6 +171,45 @@ u8 serial[CAPI_SERIAL_LEN]
 	value to return for CAPI_GET_SERIAL
 
 
+4.3 The _cmsg Structure
+
+(declared in <linux/isdn/capiutil.h>)
+
+The _cmsg structure stores the contents of a CAPI 2.0 message in an easily
+accessible form. It contains members for all possible CAPI 2.0 parameters, of
+which only those appearing in the message type currently being processed are
+actually used. Unused members should be set to zero.
+
+Members are named after the CAPI 2.0 standard names of the parameters they
+represent. See <linux/isdn/capiutil.h> for the exact spelling. Member data
+types are:
+
+u8          for CAPI parameters of type 'byte'
+
+u16         for CAPI parameters of type 'word'
+
+u32         for CAPI parameters of type 'dword'
+
+_cstruct    for CAPI parameters of type 'struct' not containing any
+	    variably-sized (struct) subparameters (eg. 'Called Party Number')
+	    The member is a pointer to a buffer containing the parameter in
+	    CAPI encoding (length + content). It may also be NULL, which will
+	    be taken to represent an empty (zero length) parameter.
+
+_cmstruct   for CAPI parameters of type 'struct' containing 'struct'
+	    subparameters ('Additional Info' and 'B Protocol')
+	    The representation is a single byte containing one of the values:
+	    CAPI_DEFAULT: the parameter is empty
+	    CAPI_COMPOSE: the values of the subparameters are stored
+	    individually in the corresponding _cmsg structure members
+
+Functions capi_cmsg2message() and capi_message2cmsg() are provided to convert
+messages between their transport encoding described in the CAPI 2.0 standard
+and their _cmsg structure representation. Note that capi_cmsg2message() does
+not know or check the size of its destination buffer. The caller must make
+sure it is big enough to accomodate the resulting CAPI message.
+
+
 5. Lower Layer Interface Functions
 
 (declared in <linux/isdn/capilli.h>)
@@ -211,3 +268,32 @@ CAPIMSG_CONTROL(m)	CAPIMSG_SETCONTROL(m, contr)	Controller/PLCI/NCCI
 							(u32)
 CAPIMSG_DATALEN(m)	CAPIMSG_SETDATALEN(m, len)	Data Length (u16)
 
+
+Library functions for working with _cmsg structures
+(from <linux/isdn/capiutil.h>):
+
+unsigned capi_cmsg2message(_cmsg *cmsg, u8 *msg)
+	Assembles a CAPI 2.0 message from the parameters in *cmsg, storing the
+	result in *msg.
+
+unsigned capi_message2cmsg(_cmsg *cmsg, u8 *msg)
+	Disassembles the CAPI 2.0 message in *msg, storing the parameters in
+	*cmsg.
+
+unsigned capi_cmsg_header(_cmsg *cmsg, u16 ApplId, u8 Command, u8 Subcommand,
+			  u16 Messagenumber, u32 Controller)
+	Fills the header part and address field of the _cmsg structure *cmsg
+	with the given values, zeroing the remainder of the structure so only
+	parameters with non-default values need to be changed before sending
+	the message.
+
+void capi_cmsg_answer(_cmsg *cmsg)
+	Sets the low bit of the Subcommand field in *cmsg, thereby converting
+	_REQ to _CONF and _IND to _RESP.
+
+char *capi_cmd2str(u8 Command, u8 Subcommand)
+	Returns the CAPI 2.0 message name corresponding to the given command
+	and subcommand values, as a static ASCII string. The return value may
+	be NULL if the command/subcommand is not one of those defined in the
+	CAPI 2.0 standard.
+
-- 
1.6.2.1.214.ge986c

[PATCH 3/4] isdn: prevent NULL ptr Oops in capi_cmsg2str()

[Tilman Schmidt]

The dereferencing of the private pointer cmsg->m in capi_cmsg2str() may
cause an Oops in case of an error, which is particularly inconvenient
as that function is typically used to format an error message. Add a
NULL pointer check to avoid this.

Impact: error handling improvement
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/capi/capiutil.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index c7c2902..16f2e46 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -989,6 +989,8 @@ _cdebbuf *capi_cmsg2str(_cmsg * cmsg)
 {
 	_cdebbuf *cdb;
 
+	if (!cmsg->m)
+		return NULL;	/* no message */
 	cdb = cdebbuf_alloc();
 	if (!cdb)
 		return NULL;
-- 
1.6.2.1.214.ge986c

Re: [PATCH 0/4] isdn: patches for 2.6.31 (v2)

[David Miller]

From: Tilman Schmidt <tilman@imap.cc>
Date: Sun,  7 Jun 2009 21:09:23 +0200 (CEST)

> Dave, Karsten,
> 
> this is the revised version of my patches to the Kernel CAPI subsystem
> I'd like to propose for inclusion in kernel release 2.6.31.
> 
> The first patch of the series, renaming capi_ctr_reseted() to
> capi_ctr_down(), is unchanged.
> 
> The patch to drivers/isdn/capi/capiutil.c has been split in two, one
> adding the kerneldoc comments and the other one, the NULL pointer
> check.
> 
> The last patch to Documentation/isdn/INTERFACE.CAPI has grown another
> capiutil function description, capi_cmd2str().

This all seems very reasonable to me, applied to net-next-2.6

Thanks!