[PATCH 00/06] staging: dgap: Digi International dgap driver

[PATCH 00/06] staging: dgap: Digi International dgap driver

[Mark Hounschell]

patch 1: remove some extra empty lines not previously caught.
patch 2: the firmware loading provess loads the config file
         for each board found. It only needs to load it once.
patch 3: fixes a checkpatch warning about a macro needing parenthesis
patch 4: removes volatiles from dgap.h as recommended by checkpatch
patch 5: fixes/changes a pr_info because it was wrong
patch 6: fixes a checkpatch warning about adding typedefs in dgap.h

-- 
1.8.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/06] staging: dgap: Removes excessive empty lines from dgap.h

[Mark Hounschell]

This patch removes excessive empty lines from dgap.h
not previously caught.

Signed-off-by: Mark Hounschell <markh@compro.net>
---
 drivers/staging/dgap/dgap.h | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 979ad40..6415680 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -282,14 +282,12 @@ typedef unsigned char		uchar;
 #define SPINTFC		0xfc		/* Reserved			*/
 #define SCOMMODE	0xfd		/* Set RS232/422 mode		*/
 
-
 /************************************************************************
  *	Modes for SCOMMODE
  ************************************************************************/
 #define MODE_232	0x00
 #define MODE_422	0x01
 
-
 /************************************************************************
  *      Event flags.
  ************************************************************************/
@@ -494,8 +492,6 @@ enum {
 	REQUESTED_CONCENTRATOR
 };
 
-
-
 /*
  * Modem line constants are defined as macros because DSR and
  * DCD are swapable using the ditty altpin option.
@@ -507,14 +503,12 @@ enum {
 #define D_RI(ch)        DM_RI           /* Ring indicator       */
 #define D_DTR(ch)       DM_DTR          /* Data terminal ready  */
 
-
 /*************************************************************************
  *
  * Structures and closely related defines.
  *
  *************************************************************************/
 
-
 /*
  * A structure to hold a statistics counter.  We also
  * compute moving averages for this counter.
@@ -526,7 +520,6 @@ struct macounter {
 	ulong		ema;	/* Exponential moving average */
 };
 
-
 /************************************************************************
  * Device flag definitions for bd_flags.
  ************************************************************************/
@@ -620,8 +613,6 @@ struct board_t {
 					/* download                       */
 };
 
-
-
 /************************************************************************
  * Unit flag definitions for un_flags.
  ************************************************************************/
@@ -659,7 +650,6 @@ struct un_t {
 	struct device *un_sysfs;
 };
 
-
 /************************************************************************
  * Device flag definitions for ch_flags.
  ************************************************************************/
@@ -686,7 +676,6 @@ struct un_t {
 #define SNIFF_WAIT_DATA	0x2
 #define SNIFF_WAIT_SPACE 0x4
 
-
 /************************************************************************
  ***	Definitions for Digi ditty(1) command.
  ************************************************************************/
@@ -727,7 +716,6 @@ struct un_t {
 #define	TIOCMBIS	(('d'<<8) | 255)	/* set modem ctrl state */
 #endif
 
-
 #if !defined(TIOCSDTR)
 #define	TIOCSDTR	(('e'<<8) | 0)		/* set DTR		*/
 #define	TIOCCDTR	(('e'<<8) | 1)		/* clear DTR		*/
@@ -762,7 +750,6 @@ struct	digiflow_t {
 	unsigned char	stopc;			/* flow cntl stop char	*/
 };
 
-
 #ifdef	FLOW_2200
 #define	F2200_GETA	(('e'<<8) | 104)	/* Get 2x36 flow cntl flags */
 #define	F2200_SETAW	(('e'<<8) | 105)	/* Set 2x36 flow cntl flags */
@@ -1004,7 +991,6 @@ struct channel_t {
 	u32 ch_open_count;		/* open count			*/
 	u32	ch_flags;		/* Channel flags                */
 
-
 	u32	ch_close_delay;		/* How long we should drop      */
 					/* RTS/DTR for                  */
 
@@ -1256,29 +1242,17 @@ struct cnode {
 		} module;
 
 		char *ttyname;
-
 		char *cuname;
-
 		char *printname;
-
 		long majornumber;
-
 		long altpin;
-
 		long ttysize;
-
 		long chsize;
-
 		long bssize;
-
 		long unsize;
-
 		long f2size;
-
 		long vpixsize;
-
 		long useintr;
 	} u;
 };
-
 #endif
-- 
1.8.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/06] staging: dgap: Only read config file dgap.conf once

