On Sun, 26 Nov 2017, Fredrik Noring wrote:
> Hi Alan Stern,
>
> > > Be aware that your driver should utilize ohci_init_driver(), like
> > > several of the other platform-specific OHCI drivers do. Unless there's
> > > some very good reason, new drivers should never use the old interface.
> >
> > Agreed, please find updated patch with the new interface. (I suppose the
> > changes to drivers/usb/host/ohci-hcd.c eventually will have to be clarified
> > and moved elsewhere too.)
>
> The original author Jürgen Urban has made several improvements to v3 of the
> driver, shown below. To make progress we had to remove this WARN_ON line:
>
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -527,7 +527,6 @@ static inline void dma_free_attrs(struct device *dev,
> size_t size,
> const struct dma_map_ops *ops = get_dma_ops(dev);
>
> BUG_ON(!ops);
> - WARN_ON(irqs_disabled());
>
> if (dma_release_from_dev_coherent(dev, get_order(size), cpu_addr))
> return;
Yeah. This has nothing to do with the USB subsystem, though; you'll
have to take it up with the people responsible for maintaining that
file.
> We also tried a change along the lines of
>
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -444,8 +444,13 @@ static int ohci_init (struct ohci_hcd *ohci)
> int ret;
> struct usb_hcd *hcd = ohci_to_hcd(ohci);
>
> - /* Accept arbitrarily long scatter-gather lists */
> - hcd->self.sg_tablesize = ~0;
> + if (hcd->self.uses_dma) {
> + /* Accept arbitrarily long scatter-gather lists */
> + hcd->self.sg_tablesize = ~0;
> + } else {
> + /* DMA not supported. */
> + hcd->self.sg_tablesize = 0;
This part isn't necessary. The storage is allocated with kzalloc so
everything gets set to 0 initially anyway.
> + }
>
> if (distrust_firmware)
> ohci->flags |= OHCI_QUIRK_HUB_POWER;
>
> to address the issue of scatter-gather in combination with HCD_LOCAL_MEM and
> dma_declare_coherent_memory().
That's quite reasonable. If you submit just this change as a separate,
individual patch, I will accept it. But do please add a comment
explaining why the uses_dma test is necessary.
> The PS2 kernel is currently somewhat unstable, but it is at the moment unclear
> whether this is due to its OHCI driver.
>
> What are your thoughts on these changes and the driver patch below?
I did not read the whole thing in detail, but it generally looks okay.
Except, of course, that the dma-mapping.h change can't be part of this
patch. That will have to be done separately.
What happened to the changes to ohci_run() and ohci_irq() that were in
v2 of this RFC? Did you decide they weren't needed?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
