Message ID | DB904C5425BA6F4E8424B3B51A1414D16D76860EA1@NWD2CMBX1.ad.analog.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Hello. On 04-01-2012 10:04, Zhang, Sonic wrote: > Hi Serge, > The MULTI_DONE_INT, UDMAIN_DONE_INT and UDMAOUT_DONE_INT are triggered independent > of the ATAPI_DEV_INT, although they bind to the same IRQ on bf548. With your patch, > these interrupts are ignored and results in kernel error "unhandled IRQ". Ah, indeed... > If you insist the BMDMA emulation on bf548 should comply with INF-8038i. It's not that I insist. It clearly follows from libata implemeting bmdma_status() method. > I would propose the following patch to disable all BF548 ATAPI specific interrupts. Yes, I agree -- they don't seem needed. And that will simplify the driver too. I'll recast the patch and add your signoff if you agree. > Sonic > --- > drivers/ata/pata_bf54x.c | 19 ++----------------- > 1 files changed, 2 insertions(+), 17 deletions(-) > > diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c > index bd987bb..9711c2a 100644 > --- a/drivers/ata/pata_bf54x.c > +++ b/drivers/ata/pata_bf54x.c > @@ -418,14 +418,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) > (tcyc_tdvs<<8 | tdvs)); > ATAPI_SET_ULTRA_TIM_2(base, (tmli<<8 | tss)); > ATAPI_SET_ULTRA_TIM_3(base, (trp<<8 | tzah)); > - > - /* Enable host ATAPI Untra DMA interrupts */ > - ATAPI_SET_INT_MASK(base, > - ATAPI_GET_INT_MASK(base) > - | UDMAIN_DONE_MASK > - | UDMAOUT_DONE_MASK > - | UDMAIN_TERM_MASK > - | UDMAOUT_TERM_MASK); > } > } > } > @@ -470,10 +462,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) > ATAPI_SET_MULTI_TIM_0(base, (tm<<8 | td)); > ATAPI_SET_MULTI_TIM_1(base, (tkr<<8 | tkw)); > ATAPI_SET_MULTI_TIM_2(base, (teoc<<8 | th)); > - > - /* Enable host ATAPI Multi DMA interrupts */ > - ATAPI_SET_INT_MASK(base, ATAPI_GET_INT_MASK(base) > - | MULTI_DONE_MASK | MULTI_TERM_MASK); > SSYNC(); > } > } > @@ -1155,13 +1143,10 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap) > void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr; > unsigned short int_status = ATAPI_GET_INT_STATUS(base); > > - if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON|ULTRA_XFER_ON)) > + if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON | ULTRA_XFER_ON)) > host_stat |= ATA_DMA_ACTIVE; > - if (int_status& (MULTI_DONE_INT|UDMAIN_DONE_INT|UDMAOUT_DONE_INT| > - ATAPI_DEV_INT)) > + if (ATAPI_GET_INT_STATUS(base)& ATAPI_DEV_INT) > host_stat |= ATA_DMA_INTR; > - if (int_status& (MULTI_TERM_INT|UDMAIN_TERM_INT|UDMAOUT_TERM_INT)) > - host_stat |= ATA_DMA_ERR|ATA_DMA_INTR; > > dev_dbg(ap->dev, "ATAPI: host_stat=0x%x\n", host_stat); > > -- > 1.7.0.4 WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 4, 2012 at 9:21 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote: > Hello. > > > On 04-01-2012 10:04, Zhang, Sonic wrote: > >> Hi Serge, > > >> The MULTI_DONE_INT, UDMAIN_DONE_INT and UDMAOUT_DONE_INT are triggered >> independent >> of the ATAPI_DEV_INT, although they bind to the same IRQ on bf548. With >> your patch, > >> these interrupts are ignored and results in kernel error "unhandled IRQ". > > Ah, indeed... > > >> If you insist the BMDMA emulation on bf548 should comply with INF-8038i. > > > It's not that I insist. It clearly follows from libata implemeting > bmdma_status() method. > > >> I would propose the following patch to disable all BF548 ATAPI specific >> interrupts. > > > Yes, I agree -- they don't seem needed. And that will simplify the driver > too. I'll recast the patch and add your signoff if you agree. No problem. Sonic > >> Sonic > > >> --- >> drivers/ata/pata_bf54x.c | 19 ++----------------- >> 1 files changed, 2 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c >> index bd987bb..9711c2a 100644 >> --- a/drivers/ata/pata_bf54x.c >> +++ b/drivers/ata/pata_bf54x.c >> @@ -418,14 +418,6 @@ static void bfin_set_dmamode(struct ata_port *ap, >> struct ata_device *adev) >> (tcyc_tdvs<<8 | tdvs)); >> ATAPI_SET_ULTRA_TIM_2(base, (tmli<<8 | >> tss)); >> ATAPI_SET_ULTRA_TIM_3(base, (trp<<8 | >> tzah)); >> - >> - /* Enable host ATAPI Untra DMA interrupts >> */ >> - ATAPI_SET_INT_MASK(base, >> - ATAPI_GET_INT_MASK(base) >> - | UDMAIN_DONE_MASK >> - | UDMAOUT_DONE_MASK >> - | UDMAIN_TERM_MASK >> - | UDMAOUT_TERM_MASK); >> } >> } >> } >> @@ -470,10 +462,6 @@ static void bfin_set_dmamode(struct ata_port *ap, >> struct ata_device *adev) >> ATAPI_SET_MULTI_TIM_0(base, (tm<<8 | td)); >> ATAPI_SET_MULTI_TIM_1(base, (tkr<<8 | tkw)); >> ATAPI_SET_MULTI_TIM_2(base, (teoc<<8 | th)); >> - >> - /* Enable host ATAPI Multi DMA interrupts */ >> - ATAPI_SET_INT_MASK(base, ATAPI_GET_INT_MASK(base) >> - | MULTI_DONE_MASK | MULTI_TERM_MASK); >> SSYNC(); >> } >> } >> @@ -1155,13 +1143,10 @@ static unsigned char bfin_bmdma_status(struct >> ata_port *ap) >> void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr; >> unsigned short int_status = ATAPI_GET_INT_STATUS(base); >> >> - if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON|ULTRA_XFER_ON)) >> + if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON | ULTRA_XFER_ON)) >> host_stat |= ATA_DMA_ACTIVE; >> - if (int_status& (MULTI_DONE_INT|UDMAIN_DONE_INT|UDMAOUT_DONE_INT| >> - ATAPI_DEV_INT)) >> + if (ATAPI_GET_INT_STATUS(base)& ATAPI_DEV_INT) >> host_stat |= ATA_DMA_INTR; >> - if (int_status& >> (MULTI_TERM_INT|UDMAIN_TERM_INT|UDMAOUT_TERM_INT)) >> - host_stat |= ATA_DMA_ERR|ATA_DMA_INTR; >> >> dev_dbg(ap->dev, "ATAPI: host_stat=0x%x\n", host_stat); >> >> -- >> 1.7.0.4 > > > WBR, Sergei > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 01/04/2012 09:04 AM, Zhang, Sonic wrote: > The MULTI_DONE_INT, UDMAIN_DONE_INT and UDMAOUT_DONE_INT are triggered independent of the ATAPI_DEV_INT, although they bind to the same IRQ on bf548. With your patch, these interrupts are ignored and results in kernel error "unhandled IRQ". > If you insist the BMDMA emulation on bf548 should comply with INF-8038i. I would propose the following patch to disable all BF548 ATAPI specific interrupts. > Sonic > --- > drivers/ata/pata_bf54x.c | 19 ++----------------- > 1 files changed, 2 insertions(+), 17 deletions(-) > diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c > index bd987bb..9711c2a 100644 > --- a/drivers/ata/pata_bf54x.c > +++ b/drivers/ata/pata_bf54x.c > @@ -418,14 +418,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) > (tcyc_tdvs<<8 | tdvs)); > ATAPI_SET_ULTRA_TIM_2(base, (tmli<<8 | tss)); > ATAPI_SET_ULTRA_TIM_3(base, (trp<<8 | tzah)); > - > - /* Enable host ATAPI Untra DMA interrupts */ > - ATAPI_SET_INT_MASK(base, > - ATAPI_GET_INT_MASK(base) > - | UDMAIN_DONE_MASK > - | UDMAOUT_DONE_MASK > - | UDMAIN_TERM_MASK > - | UDMAOUT_TERM_MASK); > } > } > } > @@ -470,10 +462,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) > ATAPI_SET_MULTI_TIM_0(base, (tm<<8 | td)); > ATAPI_SET_MULTI_TIM_1(base, (tkr<<8 | tkw)); > ATAPI_SET_MULTI_TIM_2(base, (teoc<<8 | th)); > - > - /* Enable host ATAPI Multi DMA interrupts */ > - ATAPI_SET_INT_MASK(base, ATAPI_GET_INT_MASK(base) > - | MULTI_DONE_MASK | MULTI_TERM_MASK); > SSYNC(); > } > } > @@ -1155,13 +1143,10 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap) > void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr; > unsigned short int_status = ATAPI_GET_INT_STATUS(base); > > - if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON|ULTRA_XFER_ON)) > + if (ATAPI_GET_STATUS(base)& (MULTI_XFER_ON | ULTRA_XFER_ON)) > host_stat |= ATA_DMA_ACTIVE; > - if (int_status& (MULTI_DONE_INT|UDMAIN_DONE_INT|UDMAOUT_DONE_INT| > - ATAPI_DEV_INT)) > + if (ATAPI_GET_INT_STATUS(base)& ATAPI_DEV_INT) > host_stat |= ATA_DMA_INTR; > - if (int_status& (MULTI_TERM_INT|UDMAIN_TERM_INT|UDMAOUT_TERM_INT)) > - host_stat |= ATA_DMA_ERR|ATA_DMA_INTR; > > dev_dbg(ap->dev, "ATAPI: host_stat=0x%x\n", host_stat); > Unfortunately your patch is white space damaged, with tabs converted to spaces. I'll reconstruct it... MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c index bd987bb..9711c2a 100644 --- a/drivers/ata/pata_bf54x.c +++ b/drivers/ata/pata_bf54x.c @@ -418,14 +418,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) (tcyc_tdvs<<8 | tdvs)); ATAPI_SET_ULTRA_TIM_2(base, (tmli<<8 | tss)); ATAPI_SET_ULTRA_TIM_3(base, (trp<<8 | tzah)); - - /* Enable host ATAPI Untra DMA interrupts */ - ATAPI_SET_INT_MASK(base, - ATAPI_GET_INT_MASK(base) - | UDMAIN_DONE_MASK - | UDMAOUT_DONE_MASK - | UDMAIN_TERM_MASK - | UDMAOUT_TERM_MASK); } } } @@ -470,10 +462,6 @@ static void bfin_set_dmamode(struct ata_port *ap, struct ata_device *adev) ATAPI_SET_MULTI_TIM_0(base, (tm<<8 | td)); ATAPI_SET_MULTI_TIM_1(base, (tkr<<8 | tkw)); ATAPI_SET_MULTI_TIM_2(base, (teoc<<8 | th)); - - /* Enable host ATAPI Multi DMA interrupts */ - ATAPI_SET_INT_MASK(base, ATAPI_GET_INT_MASK(base) - | MULTI_DONE_MASK | MULTI_TERM_MASK); SSYNC(); } } @@ -1155,13 +1143,10 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap) void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr; unsigned short int_status = ATAPI_GET_INT_STATUS(base); - if (ATAPI_GET_STATUS(base) & (MULTI_XFER_ON|ULTRA_XFER_ON)) + if (ATAPI_GET_STATUS(base) & (MULTI_XFER_ON | ULTRA_XFER_ON)) host_stat |= ATA_DMA_ACTIVE; - if (int_status & (MULTI_DONE_INT|UDMAIN_DONE_INT|UDMAOUT_DONE_INT| - ATAPI_DEV_INT)) + if (ATAPI_GET_INT_STATUS(base) & ATAPI_DEV_INT) host_stat |= ATA_DMA_INTR; - if (int_status & (MULTI_TERM_INT|UDMAIN_TERM_INT|UDMAOUT_TERM_INT)) - host_stat |= ATA_DMA_ERR|ATA_DMA_INTR; dev_dbg(ap->dev, "ATAPI: host_stat=0x%x\n", host_stat);