Messages in this thread | | | Date | Tue, 1 Dec 2015 20:22:40 +0100 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource |
| |
Hello,
On Tue, Dec 01, 2015 at 11:58:28AM -0700, Jason Gunthorpe wrote: > This does a request_resource under the covers which means tis holds a > lock on the memory range it is using so other drivers cannot grab it. > When doing probing it is important to ensure that other drivers are > not using the same range before tis starts touching it. > > To do this flow the actual struct resource from the device right > through to devm_ioremap_resource. This ensures all the proper resource > meta-data is carried down. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/char/tpm/tpm_tis.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 0a2d94f3d679..1032855c46b2 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -67,14 +67,16 @@ enum tis_defaults { > }; > > struct tpm_info { > - unsigned long start; > - unsigned long len; > + struct resource res; > int irq; > }; > > static struct tpm_info tis_default_info = { > - .start = TIS_MEM_BASE, > - .len = TIS_MEM_LEN, > + .res = { > + .start = TIS_MEM_BASE, > + .end = TIS_MEM_BASE + TIS_MEM_LEN - 1, > + .flags = IORESOURCE_MEM, > + }, > .irq = 0, > }; > > @@ -716,7 +718,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > chip->acpi_dev_handle = acpi_dev_handle; > #endif > > - chip->vendor.iobase = devm_ioremap(dev, tpm_info->start, tpm_info->len); > + chip->vendor.iobase = devm_ioremap_resource(dev, &tpm_info->res); > if (!chip->vendor.iobase) > return -EIO; > > @@ -899,9 +901,12 @@ static int tpm_tis_pnp_init(struct pnp_dev *pnp_dev, > { > struct tpm_info tpm_info = {}; > acpi_handle acpi_dev_handle = NULL; > + struct resource *res; > > - tpm_info.start = pnp_mem_start(pnp_dev, 0); > - tpm_info.len = pnp_mem_len(pnp_dev, 0); > + res = pnp_get_resource(pnp_dev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENODEV; > + memcpy(&tpm_info.res, res, sizeof(*res));
I think you can do
tpm_info.res = res;
here, which IMHO reads nicer and maybe is even more efficient (I don't know much about x86).
> if (pnp_irq_valid(pnp_dev, 0)) > tpm_info.irq = pnp_irq(pnp_dev, 0); > @@ -964,12 +969,9 @@ static int tpm_check_resource(struct acpi_resource *ares, void *data) > struct tpm_info *tpm_info = (struct tpm_info *) data; > struct resource res; > > - if (acpi_dev_resource_interrupt(ares, 0, &res)) { > + if (acpi_dev_resource_interrupt(ares, 0, &res)) > tpm_info->irq = res.start; > - } else if (acpi_dev_resource_memory(ares, &res)) { > - tpm_info->start = res.start; > - tpm_info->len = resource_size(&res); > - } > + acpi_dev_resource_memory(ares, &tpm_info->res); > > return 1; > } > @@ -992,6 +994,9 @@ static int tpm_tis_acpi_init(struct acpi_device *acpi_dev) > > acpi_dev_free_resource_list(&resources); > > + if (resource_size(&tpm_info.res) == 0) > + return -ENODEV; > +
Does this result in an error message from the upper layers?
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
| |