linux-kernel - Re: [PATCH] usb: dwc3: OCTEON: add support for device tree

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] [day] [month] [year] [list]

Message-ID: <8760q6ker5.fsf@linux.intel.com>
Date:   Thu, 08 Sep 2016 10:54:54 +0300
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     "Steven J. Hill" <steven.hill@...ium.com>,
        linux-usb@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, linux-mips@...ux-mips.org,
        David Daney <david.daney@...ium.com>
Subject: Re: [PATCH] usb: dwc3: OCTEON: add support for device tree


Hi Steven,

"Steven J. Hill" <steven.hill@...ium.com> writes:
> This patch adds support to parse probe data for
> the dwc3-octeon driver using device tree. The
> DWC3 IP core is found on OCTEON III processors.
>
> Signed-off-by: Steven J. Hill <Steven.Hill@...ium.com>
> ---
>  drivers/usb/dwc3/Kconfig       | 10 +++++
>  drivers/usb/dwc3/Makefile      |  1 +
>  drivers/usb/dwc3/dwc3-octeon.c | 96 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 107 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-octeon.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index a64ce1c..99db6008 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -105,4 +105,14 @@ config USB_DWC3_ST
>  	  inside (i.e. STiH407).
>  	  Say 'Y' or 'M' if you have one such device.
>  
> +config USB_DWC3_OCTEON
> +	tristate "Cavium OCTEON III Platforms"
> +	depends on CAVIUM_OCTEON_SOC

we really don't want SoC dependencies. At a minimum, you should have:

	depends on CAVIUM_OCTEON_SOC || COMPILE_TEST

> +static int dwc3_octeon_probe(struct platform_device *pdev)
> +{
> +	struct device		*dev = &pdev->dev;
> +	struct resource		*res;
> +	struct dwc3_octeon	*octeon;
> +	int			ret;
> +
> +	octeon = devm_kzalloc(dev, sizeof(*octeon), GFP_KERNEL);
> +	if (!octeon)
> +		return - ENOMEM;
> +
> +	/*
> +	 * Right now device-tree probed devices don't get dma_mask set.
> +	 * Since shared usb code relies on it, set it here for now.
> +	 */

this doesn't look correct to me. Are you, perhaps, just missing
dma-ranges and dma-coherent properties?

> +static int dwc3_octeon_remove(struct platform_device *pdev)
> +{
> +	struct dwc3_octeon *octeon = platform_get_drvdata(pdev);
> +
> +	octeon->usbctl = NULL;
> +	octeon->index = -1;

octeon is going to be freed when ->remove() gets executed. You really
don't need to do these. In fact, setting usbctl to NULL will break
iounmap(). It seems to be me you don't need a remove at all.

-- 
balbi

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