linux-kernel - Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 08 Sep 2016 11:29:04 +0300
From:   Felipe Balbi <balbi@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Peter Chen <hzpeterchen@...il.com>, Leo Li <pku.leo@...il.com>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Russell King - ARM Linux <linux@....linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        "linux-usb\@vger.kernel.org" <linux-usb@...r.kernel.org>,
        Sekhar Nori <nsekhar@...com>,
        lkml <linux-kernel@...r.kernel.org>,
        Stuart Yoder <stuart.yoder@....com>,
        Scott Wood <oss@...error.net>,
        David Fisher <david.fisher1@...opsys.com>,
        "Thang Q. Nguyen" <tqnguyen@....com>,
        Alan Stern <stern@...land.harvard.edu>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-arm-kernel\@lists.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev


Hi,

Arnd Bergmann <arnd@...db.de> 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

Download attachment "signature.asc" of type "application/pgp-signature" (801 bytes)