public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Rene Herman <rene.herman@keyaccess.nl>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: trenn@suse.de, linux-kernel <linux-kernel@vger.kernel.org>,
	akpm <akpm@linux-foundation.org>,
	Bjorn Helgaas <bjorn.helgaas@hp.com>,
	"Li, Shaohua" <shaohua.li@intel.com>,
	"yakui.zhao" <yakui.zhao@intel.com>
Subject: Re: [PATCH 3/3] PNP cleanups - Version 2 - Pass struct pnp_dev to pnp_clean_resource_table for cleanup reasons
Date: Tue, 20 Nov 2007 20:36:11 +0100	[thread overview]
Message-ID: <4743372A.6030400@keyaccess.nl> (raw)
In-Reply-To: <20071120180007.1ebe11a2@the-village.bc.nu>

On 20-11-07 19:00, Alan Cox wrote:

> On Tue, 20 Nov 2007 18:52:04 +0100
> Thomas Renninger <trenn@suse.de> wrote:
> 
>> Pass struct pnp_dev to pnp_clean_resource_table for cleanup reasons
> 
> Again I don't see the point of this change. A routine for cleaning up
> resource tables expects logically to be passed a resource table to clean
> up not some device it may be attached to.

He needs to pass the pnp_dev to later be able to replace the:

	for (idx = 0; idx < PNP_MAX_PORT; idx++)

loops with:

	for (idx = 0; pnp_port(dev, idx); idx++)

in a later patch when he introduces dynamic resource tables -- pnp-acpi can 
(and does) now sometimes require more resources than the current pnp limits 
allow but simply upping the limits uncoditionally wastes too much space in 
the resource tables. He therefore aims to krealloc() the arrays as required.

> Perhaps if you could explain where you are trying to end up, it would 
> help understand what you are trying to do.
> 
> I don't see why pnp_dma() and pnp_irq() should change either. It just 
> causes noise and breaks driver code. I don't see where it needs to change
> to make internal pnp changes work ?

As he explained in his 0/3, his pnp_port() would look like:

#define pnp_port(dev,bar)      ((dev)->res.allocated_ports > (bar) \
	? (&(dev)->res.port_resource[(bar)]) : NULL)

If the above replacement was the only use for the macros, he could as well do:

	for (idx = 0; idx < dev->res.allocated_ports; idx++)

but given that he'll need to get at the resource more generally, the simple 
pnp_port(), pnp_irq(), pnp_dma() and pnp_mem() names sound best. It would 
ofcourse be possible to call them something like pnp_port_addr() as well but 
given that he only needs to get rid of pnp_irq() and pnp_dma() to have these 
better names available, I'd say go for it.

pnp_{irq,dma}_no(), or pnp_{irq,dma}_start() as he originally proposed and 
which has consistency both with the existing pnp_{port,mem}_start() and the 
struct resource name as its plus should be fine and he then frees up the 
better names for the new use which should make for better readable code at 
the end of things.

My vote's with pnp_irq_start(). As said, consistent both with the port and 
mem variants and the struct resource usage and name.

Rene.


  reply	other threads:[~2007-11-20 19:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-20 17:52 [PATCH 3/3] PNP cleanups - Version 2 - Pass struct pnp_dev to pnp_clean_resource_table for cleanup reasons Thomas Renninger
2007-11-20 18:00 ` Alan Cox
2007-11-20 19:36   ` Rene Herman [this message]
2007-11-20 22:18     ` Alan Cox
2007-11-21  8:37       ` Rene Herman
2007-11-21  9:43         ` Thomas Renninger
2007-11-21 18:13           ` Alan Cox
2007-11-21 23:05             ` Thomas Renninger

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=4743372A.6030400@keyaccess.nl \
    --to=rene.herman@keyaccess.nl \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=bjorn.helgaas@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shaohua.li@intel.com \
    --cc=trenn@suse.de \
    --cc=yakui.zhao@intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox