| Message ID | 20170712123101.26996-1-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2017-07-12 at 14:31 +0200, Hans de Goede wrote: > WARN_ON causes a backtrace to get logged which is only useful for > kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG "...") > should be used. > > This fixes users running userspace software to monitor kernel oopses > getting a false positive bug-report every boot because of the wrong > use of WARN_ON. I'm not against the patch, looks good to me, though can you shed a bit of light what is the FW bug you are experiencing? > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/pinctrl/intel/pinctrl-baytrail.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c > b/drivers/pinctrl/intel/pinctrl-baytrail.c > index fa3c5758ac67..0f3a02495aeb 100644 > --- a/drivers/pinctrl/intel/pinctrl-baytrail.c > +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c > @@ -981,12 +981,12 @@ static int byt_gpio_request_enable(struct > pinctrl_dev *pctl_dev, > */ > value = readl(reg) & BYT_PIN_MUX; > gpio_mux = byt_get_gpio_mux(vg, offset); > - if (WARN_ON(gpio_mux != value)) { > + if (gpio_mux != value) { > value = readl(reg) & ~BYT_PIN_MUX; > value |= gpio_mux; > writel(value, reg); > > - dev_warn(&vg->pdev->dev, > + dev_warn(&vg->pdev->dev, FW_BUG > "pin %u forcibly re-configured as GPIO\n", > offset); > } >
Hi, On 12-07-17 14:38, Andy Shevchenko wrote: > On Wed, 2017-07-12 at 14:31 +0200, Hans de Goede wrote: >> WARN_ON causes a backtrace to get logged which is only useful for >> kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG "...") >> should be used. >> >> This fixes users running userspace software to monitor kernel oopses >> getting a false positive bug-report every boot because of the wrong >> use of WARN_ON. > > I'm not against the patch, looks good to me, though can you shed a bit > of light what is the FW bug you are experiencing? The bug is the code path which has the WARN_ON triggering, which means that the ACPI tables / or a driver want to use a pin as gpio and it has been setup differently (as a special function) by the firmware. Regards, Hans > >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/pinctrl/intel/pinctrl-baytrail.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c >> b/drivers/pinctrl/intel/pinctrl-baytrail.c >> index fa3c5758ac67..0f3a02495aeb 100644 >> --- a/drivers/pinctrl/intel/pinctrl-baytrail.c >> +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c >> @@ -981,12 +981,12 @@ static int byt_gpio_request_enable(struct >> pinctrl_dev *pctl_dev, >> */ >> value = readl(reg) & BYT_PIN_MUX; >> gpio_mux = byt_get_gpio_mux(vg, offset); >> - if (WARN_ON(gpio_mux != value)) { >> + if (gpio_mux != value) { >> value = readl(reg) & ~BYT_PIN_MUX; >> value |= gpio_mux; >> writel(value, reg); >> >> - dev_warn(&vg->pdev->dev, >> + dev_warn(&vg->pdev->dev, FW_BUG >> "pin %u forcibly re-configured as GPIO\n", >> offset); >> } >> > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-07-12 at 14:44 +0200, Hans de Goede wrote: > On 12-07-17 14:38, Andy Shevchenko wrote: > > On Wed, 2017-07-12 at 14:31 +0200, Hans de Goede wrote: > > > WARN_ON causes a backtrace to get logged which is only useful for > > > kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG > > > "...") > > > should be used. > > > > > > This fixes users running userspace software to monitor kernel > > > oopses > > > getting a false positive bug-report every boot because of the > > > wrong > > > use of WARN_ON. > > > > I'm not against the patch, looks good to me, though can you shed a > > bit > > of light what is the FW bug you are experiencing? > > The bug is the code path which has the WARN_ON triggering, which means > that the ACPI tables / or a driver want to use a pin as gpio and it > has > been setup differently (as a special function) by the firmware. Yes, and what I'm asking are the details of it, what product, what device needs that pin, and why do we think that is the rightful request from the OS?
Hi, On 12-07-17 15:12, Andy Shevchenko wrote: > On Wed, 2017-07-12 at 14:44 +0200, Hans de Goede wrote: >> On 12-07-17 14:38, Andy Shevchenko wrote: >>> On Wed, 2017-07-12 at 14:31 +0200, Hans de Goede wrote: >>>> WARN_ON causes a backtrace to get logged which is only useful for >>>> kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG >>>> "...") >>>> should be used. >>>> >>>> This fixes users running userspace software to monitor kernel >>>> oopses >>>> getting a false positive bug-report every boot because of the >>>> wrong >>>> use of WARN_ON. >>> >>> I'm not against the patch, looks good to me, though can you shed a >>> bit >>> of light what is the FW bug you are experiencing? >> >> The bug is the code path which has the WARN_ON triggering, which means >> that the ACPI tables / or a driver want to use a pin as gpio and it >> has >> been setup differently (as a special function) by the firmware. > > Yes, and what I'm asking are the details of it, what product, what > device needs that pin, and why do we think that is the rightful request > from the OS? I will need to do some digging to get you these details, I will get back to you on that later. This discussion really is orthogonal to this patch, calling WARN_ON for something outside of the control of the kernel clearly is the wrong thing to do. So in the mean time can I have your ack for this patch ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-07-12 at 16:18 +0200, Hans de Goede wrote: > On 12-07-17 15:12, Andy Shevchenko wrote: > > On Wed, 2017-07-12 at 14:44 +0200, Hans de Goede wrote: > > > On 12-07-17 14:38, Andy Shevchenko wrote: > > > > On Wed, 2017-07-12 at 14:31 +0200, Hans de Goede wrote: > > > > > > > > that the ACPI tables / or a driver want to use a pin as gpio and > > > it > > > has > > > been setup differently (as a special function) by the firmware. > > > > Yes, and what I'm asking are the details of it, what product, what > > device needs that pin, and why do we think that is the rightful > > request > > from the OS? > > I will need to do some digging to get you these details, I will get > back to you on that later. Thanks! > This discussion really is orthogonal to this patch, Correct. I would like to know the details to have this knowledge in case of something similar might happen with other device(s). > calling WARN_ON > for something outside of the control of the kernel clearly is the > wrong > thing to do. So in the mean time can I have your ack for this patch ? As I said, patch is okay to me. Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
On Wed, Jul 12, 2017 at 2:31 PM, Hans de Goede <hdegoede@redhat.com> wrote: > WARN_ON causes a backtrace to get logged which is only useful for > kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG "...") > should be used. > > This fixes users running userspace software to monitor kernel oopses > getting a false positive bug-report every boot because of the wrong > use of WARN_ON. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Patch applied with Andy's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" 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/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index fa3c5758ac67..0f3a02495aeb 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -981,12 +981,12 @@ static int byt_gpio_request_enable(struct pinctrl_dev *pctl_dev, */ value = readl(reg) & BYT_PIN_MUX; gpio_mux = byt_get_gpio_mux(vg, offset); - if (WARN_ON(gpio_mux != value)) { + if (gpio_mux != value) { value = readl(reg) & ~BYT_PIN_MUX; value |= gpio_mux; writel(value, reg); - dev_warn(&vg->pdev->dev, + dev_warn(&vg->pdev->dev, FW_BUG "pin %u forcibly re-configured as GPIO\n", offset); }
WARN_ON causes a backtrace to get logged which is only useful for kernel bugs. For signalling a firmware bug dev_warn(dev, FW_BUG "...") should be used. This fixes users running userspace software to monitor kernel oopses getting a false positive bug-report every boot because of the wrong use of WARN_ON. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/pinctrl/intel/pinctrl-baytrail.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)