libata: reimplement LLD init_one()'s using new init helpers

PATCH Jeff Garzik 2 participants 2 messages Sep 19, 2006 1 message not in the archive libata

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jeff Garzik

author

2006-09-19 05:42 UTC #1 lore raw

Quoted 31 lines · Tejun Heo

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.

Quoted 6 lines

* 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 <[email protected]>

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.

Tejun Heo

2006-09-19 06:11 UTC #2 lore raw

Helloooo,

Quoted 7 lines · Jeff Garzik

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

Quoted 7 lines

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.

This patchset has been one of the most painful to carry around due to dependencies on everything and the wide range of changes. I agree that there can be a better way to split this patch if I think *really* hard, but was kind of hoping I could avoid that. :-)

I'll try to reshuffle the whole set as you suggested but I can't say for sure. Please note that lots of previous EH changes were done this way - implement new framework while leaving old one alone, convert LLDs, kill old framework. I have to admit that this patchset is more drastic tho.

Thanks.

Signature · 1 line

tejun