Hi Suzuki,
On Fri, 13 Dec 2024 at 16:38, Suzuki K Poulose suzuki.poulose@arm.com wrote:
Hi Mike
On 13/12/2024 14:49, Mike Leach wrote:
Current coresight_timeout function spins on a bit on a test register, till bit value achieved or timeout hit.
Add another function to return the full value of the register being tested.
The patch looks good to me. However, this change doesn't bring much value, other than avoiding a read at the caller side. I guess it is simpler to leave things as is and let the caller read the register value while logging rather than providing another variant.
The reason for returning the register value is that you get the exact read value used at the time of the test - not potentially a different value read later.
I am inclined to keep the interface simpler.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----- include/linux/coresight.h | 2 + 2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcb..feb1a1db355f 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,32 +1017,37 @@ static void coresight_remove_conns(struct coresight_device *csdev) }
/**
- coresight_timeout - loop until a bit has changed to a specific register
state.
- coresight_timeout_retval - loop until a bit has changed to a specific register
state. Return final register value- @csa: coresight device access for the device
- @offset: Offset of the register from the base of the device.
- @position: the position of the bit of interest.
- @value: the value the bit should have.
*/
- @rval: the last read value of the register being tested.
- Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
- TIMEOUT_US has elapsed, which ever happens first.
-int coresight_timeout(struct csdev_access *csa, u32 offset,
int position, int value)+int coresight_timeout_retval(struct csdev_access *csa, u32 offset,
int position, int value, u32 *rval)
int i;u32 val;
int i, rc = -EAGAIN;u32 val = 0; for (i = TIMEOUT_US; i > 0; i--) { val = csdev_access_read32(csa, offset); /* waiting on the bit to go from 0 to 1 */ if (value) {
if (val & BIT(position))return 0;
if (val & BIT(position)) {rc = 0;goto return_rval;} /* waiting on the bit to go from 1 to 0 */ } else {
if (!(val & BIT(position)))return 0;
if (!(val & BIT(position))) {rc = 0;goto return_rval;} } /*@@ -1054,7 +1059,30 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, udelay(1); }
return -EAGAIN;+return_rval:
*rval = val;super minor nit: we could be flexible in accepting a "NULL" rval and thus keep the normal variants pass the NULL straight in. i.e,
if (rval) *rval = val;
Sure.
Mike
return rc;+} +EXPORT_SYMBOL_GPL(coresight_timeout_retval);
+/**
- coresight_timeout - loop until a bit has changed to a specific register
state
- @csa: coresight device access for the device
- @offset: Offset of the register from the base of the device.
- @position: the position of the bit of interest.
- @value: the value the bit should have.
- Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
- TIMEOUT_US has elapsed, which ever happens first.
- */
+int coresight_timeout(struct csdev_access *csa, u32 offset,
int position, int value)+{
u32 rval = 0;return coresight_timeout_retval(csa, offset, position, value, &rval);return core_timeout_retval(csa, offset, position, value, NULL);Suzuki
} EXPORT_SYMBOL_GPL(coresight_timeout);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 055ce5cd5c44..29cb71e82b0b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,8 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(struct coresight_device *csdev); extern int coresight_timeout(struct csdev_access *csa, u32 offset, int position, int value); +extern int coresight_timeout_retval(struct csdev_access *csa, u32 offset,
int position, int value, u32 *rval);extern int coresight_claim_device(struct coresight_device *csdev); extern int coresight_claim_device_unlocked(struct coresight_device *csdev);