Message ID | 1414507090-516-5-git-send-email-ulf.hansson@linaro.org (mailing list archive) |
---|---|
State | New, archived |
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
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
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
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
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
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
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
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
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(-)