headers
Mark Lord | 1 Mar 2005 04:06
Picon
Favicon

Re: data_xfer too fast? - SATA PIO

Err.. PIO data transfer within a sector (or multiple sector block)
is supposed to be synchronous in hardware.  This means the chip/board
should be using IORDY to stretch the cycle until data can be supplied
within each 256-word interrupt group.

Right?

Mat Loikkanen wrote:
> We've run into the issue of ata_mmio_data_xfer() reading our host
> controller's data fifo too fast -- requesting a data word when the fifo was
> empty, before the device had sent PIO data (in our observed case somewhere
> in the middle of a 512 byte IDENTIFY DEVICE PIO data transfer).  I can't see
> any provision in Libata for a case like ours where the processor and bus are
> "too fast".  Has anyone run into this issue before?  Any ideas on what we
> should do about it?  We can make our host controller to wait-state the bus
> ... but for how long, what if data never arrives ...  Thanks for any help.
> 
> -mat
>  
> Mat Loikkanen
> Synopsys, Inc.
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 1 comment |
headers
Tejun Heo | 1 Mar 2005 05:21
Picon

Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE

Hello, Bartlomiej.
Hello, Jeff.

On Mon, Feb 28, 2005 at 05:14:55PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Monday 28 February 2005 16:24, Tejun Heo wrote:
> > Bartlomiej Zolnierkiewicz wrote:
> > > 
> > > Nope, it works just fine because REQ_DRIVE_TASK used only
> > > no-data protocol, please check task_no_data_intr().
> > > 
> > 
> >   Sorry, I missed that.  IDE really has a lot of ways to finish a 
> > command, doesn't it?  hdio.txt is gonna look ugly. :-)
> 
> Yep but it was a lot more "fun" when there were three PIO codepaths. ;-)
> 
> hdio.txt doesn't need to know about driver internals so no problem here.
> 

 I was talking about the TASKFILE ioctl IN register result.

> > > 
> > >> IMHO, this flag-to-get-result-registers thing is way too subtle.  How
> > >>about keeping old behavior by just not copying out register outputs in
> > >>ide_taskfile_ioctl() in applicable cases instead of not reading
> > >>registers when ending commands?  That is, unless there's some
> > >>noticeable performance impacts I'm not aware of.
> > > 
> > > 
> > > This would miss whole point of not _reading_ these registers (IO is slow).
(Continue reading)

Permalink | Reply | 16 comments |
headers
Tejun Heo | 1 Mar 2005 06:29
Picon

Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE


  Oh, Bartlomiej, one more thing.

  If it isn't too much trouble, can you please set up a bk repository 
which contains the patches you've posted and whatever you're working on 
but hasn't yet made into ide-dev tree?  So that we don't have to juggle 
patches back and forth.  If you maintain your up-to-date working tree, 
I'll make my patches against that tree and if it's convinient for you, I 
can also set up an export tree you can pull from.

  Thanks.  :-)

--

-- 
tejun

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Bartlomiej Zolnierkiewicz | 1 Mar 2005 09:42
Picon

Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE

Hello,

On Tue, 1 Mar 2005 13:21:16 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
> Hello, Bartlomiej.
> Hello, Jeff.
> 
> On Mon, Feb 28, 2005 at 05:14:55PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Monday 28 February 2005 16:24, Tejun Heo wrote:
> > > Bartlomiej Zolnierkiewicz wrote:
> > > >
> > > > Nope, it works just fine because REQ_DRIVE_TASK used only
> > > > no-data protocol, please check task_no_data_intr().
> > > >
> > >
> > >   Sorry, I missed that.  IDE really has a lot of ways to finish a
> > > command, doesn't it?  hdio.txt is gonna look ugly. :-)
> >
> > Yep but it was a lot more "fun" when there were three PIO codepaths. ;-)
> >
> > hdio.txt doesn't need to know about driver internals so no problem here.
> >
> 
>  I was talking about the TASKFILE ioctl IN register result.
> 
> > > >
> > > >> IMHO, this flag-to-get-result-registers thing is way too subtle.  How
> > > >>about keeping old behavior by just not copying out register outputs in
> > > >>ide_taskfile_ioctl() in applicable cases instead of not reading
> > > >>registers when ending commands?  That is, unless there's some
> > > >>noticeable performance impacts I'm not aware of.
(Continue reading)