[Mark Hounschell]

The config file is currently read for each board found.
It only needs to be read one time. The buffer it is read
into can now be freed immediately after it is parsed
instead of at driver unload time.

Signed-off-by: Mark Hounschell <markh@compro.net>
---
 drivers/staging/dgap/dgap.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index ea920e4..07fcb8d 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -620,8 +620,6 @@ static void dgap_cleanup_module(void)
 	class_destroy(dgap_class);
 	unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
 
-	kfree(dgap_config_buf);
-
 	for (i = 0; i < dgap_NumBoards; ++i) {
 		dgap_remove_ports_sysfiles(dgap_Board[i]);
 		dgap_tty_uninit(dgap_Board[i]);
@@ -828,7 +826,7 @@ static int dgap_firmware_load(struct pci_dev *pdev, int card_type)
 	dgap_get_vpd(brd);
 	dgap_do_reset_board(brd);
 
-	if (fw_info[card_type].conf_name) {
+	if ((fw_info[card_type].conf_name) && !dgap_config_buf) {
 		ret = request_firmware(&fw, fw_info[card_type].conf_name,
 					 &pdev->dev);
 		if (ret) {
@@ -836,20 +834,22 @@ static int dgap_firmware_load(struct pci_dev *pdev, int card_type)
 				fw_info[card_type].conf_name);
 			return ret;
 		}
+
+		dgap_config_buf = kmalloc(fw->size + 1, GFP_KERNEL);
 		if (!dgap_config_buf) {
-			dgap_config_buf = kmalloc(fw->size + 1, GFP_ATOMIC);
-			if (!dgap_config_buf) {
-				release_firmware(fw);
-				return -ENOMEM;
-			}
+			release_firmware(fw);
+			return -ENOMEM;
 		}
 
 		memcpy(dgap_config_buf, fw->data, fw->size);
 		release_firmware(fw);
 		dgap_config_buf[fw->size + 1] = '\0';
 
-		if (dgap_parsefile(&dgap_config_buf, TRUE) != 0)
+		if (dgap_parsefile(&dgap_config_buf, TRUE) != 0) {
+			kfree(dgap_config_buf);
 			return -EINVAL;
+		}
+		kfree(dgap_config_buf);
 	}
 
 	ret = dgap_after_config_loaded(brd->boardnum);
-- 
1.8.4.5

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/06] staging: dgap: Macros with complex values should be enclosed in parenthesis

[Mark Hounschell]

This patch fixes a checkpatch warning
"Macros with complex values should be enclosed in parenthesis"

Signed-off-by: Mark Hounschell <markh@compro.net>
---
 drivers/staging/dgap/dgap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 6415680..01eb56f 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -186,7 +186,7 @@ typedef unsigned char		uchar;
 			ECHOCTL | ECHOKE | IEXTEN)
 
 #ifndef _POSIX_VDISABLE
-#define   _POSIX_VDISABLE '\0'
+#define _POSIX_VDISABLE ('\0')
 #endif
 
 #define SNIFF_MAX	65536		/* Sniff buffer size (2^n) */
-- 
1.8.4.5

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/06] staging: dgap: remove volatiles from dgap.h

[Mark Hounschell]

This patch fixes checkpatch warnings about using volatiles in dgap.h

