linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
@ 2015-03-09 16:29 Giedrius Statkevičius
  2015-03-10  7:39 ` Dan Carpenter
  2015-03-12 10:08 ` Greg KH
  0 siblings, 2 replies; 11+ messages in thread
From: Giedrius Statkevičius @ 2015-03-09 16:29 UTC (permalink / raw)
  To: lidza.louina, markh
  Cc: gregkh, driverdev-devel, devel, linux-kernel, dan.carpenter,
	Giedrius Statkevičius

Remove BOARD_FAILED and don't save dgnc_boards which failed to
initialize.

Assign the result of kzalloc() to brd in dgnc_found_board() and only put
it in the dgnc_Board[] if it successfully initializes. Also, remove
BOARD_FAILED enum and all ifs that check for it. Finally, remove one
final place where state was set to BOARD_FAILED which was even redundant
before this patch.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
in the first version

 drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
 drivers/staging/dgnc/dgnc_driver.h |  3 +--
 drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
 drivers/staging/dgnc/dgnc_tty.c    |  8 --------
 4 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index fa1ee79..075727d 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	unsigned long flags;
 
 	/* get the board structure and prep it */
-	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
-	brd = dgnc_Board[dgnc_NumBoards];
+	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
 
 	if (!brd)
 		return -ENOMEM;
