Hi,
Arnd Bergmann <arnd@xxxxxxxx> writes:
>> > @@ -178,7 +179,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *dwc)
>> > static void dwc3_free_one_event_buffer(struct dwc3 *dwc,
>> > struct dwc3_event_buffer *evt)
>> > {
>> > - dma_free_coherent(dwc->dev, evt->length, evt->buf, evt->dma);
>> > + dma_free_coherent(dwc->sysdev, evt->length, evt->buf, evt->dma);
>>
>> how about "dma_dev" instead? Is this used for anything other than DMA?
>
> The two other things we have discussed in this thread are:
>
> - connecting of_node pointers to usb_device structures for children
> of sysdev->of_node. Note that this can happen even for PCI devices
> in case you have a USB ethernet device hardwired to a PCI-USB bridge
> and put the mac address in DT.
>
> - finding the PHY device for a HCD
>
> There might be others. Basically sysdev here is what the USB core code
> can use for looking up any kind of properties provided by the firmware.
fair enough
>> > @@ -846,6 +847,13 @@ static int dwc3_probe(struct platform_device *pdev)
>> > dwc = PTR_ALIGN(mem, DWC3_ALIGN_MASK + 1);
>> > dwc->mem = mem;
>> > dwc->dev = dev;
>> > +#ifdef CONFIG_PCI
>> > + /* TODO: or some other way of detecting this? */
>> > + if (dwc->dev->parent && dwc->dev->parent->bus == &pci_bus_type)
>> > + dwc->sysdev = dwc->dev->parent;
>> > + else
>> > +#endif
>> > + dwc->sysdev = dwc->dev;
>>
>> Well, we can remove this ifdef and *always* use the parent. We will just
>> require that dwc3 users provide a glue layer. In that case, your check
>> becomes:
>>
>> if (dwc->dev->parent)
>> dwc->sysdev = dwc->dev->parent;
>> else
>> dev_info(dwc->dev, "Please provide a glue layer!\n");
>
> If we do that, we have to put child devices of the dwc3 devices into
> the platform glue, and it also breaks those dwc3 devices that don't
> have a parent driver.
Well, this is easy to fix:
if (dwc->dev->parent) {
dwc->sysdev = dwc->dev->parent;
} else {
dev_info(dwc->dev, "Please provide a glue layer!\n");
dwc->sysdev = dwc->dev;
}
>> > diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
>> > index 89a2f712fdfe..4d7439cb8cd8 100644
>> > --- a/drivers/usb/dwc3/dwc3-st.c
>> > +++ b/drivers/usb/dwc3/dwc3-st.c
>> > @@ -218,7 +218,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>> > if (IS_ERR(regmap))
>> > return PTR_ERR(regmap);
>> >
>> > - dma_set_coherent_mask(dev, dev->coherent_dma_mask);
>>
>> so is this.
>>
>> All in all, I like where you're going with this, we just need a matching
>> acpi_dma_configure() and problems will be sorted out.
>
> With this patch, I don't think we even need that any more, as the device
> that we use the dma-mapping API is the one that already gets configured
> correctly by the platform code for all cases: PCI, OF, ACPI and combinations
> of those.
sounds good to me
--
balbi
Attachment:
signature.asc
Description: PGP signature
