linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Phillips <phillips@arcor.de>
To: Dave Olien <dmo@osdl.org>, axboe@suse.de
Cc: _deepfire@mail.ru, linux-kernel@vger.kernel.org
Subject: Re: DAC960 in 2.5.38, with new changes
Date: Mon, 23 Sep 2002 22:39:00 +0200	[thread overview]
Message-ID: <E17tZyv-0003be-00@starship> (raw)
In-Reply-To: <20020923120400.A15452@acpi.pdx.osdl.net>

On Monday 23 September 2002 21:04, Dave Olien wrote:
> Daniel, Jens
> 
> Thanks for the feedback.  Daniel, I've incorporated your patch into my
> driver changes, and moved to linux 2.5.38.
> 
> Included are changes to put the memory mailbox and completion status
> mailboxes into dma mapped pages...

Applied, booted, up and running.  I'll attempt to follow your work -
you are clearly way out in front of everybody in understanding this
driver.

Minor whitespace suggestion: don't worry too much about breaking up
lines to fit in 80 columns.  It's nice where it works, but where it
just makes more lines, don't bother.  We are going to go do spelling
patches to shorten a lot of those names anyway.

What test do you use to determine that some blocks are incorrectly
written?

> I made a decision to remove some behavior from the functions:
> 
> 	DAC960_V1_EnableMemoryMailboxInterface
> 	DAC960_FinalizeController
> 
> Following is a description of the behavior I have removed.
> 
> In the original code, for the  "LA" and "PG" controller types,
> the FinalizeController() function does NOT free the memory for the
> mailboxes for these controllers.  Instead, the Finalize function calls
> 
> 	DAC960_LA_SaveMemoryMailboxInfo
> 		or
> 	DAC960_PG_SaveMemoryMailboxInfo
> 
> to instruct the controller to remember its memory mailbox addresses.
> 
> The driver EnableMemoryMailboxInterface() function calls 
> 
> 	"DAC960_LA_RestoreMemoryMailboxInfo"
> 		or
> 	"DAC960_PG_RestoreMemoryMailboxInfo"
> 
> functions to retrieve the controller's stored memory mailbox pointers.
> 
> This behavior is "useful" only when the driver is unloaded, and then
> reloaded.  
> 
> I changed to code to just free the memory mailboxes during Finalize,
> and reallocate new ones when the driver is reloaded.
> I feel it is much better to release this resource when the driver
> isn't loaded.

I think you're right.  It's hard to imagine why it would be good to
optimize the module unload/reload case for DAC, and if there is a
correctness argument, I personally would rather find out about it the
hard way than never know it.

> In 2.5. the driver needs both DMA addresses (that would be the address
> stored in the controller), and the CPU address.  To preserve the old
> behavior in 2.5, the driver would need to save these CPU addresses
> somewhere.  Strictly speaking, we shouldn't be translating between
> the two address types.

Yes, it seems a waste of effort.

> I don't have specifications for these devices.
> I don't know if there is some real reason for the old behavior.
>
> I don't have either of these controller types available to test this change
> either.  So, if you can think of any reason I should preserve the original
> behavior, I'd appreciate the feedback.

Hopefully somebody with one of those will jump into the thread.  Where
in the mass of DAC init output do we look for the controller type?

-- 
Daniel

  parent reply	other threads:[~2002-09-23 20:33 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-09-23 19:04 Dave Olien
2002-09-23 19:19 ` David Mosberger
2002-09-23 20:40   ` David S. Miller
2002-09-23 20:53     ` David Mosberger
2002-09-23 20:57       ` David S. Miller
2002-09-23 21:24         ` David Mosberger
2002-09-23 21:17           ` David S. Miller
2002-09-23 21:32             ` David Mosberger
2002-09-23 21:28               ` David S. Miller
2002-09-23 21:31           ` Daniel Phillips
2002-09-23 21:33             ` David Mosberger
2002-09-23 21:28               ` David S. Miller
2002-09-23 21:40               ` Daniel Phillips
2002-09-23 21:45                 ` David Mosberger
2002-09-23 20:44   ` Daniel Phillips
2002-09-23 20:54     ` David S. Miller
2002-09-24 16:54       ` Dave Olien
2002-09-24 17:11         ` David Mosberger
2002-09-24 17:28           ` Dave Olien
2002-09-24 18:21             ` David Mosberger
2002-09-24 18:25               ` Dave Olien
2002-09-24 19:12                 ` David Mosberger
2002-09-24 18:45               ` Dave Olien
2002-09-24 17:50           ` Daniel Phillips
2002-09-24 18:27             ` David Mosberger
2002-09-24 17:39         ` Daniel Phillips
2002-09-25 22:20           ` DAC960, documentation links Daniel Phillips
2002-09-25 22:23             ` Daniel Phillips
2002-09-26  0:02             ` Mr. James W. Laferriere
2002-09-26 16:13           ` DAC960 in 2.5.38, with new changes Alan Cox
2002-09-24 17:45         ` Daniel Phillips
2002-09-25 21:42           ` davide.rossetti
2002-09-23 20:39 ` Daniel Phillips [this message]
2002-09-23 21:41   ` Dave Olien
2002-09-23 21:53     ` Daniel Phillips
2002-09-23 22:03       ` Dave Olien
2002-09-23 22:22         ` Daniel Phillips

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=E17tZyv-0003be-00@starship \
    --to=phillips@arcor.de \
    --cc=_deepfire@mail.ru \
    --cc=axboe@suse.de \
    --cc=dmo@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: DAC960 in 2.5.38, with new changes' \
    /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).