| Message ID | 1487741048-24659-2-git-send-email-vivek.gautam@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
Vivek Gautam <vivek.gautam@codeaurora.org> writes: > 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@kernel.org> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> how do you plan on merging this since it depends on previous patch?
On Fri, Mar 10, 2017 at 4:54 PM, Felipe Balbi <balbi@kernel.org> wrote: > Vivek Gautam <vivek.gautam@codeaurora.org> writes: > >> 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@kernel.org> >> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> > > how do you plan on merging this since it depends on previous patch? Will ask Phillip if he is fine with the previous patch. He can then consider giving an ack so that both patches can be pulled in together. Will it be okay to do that ? Regards Vivek > > -- > balbi
Hi, Vivek Gautam <vivek.gautam@codeaurora.org> writes: > On Fri, Mar 10, 2017 at 4:54 PM, Felipe Balbi <balbi@kernel.org> wrote: >> Vivek Gautam <vivek.gautam@codeaurora.org> writes: >> >>> 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@kernel.org> >>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> >> >> how do you plan on merging this since it depends on previous patch? > > Will ask Phillip if he is fine with the previous patch. He can then consider > giving an ack so that both patches can be pulled in together. > > Will it be okay to do that ? sounds like a plan :-)
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@kernel.org> > Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.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. > + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@kernel.org> >> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.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 >
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); + 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; +} + 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; + ret = of_platform_populate(np, NULL, NULL, dev); if (ret) { for (i = 0; i < simple->num_clocks; i++) { @@ -107,6 +150,9 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) clk_put(simple->clks[i]); } + for (i = 0; i < simple->num_resets; i++) + reset_control_assert(simple->resets[i]); + return ret; } @@ -128,6 +174,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev) clk_put(simple->clks[i]); } + for (i = 0; i < simple->num_resets; i++) + reset_control_assert(simple->resets[i]); + of_platform_depopulate(dev); pm_runtime_put_sync(dev);
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@kernel.org> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org> --- Based on torvald's master branch. drivers/usb/dwc3/dwc3-of-simple.c | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)