| Message ID | 1414507090-516-5-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ulf, Rafael, On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > Typically an ->attach_dev() callback would fetch some PM resourses. > > Those operations, like for example clk_get() may fail with different > errors, including -EPROBE_DEFER. Instead of ignoring these errors and > potentially only print an error message, let's propagate them to give > callers the option to handle them. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Given that several patch series using ->attach_dev() are already floating around and will be in -next soon, what is the plan of getting this in? Doing it ASAP (in v3.18-rc3)? Delaying this to v3.19-rc2, which will require an atomic fixing of its users? Any other option? Gr{oetje,eeting}s, Geert, who's about to resubmit his pm-rmobile patches -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 October 2014 21:31, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, Rafael, > > On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> Typically an ->attach_dev() callback would fetch some PM resourses. >> >> Those operations, like for example clk_get() may fail with different >> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >> potentially only print an error message, let's propagate them to give >> callers the option to handle them. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Given that several patch series using ->attach_dev() are already floating > around and will be in -next soon, what is the plan of getting this in? > Doing it ASAP (in v3.18-rc3)? > Delaying this to v3.19-rc2, which will require an atomic fixing of its users? > Any other option? I would prefer if we consider 3.18-rc[x|. That's applies also to the below patchset, which actually fixes an issue. It would simplify the process of handling other SOC specific patches which adds PM domain support. [PATCH v3 0/9] PM / Domains: Fix race conditions during boot Obviously the patches needs to be reviewed, I guess we are still in the process of doing that. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Wed, Oct 29, 2014 at 10:26 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 28 October 2014 21:31, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Typically an ->attach_dev() callback would fetch some PM resourses. >>> >>> Those operations, like for example clk_get() may fail with different >>> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >>> potentially only print an error message, let's propagate them to give >>> callers the option to handle them. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Given that several patch series using ->attach_dev() are already floating >> around and will be in -next soon, what is the plan of getting this in? >> Doing it ASAP (in v3.18-rc3)? >> Delaying this to v3.19-rc2, which will require an atomic fixing of its users? >> Any other option? > > I would prefer if we consider 3.18-rc[x|. That's applies also to the > below patchset, which actually fixes an issue. It would simplify the > process of handling other SOC specific patches which adds PM domain > support. > > [PATCH v3 0/9] PM / Domains: Fix race conditions during boot v3.18-rc[x] sounds fine. > Obviously the patches needs to be reviewed, I guess we are still in > the process of doing that. Indeed. Perhaps we can get just the prototype change of ->attach_dev() in first? That leaves some time for reviewing the code changes to actually handle the return value, and unblocks platform patches using ->attach_dev() soon, which are planned to enter in v3.19-rc. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29 October 2014 10:32, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Ulf, > > On Wed, Oct 29, 2014 at 10:26 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 28 October 2014 21:31, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> Typically an ->attach_dev() callback would fetch some PM resourses. >>>> >>>> Those operations, like for example clk_get() may fail with different >>>> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >>>> potentially only print an error message, let's propagate them to give >>>> callers the option to handle them. >>>> >>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >>> >>> Given that several patch series using ->attach_dev() are already floating >>> around and will be in -next soon, what is the plan of getting this in? >>> Doing it ASAP (in v3.18-rc3)? >>> Delaying this to v3.19-rc2, which will require an atomic fixing of its users? >>> Any other option? >> >> I would prefer if we consider 3.18-rc[x|. That's applies also to the >> below patchset, which actually fixes an issue. It would simplify the >> process of handling other SOC specific patches which adds PM domain >> support. >> >> [PATCH v3 0/9] PM / Domains: Fix race conditions during boot > > v3.18-rc[x] sounds fine. > >> Obviously the patches needs to be reviewed, I guess we are still in >> the process of doing that. > > Indeed. > > Perhaps we can get just the prototype change of ->attach_dev() in first? > That leaves some time for reviewing the code changes to actually handle > the return value, and unblocks platform patches using ->attach_dev() soon, > which are planned to enter in v3.19-rc. That's an option. On the other hand, that would mean that the errors that the attach_dev() callback would return from you SOC specific code, would just be ignored until v3.19-rc. That's not so good, hiding errors. :-) Let's see what Rafael thinks. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Wed, Oct 29, 2014 at 11:14 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>>> Given that several patch series using ->attach_dev() are already floating >>>> around and will be in -next soon, what is the plan of getting this in? >>>> Doing it ASAP (in v3.18-rc3)? >>>> Delaying this to v3.19-rc2, which will require an atomic fixing of its users? >>>> Any other option? >>> >>> I would prefer if we consider 3.18-rc[x|. That's applies also to the >>> below patchset, which actually fixes an issue. It would simplify the >>> process of handling other SOC specific patches which adds PM domain >>> support. >>> >>> [PATCH v3 0/9] PM / Domains: Fix race conditions during boot >> >> v3.18-rc[x] sounds fine. >> >>> Obviously the patches needs to be reviewed, I guess we are still in >>> the process of doing that. >> >> Indeed. >> >> Perhaps we can get just the prototype change of ->attach_dev() in first? >> That leaves some time for reviewing the code changes to actually handle >> the return value, and unblocks platform patches using ->attach_dev() soon, >> which are planned to enter in v3.19-rc. > > That's an option. > > On the other hand, that would mean that the errors that the > attach_dev() callback would return from you SOC specific code, would > just be ignored until v3.19-rc. That's not so good, hiding errors. :-) The code to handle the return value could still get in in v3.18-rc. So the errors would only be unhandled for a short while in -next. > Let's see what Rafael thinks. Right. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Ulf, Rafael, > > On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> Typically an ->attach_dev() callback would fetch some PM resourses. >> >> Those operations, like for example clk_get() may fail with different >> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >> potentially only print an error message, let's propagate them to give >> callers the option to handle them. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > Given that several patch series using ->attach_dev() are already floating > around and will be in -next soon, what is the plan of getting this in? Shall we take this as a Reviewed-by or Acked-by for the series? :) > Doing it ASAP (in v3.18-rc3)? IMO, this isn't at all appropriate for -rc since it's not fixing a regression. Also, this series includes other cleanups that are not really fixes either. At this point of the -rc cycle, we need to focus only on regression fixes. > Delaying this to v3.19-rc2, which will require an atomic fixing of its users? > Any other option? I don't see any users of this in -next yet, so I think doing a simple patch to the prototype and fixing up any users before they hit -next is the right approach. Errors will be ignored, but that's not change from today. :) Then the rest of this cleanup and behavior change stuff can continue to be reviewed and get broader testing before merge. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kevin, On Wed, Oct 29, 2014 at 10:10 PM, Kevin Hilman <khilman@kernel.org> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: >> On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Typically an ->attach_dev() callback would fetch some PM resourses. >>> >>> Those operations, like for example clk_get() may fail with different >>> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >>> potentially only print an error message, let's propagate them to give >>> callers the option to handle them. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Given that several patch series using ->attach_dev() are already floating >> around and will be in -next soon, what is the plan of getting this in? > > Shall we take this as a Reviewed-by or Acked-by for the series? :) No, I still have to review/test this series. Changing ->attach_dev() to return an error code again is fine for me. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 29 October 2014 22:10, Kevin Hilman <khilman@kernel.org> wrote: > Geert Uytterhoeven <geert@linux-m68k.org> writes: > >> Hi Ulf, Rafael, >> >> On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> Typically an ->attach_dev() callback would fetch some PM resourses. >>> >>> Those operations, like for example clk_get() may fail with different >>> errors, including -EPROBE_DEFER. Instead of ignoring these errors and >>> potentially only print an error message, let's propagate them to give >>> callers the option to handle them. >>> >>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> >> Given that several patch series using ->attach_dev() are already floating >> around and will be in -next soon, what is the plan of getting this in? > > Shall we take this as a Reviewed-by or Acked-by for the series? :) > >> Doing it ASAP (in v3.18-rc3)? > > IMO, this isn't at all appropriate for -rc since it's not fixing a > regression. Also, this series includes other cleanups that are not > really fixes either. At this point of the -rc cycle, we need to focus > only on regression fixes. > >> Delaying this to v3.19-rc2, which will require an atomic fixing of its users? >> Any other option? > > I don't see any users of this in -next yet, so I think doing a simple > patch to the prototype and fixing up any users before they hit -next is > the right approach. Errors will be ignored, but that's not change from > today. :) > > Then the rest of this cleanup and behavior change stuff can continue to > be reviewed and get broader testing before merge. Okay, I will follow your suggestions and send a patch that only change the prototype, intended as a fix for rc[n]. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4e5fcd7..da40769 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1394,12 +1394,19 @@ static int genpd_alloc_dev_data(struct generic_pm_domain *genpd, if (ret) goto err_data; - if (genpd->attach_dev) - genpd->attach_dev(dev); + if (genpd->attach_dev) { + ret = genpd->attach_dev(dev); + if (ret) + goto err_attach; + } dev_pm_qos_add_notifier(dev, &gpd_data->nb); return 0; + err_attach: + spin_lock_irq(&dev->power.lock); + dev->power.subsys_data->domain_data = NULL; + spin_unlock_irq(&dev->power.lock); err_data: kfree(gpd_data); err_alloc: diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index e4edde1..70a3bc3 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -72,7 +72,7 @@ struct generic_pm_domain { bool max_off_time_changed; bool cached_power_down_ok; struct gpd_cpuidle_data *cpuidle_data; - void (*attach_dev)(struct device *dev); + int (*attach_dev)(struct device *dev); void (*detach_dev)(struct device *dev); };
Typically an ->attach_dev() callback would fetch some PM resourses. Those operations, like for example clk_get() may fail with different errors, including -EPROBE_DEFER. Instead of ignoring these errors and potentially only print an error message, let's propagate them to give callers the option to handle them. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/base/power/domain.c | 11 +++++++++-- include/linux/pm_domain.h | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-)