| Message ID | 1448996309-15220-2-git-send-email-jgunthorpe@obsidianresearch.com |
|---|---|
| State | New |
| Headers | show |
Hello, On Tue, Dec 01, 2015 at 11:58:27AM -0700, Jason Gunthorpe wrote: > Instead of clearing the global interrupts flag when any device > does not have an interrupt just pass -1 through tpm_info.irq. > > The only thing that asks for autoprobing is the force=1 path. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/char/tpm/tpm_tis.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 8a3509cb10da..0a2d94f3d679 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -69,7 +69,7 @@ enum tis_defaults { > struct tpm_info { > unsigned long start; > unsigned long len; > - unsigned int irq; > + int irq; I'd add a comment here about the possible values of irq and their interpretation. Something like: /* * irq > 0 means: use irq $irq; * irq = 0 means: autoprobe for an irq; * irq = -1 means: no irq support */ > }; > > static struct tpm_info tis_default_info = { > @@ -807,7 +807,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > /* INTERRUPT Setup */ > init_waitqueue_head(&chip->vendor.read_queue); > init_waitqueue_head(&chip->vendor.int_queue); > - if (interrupts) { > + if (interrupts && tpm_info->irq != -1) { > if (tpm_info->irq) { > tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED, > tpm_info->irq); > @@ -895,9 +895,9 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume); > > #ifdef CONFIG_PNP > static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, > - const struct pnp_device_id *pnp_id) > + const struct pnp_device_id *pnp_id) > { > - struct tpm_info tpm_info = tis_default_info; > + struct tpm_info tpm_info = {}; > acpi_handle acpi_dev_handle = NULL; > > tpm_info.start = pnp_mem_start(pnp_dev, 0); > @@ -906,7 +906,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, > if (pnp_irq_valid(pnp_dev, 0)) > tpm_info.irq = pnp_irq(pnp_dev, 0); > else > - interrupts = false; > + tpm_info.irq = -1; It's definitly a nice improvement of this patch that the init functions don't change the module parameter any more. (I didn't check if all changes are gone now, but at least it's two modifications less.) Best regards Uwe
On Tue, Dec 01, 2015 at 08:19:18PM +0100, Uwe Kleine-König wrote: > I'd add a comment here about the possible values of irq and their > interpretation. Something like: Done > It's definitly a nice improvement of this patch that the init functions > don't change the module parameter any more. (I didn't check if all > changes are gone now, but at least it's two modifications less.) I checked, this gets them all. Jason ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 8a3509cb10da..0a2d94f3d679 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -69,7 +69,7 @@ enum tis_defaults { struct tpm_info { unsigned long start; unsigned long len; - unsigned int irq; + int irq; }; static struct tpm_info tis_default_info = { @@ -807,7 +807,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, /* INTERRUPT Setup */ init_waitqueue_head(&chip->vendor.read_queue); init_waitqueue_head(&chip->vendor.int_queue); - if (interrupts) { + if (interrupts && tpm_info->irq != -1) { if (tpm_info->irq) { tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED, tpm_info->irq); @@ -895,9 +895,9 @@ static SIMPLE_DEV_PM_OPS(tpm_tis_pm, tpm_pm_suspend, tpm_tis_resume); #ifdef CONFIG_PNP static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, - const struct pnp_device_id *pnp_id) + const struct pnp_device_id *pnp_id) { - struct tpm_info tpm_info = tis_default_info; + struct tpm_info tpm_info = {}; acpi_handle acpi_dev_handle = NULL; tpm_info.start = pnp_mem_start(pnp_dev, 0); @@ -906,7 +906,7 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, if (pnp_irq_valid(pnp_dev, 0)) tpm_info.irq = pnp_irq(pnp_dev, 0); else - interrupts = false; + tpm_info.irq = -1; #ifdef CONFIG_ACPI if (pnp_acpi_device(pnp_dev)) { @@ -977,13 +977,14 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data) static int tpm_tis_acpi_init(struct acpi_device *acpi_dev) { struct list_head resources; - struct tpm_info tpm_info = tis_default_info; + struct tpm_info tpm_info = {}; int ret; if (!is_fifo(acpi_dev)) return -ENODEV; INIT_LIST_HEAD(&resources); + tpm_info.irq = -1; ret = acpi_dev_get_resources(acpi_dev, &resources, tpm_check_resource, &tpm_info); if (ret < 0) @@ -991,9 +992,6 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev) acpi_dev_free_resource_list(&resources); - if (!tpm_info.irq) - interrupts = false; - if (is_itpm(acpi_dev)) itpm = true;
Instead of clearing the global interrupts flag when any device does not have an interrupt just pass -1 through tpm_info.irq. The only thing that asks for autoprobing is the force=1 path. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/char/tpm/tpm_tis.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)