Signed-off-by: Mark Hounschell <markh@compro.net>
---
 drivers/staging/dgap/dgap.h | 152 ++++++++++++++++++++++----------------------
 1 file changed, 76 insertions(+), 76 deletions(-)

diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 01eb56f..2fce41c 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -1056,20 +1056,20 @@ struct channel_t {
  * Command structure definition.
  ************************************************************************/
 struct cm_t {
-	volatile unsigned short cm_head;	/* Command buffer head offset */
-	volatile unsigned short cm_tail;	/* Command buffer tail offset */
-	volatile unsigned short cm_start;	/* start offset of buffer     */
-	volatile unsigned short cm_max;		/* last offset of buffer      */
+	unsigned short cm_head;		/* Command buffer head offset */
+	unsigned short cm_tail;		/* Command buffer tail offset */
+	unsigned short cm_start;	/* start offset of buffer     */
+	unsigned short cm_max;		/* last offset of buffer      */
 };
 
 /************************************************************************
  * Event structure definition.
  ************************************************************************/
 struct ev_t {
-	volatile unsigned short ev_head;	/* Command buffer head offset */
-	volatile unsigned short ev_tail;	/* Command buffer tail offset */
-	volatile unsigned short ev_start;	/* start offset of buffer     */
-	volatile unsigned short ev_max;		/* last offset of buffer      */
+	unsigned short ev_head;		/* Command buffer head offset */
+	unsigned short ev_tail;		/* Command buffer tail offset */
+	unsigned short ev_start;	/* start offset of buffer     */
+	unsigned short ev_max;		/* last offset of buffer      */
 };
 
 /************************************************************************
@@ -1096,74 +1096,74 @@ struct downld_t {
  *        U = unknown (may be changed w/o notice)                       *
  ************************************************************************/
 struct bs_t {
-	volatile unsigned short  tp_jmp;	/* Transmit poll jump	 */
-	volatile unsigned short  tc_jmp;	/* Cooked procedure jump */
-	volatile unsigned short  ri_jmp;	/* Not currently used	 */
-	volatile unsigned short  rp_jmp;	/* Receive poll jump	 */
-
-	volatile unsigned short  tx_seg;	/* W Tx segment	 */
-	volatile unsigned short  tx_head;	/* W Tx buffer head offset */
-	volatile unsigned short  tx_tail;	/* R Tx buffer tail offset */
-	volatile unsigned short  tx_max;	/* W Tx buffer size - 1    */
-
-	volatile unsigned short  rx_seg;	/* W Rx segment	    */
-	volatile unsigned short  rx_head;	/* W Rx buffer head offset */
-	volatile unsigned short  rx_tail;	/* R Rx buffer tail offset */
-	volatile unsigned short  rx_max;	/* W Rx buffer size - 1    */
-
-	volatile unsigned short  tx_lw;		/* W Tx buffer low water mark */
-	volatile unsigned short  rx_lw;		/* W Rx buffer low water mark */
-	volatile unsigned short  rx_hw;		/* W Rx buffer high water mark*/
-	volatile unsigned short  incr;		/* W Increment to next channel*/
-
-	volatile unsigned short  fepdev;	/* U SCC device base address  */
-	volatile unsigned short  edelay;	/* W Exception delay          */
-	volatile unsigned short  blen;		/* W Break length             */
-	volatile unsigned short  btime;		/* U Break complete time      */
-
-	volatile unsigned short  iflag;		/* C UNIX input flags         */
-	volatile unsigned short  oflag;		/* C UNIX output flags        */
-	volatile unsigned short  cflag;		/* C UNIX control flags       */
-	volatile unsigned short  wfill[13];	/* U Reserved for expansion   */
-
-	volatile unsigned char   num;		/* U Channel number           */
-	volatile unsigned char   ract;		/* U Receiver active counter  */
-	volatile unsigned char   bstat;		/* U Break status bits        */
-	volatile unsigned char   tbusy;		/* W Transmit busy            */
-	volatile unsigned char   iempty;	/* W Transmit empty event     */
-						/* enable                     */
-	volatile unsigned char   ilow;		/* W Transmit low-water event */
-						/* enable                     */
-	volatile unsigned char   idata;		/* W Receive data interrupt   */
-						/* enable                     */
-	volatile unsigned char   eflag;		/* U Host event flags         */
-
-	volatile unsigned char   tflag;		/* U Transmit flags           */
-	volatile unsigned char   rflag;		/* U Receive flags            */
-	volatile unsigned char   xmask;		/* U Transmit ready flags     */
-	volatile unsigned char   xval;		/* U Transmit ready value     */
-	volatile unsigned char   m_stat;	/* RC Modem status bits       */
-	volatile unsigned char   m_change;	/* U Modem bits which changed */
-	volatile unsigned char   m_int;		/* W Modem interrupt enable   */
-						/* bits                       */
-	volatile unsigned char   m_last;	/* U Last modem status        */
-
-	volatile unsigned char   mtran;		/* C Unreported modem trans   */
-	volatile unsigned char   orun;		/* C Buffer overrun occurred  */
-	volatile unsigned char   astartc;	/* W Auxiliary Xon char       */
-	volatile unsigned char   astopc;	/* W Auxiliary Xoff char      */
-	volatile unsigned char   startc;	/* W Xon character            */
-	volatile unsigned char   stopc;		/* W Xoff character           */
-	volatile unsigned char   vnextc;	/* W Vnext character          */
-	volatile unsigned char   hflow;		/* C Software flow control    */
-
-	volatile unsigned char   fillc;		/* U Delay Fill character     */
-	volatile unsigned char   ochar;		/* U Saved output character   */
-	volatile unsigned char   omask;		/* U Output character mask    */
-
-	volatile unsigned char   bfill[13];	/* U Reserved for expansion   */
-
-	volatile unsigned char   scc[16];	/* U SCC registers            */
+	unsigned short  tp_jmp;		/* Transmit poll jump	 */
+	unsigned short  tc_jmp;		/* Cooked procedure jump */
+	unsigned short  ri_jmp;		/* Not currently used	 */
+	unsigned short  rp_jmp;		/* Receive poll jump	 */
+
+	unsigned short  tx_seg;		/* W Tx segment	 */
+	unsigned short  tx_head;	/* W Tx buffer head offset */
+	unsigned short  tx_tail;	/* R Tx buffer tail offset */
+	unsigned short  tx_max;		/* W Tx buffer size - 1    */
+
+	unsigned short  rx_seg;		/* W Rx segment	    */
+	unsigned short  rx_head;	/* W Rx buffer head offset */
+	unsigned short  rx_tail;	/* R Rx buffer tail offset */
+	unsigned short  rx_max;		/* W Rx buffer size - 1    */
+
+	unsigned short  tx_lw;		/* W Tx buffer low water mark */
+	unsigned short  rx_lw;		/* W Rx buffer low water mark */
+	unsigned short  rx_hw;		/* W Rx buffer high water mark*/
+	unsigned short  incr;		/* W Increment to next channel*/
+
+	unsigned short  fepdev;		/* U SCC device base address  */
+	unsigned short  edelay;		/* W Exception delay          */
+	unsigned short  blen;		/* W Break length             */
+	unsigned short  btime;		/* U Break complete time      */
+
+	unsigned short  iflag;		/* C UNIX input flags         */
+	unsigned short  oflag;		/* C UNIX output flags        */
+	unsigned short  cflag;		/* C UNIX control flags       */
+	unsigned short  wfill[13];	/* U Reserved for expansion   */
+
+	unsigned char   num;		/* U Channel number           */
+	unsigned char   ract;		/* U Receiver active counter  */
+	unsigned char   bstat;		/* U Break status bits        */
+	unsigned char   tbusy;		/* W Transmit busy            */
+	unsigned char   iempty;		/* W Transmit empty event     */
+					/* enable                     */
+	unsigned char   ilow;		/* W Transmit low-water event */
+					/* enable                     */
+	unsigned char   idata;		/* W Receive data interrupt   */
+					/* enable                     */
+	unsigned char   eflag;		/* U Host event flags         */
+
+	unsigned char   tflag;		/* U Transmit flags           */
+	unsigned char   rflag;		/* U Receive flags            */
+	unsigned char   xmask;		/* U Transmit ready flags     */
+	unsigned char   xval;		/* U Transmit ready value     */
+	unsigned char   m_stat;		/* RC Modem status bits       */
+	unsigned char   m_change;	/* U Modem bits which changed */
+	unsigned char   m_int;		/* W Modem interrupt enable   */
+					/* bits                       */
+	unsigned char   m_last;		/* U Last modem status        */
+
+	unsigned char   mtran;		/* C Unreported modem trans   */
+	unsigned char   orun;		/* C Buffer overrun occurred  */
+	unsigned char   astartc;	/* W Auxiliary Xon char       */
+	unsigned char   astopc;		/* W Auxiliary Xoff char      */
+	unsigned char   startc;		/* W Xon character            */
+	unsigned char   stopc;		/* W Xoff character           */
+	unsigned char   vnextc;		/* W Vnext character          */
+	unsigned char   hflow;		/* C Software flow control    */
+
+	unsigned char   fillc;		/* U Delay Fill character     */
+	unsigned char   ochar;		/* U Saved output character   */
+	unsigned char   omask;		/* U Output character mask    */
+
+	unsigned char   bfill[13];	/* U Reserved for expansion   */
+
+	unsigned char   scc[16];	/* U SCC registers            */
 };
 
 struct cnode {
-- 
1.8.1.4

[PATCH 05/06] staging: dgap: fix/change a pr_info

[Mark Hounschell]

This patch just fixes a pr_info. brd->state is not proper
at this point in time.

Signed-off-by: Mark Hounschell <markh@compro.net>
---
 drivers/staging/dgap/dgap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 07fcb8d..4bbedae 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -782,9 +782,8 @@ static int dgap_found_board(struct pci_dev *pdev, int id)
 	if (i)
 		brd->state = BOARD_FAILED;
 
-	pr_info("dgap: board %d: %s (rev %d), irq %ld, %s\n",
-		dgap_NumBoards, brd->name, brd->rev, brd->irq,
-		brd->state ? "NOT READY\0" : "READY\0");
+	pr_info("dgap: board %d: %s (rev %d), irq %ld\n",
+		dgap_NumBoards, brd->name, brd->rev, brd->irq);
 
 	return 0;
 }
-- 
1.8.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/06] staging: dgap: fix checkpatch warning about adding typedefs

