From: Jeff Garzik <jgarzik@pobox.com>
To: Tejun Heo <htejun@gmail.com>
Cc: alan@lxorguk.ukuu.org.uk, mlord@pobox.com, albertcc@tw.ibm.com,
uchang@tw.ibm.com, forrest.zhao@intel.com, brking@us.ibm.com,
linux-ide@vger.kernel.org
Subject: Re: [PATCH 9/20] libata: reimplement LLD init_one()'s using new init helpers
Date: Tue, 19 Sep 2006 01:41:58 -0400 [thread overview]
Message-ID: <450F8326.7090108@pobox.com> (raw)
In-Reply-To: <11559779721014-git-send-email-htejun@gmail.com>
Tejun Heo wrote:
> Reimplement LLD init_one()'s using new init helpers. LLDs now follow
> the following init model.
>
> 1. allocate host
> 2. acquire resources and initialize controller and host accordingly
> 3. attach host
>
> At each step, libata provides comprehensive helpers and helps tracking
> allocated resources.
>
> The new init model...
>
> * allows LLD init_one()'s to be more flexible and concentrate on
> higher-level logic
>
> * results in code reduction in LLD init_one()'s even with more
> comments and proper error hanlding
>
> * makes init_one()'s of different LLDs much more consistent making
> them easier to understand and harder to mess up
>
> * makes init/deinit codes more easily shared with other parts by
> using the same target data structure (host and ap)
>
> The following bug fixes/behavior changes are introduced by this patch.
>
> * all LLDs deallocate resources properly on init failure
>
> * pdev_busy handling made generic. pdc_adma, sata_sil24, sata_nv,
> sata_qstor now deals with pdev_busy condition.
>
> * PDC20619 (PATA) doesn't set ATA_FLAG_SATA for its lower two ports
> and doesn't initialize scr_addr's.
>
> * PDC20375/20575 PATA port uses pdc_pata_ops instead of pdc_sata_ops
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
Unfortunately, while this patch seems correct, it is also difficult to
review, and I'm thinking it reflects that another approach may need to
be considered.
Usually the drivers and API are often change in small chunks, whereas
this patchset is more along the lines of
1) add new helpers
2) update everything to use new helpers
3) delete old helpers
The previous approach was closer to
* small update to API & drivers
* small update to API & drivers
* small update to API & drivers
...
I recognize that we are changing the LLDD init model, but I bet if you
think really hard, you could come up with a less-drastic transition path.
next parent reply other threads:[~2006-09-19 5:42 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <11559779721014-git-send-email-htejun@gmail.com>
2006-09-19 5:41 ` Jeff Garzik [this message]
2006-09-19 6:11 ` [PATCH 9/20] libata: reimplement LLD init_one()'s using new init helpers Tejun Heo
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=450F8326.7090108@pobox.com \
--to=jgarzik@pobox.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=albertcc@tw.ibm.com \
--cc=brking@us.ibm.com \
--cc=forrest.zhao@intel.com \
--cc=htejun@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=mlord@pobox.com \
--cc=uchang@tw.ibm.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;
as well as URLs for NNTP newsgroup(s).