lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 16 Mar 2017 11:04:20 +0530 From: Vivek Gautam <vivek.gautam@...eaurora.org> To: Philipp Zabel <p.zabel@...gutronix.de> Cc: balbi@...nel.org, gregkh@...uxfoundation.org, linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org Subject: Re: [PATCH 2/2] usb; dwc3: of-simple: Add support to get resets for the device Hi, On 03/15/2017 04:15 PM, Philipp Zabel wrote: > 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 <balbi@...nel.org> >> Signed-off-by: Vivek Gautam <vivek.gautam@...eaurora.org> >> --- >> >> 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 <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; >> + 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. Sure, will make use of *exclusive version of the api. > >> + 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? Yes, i think we should. Something on the lines of 'regulator_bulk_*' interface? > >> 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_...). You are right, it looks inconsistent. I thought of using a resource managed API. But now i think it doesn't make much sense. Best Regards Vivek > > regards > Philipp > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Powered by blists - more mailing lists