Re: [PATCH v8] usb: dwc3: of-simple: Add support to get resets for the device

[Philipp Zabel]

On Fri, 2017-10-20 at 17:10 +0530, Manu Gautam wrote:
> Hi,
> 
> 
> On 10/19/2017 5:17 PM, Philipp Zabel wrote:
> > From: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx>
> > 
> > Add support to get a list of resets available for the device.
> > These resets must be kept de-asserted until the device is
> > in use.
> > 
> > Signed-off-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx>
> > [p.zabel@xxxxxxxxxxxxxx: switch to hidden reset control array]
> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > ---
> > v7: Rebased onto git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
> > ---
> >  drivers/usb/dwc3/dwc3-of-simple.c | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
> > index e129c32780818..ceea1619f8aa3 100644
> > --- a/drivers/usb/dwc3/dwc3-of-simple.c
> > +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> > @@ -28,11 +28,13 @@
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> >  
> >  struct dwc3_of_simple {
> >  	struct device		*dev;
> >  	struct clk		**clks;
> >  	int			num_clocks;
> > +	struct reset_control	*resets;
> >  };
> >  
> >  static int dwc3_of_simple_clk_init(struct dwc3_of_simple *simple, int count)
> > @@ -95,10 +97,21 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
> >  	platform_set_drvdata(pdev, simple);
> >  	simple->dev = dev;
> >  
> > +	simple->resets = of_reset_control_array_get_optional_exclusive(np);
> > +	if (IS_ERR(simple->resets)) {
> > +		ret = PTR_ERR(simple->resets);
> > +		dev_err(dev, "failed to get device resets, err=%d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = reset_control_deassert(simple->resets);
> > +	if (ret)
> > +		goto err_resetc_put;
> > +
> 
> Are these reset signals asserted by default after power on?
> I think correct way to handle this should be to have explicit reset_control_assert
> before deassert.

I can only assume. If the resets can be initially deasserted (for
example because a bootloader may leave them in this state) and it is
critical for the device internal state to be reset for the driver to
function, the control should be asserted explicitly.

regards
Philipp