On Fri, Jan 19, 2024 at 11:49:17AM +0000, Charles Keepax wrote:
On Thu, Jan 18, 2024 at 07:06:13PM +0200, andy.shevchenko@gmail.com wrote:
Fri, Aug 04, 2023 at 11:46:01AM +0100, Charles Keepax kirjoitti:
while (buf < block) {const u8 *word = min(buf + sizeof(u32), block);int pad = (buf + sizeof(u32)) - word;while (buf < word) {val >>= BITS_PER_BYTE;val |= FIELD_PREP(GENMASK(31, 24), *buf);buf++;}Is this a reinvented way of get_unaligned_*() APIs?
val >>= pad * BITS_PER_BYTE;regmap_write(regmap, CS42L43_TX_DATA, val);}...
while (buf < word) {*buf = FIELD_GET(GENMASK(7, 0), val);val >>= BITS_PER_BYTE;buf++;}put_unaligned_*() ?
Alas as it has been a while I have forgetten the exact context here and this one will take a little more time. I will try to find some spare time to work out if that would actual do the same thing, I have a vague feeling there was something here.
Ok found some time to refresh my memory on this.
The main issue here was as this is processing raw data for the SPI we shouldn't assume the data is a multiple of 4-bytes. You could write the code such that you used put_unaligned_le32 for most of the data and then special case the remainder, which would probably be slightly faster. But for the remainder you either end with the code here anyway or a special case for each of the cases 8,16,24 bits. So the code ends up looking much messier. Personally I am inclined to leave this unless performance on the SPI becomes a major issue.
There is perhaps an argument for a comment in the code to explain this given it took me time to remember what was going on.
Thanks, Charles