All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: "Adam J. Richter" <adam@yggdrasil.com>
Cc: htejun@gmail.com, linux-ide@vger.kernel.org
Subject: Re: Patch: slight shrink of vt6421_init_addrs()
Date: Fri, 23 Feb 2007 05:30:03 -0500	[thread overview]
Message-ID: <45DEC22B.3010005@pobox.com> (raw)
In-Reply-To: <20070216194656.A4375@freya>

Adam J. Richter wrote:
> Hi Tejun and Jeff,
> 
> 	The discussion about the vt6421 problems caused me to glance
> at the code and try a very minor clean-up to vt6421_init_addrs().
> Please note that I don't have a vt6421, so this patch is UNTESTED.
> The patch does two things:
> 
> 	1. Elminate the bmdma_addr variable, which was only used once,
> 	   in an assignment to a field also named bmdma_addr.  So having
> 	   a separate variable served no documentary purpose.  This change
> 	   causes the value to be computed a little later, so please make
> 	   sure that that is OK.  The effect is just to make the source
> 	   code one line smaller.  The binary size is unchanged by this
> 	   modification, at least on my x86 configuration, which has
> 	   SMP and lots of debugging options activated.
> 
> 	2. Add a variable ata_ports, to replace the six times the
> 	   same value appeared to be computed.  I assume the compiler
> 	   was smart enough to avoid most of the recomputation, but
> 	   this change shrinks the .text by 7 bytes in my configuration,
> 	   makes the routine more readable and reduces opportunities for
> 	   typos.
> 
> 	If it looks OK to everyone, please forward it upstream as
> appropriate.
> 
> Adam Richter

Patch seems OK in theory, except for two procedural stumbling blocks:

1) always always always include a signed-off-by line in your kernel 
patches.  see http://linux.yyz.us/patch-format.html or 
Documentation/SubmittingPatches in the kernel tree.

2) this was stirred a bit more by recent changes, so a rediff + resend 
would be appreciated



  reply	other threads:[~2007-02-23 10:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-16 11:46 Patch: slight shrink of vt6421_init_addrs() Adam J. Richter
2007-02-23 10:30 ` Jeff Garzik [this message]
2007-02-24  0:23   ` Adam J. Richter
  -- strict thread matches above, loose matches on Subject: below --
2007-02-16 11:52 Adam J. Richter

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=45DEC22B.3010005@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=adam@yggdrasil.com \
    --cc=htejun@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.