| Message ID | 20230327050537.30861-3-anshuman.khandual@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | coresight: etm4x: Migrate ACPI AMBA devices to platform driver | expand |
On 27/03/2023 06:05, Anshuman Khandual wrote: > 'struct etm4_drvdata' itself can carry the base address before etm4_probe() > gets called. Just drop that redundant argument from etm4_probe(). > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Leo Yan <leo.yan@linaro.org> > Cc: coresight@lists.linaro.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 10119c223fbe..5d77571a8df9 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -2048,7 +2048,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) > return 0; > } > > -static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) > +static int etm4_probe(struct device *dev, u32 etm_pid) > { > struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); > struct csdev_access access = { 0 }; > @@ -2069,8 +2069,6 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) > return -ENOMEM; > } > > - drvdata->base = base; > - > spin_lock_init(&drvdata->spinlock); > > drvdata->cpu = coresight_get_cpu(dev); > @@ -2124,8 +2122,9 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) > if (!drvdata) > return -ENOMEM; > > + drvdata->base = base; > dev_set_drvdata(dev, drvdata); > - ret = etm4_probe(dev, base, id->id); > + ret = etm4_probe(dev, id->id); > if (!ret) > pm_runtime_put(&adev->dev); > > @@ -2141,6 +2140,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) > if (!drvdata) > return -ENOMEM; > > + drvdata->base = NULL; Very minor point, drvdata is zero alloced so it doesn't make sense to zero this field but not the others. It's harmless, but it might imply something and confuse someone. Either way: Reviewed-by: James Clark <james.clark@arm.com> > dev_set_drvdata(&pdev->dev, drvdata); > pm_runtime_get_noresume(&pdev->dev); > pm_runtime_set_active(&pdev->dev); > @@ -2151,7 +2151,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) > * HW by reading appropriate registers on the HW > * and thus we could skip the PID. > */ > - ret = etm4_probe(&pdev->dev, NULL, 0); > + ret = etm4_probe(&pdev->dev, 0); > > pm_runtime_put(&pdev->dev); > return ret;
On 4/4/23 20:52, James Clark wrote: > > On 27/03/2023 06:05, Anshuman Khandual wrote: >> 'struct etm4_drvdata' itself can carry the base address before etm4_probe() >> gets called. Just drop that redundant argument from etm4_probe(). >> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >> Cc: Mike Leach <mike.leach@linaro.org> >> Cc: Leo Yan <leo.yan@linaro.org> >> Cc: coresight@lists.linaro.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index 10119c223fbe..5d77571a8df9 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -2048,7 +2048,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) >> return 0; >> } >> >> -static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) >> +static int etm4_probe(struct device *dev, u32 etm_pid) >> { >> struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); >> struct csdev_access access = { 0 }; >> @@ -2069,8 +2069,6 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) >> return -ENOMEM; >> } >> >> - drvdata->base = base; >> - >> spin_lock_init(&drvdata->spinlock); >> >> drvdata->cpu = coresight_get_cpu(dev); >> @@ -2124,8 +2122,9 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) >> if (!drvdata) >> return -ENOMEM; >> >> + drvdata->base = base; >> dev_set_drvdata(dev, drvdata); >> - ret = etm4_probe(dev, base, id->id); >> + ret = etm4_probe(dev, id->id); >> if (!ret) >> pm_runtime_put(&adev->dev); >> >> @@ -2141,6 +2140,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) >> if (!drvdata) >> return -ENOMEM; >> >> + drvdata->base = NULL; > Very minor point, drvdata is zero alloced so it doesn't make sense to > zero this field but not the others. It's harmless, but it might imply > something and confuse someone. Just to keep changes to both call sites of etm4_probe() similar i.e etm4_probe()'s 'base' argument being pre-assigned as drvdata->base, let's keep the NULL assignment above unchanged. > > Either way: > Reviewed-by: James Clark <james.clark@arm.com> >
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 10119c223fbe..5d77571a8df9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2048,7 +2048,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) return 0; } -static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +static int etm4_probe(struct device *dev, u32 etm_pid) { struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct csdev_access access = { 0 }; @@ -2069,8 +2069,6 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) return -ENOMEM; } - drvdata->base = base; - spin_lock_init(&drvdata->spinlock); drvdata->cpu = coresight_get_cpu(dev); @@ -2124,8 +2122,9 @@ static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) if (!drvdata) return -ENOMEM; + drvdata->base = base; dev_set_drvdata(dev, drvdata); - ret = etm4_probe(dev, base, id->id); + ret = etm4_probe(dev, id->id); if (!ret) pm_runtime_put(&adev->dev); @@ -2141,6 +2140,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) if (!drvdata) return -ENOMEM; + drvdata->base = NULL; dev_set_drvdata(&pdev->dev, drvdata); pm_runtime_get_noresume(&pdev->dev); pm_runtime_set_active(&pdev->dev); @@ -2151,7 +2151,7 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) * HW by reading appropriate registers on the HW * and thus we could skip the PID. */ - ret = etm4_probe(&pdev->dev, NULL, 0); + ret = etm4_probe(&pdev->dev, 0); pm_runtime_put(&pdev->dev); return ret;
'struct etm4_drvdata' itself can carry the base address before etm4_probe() gets called. Just drop that redundant argument from etm4_probe(). Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: Mike Leach <mike.leach@linaro.org> Cc: Leo Yan <leo.yan@linaro.org> Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)