Hi Takashi, Thank you for this patch, but it clashes with another patch trying to do the same thing that has already been merged in our tree:
https://patchwork.linuxtv.org/project/linux-media/patch/20210104170007.20625-1-mat...@sai.msu.ru/ I do prefer your patch over the one already merged since it is a bit simpler, but shouldn't the calls to dma_sync_single_for_cpu() and dma_sync_single_for_device() in pwc-if.c also use urb->dev->bus->controller? Also, Matwey's patch uses urb->dev->bus->sysdev instead of urb->dev->bus->controller. How does 'sysdev' relate to 'controller'? I think 'controller' is the right device to use, but either seems to work when I test it with my pwc webcam. Andrew, your patch: https://patchwork.linuxtv.org/project/linux-media/patch/20210204232851.1020416-1-and...@lunn.ch/ is effectively identical to Takashi's, so I'll mark your patch as Obsoleted. Just so you know :-) Regards, Hans On 21/01/2021 21:28, Takashi Iwai wrote: > The URB buffer allocation of pwc driver involves with the > dma_map_single(), and it needs to pass the right device. Currently it > passes usb_device.dev, but it's no real device that manages the DMA. > Since the passed device has no DMA mask set up, now the pwc driver > hits the WARN_ON_ONCE() check in dma_map_page_attrs() (that was > introduced in 5.10), resulting in an error at URB allocations. > Eventually this ended up with the black output. > > This patch fixes the bug by passing the proper device, the bus > controller, to make the URB allocation and map working again. > > BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1181133 > Cc: <sta...@vger.kernel.org> > Signed-off-by: Takashi Iwai <ti...@suse.de> > --- > drivers/media/usb/pwc/pwc-if.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c > index 61869636ec61..d771160bb168 100644 > --- a/drivers/media/usb/pwc/pwc-if.c > +++ b/drivers/media/usb/pwc/pwc-if.c > @@ -461,7 +461,7 @@ static int pwc_isoc_init(struct pwc_device *pdev) > urb->pipe = usb_rcvisocpipe(udev, pdev->vendpoint); > urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP; > urb->transfer_buffer_length = ISO_BUFFER_SIZE; > - urb->transfer_buffer = pwc_alloc_urb_buffer(&udev->dev, > + urb->transfer_buffer = > pwc_alloc_urb_buffer(udev->bus->controller, > > urb->transfer_buffer_length, > &urb->transfer_dma); > if (urb->transfer_buffer == NULL) { > @@ -524,7 +524,7 @@ static void pwc_iso_free(struct pwc_device *pdev) > if (urb) { > PWC_DEBUG_MEMORY("Freeing URB\n"); > if (urb->transfer_buffer) > - pwc_free_urb_buffer(&urb->dev->dev, > + pwc_free_urb_buffer(urb->dev->bus->controller, > urb->transfer_buffer_length, > urb->transfer_buffer, > urb->transfer_dma); >
