The driver is using 256 as the size while calling devm_ioremap(). The
maximum offset is already part of struct isst_mmio_range. Use the
maximum offset (end field of the struct) plus 4 as the map size to remove
hardcoded value of 256.
Signed-off-by: Srinivas Pandruvada <[email protected]>
---
drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c b/drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c
index ff49025ec085..be709e0c0c00 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c
@@ -114,13 +114,16 @@ static int isst_if_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pcu_base &= GENMASK(10, 0);
base_addr = (u64)mmio_base << 23 | (u64) pcu_base << 12;
- punit_dev->punit_mmio = devm_ioremap(&pdev->dev, base_addr, 256);
+
+ punit_dev->mmio_range = (struct isst_mmio_range *) ent->driver_data;
+
+ punit_dev->punit_mmio = devm_ioremap(&pdev->dev, base_addr,
+ punit_dev->mmio_range[1].end + sizeof(u32));
if (!punit_dev->punit_mmio)
return -ENOMEM;
mutex_init(&punit_dev->mutex);
pci_set_drvdata(pdev, punit_dev);
- punit_dev->mmio_range = (struct isst_mmio_range *) ent->driver_data;
memset(&cb, 0, sizeof(cb));
cb.cmd_size = sizeof(struct isst_if_io_reg);
--
2.41.0
On Mon, Sep 25, 2023 at 12:45:55PM -0700, Srinivas Pandruvada wrote:
> The driver is using 256 as the size while calling devm_ioremap(). The
> maximum offset is already part of struct isst_mmio_range. Use the
> maximum offset (end field of the struct) plus 4 as the map size to remove
> hardcoded value of 256.
...
> + punit_dev->mmio_range = (struct isst_mmio_range *) ent->driver_data;
> +
> + punit_dev->punit_mmio = devm_ioremap(&pdev->dev, base_addr,
> + punit_dev->mmio_range[1].end + sizeof(u32));
Can we rather fix the mmio_range driver data to have end be actually not the
offset of the last dword? (Better maybe to keep length there.)
With help of
struct resource r;
...
r = DEFINE_RES_MEM(base_addr, mmio_range.beg + mmio_range.len);
you can switch to devm_ioremap_resource() API.
--
With Best Regards,
Andy Shevchenko
On Tue, Sep 26, 2023 at 08:04:46AM -0700, srinivas pandruvada wrote:
> On Tue, 2023-09-26 at 16:16 +0300, Andy Shevchenko wrote:
> > On Mon, Sep 25, 2023 at 12:45:55PM -0700, Srinivas Pandruvada wrote:
> > > The driver is using 256 as the size while calling devm_ioremap().
> > > The
> > > maximum offset is already part of struct isst_mmio_range. Use the
> > > maximum offset (end field of the struct) plus 4 as the map size to
> > > remove
> > > hardcoded value of 256.
...
> > > +???????punit_dev->mmio_range = (struct isst_mmio_range *) ent-
> > > >driver_data;
> > > +
> > > +???????punit_dev->punit_mmio = devm_ioremap(&pdev->dev, base_addr,
> > > +??????????????????????????????????????????? punit_dev-
> > > >mmio_range[1].end + sizeof(u32));
> >
> > Can we rather fix the mmio_range driver data to have end be actually
> > not the
> > offset of the last dword? (Better maybe to keep length there.)
> >
> We can. But that has to be separate patch on top as there are other
> places this range is used.
Still you can add a third member for now and then clean up it later as it's all
in one file.
...
> > With help of
> >
> > ????????struct resource r;
> > ????????...
> > ????????r = DEFINE_RES_MEM(base_addr, mmio_range.beg +
> > mmio_range.len);
> >
> > you can switch to devm_ioremap_resource() API.
> What is the advantage of creating a resource and then call
> devm_ioremap_resource()?
It manages resource via global resource management and also prints an error
messages in case of errors.
--
With Best Regards,
Andy Shevchenko
On Tue, 2023-09-26 at 16:16 +0300, Andy Shevchenko wrote:
> On Mon, Sep 25, 2023 at 12:45:55PM -0700, Srinivas Pandruvada wrote:
> > The driver is using 256 as the size while calling devm_ioremap().
> > The
> > maximum offset is already part of struct isst_mmio_range. Use the
> > maximum offset (end field of the struct) plus 4 as the map size to
> > remove
> > hardcoded value of 256.
>
> ...
>
> > + punit_dev->mmio_range = (struct isst_mmio_range *) ent-
> > >driver_data;
> > +
> > + punit_dev->punit_mmio = devm_ioremap(&pdev->dev, base_addr,
> > + punit_dev-
> > >mmio_range[1].end + sizeof(u32));
>
> Can we rather fix the mmio_range driver data to have end be actually
> not the
> offset of the last dword? (Better maybe to keep length there.)
>
We can. But that has to be separate patch on top as there are other
places this range is used.
> With help of
>
> struct resource r;
> ...
> r = DEFINE_RES_MEM(base_addr, mmio_range.beg +
> mmio_range.len);
>
> you can switch to devm_ioremap_resource() API.
What is the advantage of creating a resource and then call
devm_ioremap_resource()?
Thanks,
Srinivas
>