[Mark Hounschell]

This patch fixes a checkpatch warning about adding typedefs
in dgap.h

Signed-off-by: Mark Hounschell <markh@compro.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/dgap/dgap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
index 2fce41c..e9ba2a4 100644
--- a/drivers/staging/dgap/dgap.h
+++ b/drivers/staging/dgap/dgap.h
@@ -40,7 +40,7 @@
 #endif
 
 /* Required for our shared headers! */
-typedef unsigned char		uchar;
+#define uchar unsigned char
 
 #if !defined(TTY_FLIPBUF_SIZE)
 # define TTY_FLIPBUF_SIZE 512
-- 
1.8.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/06] staging: dgap: Macros with complex values should be enclosed in parenthesis

[Dan Carpenter]

On Tue, Mar 25, 2014 at 04:38:15PM -0400, Mark Hounschell wrote:
> This patch fixes a checkpatch warning
> "Macros with complex values should be enclosed in parenthesis"
> 

Really this is a bug in checkpatch.pl.  We have the source for that, so
we should fix it instead of working around it.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/06] staging: dgap: fix checkpatch warning about adding typedefs

[Dan Carpenter]

On Tue, Mar 25, 2014 at 04:38:18PM -0400, Mark Hounschell wrote:
> This patch fixes a checkpatch warning about adding typedefs
> in dgap.h
> 
> Signed-off-by: Mark Hounschell <markh@compro.net>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/staging/dgap/dgap.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
> index 2fce41c..e9ba2a4 100644
> --- a/drivers/staging/dgap/dgap.h
> +++ b/drivers/staging/dgap/dgap.h
> @@ -40,7 +40,7 @@
>  #endif
>  
>  /* Required for our shared headers! */
> -typedef unsigned char		uchar;
> +#define uchar unsigned char
>  

