Re: [PATCH v4 2/3] virtio-spi: Add virtio-spi.h

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Aug 25, 2025 at 05:19:03PM +0800, Haixu Cui wrote:
> 
> 
> On 8/21/2025 4:45 PM, Michael S. Tsirkin wrote:
> 
> 
> > > +
> > > +/* Sample data on trailing clock edge */
> > > +#define VIRTIO_SPI_CPHA			_BITUL(0)
> > > +/* Clock is high when IDLE */
> > > +#define VIRTIO_SPI_CPOL			_BITUL(1)
> > > +/* Chip Select is active high */
> > > +#define VIRTIO_SPI_CS_HIGH			_BITUL(2)
> > > +/* Transmit LSB first */
> > > +#define VIRTIO_SPI_MODE_LSB_FIRST		_BITUL(3)
> > > +/* Loopback mode */
> > > +#define VIRTIO_SPI_MODE_LOOP			_BITUL(4)
> > 
> > It is generally preferable to have an enum with just bit
> > numbers.
> > 
> > 
> > E.g.
> > 
> > enum {
> > VIRTIO_SPI_F_CPHA = 0,
> > }
> > 
> > 
> > Userspace can add _BITUL wrappers itself if it
> > wants.
> > 
> > 
> 
> Hi Michael,
> 
> Thank you for the suggestion regarding the bit definitions.
> 
> Would it be acceptable to keep the current macro definitions with _BITUL()
> because these macros are also used within the virtio SPI driver itself?
> 
> Looking forward to your guidance.
> 
> Best regards,
> Haixu Cui
> 


move them to the .c file if you want them.