Path: csiph.com!1.us.feeder.erje.net!feeder.erje.net!2.eu.feeder.erje.net!news.roellig-ltd.de!open-news-network.org!weretis.net!feeder4.news.weretis.net!storethat.news.telefonica.de!telefonica.de!news.panservice.it!bofh.it!news.nic.it!robomod From: Philipp Zabel Newsgroups: linux.kernel Subject: Re: [PATCH 2/2] usb; dwc3: of-simple: Add support to get resets for the device Date: Wed, 15 Mar 2017 11:50:02 +0100 Message-ID: References: Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Sa-Exim-Connect-IP: 2001:67c:670:100:3ad5:47ff:feaf:1a17 X-Sa-Exim-Mail-From: p.zabel@pengutronix.de X-Sa-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-Ptx-Original-Recipient: linux-kernel@vger.kernel.org Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 94 Organization: linux.* mail to news gateway X-Original-Cc: balbi@kernel.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org X-Original-Date: Wed, 15 Mar 2017 11:45:27 +0100 X-Original-Message-ID: <1489574727.2528.13.camel@pengutronix.de> X-Original-References: <1487741048-24659-1-git-send-email-vivek.gautam@codeaurora.org> <1487741048-24659-2-git-send-email-vivek.gautam@codeaurora.org> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1601259 On Wed, 2017-02-22 at 10:54 +0530, Vivek Gautam wrote: > 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. > > Cc: Felipe Balbi > Signed-off-by: Vivek Gautam > --- > > Based on torvald's master branch. > > drivers/usb/dwc3/dwc3-of-simple.c | 49 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c > index fe414e7a9c78..025de7342d28 100644 > --- a/drivers/usb/dwc3/dwc3-of-simple.c > +++ b/drivers/usb/dwc3/dwc3-of-simple.c > @@ -29,13 +29,52 @@ > #include > #include > #include > +#include > > struct dwc3_of_simple { > struct device *dev; > struct clk **clks; > int num_clocks; > + struct reset_control **resets; > + int num_resets; > }; > > +static int dwc3_of_simple_reset_init(struct dwc3_of_simple *simple, int count) > +{ > + struct device *dev = simple->dev; > + int i; > + > + simple->num_resets = count; > + > + if (!count) > + return 0; > + > + simple->resets = devm_kcalloc(dev, simple->num_resets, > + sizeof(struct reset_control *), GFP_KERNEL); > + if (!simple->resets) > + return -ENOMEM; > + > + for (i = 0; i < simple->num_resets; i++) { > + struct reset_control *reset; > + int ret; > + > + reset = devm_reset_control_get_by_index(dev, i); Please use devm_reset_control_get_exclusive_by_index instead. See include/linux/reset.h for details. > + if (IS_ERR(reset)) > + return PTR_ERR(reset); > + > + simple->resets[i] = reset; > + > + ret = reset_control_deassert(reset); > + if (ret) { > + while (--i >= 0) > + reset_control_assert(reset); > + return ret; > + } > + } > + > + return 0; > +} This looks rather generic. Should we have a reset_control_get/assert/deassert_array functionality at the reset API level? > static int dwc3_of_simple_clk_init(struct dwc3_of_simple *simple, int count) > { > struct device *dev = simple->dev; > @@ -100,6 +139,10 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) > if (ret) > return ret; > > + ret = dwc3_of_simple_reset_init(simple, of_reset_control_get_count(np)); > + if (ret) > + return ret; > + Not a blocker, but it seems a bit inconsistent to count the reset controls via the device node (of_...), but then get them via the device (devm_reset_control_get_... instead of of_reset_control_get_...). regards Philipp