On 02/22/2018 06:26 AM, Eugeniy Paltsev wrote:
To read any data from ARconnect we have special interface
which includes two AUX registers: MCIP_CMD and MCIP_READBACK.
We write command to MCIP_CMD and read data from MCIP_READBACK
after that.

We have only one instance of this registers per cluster, so we need
to held global lock before access them. This lock is defined
in arch/arc/kernel/mcip.c

To read GFRC value we also use MCIP_CMD/MCIP_READBACK pair, but
we take only local lock instead of global 'mcip_lock' lock:

This was not an omission but on purpose given the hardware is designed that way. We obviously want to avoid the spinlock when possible specially when everyone is only reading it and timer readout needs to be super fast. Thing is each core has a "private snapshot" of internal counter. So the READ_H* cmd read their own snapshots, without need for serializing globally. We do however want a consistent LO-HI - hence the need to disable local interrupts around.

static u64 arc_read_gfrc(struct clocksource *cs)
        unsigned long flags;
        u32 l, h;


        __mcip_cmd(CMD_GFRC_READ_LO, 0);
        l = read_aux_reg(ARC_REG_MCIP_READBACK);

        __mcip_cmd(CMD_GFRC_READ_HI, 0);
        h = read_aux_reg(ARC_REG_MCIP_READBACK);


        return (((u64)h) << 32) | l;

So we can break any command (like inter core interrupt send)
which uses MCIP_CMD/MCIP_READBACK pair when we read time from GFRC.

One possible solution is to create function like gfrc_read() in mcip.c
which will use global 'mcip_lock' and call it from current 'arc_read_gfrc'

Or we can create a wrapper like 'mcip_read' in arch/arc/kernel/mcip.c
with next functionality:
u32 mcip_read(u32 cmd)
        u32 ret;


        __mcip_cmd(cmd, 0);
        ret = read_aux_reg(ARC_REG_MCIP_READBACK);


        return ret;

This is a good idea in general. Mind you however that all use cases are different - as in we do multiple things not juct cmd + readback. e.g. for GFRC halt progrog we do cmd + readback + cmd. but see if you can come up with something reasonable to encapsulate the spin lock.

Reply via email to