Re: [PATCH] libata-sff: Reenable Port Multiplier after libata-sff remodeling.

[Gwendal Grignou <gwendal@google.com>]

On Tue, Aug 31, 2010 at 1:04 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On 08/30/2010 07:17 PM, Gwendal Grignou wrote:
>> Keep track of the link on the which the current request is in progress.
>> It allows support of links behind port multiplier.
>>
>> Not all libata-sff is PMP compliant. Code for native BMDMA controller
>> does not take in accound PMP.
>
> Can you please elaborate a bit more on what broke and how this patch
> fixes the problem?
Before this patch, all libata-sff assumes the qc in progess is tied to
ap->link, the host port link.
That's fine as long as the controllers do not support port multiplier,
which is the case of all controller inheriting ata_sff_port_ops except
some controllers managed by sata_mv.
Also, before the libata-ssf reorg, it did not matter, qc was given the
sff task directly.

However, sata_mv supports port multiplier and use part of libata-sff
to hanlde PIO commands to disks. qc sent to disk behind port
multiplier are tight to one of element pmp_link array.
Therefore, the part of libata-sff sata_mv exercises must be retrieve
qc from the provided link instead of ap->link.

>
>> -void ata_sff_queue_pio_task(struct ata_port *ap, unsigned long delay)
>> +void ata_sff_queue_pio_task(struct ata_link *link, unsigned long delay)
>>  {
>> +     struct ata_port *ap = link->ap;
>
> New line here, please.
Done
>
>> +     ap->sff_pio_task_link = link;
>
> It would also be useful to have WARN/BUG_ON() to make sure no two
> links try to use pio_task at the same time.  ie. Set
> ap->sff_pio_task_link here and clear it with NULL when done and make
> sure it's NULL before setting it.
Add some WARN/BUG. I set link to NULL very early, I believe it is
cleaner than setting it in hsm_move() itself.
Patch after the break.

Gwendal.
>
> Otherwise, looks good to me.
>
> Thanks!
>
> --
> tejun
>