@@ -574,17 +573,12 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	if (rc < 0) {
 		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
 		goto failed;
 	}
 
 	rc = dgnc_finalize_board_init(brd);
 	if (rc < 0) {
 		pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
@@ -592,9 +586,6 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	if (rc < 0) {
 		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
@@ -624,6 +615,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	brd->flipbuf = kzalloc(MYFLIPLEN, GFP_KERNEL);
 
 	wake_up_interruptible(&brd->state_wait);
+	dgnc_Board[dgnc_NumBoards] = brd;
 
 	return 0;
 
@@ -648,8 +640,6 @@ static int dgnc_finalize_board_init(struct dgnc_board *brd)
 		if (rc) {
 			dev_err(&brd->pdev->dev,
 				"Failed to hook IRQ %d\n", brd->irq);
-			brd->state = BOARD_FAILED;
-			brd->dpastatus = BD_NOFEP;
 			rc = -ENODEV;
 		}
 	}
@@ -708,12 +698,6 @@ static void dgnc_poll_handler(ulong dummy)
 
 		spin_lock_irqsave(&brd->bd_lock, flags);
 
-		/* If board is in a failed state, don't bother scheduling a tasklet */
-		if (brd->state == BOARD_FAILED) {
-			spin_unlock_irqrestore(&brd->bd_lock, flags);
-			continue;
-		}
-
 		/* Schedule a poll helper task */
 		tasklet_schedule(&brd->helper_tasklet);
 
diff --git a/drivers/staging/dgnc/dgnc_driver.h b/drivers/staging/dgnc/dgnc_driver.h
index 734bdc2..9f0d97b 100644
--- a/drivers/staging/dgnc/dgnc_driver.h
+++ b/drivers/staging/dgnc/dgnc_driver.h
@@ -107,8 +107,7 @@ enum {
  * All the possible states the board can be while booting up.
  */
 enum {
-	BOARD_FAILED = 0,
-	BOARD_FOUND,
+	BOARD_FOUND = 0,
 	BOARD_READY
 };
 
diff --git a/drivers/staging/dgnc/dgnc_mgmt.c b/drivers/staging/dgnc/dgnc_mgmt.c
index 5544a8e..8c3a1bf 100644
--- a/drivers/staging/dgnc/dgnc_mgmt.c
+++ b/drivers/staging/dgnc/dgnc_mgmt.c
@@ -168,10 +168,7 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		di.info_ioport = 0;
 		di.info_physaddr = (ulong) dgnc_Board[brd]->membase;
 		di.info_physsize = (ulong) dgnc_Board[brd]->membase - dgnc_Board[brd]->membase_end;
-		if (dgnc_Board[brd]->state != BOARD_FAILED)
-			di.info_nports = dgnc_Board[brd]->nasync;
-		else
-			di.info_nports = 0;
+		di.info_nports = dgnc_Board[brd]->nasync;
 
 		spin_unlock_irqrestore(&dgnc_Board[brd]->bd_lock, flags);
 
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 8179342..570eb7e 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1253,14 +1253,6 @@ static int dgnc_block_til_ready(struct tty_struct *tty, struct file *file, struc
 
 		sleep_on_un_flags = 0;
 
-		/*
-		 * If board has failed somehow during our sleep, bail with error.
-		 */
-		if (ch->ch_bd->state == BOARD_FAILED) {
-			retval = -ENXIO;
-			break;
-		}
-
 		/* If tty was hung up, break out of loop and set error. */
 		if (tty_hung_up_p(file)) {
 			retval = -EAGAIN;
-- 
2.3.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-09 16:29 [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize Giedrius Statkevičius
@ 2015-03-10  7:39 ` Dan Carpenter
  2015-03-12 10:08 ` Greg KH
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2015-03-10  7:39 UTC (permalink / raw)
  To: Giedrius Statkevičius
  Cc: lidza.louina, markh, devel, gregkh, driverdev-devel, linux-kernel

On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
> Remove BOARD_FAILED and don't save dgnc_boards which failed to
> initialize.
> 
> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
> it in the dgnc_Board[] if it successfully initializes. Also, remove
> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
> final place where state was set to BOARD_FAILED which was even redundant
> before this patch.
> 
> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>

Great!  Very thourough.  Thanks.  :)

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-09 16:29 [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize Giedrius Statkevičius
  2015-03-10  7:39 ` Dan Carpenter
@ 2015-03-12 10:08 ` Greg KH
  2015-03-12 10:20   ` Dan Carpenter
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Greg KH @ 2015-03-12 10:08 UTC (permalink / raw)
  To: Giedrius Statkevičius
  Cc: lidza.louina, markh, devel, driverdev-devel, linux-kernel, dan.carpenter

On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
> Remove BOARD_FAILED and don't save dgnc_boards which failed to
> initialize.
> 
> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
> it in the dgnc_Board[] if it successfully initializes. Also, remove
> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
> final place where state was set to BOARD_FAILED which was even redundant
> before this patch.
> 
> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
> ---
> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
> in the first version
> 
>  drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>  drivers/staging/dgnc/dgnc_driver.h |  3 +--
>  drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>  drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>  4 files changed, 4 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> index fa1ee79..075727d 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>  	unsigned long flags;
>  
>  	/* get the board structure and prep it */
> -	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
> -	brd = dgnc_Board[dgnc_NumBoards];
> +	brd = kzalloc(sizeof(*brd), GFP_KERNEL);

You've done a great job here, but...

Yeah, sorry...

I really want to see this whole "static list of boards/cards" go away.
There should not be any need for that in any in-kernel driver.  Your
patch here is a sign that things are really wrong with this whole static
array mess.

So could you do that instead?  I don't want to take patches around this
whole "board state" mess anymore, as it should all not be needed at all.

If you need pointers on what needs to be done here, just let me know.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-12 10:08 ` Greg KH
@ 2015-03-12 10:20   ` Dan Carpenter
  2015-03-12 16:14   ` Giedrius Statkevičius
  2015-03-13 20:06   ` Giedrius Statkevičius
  2 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2015-03-12 10:20 UTC (permalink / raw)
  To: Greg KH
  Cc: Giedrius Statkevičius, devel, lidza.louina, driverdev-devel,
	linux-kernel

On Thu, Mar 12, 2015 at 11:08:49AM +0100, Greg KH wrote:
> I really want to see this whole "static list of boards/cards" go away.

Yes, but I feel like this is a step in that direction.  It just deletes
dead code, it doesn't add anything.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-12 10:08 ` Greg KH
  2015-03-12 10:20   ` Dan Carpenter
@ 2015-03-12 16:14   ` Giedrius Statkevičius
  2015-03-13 20:55     ` Mark Hounschell
  2015-03-13 20:06   ` Giedrius Statkevičius
  2 siblings, 1 reply; 11+ messages in thread
From: Giedrius Statkevičius @ 2015-03-12 16:14 UTC (permalink / raw)
  To: Greg KH
  Cc: lidza.louina, markh, devel, driverdev-devel, linux-kernel, dan.carpenter

On 2015.03.12 12:08, Greg KH wrote:
> On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
>> Remove BOARD_FAILED and don't save dgnc_boards which failed to
>> initialize.
>>
>> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
>> it in the dgnc_Board[] if it successfully initializes. Also, remove
>> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
>> final place where state was set to BOARD_FAILED which was even redundant
>> before this patch.
>>
>> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
>> ---
>> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
>> in the first version
>>
>>  drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>>  drivers/staging/dgnc/dgnc_driver.h |  3 +--
>>  drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>>  drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>>  4 files changed, 4 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
>> index fa1ee79..075727d 100644
>> --- a/drivers/staging/dgnc/dgnc_driver.c
>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>>  	unsigned long flags;
>>  
>>  	/* get the board structure and prep it */
>> -	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
>> -	brd = dgnc_Board[dgnc_NumBoards];
>> +	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
> 
> You've done a great job here, but...
> 
> Yeah, sorry...
> 
> I really want to see this whole "static list of boards/cards" go away.
> There should not be any need for that in any in-kernel driver.  Your
> patch here is a sign that things are really wrong with this whole static
> array mess.
> 
> So could you do that instead?  I don't want to take patches around this
> whole "board state" mess anymore, as it should all not be needed at all.
> 
> If you need pointers on what needs to be done here, just let me know.
> 
> thanks,
> 
> greg k-h
> 

I can try :) But my main concern is the lack of dgnc driver maintainers
activity and that I don't own the hardware this driver is written for as
it's quite expensive (cheapest cards I've found start at 300$~) and I
can't afford it ATM. But I guess if I keep the patches small and logical
everything will be okay.

-- 
Thanks,
Giedrius

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-12 10:08 ` Greg KH
  2015-03-12 10:20   ` Dan Carpenter
  2015-03-12 16:14   ` Giedrius Statkevičius
@ 2015-03-13 20:06   ` Giedrius Statkevičius
  2 siblings, 0 replies; 11+ messages in thread
From: Giedrius Statkevičius @ 2015-03-13 20:06 UTC (permalink / raw)
  To: Greg KH
  Cc: lidza.louina, markh, devel, driverdev-devel, linux-kernel, dan.carpenter

Hi Greg,

On 2015.03.12 12:08, Greg KH wrote:
> On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
>> Remove BOARD_FAILED and don't save dgnc_boards which failed to
>> initialize.
>>
>> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
>> it in the dgnc_Board[] if it successfully initializes. Also, remove
>> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
>> final place where state was set to BOARD_FAILED which was even redundant
>> before this patch.
>>
>> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
>> ---
>> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
>> in the first version
>>
>>  drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>>  drivers/staging/dgnc/dgnc_driver.h |  3 +--
>>  drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>>  drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>>  4 files changed, 4 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
>> index fa1ee79..075727d 100644
>> --- a/drivers/staging/dgnc/dgnc_driver.c
>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>>  	unsigned long flags;
>>  
>>  	/* get the board structure and prep it */
>> -	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
>> -	brd = dgnc_Board[dgnc_NumBoards];
>> +	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
> 
> You've done a great job here, but...
> 
> Yeah, sorry...
> 
> I really want to see this whole "static list of boards/cards" go away.
> There should not be any need for that in any in-kernel driver.  Your
> patch here is a sign that things are really wrong with this whole static
> array mess.
> 
> So could you do that instead?  I don't want to take patches around this
> whole "board state" mess anymore, as it should all not be needed at all.
> 
> If you need pointers on what needs to be done here, just let me know.
> 
> thanks,
> 
> greg k-h
> 

Hmm,
The only thing I thought of that is relevant is that the one big
file/ioctl that handles getting info of any board could be split into
many and each could return information about that own board
(DIGI_GET_NI_INFO => get non-intelligent state info and DIGI_GETBD =>
get board info). The last ioctl that returns number of boards could be
deduced from special files (of each board) in the system.

I don't know how we can avoid a list of boards because we create sysfs
files for each board, register each board with tty core and poll all the
boards each 20ms.

Could you elaborate more, give some more info?

-- 
Thanks,
Giedrius

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-12 16:14   ` Giedrius Statkevičius
@ 2015-03-13 20:55     ` Mark Hounschell
  2015-03-14  8:44       ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Hounschell @ 2015-03-13 20:55 UTC (permalink / raw)
  To: Giedrius Statkevičius, Greg KH
  Cc: lidza.louina, devel, driverdev-devel, linux-kernel, dan.carpenter

On 03/12/2015 12:14 PM, Giedrius Statkevičius wrote:
> On 2015.03.12 12:08, Greg KH wrote:
>> On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
>>> Remove BOARD_FAILED and don't save dgnc_boards which failed to
>>> initialize.
>>>
>>> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
>>> it in the dgnc_Board[] if it successfully initializes. Also, remove
>>> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
>>> final place where state was set to BOARD_FAILED which was even redundant
>>> before this patch.
>>>
>>> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
>>> ---
>>> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
>>> in the first version
>>>
>>>   drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>>>   drivers/staging/dgnc/dgnc_driver.h |  3 +--
>>>   drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>>>   drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>>>   4 files changed, 4 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
>>> index fa1ee79..075727d 100644
>>> --- a/drivers/staging/dgnc/dgnc_driver.c
>>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>>> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>>>   	unsigned long flags;
>>>
>>>   	/* get the board structure and prep it */
>>> -	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
>>> -	brd = dgnc_Board[dgnc_NumBoards];
>>> +	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
>>
>> You've done a great job here, but...
>>
>> Yeah, sorry...
>>
>> I really want to see this whole "static list of boards/cards" go away.
>> There should not be any need for that in any in-kernel driver.  Your
>> patch here is a sign that things are really wrong with this whole static
>> array mess.
>>
>> So could you do that instead?  I don't want to take patches around this
>> whole "board state" mess anymore, as it should all not be needed at all.
>>
>> If you need pointers on what needs to be done here, just let me know.
>>
>> thanks,
>>
>> greg k-h
>>
>
> I can try :) But my main concern is the lack of dgnc driver maintainers
> activity and that I don't own the hardware this driver is written for as
> it's quite expensive (cheapest cards I've found start at 300$~) and I
> can't afford it ATM. But I guess if I keep the patches small and logical
> everything will be okay.
>

I am on the maintainers list and actually have hardware. I have just 
been so swamped the last few months that I haven't been able to do 
ANYTHING here. Even work on the dgap driver that I was working on, I 
just haven't had the time. I can tell you that the dgnc driver does NOT 
work at all and hasn't worked since it was introduced into staging by Greg.

Greg, while I'm here, how can we get Digi firmware into the kernel 
firmware tree? The dgap card is useless without it and I suspect the 
dgnc is as well.

Regards
mark


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-13 20:55     ` Mark Hounschell
@ 2015-03-14  8:44       ` Greg KH
  2015-03-15 12:07         ` Mark Hounschell
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2015-03-14  8:44 UTC (permalink / raw)
  To: Mark Hounschell
  Cc: Giedrius Statkevičius, lidza.louina, devel, driverdev-devel,
	linux-kernel, dan.carpenter

On Fri, Mar 13, 2015 at 04:55:55PM -0400, Mark Hounschell wrote:
> On 03/12/2015 12:14 PM, Giedrius Statkevičius wrote:
> >On 2015.03.12 12:08, Greg KH wrote:
> >>On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
> >>>Remove BOARD_FAILED and don't save dgnc_boards which failed to
> >>>initialize.
> >>>
> >>>Assign the result of kzalloc() to brd in dgnc_found_board() and only put
> >>>it in the dgnc_Board[] if it successfully initializes. Also, remove
> >>>BOARD_FAILED enum and all ifs that check for it. Finally, remove one
> >>>final place where state was set to BOARD_FAILED which was even redundant
> >>>before this patch.
> >>>
> >>>Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
> >>>---
> >>>v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
> >>>in the first version
> >>>
> >>>  drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
> >>>  drivers/staging/dgnc/dgnc_driver.h |  3 +--
> >>>  drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
> >>>  drivers/staging/dgnc/dgnc_tty.c    |  8 --------
> >>>  4 files changed, 4 insertions(+), 32 deletions(-)
> >>>
> >>>diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> >>>index fa1ee79..075727d 100644
> >>>--- a/drivers/staging/dgnc/dgnc_driver.c
> >>>+++ b/drivers/staging/dgnc/dgnc_driver.c
> >>>@@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
> >>>  	unsigned long flags;
> >>>
> >>>  	/* get the board structure and prep it */
> >>>-	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
> >>>-	brd = dgnc_Board[dgnc_NumBoards];
> >>>+	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
> >>
> >>You've done a great job here, but...
> >>
> >>Yeah, sorry...
> >>
> >>I really want to see this whole "static list of boards/cards" go away.
> >>There should not be any need for that in any in-kernel driver.  Your
> >>patch here is a sign that things are really wrong with this whole static
> >>array mess.
> >>
> >>So could you do that instead?  I don't want to take patches around this
> >>whole "board state" mess anymore, as it should all not be needed at all.
> >>
> >>If you need pointers on what needs to be done here, just let me know.
> >>
> >>thanks,
> >>
> >>greg k-h
> >>
> >
> >I can try :) But my main concern is the lack of dgnc driver maintainers
> >activity and that I don't own the hardware this driver is written for as
> >it's quite expensive (cheapest cards I've found start at 300$~) and I
> >can't afford it ATM. But I guess if I keep the patches small and logical
> >everything will be okay.
> >
> 
> I am on the maintainers list and actually have hardware. I have just been so
> swamped the last few months that I haven't been able to do ANYTHING here.
> Even work on the dgap driver that I was working on, I just haven't had the
> time. I can tell you that the dgnc driver does NOT work at all and hasn't
> worked since it was introduced into staging by Greg.

Really?  It's always been broken?  Why don't we just delete the thing?

> Greg, while I'm here, how can we get Digi firmware into the kernel firmware
> tree? The dgap card is useless without it and I suspect the dgnc is as well.

Submit a patch to the linux-firmware git tree with the firmware in it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-14  8:44       ` Greg KH
@ 2015-03-15 12:07         ` Mark Hounschell
  2015-03-15 16:38           ` Greg KH
  2015-03-16 17:18           ` Mark Hounschell
  0 siblings, 2 replies; 11+ messages in thread
From: Mark Hounschell @ 2015-03-15 12:07 UTC (permalink / raw)
  To: Greg KH
  Cc: Giedrius Statkevičius, lidza.louina, devel, driverdev-devel,
	linux-kernel, dan.carpenter

On 03/14/2015 04:44 AM, Greg KH wrote:
> On Fri, Mar 13, 2015 at 04:55:55PM -0400, Mark Hounschell wrote:
>> On 03/12/2015 12:14 PM, Giedrius Statkevičius wrote:
>>> On 2015.03.12 12:08, Greg KH wrote:
>>>> On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
>>>>> Remove BOARD_FAILED and don't save dgnc_boards which failed to
>>>>> initialize.
>>>>>
>>>>> Assign the result of kzalloc() to brd in dgnc_found_board() and only put
>>>>> it in the dgnc_Board[] if it successfully initializes. Also, remove
>>>>> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
>>>>> final place where state was set to BOARD_FAILED which was even redundant
>>>>> before this patch.
>>>>>
>>>>> Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
>>>>> ---
>>>>> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot to do
>>>>> in the first version
>>>>>
>>>>>   drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>>>>>   drivers/staging/dgnc/dgnc_driver.h |  3 +--
>>>>>   drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>>>>>   drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>>>>>   4 files changed, 4 insertions(+), 32 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
>>>>> index fa1ee79..075727d 100644
>>>>> --- a/drivers/staging/dgnc/dgnc_driver.c
>>>>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>>>>> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>>>>>   	unsigned long flags;
>>>>>
>>>>>   	/* get the board structure and prep it */
>>>>> -	dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
>>>>> -	brd = dgnc_Board[dgnc_NumBoards];
>>>>> +	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
>>>>
>>>> You've done a great job here, but...
>>>>
>>>> Yeah, sorry...
>>>>
>>>> I really want to see this whole "static list of boards/cards" go away.
>>>> There should not be any need for that in any in-kernel driver.  Your
>>>> patch here is a sign that things are really wrong with this whole static
>>>> array mess.
>>>>
>>>> So could you do that instead?  I don't want to take patches around this
>>>> whole "board state" mess anymore, as it should all not be needed at all.
>>>>
>>>> If you need pointers on what needs to be done here, just let me know.
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>>
>>> I can try :) But my main concern is the lack of dgnc driver maintainers
>>> activity and that I don't own the hardware this driver is written for as
>>> it's quite expensive (cheapest cards I've found start at 300$~) and I
>>> can't afford it ATM. But I guess if I keep the patches small and logical
>>> everything will be okay.
>>>
>>
>> I am on the maintainers list and actually have hardware. I have just been so
>> swamped the last few months that I haven't been able to do ANYTHING here.
>> Even work on the dgap driver that I was working on, I just haven't had the
>> time. I can tell you that the dgnc driver does NOT work at all and hasn't
>> worked since it was introduced into staging by Greg.
>
> Really?  It's always been broken?  Why don't we just delete the thing?
>

Well, broken might not be the best word. I should have just said, 
doesn't work for me.
It did work on a 3.4 kernel when I provided it to you. I have spent no 
time as to why but will take a look on Monday.

>> Greg, while I'm here, how can we get Digi firmware into the kernel firmware
>> tree? The dgap card is useless without it and I suspect the dgnc is as well.
>
> Submit a patch to the linux-firmware git tree with the firmware in it.
>

I don't know if the binary blobs are GPL compatable?

Regards
Mark



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-15 12:07         ` Mark Hounschell
@ 2015-03-15 16:38           ` Greg KH
  2015-03-16 17:18           ` Mark Hounschell
  1 sibling, 0 replies; 11+ messages in thread
From: Greg KH @ 2015-03-15 16:38 UTC (permalink / raw)
  To: Mark Hounschell
  Cc: Giedrius Statkevičius, lidza.louina, devel, driverdev-devel,
	linux-kernel, dan.carpenter

On Sun, Mar 15, 2015 at 08:07:09AM -0400, Mark Hounschell wrote:
> >>Greg, while I'm here, how can we get Digi firmware into the kernel firmware
> >>tree? The dgap card is useless without it and I suspect the dgnc is as well.
> >
> >Submit a patch to the linux-firmware git tree with the firmware in it.
> >
> 
> I don't know if the binary blobs are GPL compatable?

Look at the hundreds of different licenses in the linux-firmware tree,
they are all different for different reasons / companies.  Use whatever
license Digi uses for their firmware, it might already be part of the
tree.

hope this helps,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
  2015-03-15 12:07         ` Mark Hounschell
  2015-03-15 16:38           ` Greg KH
@ 2015-03-16 17:18           ` Mark Hounschell
  1 sibling, 0 replies; 11+ messages in thread
From: Mark Hounschell @ 2015-03-16 17:18 UTC (permalink / raw)
  To: Greg KH; +Cc: devel, lidza.louina, driverdev-devel, linux-kernel, dan.carpenter

On 03/15/2015 08:07 AM, Mark Hounschell wrote:
> On 03/14/2015 04:44 AM, Greg KH wrote:
>> On Fri, Mar 13, 2015 at 04:55:55PM -0400, Mark Hounschell wrote:
>>> On 03/12/2015 12:14 PM, Giedrius Statkevičius wrote:
>>>> On 2015.03.12 12:08, Greg KH wrote:
>>>>> On Mon, Mar 09, 2015 at 06:29:38PM +0200, Giedrius Statkevičius wrote:
>>>>>> Remove BOARD_FAILED and don't save dgnc_boards which failed to
>>>>>> initialize.
>>>>>>
>>>>>> Assign the result of kzalloc() to brd in dgnc_found_board() and
>>>>>> only put
>>>>>> it in the dgnc_Board[] if it successfully initializes. Also, remove
>>>>>> BOARD_FAILED enum and all ifs that check for it. Finally, remove one
>>>>>> final place where state was set to BOARD_FAILED which was even
>>>>>> redundant
>>>>>> before this patch.
>>>>>>
>>>>>> Signed-off-by: Giedrius Statkevičius
>>>>>> <giedrius.statkevicius@gmail.com>
>>>>>> ---
>>>>>> v2: Remove "brd = dgnc_Board[dgnc_NumBoards];" line which I forgot
>>>>>> to do
>>>>>> in the first version
>>>>>>
>>>>>>   drivers/staging/dgnc/dgnc_driver.c | 20 ++------------------
>>>>>>   drivers/staging/dgnc/dgnc_driver.h |  3 +--
>>>>>>   drivers/staging/dgnc/dgnc_mgmt.c   |  5 +----
>>>>>>   drivers/staging/dgnc/dgnc_tty.c    |  8 --------
>>>>>>   4 files changed, 4 insertions(+), 32 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/staging/dgnc/dgnc_driver.c
>>>>>> b/drivers/staging/dgnc/dgnc_driver.c
>>>>>> index fa1ee79..075727d 100644
>>>>>> --- a/drivers/staging/dgnc/dgnc_driver.c
>>>>>> +++ b/drivers/staging/dgnc/dgnc_driver.c
>>>>>> @@ -401,8 +401,7 @@ static int dgnc_found_board(struct pci_dev
>>>>>> *pdev, int id)
>>>>>>       unsigned long flags;
>>>>>>
>>>>>>       /* get the board structure and prep it */
>>>>>> -    dgnc_Board[dgnc_NumBoards] = kzalloc(sizeof(*brd), GFP_KERNEL);
>>>>>> -    brd = dgnc_Board[dgnc_NumBoards];
>>>>>> +    brd = kzalloc(sizeof(*brd), GFP_KERNEL);
>>>>>
>>>>> You've done a great job here, but...
>>>>>
>>>>> Yeah, sorry...
>>>>>
>>>>> I really want to see this whole "static list of boards/cards" go away.
>>>>> There should not be any need for that in any in-kernel driver.  Your
>>>>> patch here is a sign that things are really wrong with this whole
>>>>> static
>>>>> array mess.
>>>>>
>>>>> So could you do that instead?  I don't want to take patches around
>>>>> this
>>>>> whole "board state" mess anymore, as it should all not be needed at
>>>>> all.
>>>>>
>>>>> If you need pointers on what needs to be done here, just let me know.
>>>>>
>>>>> thanks,
>>>>>
>>>>> greg k-h
>>>>>
>>>>
>>>> I can try :) But my main concern is the lack of dgnc driver maintainers
>>>> activity and that I don't own the hardware this driver is written
>>>> for as
>>>> it's quite expensive (cheapest cards I've found start at 300$~) and I
>>>> can't afford it ATM. But I guess if I keep the patches small and
>>>> logical
>>>> everything will be okay.
>>>>
>>>
>>> I am on the maintainers list and actually have hardware. I have just
>>> been so
>>> swamped the last few months that I haven't been able to do ANYTHING
>>> here.
>>> Even work on the dgap driver that I was working on, I just haven't
>>> had the
>>> time. I can tell you that the dgnc driver does NOT work at all and
>>> hasn't
>>> worked since it was introduced into staging by Greg.
>>
>> Really?  It's always been broken?  Why don't we just delete the thing?
>>
>
> Well, broken might not be the best word. I should have just said,
> doesn't work for me.
> It did work on a 3.4 kernel when I provided it to you. I have spent no
> time as to why but will take a look on Monday.
>

The driver actually does work. It didn't work for me because the device 
entries were not as they used to be. So if you want to make a change you 
think needs testing, I can test it.
I only have a single card 8 port though.

Regards
Mark


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-03-16 17:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 16:29 [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize Giedrius Statkevičius
2015-03-10  7:39 ` Dan Carpenter
2015-03-12 10:08 ` Greg KH
2015-03-12 10:20   ` Dan Carpenter
2015-03-12 16:14   ` Giedrius Statkevičius
2015-03-13 20:55     ` Mark Hounschell
2015-03-14  8:44       ` Greg KH
2015-03-15 12:07         ` Mark Hounschell
2015-03-15 16:38           ` Greg KH
2015-03-16 17:18           ` Mark Hounschell
2015-03-13 20:06   ` Giedrius Statkevičius

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).