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

[Felipe Balbi <felipe.balbi@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]


Hi Steven,

"Steven J. Hill" <steven.hill@cavium.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@cavium.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]