| Message ID | 20230925194555.966743-4-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Minor SST optimizations | expand |
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.
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 >
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.
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);
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 <srinivas.pandruvada@linux.intel.com> --- drivers/platform/x86/intel/speed_select_if/isst_if_mmio.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)