From: Jeff Garzik <jgarzik@pobox.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] ata: add CONFIG_SATA_HOST config option
Date: Fri, 14 Oct 2011 13:46:29 -0400 [thread overview]
Message-ID: <4E987575.5080509@pobox.com> (raw)
In-Reply-To: <201110131658.08732.bzolnier@gmail.com>
On 10/13/2011 10:58 AM, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> Subject: [PATCH v3] ata: add CONFIG_SATA_HOST config option
>
> Add CONFIG_SATA_HOST config option (for selecting SATA Host
> support) to make setup easier on PATA-only systems.
>
> Additionally move SATA-specific code to libata-sata.c which
> allows us to save ~11.5k of the output code size (x86-64) on
> PATA-only systems for CONFIG_SATA_HOST=n:
>
> CONFIG_SATA_HOST=y:
> text data bss dec hex filename
> 44283 6576 57 50916 c6e4 drivers/ata/libata-core.o
> 29054 16 2 29072 7190 drivers/ata/libata-eh.o
> 20085 0 19 20104 4e88 drivers/ata/libata-sff.o
> 8699 0 0 8699 21fb drivers/ata/libata-sata.o
>
> CONFIG_SATA_HOST=n:
> text data bss dec hex filename
> 43754 6576 57 50387 c4d3 drivers/ata/libata-core.o
> 26775 16 2 26793 68a9 drivers/ata/libata-eh.o
> 20144 0 19 20163 4ec3 drivers/ata/libata-sff.o
>
> Signed-off-by: Bartlomiej Zolnierkiewicz<bzolnier@gmail.com>
> ---
> v2:
> - Kconfig fixups per Sergei's comments
> - add non-SATA version of ata_std_postreset()
> - drop non-SATA versions of sata_link_[debounce,hardreset,resume](),
> sata_set_spd(), sata_print_link_status() and ata_tf_from_fis()
> - move SATA-specific code to libata-sata.c
> v3:
> - require SATA_HOST for ata_piix for now
>
> earlier references:
> https://lkml.org/lkml/2011/2/9/149
> https://lkml.org/lkml/2011/2/11/103
>
> drivers/ata/Kconfig | 19
> drivers/ata/Makefile | 1
> drivers/ata/libata-core.c | 814 ------------------------------
> drivers/ata/libata-eh.c | 412 ---------------
> drivers/ata/libata-sata.c | 1243 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/ata/libata-sff.c | 32 -
> drivers/ata/libata.h | 52 +
> include/linux/libata.h | 32 -
> 8 files changed, 1365 insertions(+), 1240 deletions(-)
You did a good job of avoiding the "#ifdef ugliness" problem that often
plagues patches like this.
However, let's not mix code movement in with the rest of the separation.
Code movement into drivers/ata/libata-sata.c should be a separate,
first patch. Patch #1 should link and behave as before (excepting the
addition of libata-sata.c to the build of course).
Creating no-op stubs and CONFIG_SATA_HOST is a separate change after
code movement.
Jeff
prev parent reply other threads:[~2011-10-14 17:46 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-13 14:58 [PATCH v3] ata: add CONFIG_SATA_HOST config option Bartlomiej Zolnierkiewicz
2011-10-14 17:46 ` Jeff Garzik [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=4E987575.5080509@pobox.com \
--to=jgarzik@pobox.com \
--cc=bzolnier@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@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 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).