Permalink | Reply | 12 comments |
headers
Tejun Heo | 1 Mar 2005 10:29
Picon

Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE

 Hello,

On Tue, Mar 01, 2005 at 09:42:18AM +0100, Bartlomiej Zolnierkiewicz wrote:
> Hello,
> 
> On Tue, 1 Mar 2005 13:21:16 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
> > 
> >  So, how do you like the following set of TFLAG's?
> > 
> > /* struct ata_taskfile flags */
> > 
> > /* The following six flags are used by IDE driver to control register IO. */
> > ATA_TFLAG_OUT_LBA48             = (1 <<  0), /* enable 48-bit LBA and HOB out */
> 
> aggregate ATA_TFLAG_OUT_HOB_LBA{L,M,H}?
> 
> > ATA_TFLAG_OUT_ADDR              = (1 <<  1), /* enable out to nsect/lba regs */
> 
> not needed currently, add it {when,if} it is needed
> 

 Sure, I'll add flags on as-needed basis.  I was trying to show where
I'm heading.

> > ATA_TFLAG_OUT_DEVICE            = (1 <<  2), /* enable out to device reg */
> > ATA_TFLAG_IN_LBA48              = (1 <<  3), /* enable 48-bit LBA and HOB in */
> 
> aggregate ATA_TFLAG_IN_HOB_LBA_{L,M,H}?
> 
> > ATA_TFLAG_IN_ADDR               = (1 <<  4), /* enable in from nsect/lba regs */
(Continue reading)

Permalink | Reply | 10 comments |
headers
Bartlomiej Zolnierkiewicz | 1 Mar 2005 10:59
Picon

Re: [patch ide-dev 8/9] make ide_task_ioctl() use REQ_DRIVE_TASKFILE

On Tue, 1 Mar 2005 18:29:15 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
>  Hello,
> 
> On Tue, Mar 01, 2005 at 09:42:18AM +0100, Bartlomiej Zolnierkiewicz wrote:
> > Hello,
> >
> > On Tue, 1 Mar 2005 13:21:16 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
> > >
> > >  So, how do you like the following set of TFLAG's?
> > >
> > > /* struct ata_taskfile flags */
> > >
> > > /* The following six flags are used by IDE driver to control register IO. */
> > > ATA_TFLAG_OUT_LBA48             = (1 <<  0), /* enable 48-bit LBA and HOB out */
> >
> > aggregate ATA_TFLAG_OUT_HOB_LBA{L,M,H}?
> >
> > > ATA_TFLAG_OUT_ADDR              = (1 <<  1), /* enable out to nsect/lba regs */
> >
> > not needed currently, add it {when,if} it is needed
> >
> 
>  Sure, I'll add flags on as-needed basis.  I was trying to show where
> I'm heading.
> 
> > > ATA_TFLAG_OUT_DEVICE            = (1 <<  2), /* enable out to device reg */
> > > ATA_TFLAG_IN_LBA48              = (1 <<  3), /* enable 48-bit LBA and HOB in */
> >
> > aggregate ATA_TFLAG_IN_HOB_LBA_{L,M,H}?
> >
(Continue reading)

Permalink | Reply | 8 comments |
headers
Bartlomiej Zolnierkiewicz | 1 Mar 2005 15:30
Picon

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
> On Thu, Feb 24, 2005 at 04:58:03PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Thu, 10 Feb 2005 17:38:14 +0900 (KST), Tejun Heo <htejun <at> gmail.com> wrote:
> > >
> > > 01_ide_task_end_request_fix.patch
> > >
> > >         task_end_request() modified to always call ide_end_drive_cmd()
> > >         for taskfile requests.  Previously, ide_end_drive_cmd() was
> > >         called only when task->tf_out_flags.all was set.  Also,
> > >         ide_dma_intr() is modified to use task_end_request().
> > >
> > >         * fixes taskfile ioctl oops bug which was caused by referencing
> > >           NULL rq->rq_disk of taskfile requests.
> >
> > I fixed it in slightly different way in ide-dev-2.6 - by calling
> > ide_end_request() instead of ->end_request().
> 
>  Taskfile DMA path is still broken.  Also calling ide_end_request()
> will work there, but IMHO it's just cleaner to finish special commands
> inside ide_end_drive_cmd().  Currently,

