Messages in this thread | | | Date | Wed, 29 Jul 2020 15:01:05 -0600 | From | Mathieu Poirier <> | Subject | Re: [RFC PATCH 03/14] coresight: tpiu: Use coresight device access abstraction |
| |
On Wed, Jul 22, 2020 at 06:20:29PM +0100, Suzuki K Poulose wrote: > TPIU driver access the device before the coresight device > is registered. In other words, before the drvdata->csdev > is valid. Thus, we need to make sure that the csdev_access > is valid for both the invocations. Switch to using the > csdev_access directly instead of relying on availability > of drvdata->csdev.
I'm not sure all of the above is needed and based on the wording I could easily see this patch being selected for stable backport, which would be a mistak.
The gist of this patch is that we are moving to the access abstraction and the changelog should reflect that.
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Mike Leach <mike.leach@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/coresight-tpiu.c | 30 +++++++++----------- > 1 file changed, 13 insertions(+), 17 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c > index 7ef7649f48ad..84ff4bf5d3b8 100644 > --- a/drivers/hwtracing/coresight/coresight-tpiu.c > +++ b/drivers/hwtracing/coresight/coresight-tpiu.c > @@ -60,49 +60,45 @@ struct tpiu_drvdata { > struct coresight_device *csdev; > }; > > -static void tpiu_enable_hw(struct tpiu_drvdata *drvdata) > +static void tpiu_enable_hw(struct csdev_access *csa) > { > - CS_UNLOCK(drvdata->base); > + CS_UNLOCK(csa->base); > > /* TODO: fill this up */ > > - CS_LOCK(drvdata->base); > + CS_LOCK(csa->base); > } > > static int tpiu_enable(struct coresight_device *csdev, u32 mode, void *__unused) > { > - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > - > - tpiu_enable_hw(drvdata); > + tpiu_enable_hw(&csdev->access); > atomic_inc(csdev->refcnt); > dev_dbg(&csdev->dev, "TPIU enabled\n"); > return 0; > } > > -static void tpiu_disable_hw(struct tpiu_drvdata *drvdata) > +static void tpiu_disable_hw(struct csdev_access *csa) > { > - CS_UNLOCK(drvdata->base); > + CS_UNLOCK(csa->base); > > /* Clear formatter and stop on flush */ > - writel_relaxed(FFCR_STOP_FI, drvdata->base + TPIU_FFCR); > + csdev_access_relaxed_write32(csa, FFCR_STOP_FI, TPIU_FFCR); > /* Generate manual flush */ > - writel_relaxed(FFCR_STOP_FI | FFCR_FON_MAN, drvdata->base + TPIU_FFCR); > + csdev_access_relaxed_write32(csa, FFCR_STOP_FI | FFCR_FON_MAN, TPIU_FFCR); > /* Wait for flush to complete */ > - coresight_timeout(drvdata->base, TPIU_FFCR, FFCR_FON_MAN_BIT, 0); > + coresight_timeout(csa->base, TPIU_FFCR, FFCR_FON_MAN_BIT, 0); > /* Wait for formatter to stop */ > - coresight_timeout(drvdata->base, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1); > + coresight_timeout(csa->base, TPIU_FFSR, FFSR_FT_STOPPED_BIT, 1); > > - CS_LOCK(drvdata->base); > + CS_LOCK(csa->base); > } > > static int tpiu_disable(struct coresight_device *csdev) > { > - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > - > if (atomic_dec_return(csdev->refcnt)) > return -EBUSY; > > - tpiu_disable_hw(drvdata); > + tpiu_disable_hw(&csdev->access); > > dev_dbg(&csdev->dev, "TPIU disabled\n"); > return 0; > @@ -152,7 +148,7 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) > desc.access.base = base;
Any reason for introducing the above in patch 02? I would have done that as part of this patch... Also part of this patch I would remove drvdata::base since it is no longer needed.
I'm out of time for today - I will continue tomorrow.
Regards, Mathieu
> > /* Disable tpiu to support older devices */ > - tpiu_disable_hw(drvdata); > + tpiu_disable_hw(&desc.access); > > pdata = coresight_get_platform_data(dev); > if (IS_ERR(pdata)) > -- > 2.24.1 >
| |