GIC v3 function gicd_set_ipriorityr funtion is buggy ! need update like above

[mariusxp]

void gicd_set_ipriorityr(unsigned int base, unsigned int id, unsigned int pri)
{
//read/modify/write 8 bits
unsigned n = id >> IPRIORITYR_SHIFT;
// select the byte within the word
unsigned int bit_shift = (id & 0x3) << 3;
unsigned int prior_val = GET_REG_WORD_VAL(base + GICD_IPRIORITYR + (n << 2));
prior_val &= ~(GIC_PRI_MASK << bit_shift);
prior_val |= ((pri & GIC_PRI_MASK) << bit_shift);
SET_REG_WORD(base + GICD_IPRIORITYR + (n << 2), prior_val);
}

[ghost]

Why do you need to do the read, mask and write manually? The register is R/W according to ARM Generic Interrupt Controller Architecture Specification (IHI 0069C), so just writing a byte should work.

[mariusxp]

please read with care the specs:
For interrupt ID m, when DIV and MOD are the integer division and modulo operations:
• The corresponding GICD_IPRIORITYR number, n, is given by n = m DIV 4.
• The offset of the required GICD_IPRIORITYR register is (0x400 + (4*n)).
• The byte offset of the required Priority field in this register is m MOD 4, where:
— Byte offset 0 refers to register bits [7:0].
— Byte offset 1 refers to register bits [15:8].
— Byte offset 2 refers to register bits [23:16].
— Byte offset 3 refers to register bits [31:24].

it is not so easy you pointed it

[ghost]

Well, but that only means that for interrupt ID m the corresponding address is GICD_IPRIORITYR + id, just explained in a really convoluted way. That explanation considers each register as a 32-bit register. If you consider them as 8-bit registers then it is just like I have explained.

[soby-mathew]

• The corresponding GICD_IPRIORITYR number, n, is given by n = m DIV 4.
• The offset of the required GICD_IPRIORITYR register is (0x400 + (4*n)).
• The byte offset of the required Priority field in this register is m MOD 4, where:
— Byte offset 0 refers to register bits [7:0].
— Byte offset 1 refers to register bits [15:8].
— Byte offset 2 refers to register bits [23:16].
— Byte offset 3 refers to register bits [31:24].

Yes, its is a bitfield of 4 fields and each field is accessible as a byte. The read-modify-write sequence will not work in a multi core system unless there can be global locking scheme shared across multiple software stacks.

void gicd_set_ipriorityr(uintptr_t base, unsigned int id, unsigned int pri)
{
	uint8_t val = pri & GIC_PRI_MASK;
	mmio_write_8(base + GICD_IPRIORITYR + id, val);
}

Please let us know if you see any corruption of priority values with that implementation.