[RFC 1/5] ARM: CORESIGHT: Add generic lock/unlock helpers

[Jean Pihet]

Hi Jon, Will,

On Thu, Dec 13, 2012 at 8:18 PM, Jon Hunter <jon-hunter at ti.com> wrote:
>
> On 12/13/2012 08:58 AM, Will Deacon wrote:
>> Hi Jon,
>>
>> On Wed, Dec 12, 2012 at 09:43:04PM +0000, Jon Hunter wrote:
>>> The Cross Trigger Interface (CTI) helpers in cti.h include definitions
>>> for the Coresight Lock Access Register (LAR) and Lock Status Register
>>> (LSR). These registers are already defined in coresight.h and so rather
>>> than having multiple definitions, just use the definitions from
>>> coresight.h.
>>>
>>> Add the following coresight macros ...
>>> - coresight_unlock() --> Unlocks coresight module
>>> - coresight_lock()   --> Locks coresight module
>>>
>>> Use the above macros for ETB, ETM and CTI. The do-while(0) statement
>>> has been removed from the macro as it is not a multi-line macro.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter at ti.com>
>>> ---
>>>  arch/arm/include/asm/cti.h                |   16 +++-------------
>>>  arch/arm/include/asm/hardware/coresight.h |   16 ++++++++--------
>>>  2 files changed, 11 insertions(+), 21 deletions(-)
>>
>> [...]
>>
>>> diff --git a/arch/arm/include/asm/hardware/coresight.h b/arch/arm/include/asm/hardware/coresight.h
>>> index 7ecd793..dcd0e66 100644
>>> --- a/arch/arm/include/asm/hardware/coresight.h
>>> +++ b/arch/arm/include/asm/hardware/coresight.h
>>> @@ -141,17 +141,17 @@
>>>  #define ETBFF_TRIGEVT               BIT(9)
>>>  #define ETBFF_TRIGFL                BIT(10)
>>>
>>> -#define etb_writel(t, v, x) \
>>> -    (__raw_writel((v), (t)->etb_regs + (x)))
>>> +#define etb_writel(t, v, x) (__raw_writel((v), (t)->etb_regs + (x)))
>>>  #define etb_readl(t, x) (__raw_readl((t)->etb_regs + (x)))
>>>
>>> -#define etm_lock(t) do { etm_writel((t), 0, CSMR_LOCKACCESS); } while (0)
>>> -#define etm_unlock(t) \
>>> -    do { etm_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
>>> +#define etb_lock(t) coresight_lock((t)->etb_regs)
>>> +#define etb_unlock(t) coresight_unlock((t)->etb_regs)
>>> +#define etm_lock(t) coresight_lock((t)->etm_regs)
>>> +#define etm_unlock(t) coresight_unlock((t)->etm_regs)
>>>
>>> -#define etb_lock(t) do { etb_writel((t), 0, CSMR_LOCKACCESS); } while (0)
>>> -#define etb_unlock(t) \
>>> -    do { etb_writel((t), UNLOCK_MAGIC, CSMR_LOCKACCESS); } while (0)
>>> +#define coresight_lock(base) (__raw_writel(0, base + CSMR_LOCKACCESS))
>>> +#define coresight_unlock(base) \
>>> +    (__raw_writel(UNLOCK_MAGIC, base + CSMR_LOCKACCESS))
>>>
>>>  #endif /* __ASM_HARDWARE_CORESIGHT_H */
>>
>> How about we take this opportunity to divorce the more general coresight
>> bits from the etb specific parts, like you've done for the cti. We could
>> move the ETB stuff out into asm/etb.h (although it might be nice to keep all
>> the coresight device headers in one place... answers on a postcard) and just
>> have the architected coresight functionality in this header.
>
> Yes I have been wondering about that too. Long term it would be good to
> find a home in the drivers directory for all these coresight devices
> too. For now, we could extract the etb/etm parts from coresight.h into
> etb.h and etm.h, respectively.

I am now writing a HW trace driver that extracts traces from the CMI
and PMI IP blocks (and later L2CC). It is still in early development
status.

The idea I have in mind is to have the implementation in
drivers/power/hw_trace. Eventually this directory would contain the
drivers for ETM/ETB, CMI, PMI and also the coresight support. It would
be architected so that the lower level drivers export the necessary
functions for the higher level code to use the resource. Example: the
PMI driver would use the ETB and coresight drivers.

What do you think?

Regards,
Jean

>
> Cheers
> Jon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html