| Message ID | 1446740353-15235-5-git-send-email-martin.wilck@ts.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 05, 2015 at 05:19:11PM +0100, martin.wilck@ts.fujitsu.com wrote: > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com> > > IRQ probing can take a long time and irrtitate users. > Inform users what's going on. > > Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com> > --- > drivers/char/tpm/tpm_tis.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 7619035..f5d7d52 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -779,6 +779,10 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > irq_e = 15; > } > > + dev_info(dev, "Probing IRQ - this may take some time.\n"); This is the only line that makes sense to me. Other lines add more clutter than bring benefit. > + dev_info(dev, "\"genirq: Flags mismatch\" warnings may be logged while probing;\n"); > + dev_info(dev, "they can be safely ignored.\n"); > + dev_info(dev, "You may skip IRQ probing with parameter \"tpm_tis.interrupts=0\"\n"); > for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) { > dev_dbg(dev, "Probing irq %d\n", i); > iowrite8(i, chip->vendor.iobase + > -- > 1.8.3.1 /Jarkko ------------------------------------------------------------------------------
Hi Jarkko, On Do, 2015-11-05 at 23:44 +0200, Jarkko Sakkinen wrote: > On Thu, Nov 05, 2015 at 05:19:11PM +0100, martin.wilck@ts.fujitsu.com wrote: > > From: Martin Wilck <Martin.Wilck@ts.fujitsu.com> > > > > IRQ probing can take a long time and irrtitate users. > > Inform users what's going on. > > > > Signed-off-by: Martin Wilck <Martin.Wilck@ts.fujitsu.com> > > --- > > drivers/char/tpm/tpm_tis.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > > index 7619035..f5d7d52 100644 > > --- a/drivers/char/tpm/tpm_tis.c > > +++ b/drivers/char/tpm/tpm_tis.c > > @@ -779,6 +779,10 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > > irq_e = 15; > > } > > > > + dev_info(dev, "Probing IRQ - this may take some time.\n"); > > This is the only line that makes sense to me. Other lines add more > clutter than bring benefit. If any of the probed IRQs has already been taken by a handler that hasn't set IRQF_SHARED, probing will cause seriously ugly kernel stack traces starting with a "genirq: Flags mismatch" message. Have you seen those? They look highly irritating even to experienced users. I see no way to avoid this, therefore I thought a "don't worry" message would do no harm. I know there's sort of a contradiction between this statement and my other patch that actually introduces IRQF_SHARED. The intention of the other patch is to avoid false probing. The intention of this one is just to make the (almost) necessarily generated kernel traces look less scary. Regards Martin > > > + dev_info(dev, "\"genirq: Flags mismatch\" warnings may be logged while probing;\n"); > > + dev_info(dev, "they can be safely ignored.\n"); > > + dev_info(dev, "You may skip IRQ probing with parameter \"tpm_tis.interrupts=0\"\n"); > > for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) { > > dev_dbg(dev, "Probing irq %d\n", i); > > iowrite8(i, chip->vendor.iobase + ------------------------------------------------------------------------------
On Tue, Nov 17, 2015 at 08:51:30PM +0100, Wilck, Martin wrote: > If any of the probed IRQs has already been taken by a handler that > hasn't set IRQF_SHARED, probing will cause seriously ugly kernel stack > traces starting with a "genirq: Flags mismatch" message. Have you seen > those? They look highly irritating even to experienced users. I see no > way to avoid this, therefore I thought a "don't worry" message would do > no harm. > > I know there's sort of a contradiction between this statement and my > other patch that actually introduces IRQF_SHARED. The intention of the > other patch is to avoid false probing. The intention of this one is just > to make the (almost) necessarily generated kernel traces look less > scary. And adds more traffic to klog without any major benefit. I don't even know if we can talk about user convenience here because no standard user reads klog. And those who read it would probably appreciate that each subsystem keeps its traffic minimal and to the point. > Regards > Martin /Jarkko ------------------------------------------------------------------------------
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 7619035..f5d7d52 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -779,6 +779,10 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, irq_e = 15; } + dev_info(dev, "Probing IRQ - this may take some time.\n"); + dev_info(dev, "\"genirq: Flags mismatch\" warnings may be logged while probing;\n"); + dev_info(dev, "they can be safely ignored.\n"); + dev_info(dev, "You may skip IRQ probing with parameter \"tpm_tis.interrupts=0\"\n"); for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) { dev_dbg(dev, "Probing irq %d\n", i); iowrite8(i, chip->vendor.iobase +