Re: [PATCH] usb: dwc3: Addition of "dr_mode" dt property.

[Felipe Balbi <balbi@ti.com>]

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

Hi,

On Thu, May 30, 2013 at 02:53:10PM -0500, Dan Murphy wrote:
> > @@ -520,9 +520,23 @@ static int dwc3_probe(struct platform_device *pdev)
> >  		mode = DWC3_MODE_HOST;
> >  	else if (IS_ENABLED(CONFIG_USB_DWC3_GADGET))
> >  		mode = DWC3_MODE_DEVICE;
> > -	else
> > -		mode = DWC3_MODE_DRD;
> > -
> > +	else {
> > +		if (of_property_read_string(node, "dr_mode", &dr_mode)) {
> This will not execute if the either CONFIG options are set and then
> the DT property is not even honored
> Did you test this with multiple CONFIG options?
> There seems to be a conflict between CONFIGs and runtime operation.

this is alright. We still want to honor the users who chose to compile
the driver for gadget-only. In that case, there is no choice to be made.

Now, if you build the driver in its entirety (meaning, DRD), you can
still choose in runtime if you want the driver to behave as host-only or
gadget-only.

Picture a situation where you have a single SoC with multiple instances
of this IP and you want to make sure that e.g. ports 1-3 are host-only,
port 4 is peripheral-only and port 5 is DRD.

> > +			dev_warn(dev, "Missing dr_mode so assuming DWC3_MODE_DRD\n");
> If dr_mode is an optional parameter why would the dev_warn say it is missing?
> Do we even want to warn here?

Yes. Definitely yes. That would mean a less than optimal DTS file.
Still, for the sake of sensible defaults, we can still choose to work on
DRD mode, assuming full capabilities in case user didn't write proper
DTS, still user should be notified about it.

> > +			mode = DWC3_MODE_DRD;
> > +		} else {
> > +			if (strcmp(dr_mode, "host") == 0)
> > +				mode = DWC3_MODE_HOST;
> What if CONFIG_USB_DWC3_HOST is not enabled?

No issues, this will only execute if DRD is enabled, which means both
Host and Device are built in the final binary.

> > +			else if (strcmp(dr_mode, "gadget") == 0)
> > +				mode = DWC3_MODE_DEVICE;
> What if CONFIG_USB_DWC3_GADGET is not enabled?

see above.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]