On Tue, 2007-05-01 at 17:29 +0200, Oliver Endriss wrote:
> Stone wrote:
> > > Are there any objections to checking these fixes into main?
> >
> > I'm not trying to get credit for this contribution, which is why I did not
> > include a "signed off by". But, I would sure like to see it added if its
> > the right thing to do. Ok, Im going to shut up about it now. Maybe I
> > shouldn't have jumped in the middle of this and disturbed the process.
>
> Well, I just wanted to give Jon the chance to submit a complete patch
> and receive all credits.
>
> Imho the person who found the bug deserves to appear as the patch author
> in the changelog. It's not a problem to insert 2 lines of code. ;-)
>
> Btw, your patch is not quite correct:
>
> - if (!av7110->grabbing)
> + if (av7110->grabbing)
> saa7146_pgtable_free(pdev, &av7110->pt);
> + vfree(av7110->grabbing);
>
> Should read:
> + if (av7110->grabbing) {
> saa7146_pgtable_free(pdev, &av7110->pt);
> + vfree(av7110->grabbing);
> + }
>
> Otherwise indention would not match what the compiler does.
>
> If Jon doesn't submit his signed-off until tomorrow evening, I'll apply
> the patch without it.
>
> CU
> Oliver
>
Sorry for the delay. I wanted to track down the DMA sync bug before I
came back to look at this again.
I have added saa7146_vmalloc_destroy_pgtable() which frees the resources
allocated by saa7146_vmalloc_build_pgtable() and updated the callers in
budget-core.c and av711o.c. I have also been through the updated
functions and updated the error paths to ensure they free all allocated
resources on error.
Also included in this patch are the previous fixes for pci_unmap_sg()
and syncing the PCI streamed data to work with a SWIOTLB and match the
requirements documented in DMA-API.txt.
Jon
Signed-off-by: Jon Burgess <jburgess777@xxxxxxxxxxxxxx>
diff -r e3779e21a314 linux/drivers/media/common/saa7146_core.c
--- a/linux/drivers/media/common/saa7146_core.c Tue May 01 10:48:09 2007 -0300
+++ b/linux/drivers/media/common/saa7146_core.c Tue May 01 21:54:19 2007 +0100
@@ -136,28 +136,45 @@ char *saa7146_vmalloc_build_pgtable(stru
char *mem = vmalloc_32(length);
int slen = 0;
- if (NULL == mem) {
- return NULL;
- }
-
- if (!(pt->slist = vmalloc_to_sg(mem, pages))) {
- vfree(mem);
- return NULL;
- }
-
- if (saa7146_pgtable_alloc(pci, pt)) {
- kfree(pt->slist);
- pt->slist = NULL;
- vfree(mem);
- return NULL;
- }
-
- slen = pci_map_sg(pci,pt->slist,pages,PCI_DMA_FROMDEVICE);
- if (0 != saa7146_pgtable_build_single(pci, pt, pt->slist, slen)) {
- return NULL;
- }
+ if (NULL == mem)
+ goto err_null;
+
+ if (!(pt->slist = vmalloc_to_sg(mem, pages)))
+ goto err_free_mem;
+
+ if (saa7146_pgtable_alloc(pci, pt))
+ goto err_free_slist;
+
+ pt->nents = pages;
+ slen = pci_map_sg(pci,pt->slist,pt->nents,PCI_DMA_FROMDEVICE);
+ if (0 == slen)
+ goto err_free_pgtable;
+
+ if (0 != saa7146_pgtable_build_single(pci, pt, pt->slist, slen))
+ goto err_unmap_sg;
return mem;
+
+err_unmap_sg:
+ pci_unmap_sg(pci, pt->slist, pt->nents, PCI_DMA_FROMDEVICE);
+err_free_pgtable:
+ saa7146_pgtable_free(pci, pt);
+err_free_slist:
+ kfree(pt->slist);
+ pt->slist = NULL;
+err_free_mem:
+ vfree(mem);
+err_null:
+ return NULL;
+}
+
+void saa7146_vmalloc_destroy_pgtable(struct pci_dev *pci, char *mem, struct saa7146_pgtable *pt)
+{
+ pci_unmap_sg(pci, pt->slist, pt->nents, PCI_DMA_FROMDEVICE);
+ saa7146_pgtable_free(pci, pt);
+ kfree(pt->slist);
+ pt->slist = NULL;
+ vfree(mem);
}
void saa7146_pgtable_free(struct pci_dev *pci, struct saa7146_pgtable *pt)
@@ -166,8 +183,6 @@ void saa7146_pgtable_free(struct pci_dev
return;
pci_free_consistent(pci, pt->size, pt->cpu, pt->dma);
pt->cpu = NULL;
- kfree(pt->slist);
- pt->slist = NULL;
}
int saa7146_pgtable_alloc(struct pci_dev *pci, struct saa7146_pgtable *pt)
@@ -532,6 +547,7 @@ EXPORT_SYMBOL_GPL(saa7146_pgtable_free);
EXPORT_SYMBOL_GPL(saa7146_pgtable_free);
EXPORT_SYMBOL_GPL(saa7146_pgtable_build_single);
EXPORT_SYMBOL_GPL(saa7146_vmalloc_build_pgtable);
+EXPORT_SYMBOL_GPL(saa7146_vmalloc_destroy_pgtable);
EXPORT_SYMBOL_GPL(saa7146_wait_for_debi_done);
EXPORT_SYMBOL_GPL(saa7146_setgpio);
diff -r e3779e21a314 linux/drivers/media/dvb/ttpci/av7110.c
--- a/linux/drivers/media/dvb/ttpci/av7110.c Tue May 01 10:48:09 2007 -0300
+++ b/linux/drivers/media/dvb/ttpci/av7110.c Tue May 01 22:29:11 2007 +0100
@@ -1245,6 +1245,9 @@ static void vpeirq(unsigned long data)
if (!budget->feeding1 || (newdma == olddma))
return;
+
+ /* Ensure streamed PCI data is synced to CPU */
+ pci_dma_sync_sg_for_cpu(budget->dev->pci, budget->pt.slist, budget->pt.nents, PCI_DMA_FROMDEVICE);
#if 0 /* keep */
/* track rps1 activity */
@@ -2679,8 +2682,8 @@ err_pci_free_5:
err_pci_free_5:
pci_free_consistent(pdev, 8192, av7110->debi_virt, av7110->debi_bus);
err_saa71466_vfree_4:
- if (!av7110->grabbing)
- saa7146_pgtable_free(pdev, &av7110->pt);
+ if (av7110->grabbing)
+ saa7146_vmalloc_destroy_pgtable(pdev, av7110->grabbing, &av7110->pt);
err_i2c_del_3:
i2c_del_adapter(&av7110->i2c_adap);
err_dvb_unregister_adapter_2:
@@ -2710,7 +2713,7 @@ static int __devexit av7110_detach(struc
SAA7146_ISR_CLEAR(saa, MASK_10);
msleep(50);
tasklet_kill(&av7110->vpe_tasklet);
- saa7146_pgtable_free(saa->pci, &av7110->pt);
+ saa7146_vmalloc_destroy_pgtable(saa->pci, av7110->grabbing, &av7110->pt);
}
av7110_exit_v4l(av7110);
diff -r e3779e21a314 linux/drivers/media/dvb/ttpci/budget-core.c
--- a/linux/drivers/media/dvb/ttpci/budget-core.c Tue May 01 10:48:09 2007 -0300
+++ b/linux/drivers/media/dvb/ttpci/budget-core.c Tue May 01 22:28:31 2007 +0100
@@ -195,6 +195,9 @@ static void vpeirq(unsigned long data)
u32 newdma = saa7146_read(budget->dev, PCI_VDP3);
u32 count;
+ /* Ensure streamed PCI data is synced to CPU */
+ pci_dma_sync_sg_for_cpu(budget->dev->pci, budget->pt.slist, budget->pt.nents, PCI_DMA_FROMDEVICE);
+
/* nearest lower position divisible by 188 */
newdma -= newdma % 188;
@@ -504,16 +507,16 @@ int ttpci_budget_init(struct budget *bud
strcpy(budget->i2c_adap.name, budget->card->name);
if (i2c_add_adapter(&budget->i2c_adap) < 0) {
- dvb_unregister_adapter(&budget->dvb_adapter);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_dvb_unregister;
}
ttpci_eeprom_parse_mac(&budget->i2c_adap, budget->dvb_adapter.proposed_mac);
- if (NULL ==
- (budget->grabbing = saa7146_vmalloc_build_pgtable(dev->pci, budget->buffer_size, &budget->pt))) {
+ budget->grabbing = saa7146_vmalloc_build_pgtable(dev->pci, budget->buffer_size, &budget->pt);
+ if (NULL == budget->grabbing) {
ret = -ENOMEM;
- goto err;
+ goto err_del_i2c;
}
saa7146_write(dev, PCI_BT_V1, 0x001c0000);
@@ -526,14 +529,16 @@ int ttpci_budget_init(struct budget *bud
if (bi->type != BUDGET_FS_ACTIVY)
saa7146_setgpio(dev, 2, SAA7146_GPIO_OUTHI);
- if (budget_register(budget) == 0) {
- return 0;
- }
-err:
+ if (budget_register(budget) == 0)
+ return 0; /* Everything OK */
+
+ /* An error occurred, cleanup resources */
+ saa7146_vmalloc_destroy_pgtable(dev->pci, budget->grabbing, &budget->pt);
+
+err_del_i2c:
i2c_del_adapter(&budget->i2c_adap);
- vfree(budget->grabbing);
-
+err_dvb_unregister:
dvb_unregister_adapter(&budget->dvb_adapter);
return ret;
@@ -555,15 +560,13 @@ int ttpci_budget_deinit(struct budget *b
budget_unregister(budget);
+ tasklet_kill(&budget->vpe_tasklet);
+
+ saa7146_vmalloc_destroy_pgtable(dev->pci, budget->grabbing, &budget->pt);
+
i2c_del_adapter(&budget->i2c_adap);
dvb_unregister_adapter(&budget->dvb_adapter);
-
- tasklet_kill(&budget->vpe_tasklet);
-
- saa7146_pgtable_free(dev->pci, &budget->pt);
-
- vfree(budget->grabbing);
return 0;
}
diff -r e3779e21a314 linux/include/media/saa7146.h
--- a/linux/include/media/saa7146.h Tue May 01 10:48:09 2007 -0300
+++ b/linux/include/media/saa7146.h Tue May 01 22:20:38 2007 +0100
@@ -70,6 +70,7 @@ struct saa7146_pgtable {
unsigned long offset;
/* used for custom pagetables (used for example by budget dvb cards) */
struct scatterlist *slist;
+ int nents;
};
struct saa7146_pci_extension_data {
@@ -181,6 +182,7 @@ void saa7146_pgtable_free(struct pci_dev
void saa7146_pgtable_free(struct pci_dev *pci, struct saa7146_pgtable *pt);
int saa7146_pgtable_build_single(struct pci_dev *pci, struct saa7146_pgtable *pt, struct scatterlist *list, int length );
char *saa7146_vmalloc_build_pgtable(struct pci_dev *pci, long length, struct saa7146_pgtable *pt);
+void saa7146_vmalloc_destroy_pgtable(struct pci_dev *pci, char *mem, struct saa7146_pgtable *pt);
void saa7146_setgpio(struct saa7146_dev *dev, int port, u32 data);
int saa7146_wait_for_debi_done(struct saa7146_dev *dev, int nobusyloop);
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
