Skip to contentSkip to conversation

sil680 MMIO changes moved to branch

DISCUSSION Jeff Garzik 3 participants 7 messages May 25, 2007 libata

lore
Share & tools
lore
#0 lore raw

I moved the sil680 MMIO changes from 'upstream' (queued for 2.6.23) to a side branch 'sil680-mmio'.

To be clear, I am still quite interested in the patch (w/ the reset fix), but I did not want to delay other unrelated stuff that was about to be queued into #upstream.

The patch will still appear in -mm, and it is trivial to pull it from #sil680-mmio into #upstream once people are happy.

Jeff

#1 lore raw
Quoted 11 lines · Jeff Garzik

I moved the sil680 MMIO changes from 'upstream' (queued for 2.6.23) to a
side branch 'sil680-mmio'.

To be clear, I am still quite interested in the patch (w/ the reset fix), but I did not want to delay other unrelated stuff that was about
to be queued into #upstream.

The patch will still appear in -mm, and it is trivial to pull it from #sil680-mmio into #upstream once people are happy.

Fine with me. I've been busy with other things and waiting for you and Alan to decide what should be done for the reset stuff anway :-)

So Alan, do you think we should add something or do you think that just reading altstat like ata_pause does is good enough ?

Cheers,
Ben.

#2 lore raw
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 7 lines · Jeff Garzik

I moved the sil680 MMIO changes from 'upstream' (queued for 2.6.23) to a side branch 'sil680-mmio'.

To be clear, I am still quite interested in the patch (w/ the reset fix), but I did not want to delay other unrelated stuff that was about to be queued into #upstream.

The patch will still appear in -mm, and it is trivial to pull it from #sil680-mmio into #upstream once people are happy.

Fine with me. I've been busy with other things and waiting for you and Alan to decide what should be done for the reset stuff anway :-)

Just replace the ata_std_softreset() call with code that does MMIO flushes on a safe register?

Quoted 2 lines

So Alan, do you think we should add something or do you think that just reading altstat like ata_pause does is good enough ?

Reading AltStatus is fine for this.

Jeff

#3 lore raw
Quoted 2 lines · Jeff Garzik

Just replace the ata_std_softreset() call with code that does MMIO flushes on a safe register?

Yeah well... if just reading altstatus is enough, I don't see the point of doing anything MMIO-dependant or whatever. I can just add reading of it after all 3 SRST bit toggles unconditionally.

Ben.

#4 lore raw
Quoted 6 lines · Benjamin Herrenschmidt
Quoted 1 line · Jeff Garzik

Just replace the ata_std_softreset() call with code that does MMIO flushes on a safe register?

Yeah well... if just reading altstatus is enough, I don't see the point of doing anything MMIO-dependant or whatever. I can just add reading of it after all 3 SRST bit toggles unconditionally.

Sorry, to be clear I would not want to change the core code to bang AltStatus in the middle of the SRST twiddles...

The core code only fails (in theory) for PATA+MMIO cases, and the easiest fix should be to read some innocuous vendor-specific register. That allows the core code to stay unchanged, requiring only two drivers (pdc and sil680) to be updated with this flushing behavior.

All other discussion of ata_pause() seemed like an unrelated side discussion, unless I missed something?

Jeff

#5 lore raw
Quoted 9 lines · Jeff Garzik

Sorry, to be clear I would not want to change the core code to bang AltStatus in the middle of the SRST twiddles...

The core code only fails (in theory) for PATA+MMIO cases, and the easiest fix should be to read some innocuous vendor-specific register.
That allows the core code to stay unchanged, requiring only two drivers
(pdc and sil680) to be updated with this flushing behavior.

Ok. Will do that then.

Quoted 2 lines

All other discussion of ata_pause() seemed like an unrelated side discussion, unless I missed something?

Yes, I think we drifted because ata_pause is the other known case where MMIO needs flushing.

#6 lore raw
In reply to Benjamin Herrenschmidt
Quoted 5 lines

Fine with me. I've been busy with other things and waiting for you and Alan to decide what should be done for the reset stuff anway :-)

So Alan, do you think we should add something or do you think that just reading altstat like ata_pause does is good enough ?

Reading altstat is unneccessary for PIO so we do want to get these changes in so we can sort out performance of command sending, pause and anything else where it would help.

The PDC driver may well want to use altstatus reads for this but thats driver specific.