That's not the point of the warning.  Just replace all the instances of
uchar with u8.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/06] staging: dgap: fix checkpatch warning about adding typedefs

[Mark Hounschell]

On 04/03/2014 04:56 AM, Dan Carpenter wrote:
> On Tue, Mar 25, 2014 at 04:38:18PM -0400, Mark Hounschell wrote:
>> This patch fixes a checkpatch warning about adding typedefs
>> in dgap.h
>>
>> Signed-off-by: Mark Hounschell <markh@compro.net>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>   drivers/staging/dgap/dgap.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/dgap/dgap.h b/drivers/staging/dgap/dgap.h
>> index 2fce41c..e9ba2a4 100644
>> --- a/drivers/staging/dgap/dgap.h
>> +++ b/drivers/staging/dgap/dgap.h
>> @@ -40,7 +40,7 @@
>>   #endif
>>
>>   /* Required for our shared headers! */
>> -typedef unsigned char		uchar;
>> +#define uchar unsigned char
>>
>
> That's not the point of the warning.  Just replace all the instances of
> uchar with u8.
>

OK, I'll send a patch for this.

Mark

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/06] staging: dgap: Macros with complex values should be enclosed in parenthesis

[Mark Hounschell]

On 04/03/2014 04:51 AM, Dan Carpenter wrote:
> On Tue, Mar 25, 2014 at 04:38:15PM -0400, Mark Hounschell wrote:
>> This patch fixes a checkpatch warning
>> "Macros with complex values should be enclosed in parenthesis"
>>
>
> Really this is a bug in checkpatch.pl.  We have the source for that, so
> we should fix it instead of working around it.
>

