Re: [PATCH v6 1/4] bswap: Add the ability to store to an unaligned 24 bit field

[Jonathan Cameron]

On Sat, 20 May 2023 19:08:22 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 20/5/23 17:15, Richard Henderson wrote:
> > On 5/20/23 06:15, BALATON Zoltan wrote:  
> >> On Sat, 20 May 2023, Peter Maydell wrote:  
> >>> On Fri, 19 May 2023 at 15:19, Jonathan Cameron via
> >>> <qemu-devel@nongnu.org> wrote:  
> >>>>
> >>>> From: Ira Weiny <ira.weiny@intel.com>
> >>>>
> >>>> CXL has 24 bit unaligned fields which need to be stored to.  CXL is
> >>>> specified as little endian.
> >>>>
> >>>> Define st24_le_p() and the supporting functions to store such a field
> >>>> from a 32 bit host native value.
> >>>>
> >>>> The use of b, w, l, q as the size specifier is limiting.  So "24" was
> >>>> used for the size part of the function name.  
> >>
> >> Maybe it's clearer to use 24 but if we want to keep these somewhat 
> >> consistent how about using t for Triplet, Three-bytes or Twenty-four?  
> > 
> > I think it's clearer to use '3'.
> > When I added 128-bit support I used cpu_ld16_mmu.  

As an aside on that - you didn't update the docs when you added that
(I was looking for it to copy your regex ;)

> 
> There is also ld8u / ld8s / st8.
> 
> > I think it would be clearer to not use letters anywhere, and to use 
> > units of bytes instead of units of bits (no one can store just a bit), 
> > but changing everything is a big job.  
> 
> So:
> 
> ldub ->  ld1u,
> 
> lduw_le -> ld2u_le,
> 
> virtio_stl -> virtio_st4,
> 
> stq_be -> st8_be.
> 
> Right?
> 
> Also we have:
> 
> cpu_ld/st_*
> virtio_ld/st_*
> ld/st_*_phys
> ld/st_*_pci_dma
> address_space_ld/st
> 
> While mass-changing, we could use FOO_ld/st_BAR with FOO
> for API and BAR for API variant (endian, mmuidx, ra, ...):
> 
> So:
> 
> ld/st_*_pci_dma -> pci_dma_ld/st_*
> 
> for ld/st_*_phys I'm not sure.