LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Giedrius Statkevičius" <giedrius.statkevicius@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: lidza.louina@gmail.com, markh@compro.net,
	devel@driverdev.osuosl.org,
	driverdev-devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org, dan.carpenter@oracle.com
Subject: Re: [PATCH v2] dgnc: Don't save boards in memory that have failed to initialize
Date: Fri, 13 Mar 2015 22:06:39 +0200	[thread overview]
Message-ID: <5503434F.8050800@gmail.com> (raw)
In-Reply-To: <20150312100849.GA16122@kroah.com>

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

      parent reply	other threads:[~2015-03-13 20:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 16:29 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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5503434F.8050800@gmail.com \
    --to=giedrius.statkevicius@gmail.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=driverdev-devel@linuxdriverproject.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=lidza.louina@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markh@compro.net \
    --subject='Re: [PATCH v2] dgnc: Don'\''t save boards in memory that have failed to initialize' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).