Ya, I didn't think it was very complex. Want me to change it back?

Mark

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/06] staging: dgap: Macros with complex values should be enclosed in parenthesis

[Dan Carpenter]

On Thu, Apr 03, 2014 at 07:53:30AM -0400, Mark Hounschell wrote:
> On 04/03/2014 04:51 AM, Dan Carpenter wrote:
> >On Tue, Mar 25, 2014 at 04:38:15PM -0400, Mark Hounschell wrote:
> >>This patch fixes a checkpatch warning
> >>"Macros with complex values should be enclosed in parenthesis"
> >>
> >
> >Really this is a bug in checkpatch.pl.  We have the source for that, so
> >we should fix it instead of working around it.
> >
> 
> Ya, I didn't think it was very complex. Want me to change it back?

No.  It's fine.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/06] staging: dgap: Only read config file dgap.conf once

[Greg Kroah-Hartman]

On Tue, Mar 25, 2014 at 04:38:14PM -0400, Mark Hounschell wrote:
> The config file is currently read for each board found.
> It only needs to be read one time. The buffer it is read
> into can now be freed immediately after it is parsed
> instead of at driver unload time.
> 
> Signed-off-by: Mark Hounschell <markh@compro.net>
> ---
>  drivers/staging/dgap/dgap.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Kernel drivers shouldn't be reading config files in the first place :)

But I'll take this for now, as it does fix the code up.

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/06] staging: dgap: Only read config file dgap.conf once

[Mark Hounschell]

