[PATCH 4/9] usb: chipidea: ci13xxx-imx: add "dr_mode" property to device tree bindings

Alexander Shishkin alexander.shishkin at linux.intel.com
Fri Nov 16 22:53:09 EST 2012


Michael Grzeschik <m.grzeschik at pengutronix.de> writes:

> From: Marc Kleine-Budde <mkl at pengutronix.de>
>
> Its necessary to limit a hostonly soc to its single role, since
> debugging has shown that reading on the "CAP_DCCPARAMS" register inside
> a host-only port, what ci_hdrc_gadget_init does, can lead to an
> instable behaviour of the IC.

Probably typos: should be "it's" and "unstable".

[snip]

> --- a/drivers/usb/chipidea/ci13xxx_imx.c
> +++ b/drivers/usb/chipidea/ci13xxx_imx.c
> @@ -237,6 +237,8 @@ static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	ci13xxx_get_dr_mode(pdev->dev.of_node, pdata);
> +
>  	plat_ci = ci13xxx_add_device(&pdev->dev,
>  				pdev->resource, pdev->num_resources,
>  				pdata);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index b50b77a..3e3e159 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -63,6 +63,7 @@
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of_platform.h>
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
>  #include <linux/usb/otg.h>
> @@ -521,6 +522,23 @@ void ci13xxx_remove_device(struct platform_device *pdev)
>  }
>  EXPORT_SYMBOL_GPL(ci13xxx_remove_device);
>  
> +void ci13xxx_get_dr_mode(struct device_node *of_node, struct ci13xxx_platform_data *pdata)
> +{
> +	const unsigned char *dr_mode;
> +
> +	dr_mode = of_get_property(of_node, "dr_mode", NULL);
> +	if (!dr_mode)
> +		return;
> +
> +	if (!strcmp(dr_mode, "host"))
> +		pdata->flags |= CI13XXX_DR_MODE_HOST;
> +	else if (!strcmp(dr_mode, "peripheral"))
> +		pdata->flags |= CI13XXX_DR_MODE_PERIPHERAL;
> +	else if (!strcmp(dr_mode, "otg"))
> +		pdata->flags |= CI13XXX_DR_MODE_HOST | CI13XXX_DR_MODE_PERIPHERAL;
> +}
> +EXPORT_SYMBOL_GPL(ci13xxx_get_dr_mode);
> +

I'd prefer this function to live in ci13xxx_imx, since that's where it's
used and it doesn't really need anything from core.c anyway. Or maybe it
would make sense to make it even more generic (for other devitetree
users), since you're saying that other drivers are using this already.

Looks good to me otherwise.

Regards,
--
Alex


More information about the devicetree-discuss mailing list