Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
Hi Xinhui, > Hi all, > I am xinhui pan. Thanks to the VPN problem, I can't access Intel's > network. So I have to send you this email by personal email address. > I am on Spring Festival vacation until Feb 9th. Sorry for that. > Your suggestion is very nice,thanks :) > I will generate V2 patch ASAP when my vocation is over. Thanks for > all your help. Since I got your ack you agree with this change, I can resend on behalf of you. Just enjoy your holidays :) Br, David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
Hi Xinhui, Hi all, I am xinhui pan. Thanks to the VPN problem, I can't access Intel's network. So I have to send you this email by personal email address. I am on Spring Festival vacation until Feb 9th. Sorry for that. Your suggestion is very nice,thanks :) I will generate V2 patch ASAP when my vocation is over. Thanks for all your help. Since I got your ack you agree with this change, I can resend on behalf of you. Just enjoy your holidays :) Br, David -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, 29 Jan 2014 13:06, David Cohen wrote: > On Wed, Jan 29, 2014 at 01:52:30PM -0600, Felipe Balbi wrote: >> On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: >>> On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: 于 2014年01月29日 08:13, David Cohen 写道: > On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: >> On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: >>> On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > From: "xinhui.pan" > > intel_gpio_runtime_idle should return correct error code if it do > fail. > make it more correct even though -EBUSY is the most possible return > value. > > Signed-off-by: bo.he > Signed-off-by: xinhui.pan > --- > drivers/gpio/gpio-intel-mid.c |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-intel-mid.c > b/drivers/gpio/gpio-intel-mid.c > index d1b50ef..05749a3 100644 > --- a/drivers/gpio/gpio-intel-mid.c > +++ b/drivers/gpio/gpio-intel-mid.c > @@ -394,7 +394,9 @@ static const struct irq_domain_ops > intel_gpio_irq_ops = { > > static int intel_gpio_runtime_idle(struct device *dev) > { > - pm_schedule_suspend(dev, 500); > + int err = pm_schedule_suspend(dev, 500); > + if (err) > + return err; > return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } >>> >>> The reply to your suggestion is probably in this commit :) >>> >>> --- >>> commit 45f0a85c8258741d11bda25c0a5669c06267204a >>> Author: Rafael J. Wysocki >>> Date: Mon Jun 3 21:49:52 2013 +0200 >>> >>> PM / Runtime: Rework the "runtime idle" helper routine >>> --- >>> >>> We won't return 0 from here. >> >> so you never want to return 0, why don't you, then: >> >> static int intel_gpio_runtime_idle(struct device *dev) >> { >> pm_schedule_suspend(dev, 500); >> return -EBUSY; >> } > > That's how it is currently :) > > But this patch is making the function to return a different code in case > of error. IMHO there is not much fuctional gain with it, but I see > perhaps one extra info for tracing during development. > > Anyway, I'll let Xinhui to do further comment since he's the author. > > Br, David > hi ,David & Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. >>> >>> In my experience, when I'm using fancy things like lauterbach a proper >>> error code may save couple of minutes in my life :) >>> >>> I keep my ack here. >> >> fair enough, sorry for the noise ;-) It could still be simplified a bit: >> >> return err ?: -EBUSY; > > Agreed :) > Xinhui, could we have this suggestion in your patch? > > Br, David > Hi all, I am xinhui pan. Thanks to the VPN problem, I can't access Intel's network. So I have to send you this email by personal email address. I am on Spring Festival vacation until Feb 9th. Sorry for that. Your suggestion is very nice,thanks :) I will generate V2 patch ASAP when my vocation is over. Thanks for all your help. >> >> -- >> balbi > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, 29 Jan 2014 13:06, David Cohen wrote: On Wed, Jan 29, 2014 at 01:52:30PM -0600, Felipe Balbi wrote: On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: 于 2014年01月29日 08:13, David Cohen 写道: On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David hi ,David Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. In my experience, when I'm using fancy things like lauterbach a proper error code may save couple of minutes in my life :) I keep my ack here. fair enough, sorry for the noise ;-) It could still be simplified a bit: return err ?: -EBUSY; Agreed :) Xinhui, could we have this suggestion in your patch? Br, David Hi all, I am xinhui pan. Thanks to the VPN problem, I can't access Intel's network. So I have to send you this email by personal email address. I am on Spring Festival vacation until Feb 9th. Sorry for that. Your suggestion is very nice,thanks :) I will generate V2 patch ASAP when my vocation is over. Thanks for all your help. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 01:52:30PM -0600, Felipe Balbi wrote: > On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: > > On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: > > > > > > 于 2014年01月29日 08:13, David Cohen 写道: > > > > On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: > > > >> On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: > > > >>> On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > > > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > > > From: "xinhui.pan" > > > > > > > > intel_gpio_runtime_idle should return correct error code if it do > > > > fail. > > > > make it more correct even though -EBUSY is the most possible return > > > > value. > > > > > > > > Signed-off-by: bo.he > > > > Signed-off-by: xinhui.pan > > > > --- > > > > drivers/gpio/gpio-intel-mid.c |4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpio/gpio-intel-mid.c > > > > b/drivers/gpio/gpio-intel-mid.c > > > > index d1b50ef..05749a3 100644 > > > > --- a/drivers/gpio/gpio-intel-mid.c > > > > +++ b/drivers/gpio/gpio-intel-mid.c > > > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops > > > > intel_gpio_irq_ops = { > > > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > > { > > > > - pm_schedule_suspend(dev, 500); > > > > + int err = pm_schedule_suspend(dev, 500); > > > > + if (err) > > > > + return err; > > > > return -EBUSY; > > > > > > wait, is it only me or this would look a lot better as: > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > { > > > return pm_schedule_suspend(dev, 500); > > > } > > > >>> > > > >>> The reply to your suggestion is probably in this commit :) > > > >>> > > > >>> --- > > > >>> commit 45f0a85c8258741d11bda25c0a5669c06267204a > > > >>> Author: Rafael J. Wysocki > > > >>> Date: Mon Jun 3 21:49:52 2013 +0200 > > > >>> > > > >>> PM / Runtime: Rework the "runtime idle" helper routine > > > >>> --- > > > >>> > > > >>> We won't return 0 from here. > > > >> > > > >> so you never want to return 0, why don't you, then: > > > >> > > > >> static int intel_gpio_runtime_idle(struct device *dev) > > > >> { > > > >>pm_schedule_suspend(dev, 500); > > > >>return -EBUSY; > > > >> } > > > > > > > > That's how it is currently :) > > > > > > > > But this patch is making the function to return a different code in case > > > > of error. IMHO there is not much fuctional gain with it, but I see > > > > perhaps one extra info for tracing during development. > > > > > > > > Anyway, I'll let Xinhui to do further comment since he's the author. > > > > > > > > Br, David > > > > > > > hi ,David & Balbi > > > I checked several drivers yesterday to see how they use > > > pm_schedule_suspend > > > then found one bug in i2c. Also I noticed gpio. > > > I think returning a correct error code is important.So I change -EBUSY > > > to *err*. To be honest,current code works well. > > > > In my experience, when I'm using fancy things like lauterbach a proper > > error code may save couple of minutes in my life :) > > > > I keep my ack here. > > fair enough, sorry for the noise ;-) It could still be simplified a bit: > > return err ?: -EBUSY; Agreed :) Xinhui, could we have this suggestion in your patch? Br, David > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: > On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: > > > > 于 2014年01月29日 08:13, David Cohen 写道: > > > On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: > > >> On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: > > >>> On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > > From: "xinhui.pan" > > > > > > intel_gpio_runtime_idle should return correct error code if it do > > > fail. > > > make it more correct even though -EBUSY is the most possible return > > > value. > > > > > > Signed-off-by: bo.he > > > Signed-off-by: xinhui.pan > > > --- > > > drivers/gpio/gpio-intel-mid.c |4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpio/gpio-intel-mid.c > > > b/drivers/gpio/gpio-intel-mid.c > > > index d1b50ef..05749a3 100644 > > > --- a/drivers/gpio/gpio-intel-mid.c > > > +++ b/drivers/gpio/gpio-intel-mid.c > > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops > > > intel_gpio_irq_ops = { > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > { > > > - pm_schedule_suspend(dev, 500); > > > + int err = pm_schedule_suspend(dev, 500); > > > + if (err) > > > + return err; > > > return -EBUSY; > > > > wait, is it only me or this would look a lot better as: > > > > static int intel_gpio_runtime_idle(struct device *dev) > > { > > return pm_schedule_suspend(dev, 500); > > } > > >>> > > >>> The reply to your suggestion is probably in this commit :) > > >>> > > >>> --- > > >>> commit 45f0a85c8258741d11bda25c0a5669c06267204a > > >>> Author: Rafael J. Wysocki > > >>> Date: Mon Jun 3 21:49:52 2013 +0200 > > >>> > > >>> PM / Runtime: Rework the "runtime idle" helper routine > > >>> --- > > >>> > > >>> We won't return 0 from here. > > >> > > >> so you never want to return 0, why don't you, then: > > >> > > >> static int intel_gpio_runtime_idle(struct device *dev) > > >> { > > >> pm_schedule_suspend(dev, 500); > > >> return -EBUSY; > > >> } > > > > > > That's how it is currently :) > > > > > > But this patch is making the function to return a different code in case > > > of error. IMHO there is not much fuctional gain with it, but I see > > > perhaps one extra info for tracing during development. > > > > > > Anyway, I'll let Xinhui to do further comment since he's the author. > > > > > > Br, David > > > > > hi ,David & Balbi > > I checked several drivers yesterday to see how they use > > pm_schedule_suspend > > then found one bug in i2c. Also I noticed gpio. > > I think returning a correct error code is important.So I change -EBUSY > > to *err*. To be honest,current code works well. > > In my experience, when I'm using fancy things like lauterbach a proper > error code may save couple of minutes in my life :) > > I keep my ack here. fair enough, sorry for the noise ;-) It could still be simplified a bit: return err ?: -EBUSY; -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: > > 于 2014年01月29日 08:13, David Cohen 写道: > > On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: > >> On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: > >>> On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > From: "xinhui.pan" > > > > intel_gpio_runtime_idle should return correct error code if it do fail. > > make it more correct even though -EBUSY is the most possible return > > value. > > > > Signed-off-by: bo.he > > Signed-off-by: xinhui.pan > > --- > > drivers/gpio/gpio-intel-mid.c |4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpio-intel-mid.c > > b/drivers/gpio/gpio-intel-mid.c > > index d1b50ef..05749a3 100644 > > --- a/drivers/gpio/gpio-intel-mid.c > > +++ b/drivers/gpio/gpio-intel-mid.c > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops > > intel_gpio_irq_ops = { > > > > static int intel_gpio_runtime_idle(struct device *dev) > > { > > - pm_schedule_suspend(dev, 500); > > + int err = pm_schedule_suspend(dev, 500); > > + if (err) > > + return err; > > return -EBUSY; > > wait, is it only me or this would look a lot better as: > > static int intel_gpio_runtime_idle(struct device *dev) > { > return pm_schedule_suspend(dev, 500); > } > >>> > >>> The reply to your suggestion is probably in this commit :) > >>> > >>> --- > >>> commit 45f0a85c8258741d11bda25c0a5669c06267204a > >>> Author: Rafael J. Wysocki > >>> Date: Mon Jun 3 21:49:52 2013 +0200 > >>> > >>> PM / Runtime: Rework the "runtime idle" helper routine > >>> --- > >>> > >>> We won't return 0 from here. > >> > >> so you never want to return 0, why don't you, then: > >> > >> static int intel_gpio_runtime_idle(struct device *dev) > >> { > >>pm_schedule_suspend(dev, 500); > >>return -EBUSY; > >> } > > > > That's how it is currently :) > > > > But this patch is making the function to return a different code in case > > of error. IMHO there is not much fuctional gain with it, but I see > > perhaps one extra info for tracing during development. > > > > Anyway, I'll let Xinhui to do further comment since he's the author. > > > > Br, David > > > hi ,David & Balbi > I checked several drivers yesterday to see how they use pm_schedule_suspend > then found one bug in i2c. Also I noticed gpio. > I think returning a correct error code is important.So I change -EBUSY > to *err*. To be honest,current code works well. In my experience, when I'm using fancy things like lauterbach a proper error code may save couple of minutes in my life :) I keep my ack here. Br, David > >> > >> just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? > >> > >> -- > >> balbi > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: 于 2014年01月29日 08:13, David Cohen 写道: On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David hi ,David Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. In my experience, when I'm using fancy things like lauterbach a proper error code may save couple of minutes in my life :) I keep my ack here. Br, David just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: 于 2014年01月29日 08:13, David Cohen 写道: On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David hi ,David Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. In my experience, when I'm using fancy things like lauterbach a proper error code may save couple of minutes in my life :) I keep my ack here. fair enough, sorry for the noise ;-) It could still be simplified a bit: return err ?: -EBUSY; -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Wed, Jan 29, 2014 at 01:52:30PM -0600, Felipe Balbi wrote: On Wed, Jan 29, 2014 at 11:12:32AM -0800, David Cohen wrote: On Wed, Jan 29, 2014 at 03:23:40PM +0800, xinhui.pan wrote: 于 2014年01月29日 08:13, David Cohen 写道: On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David hi ,David Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. In my experience, when I'm using fancy things like lauterbach a proper error code may save couple of minutes in my life :) I keep my ack here. fair enough, sorry for the noise ;-) It could still be simplified a bit: return err ?: -EBUSY; Agreed :) Xinhui, could we have this suggestion in your patch? Br, David -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
于 2014年01月29日 08:13, David Cohen 写道: > On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: >> On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: >>> On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > From: "xinhui.pan" > > intel_gpio_runtime_idle should return correct error code if it do fail. > make it more correct even though -EBUSY is the most possible return value. > > Signed-off-by: bo.he > Signed-off-by: xinhui.pan > --- > drivers/gpio/gpio-intel-mid.c |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c > index d1b50ef..05749a3 100644 > --- a/drivers/gpio/gpio-intel-mid.c > +++ b/drivers/gpio/gpio-intel-mid.c > @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops > = { > > static int intel_gpio_runtime_idle(struct device *dev) > { > - pm_schedule_suspend(dev, 500); > + int err = pm_schedule_suspend(dev, 500); > + if (err) > + return err; > return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } >>> >>> The reply to your suggestion is probably in this commit :) >>> >>> --- >>> commit 45f0a85c8258741d11bda25c0a5669c06267204a >>> Author: Rafael J. Wysocki >>> Date: Mon Jun 3 21:49:52 2013 +0200 >>> >>> PM / Runtime: Rework the "runtime idle" helper routine >>> --- >>> >>> We won't return 0 from here. >> >> so you never want to return 0, why don't you, then: >> >> static int intel_gpio_runtime_idle(struct device *dev) >> { >> pm_schedule_suspend(dev, 500); >> return -EBUSY; >> } > > That's how it is currently :) > > But this patch is making the function to return a different code in case > of error. IMHO there is not much fuctional gain with it, but I see > perhaps one extra info for tracing during development. > > Anyway, I'll let Xinhui to do further comment since he's the author. > > Br, David > hi ,David & Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. >> >> just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? >> >> -- >> balbi > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: > > On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > > > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > > > From: "xinhui.pan" > > > > > > > > intel_gpio_runtime_idle should return correct error code if it do fail. > > > > make it more correct even though -EBUSY is the most possible return > > > > value. > > > > > > > > Signed-off-by: bo.he > > > > Signed-off-by: xinhui.pan > > > > --- > > > > drivers/gpio/gpio-intel-mid.c |4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpio/gpio-intel-mid.c > > > > b/drivers/gpio/gpio-intel-mid.c > > > > index d1b50ef..05749a3 100644 > > > > --- a/drivers/gpio/gpio-intel-mid.c > > > > +++ b/drivers/gpio/gpio-intel-mid.c > > > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops > > > > intel_gpio_irq_ops = { > > > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > > { > > > > - pm_schedule_suspend(dev, 500); > > > > + int err = pm_schedule_suspend(dev, 500); > > > > + if (err) > > > > + return err; > > > > return -EBUSY; > > > > > > wait, is it only me or this would look a lot better as: > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > { > > > return pm_schedule_suspend(dev, 500); > > > } > > > > The reply to your suggestion is probably in this commit :) > > > > --- > > commit 45f0a85c8258741d11bda25c0a5669c06267204a > > Author: Rafael J. Wysocki > > Date: Mon Jun 3 21:49:52 2013 +0200 > > > > PM / Runtime: Rework the "runtime idle" helper routine > > --- > > > > We won't return 0 from here. > > so you never want to return 0, why don't you, then: > > static int intel_gpio_runtime_idle(struct device *dev) > { > pm_schedule_suspend(dev, 500); > return -EBUSY; > } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David > > just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: > On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > > From: "xinhui.pan" > > > > > > intel_gpio_runtime_idle should return correct error code if it do fail. > > > make it more correct even though -EBUSY is the most possible return value. > > > > > > Signed-off-by: bo.he > > > Signed-off-by: xinhui.pan > > > --- > > > drivers/gpio/gpio-intel-mid.c |4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c > > > index d1b50ef..05749a3 100644 > > > --- a/drivers/gpio/gpio-intel-mid.c > > > +++ b/drivers/gpio/gpio-intel-mid.c > > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops > > > = { > > > > > > static int intel_gpio_runtime_idle(struct device *dev) > > > { > > > - pm_schedule_suspend(dev, 500); > > > + int err = pm_schedule_suspend(dev, 500); > > > + if (err) > > > + return err; > > > return -EBUSY; > > > > wait, is it only me or this would look a lot better as: > > > > static int intel_gpio_runtime_idle(struct device *dev) > > { > > return pm_schedule_suspend(dev, 500); > > } > > The reply to your suggestion is probably in this commit :) > > --- > commit 45f0a85c8258741d11bda25c0a5669c06267204a > Author: Rafael J. Wysocki > Date: Mon Jun 3 21:49:52 2013 +0200 > > PM / Runtime: Rework the "runtime idle" helper routine > --- > > We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: > On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > > From: "xinhui.pan" > > > > intel_gpio_runtime_idle should return correct error code if it do fail. > > make it more correct even though -EBUSY is the most possible return value. > > > > Signed-off-by: bo.he > > Signed-off-by: xinhui.pan > > --- > > drivers/gpio/gpio-intel-mid.c |4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c > > index d1b50ef..05749a3 100644 > > --- a/drivers/gpio/gpio-intel-mid.c > > +++ b/drivers/gpio/gpio-intel-mid.c > > @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = > > { > > > > static int intel_gpio_runtime_idle(struct device *dev) > > { > > - pm_schedule_suspend(dev, 500); > > + int err = pm_schedule_suspend(dev, 500); > > + if (err) > > + return err; > > return -EBUSY; > > wait, is it only me or this would look a lot better as: > > static int intel_gpio_runtime_idle(struct device *dev) > { > return pm_schedule_suspend(dev, 500); > } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the "runtime idle" helper routine --- We won't return 0 from here. Br, David > > cheers > > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > From: "xinhui.pan" > > intel_gpio_runtime_idle should return correct error code if it do fail. > make it more correct even though -EBUSY is the most possible return value. > > Signed-off-by: bo.he > Signed-off-by: xinhui.pan > --- > drivers/gpio/gpio-intel-mid.c |4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c > index d1b50ef..05749a3 100644 > --- a/drivers/gpio/gpio-intel-mid.c > +++ b/drivers/gpio/gpio-intel-mid.c > @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { > > static int intel_gpio_runtime_idle(struct device *dev) > { > - pm_schedule_suspend(dev, 500); > + int err = pm_schedule_suspend(dev, 500); > + if (err) > + return err; > return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
Hi, Thanks for the patch :) On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: > From: "xinhui.pan" > > intel_gpio_runtime_idle should return correct error code if it do fail. > make it more correct even though -EBUSY is the most possible return value. > > Signed-off-by: bo.he > Signed-off-by: xinhui.pan Acked-by: David Cohen -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio-intel-mid: fix the incorrect return of idle callback
From: "xinhui.pan" intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he Signed-off-by: xinhui.pan --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio-intel-mid: fix the incorrect return of idle callback
From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
Hi, Thanks for the patch :) On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com Acked-by: David Cohen david.a.co...@linux.intel.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. Br, David cheers -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio-intel-mid: fix the incorrect return of idle callback
于 2014年01月29日 08:13, David Cohen 写道: On Tue, Jan 28, 2014 at 12:12:06PM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 09:24:13AM -0800, David Cohen wrote: On Tue, Jan 28, 2014 at 10:49:37AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 04:50:57PM +0800, xinhui.pan wrote: From: xinhui.pan xinhuix@intel.com intel_gpio_runtime_idle should return correct error code if it do fail. make it more correct even though -EBUSY is the most possible return value. Signed-off-by: bo.he bo...@intel.com Signed-off-by: xinhui.pan xinhuix@intel.com --- drivers/gpio/gpio-intel-mid.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-intel-mid.c b/drivers/gpio/gpio-intel-mid.c index d1b50ef..05749a3 100644 --- a/drivers/gpio/gpio-intel-mid.c +++ b/drivers/gpio/gpio-intel-mid.c @@ -394,7 +394,9 @@ static const struct irq_domain_ops intel_gpio_irq_ops = { static int intel_gpio_runtime_idle(struct device *dev) { - pm_schedule_suspend(dev, 500); + int err = pm_schedule_suspend(dev, 500); + if (err) + return err; return -EBUSY; wait, is it only me or this would look a lot better as: static int intel_gpio_runtime_idle(struct device *dev) { return pm_schedule_suspend(dev, 500); } The reply to your suggestion is probably in this commit :) --- commit 45f0a85c8258741d11bda25c0a5669c06267204a Author: Rafael J. Wysocki rafael.j.wyso...@intel.com Date: Mon Jun 3 21:49:52 2013 +0200 PM / Runtime: Rework the runtime idle helper routine --- We won't return 0 from here. so you never want to return 0, why don't you, then: static int intel_gpio_runtime_idle(struct device *dev) { pm_schedule_suspend(dev, 500); return -EBUSY; } That's how it is currently :) But this patch is making the function to return a different code in case of error. IMHO there is not much fuctional gain with it, but I see perhaps one extra info for tracing during development. Anyway, I'll let Xinhui to do further comment since he's the author. Br, David hi ,David Balbi I checked several drivers yesterday to see how they use pm_schedule_suspend then found one bug in i2c. Also I noticed gpio. I think returning a correct error code is important.So I change -EBUSY to *err*. To be honest,current code works well. just like drivers/tty/serial/mfd.c::serial_hsu_runtime_idle() is doing ? -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/