I agree but please note that your patch makes *all* taskfile registers to
be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
(and obviously later you can't revert this change).

>  * Successful flagged taskfile                  -> ide_end_drive_cmd()
>  * All other successful non-DMA special cmds    -> ide_end_request()
>  * Successful DMA taskfile                      -> segfault

Have you tested it?  Why would it segfault?
(Continue reading)

Permalink | Reply | 8 comments |
headers
Tejun Heo | 1 Mar 2005 17:49
Picon

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

 Hello, Bartlomiej.

On Tue, Mar 01, 2005 at 03:30:32PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <htejun <at> gmail.com> wrote:
> > 
> >  Taskfile DMA path is still broken.  Also calling ide_end_request()
> > will work there, but IMHO it's just cleaner to finish special commands
> > inside ide_end_drive_cmd().  Currently,
> 
> I agree but please note that your patch makes *all* taskfile registers to
> be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
> (and obviously later you can't revert this change).
> 
> >  * Successful flagged taskfile                  -> ide_end_drive_cmd()
> >  * All other successful non-DMA special cmds    -> ide_end_request()
> >  * Successful DMA taskfile                      -> segfault
> 
> Have you tested it?  Why would it segfault?
> 

 It's the same reason why PIO taskfiles were broken.  rq->rq_disk is
NULL for taskfile requests.

ide_startstop_t ide_dma_intr (ide_drive_t *drive)
{
...
			printk("**HERE0***\n");
			drv = *(ide_driver_t **)rq->rq_disk->private_data;;
			printk("**HERE1***\n");
			drv->end_request(drive, 1, rq->nr_sectors);
(Continue reading)

Permalink | Reply | 2 comments |
headers
Mat Loikkanen | 1 Mar 2005 18:49

RE: data_xfer too fast? - SATA PIO

Thanks, Mark.  I think the hardware designer plans to do this -- add wait
states on the bus (we sell soft IP, so our controller is on an FPGA on our
test platform).  I wasn't sure if there were any PCI/ISA-isms that were at
play here.  We are ARM AMBA/AHB bus based, targeting embedded applications.

-----Original Message-----
From: Mark Lord

Err.. PIO data transfer within a sector (or multiple sector block)
is supposed to be synchronous in hardware.  This means the chip/board
should be using IORDY to stretch the cycle until data can be supplied
within each 256-word interrupt group.

Right?

Mat Loikkanen wrote:
> We've run into the issue of ata_mmio_data_xfer() reading our host
> controller's data fifo too fast -- requesting a data word when the fifo
was
> empty, before the device had sent PIO data (in our observed case somewhere
> in the middle of a 512 byte IDENTIFY DEVICE PIO data transfer).  I can't
see
> any provision in Libata for a case like ours where the processor and bus
are
> "too fast".  Has anyone run into this issue before?  Any ideas on what we
> should do about it?  We can make our host controller to wait-state the bus
> ... but for how long, what if data never arrives ...  Thanks for any help.
> 
> -mat
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Mat Loikkanen | 1 Mar 2005 19:06

SATA Plugfest

Thought I'd pass along our experience at the SATA Plugfest in Milpitas, CA,
recently.  The Plugfest was a 2 1/2 day event where some companies roamed
with their devices, plugging into host controllers from other companies who
stayed put in their hotel suites.  The days were scheduled with 1-hour time
slots.  As far as I could tell, we were the only Linux based platform at the
show.  I'm running Linux on an ARM Versatile dev board; our host controller
is in an FPGA; the PHY is from ST.  All driver layers except my low-level
driver are from the 2.6.10 kernel.  Most devices were GenII.  Aside from an
unexpected (by my driver) interrupt from some ATAPI ODDs in response to a
soft reset, no software issues were encountered.  Thanks to Jeff and all
others who have contributed to Libata, etc.  

-mat

Mat Loikkanen
Synopsys, Inc.

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |

Gmane