On 11/8/24 21:00, Stephen Boyd wrote:
Quoting Eugen Hristev (2024-10-30 01:28:14)
On 10/30/24 02:40, Stephen Boyd wrote:
If the rpmh-rsc code didn't use writel() or readl() I'd believe
that the data member is simply a u32 container. But those
writel() and readl() functions are doing a byte swap, which
seems to imply that the data member is a native CPU endian u32
that needs to be converted to little-endian. Sounds like
BCM_TCS_CMD() should just pack things into a u32 and we can
simply remove the cpu_to_l32() stuff in the macro?
This review [1] from Evan Green on the original patch submission
requested the use of cpu_to_le32
So that's how it ended up there.
Thanks. I still don't see why this can't just be treated as a u32
and then we have writel() take care of it for us.
If the values are in the wrong endianness, e.g. 0xff11 instead of
0x11ff, the corresponding field would be filled up wrongly, even
possibly writing unwanted bits. vote_x and vote_y have a mask of length
14, so there is one byte and another 6 more bits. If the endianness of
the value is not correct, the one byte might end up written over the 6
bits and 2 extra bits which are supposed to be for another field.
In my example 0x11 should be in the first 6 bits and the 0xff in the
next byte, but if the endianness of the cpu is different, we might write
0xff on the 6 bit field.
So we must ensure that the multi-byte fields are in the correct
endianness that the hardware expects.
In other words, writel does not know about the multi-byte fields inside
this u32 which have a specific bit shift, and those fields are expected
to be in le32 order written to the hardware. Whether or not the cpu is
le32 is not important because using cpu_to_le32 will make it safe either
way.
I apologize for my not so great explanation