On 04/14/2014 11:49 AM, Greg Kroah-Hartman wrote:
> On Tue, Mar 25, 2014 at 04:38:14PM -0400, Mark Hounschell wrote:
>> The config file is currently read for each board found.
>> It only needs to be read one time. The buffer it is read
>> into can now be freed immediately after it is parsed
>> instead of at driver unload time.
>>
>> Signed-off-by: Mark Hounschell <markh@compro.net>
>> ---
>>  drivers/staging/dgap/dgap.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> Kernel drivers shouldn't be reading config files in the first place :)
> 
> But I'll take this for now, as it does fix the code up.
> 

Ya, I know. I'm just not sure how to work around that yet.

Thanks
Mark

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/06] staging: dgap: Only read config file dgap.conf once

[Dan Carpenter]

On Mon, Apr 14, 2014 at 03:17:19PM -0400, Mark Hounschell wrote:
> On 04/14/2014 11:49 AM, Greg Kroah-Hartman wrote:
> > On Tue, Mar 25, 2014 at 04:38:14PM -0400, Mark Hounschell wrote:
> >> The config file is currently read for each board found.
> >> It only needs to be read one time. The buffer it is read
> >> into can now be freed immediately after it is parsed
> >> instead of at driver unload time.
> >>
> >> Signed-off-by: Mark Hounschell <markh@compro.net>
> >> ---
> >>  drivers/staging/dgap/dgap.c | 18 +++++++++---------
> >>  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > Kernel drivers shouldn't be reading config files in the first place :)
> > 
> > But I'll take this for now, as it does fix the code up.
> > 
> 
> Ya, I know. I'm just not sure how to work around that yet.
> 

You're probably going to have different solutions for different bits.
For example, can we auto detect the board type?  If so then remove that
chunk from the config file.  We'll have to have a struct somewhere with
the number of ports each type has.  Some things may have to be module
init options and somethings maybe we can configure with the tty ioctls
and some things through sysfs.

Creating your own driver specific api is bad so always prefer shared
infrastructure over, for example, doing your own thing in sysfs.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/06] staging: dgap: Only read config file dgap.conf once

[Mark Hounschell]

On 04/15/2014 06:01 AM, Dan Carpenter wrote:
> On Mon, Apr 14, 2014 at 03:17:19PM -0400, Mark Hounschell wrote:
>> On 04/14/2014 11:49 AM, Greg Kroah-Hartman wrote:
>>> On Tue, Mar 25, 2014 at 04:38:14PM -0400, Mark Hounschell wrote:
>>>> The config file is currently read for each board found.
>>>> It only needs to be read one time. The buffer it is read
>>>> into can now be freed immediately after it is parsed
>>>> instead of at driver unload time.
>>>>
>>>> Signed-off-by: Mark Hounschell <markh@compro.net>
>>>> ---
>>>>  drivers/staging/dgap/dgap.c | 18 +++++++++---------
>>>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> Kernel drivers shouldn't be reading config files in the first place :)
>>>
>>> But I'll take this for now, as it does fix the code up.
>>>
>>
>> Ya, I know. I'm just not sure how to work around that yet.
>>
> 
> You're probably going to have different solutions for different bits.
> For example, can we auto detect the board type?  If so then remove that
> chunk from the config file.  We'll have to have a struct somewhere with
> the number of ports each type has.  Some things may have to be module
> init options and somethings maybe we can configure with the tty ioctls
> and some things through sysfs.
> 
> Creating your own driver specific api is bad so always prefer shared
> infrastructure over, for example, doing your own thing in sysfs.
> 

Understood. This driver supports many boards that I do not have access
to.  Some of the other boards support external port expanders that allow
external control of the port configuration. These boards also require
different "concentrator" firmware depending on what external port
configuration you have connected.

In any case, I'm not quite ready to dive into all that just yet. There
are still lots of little things to do. Probably a lot more than I know of.

Regards
Mark

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel