[Question / Potential Issue] Missing sd_dma_addr field on 64-bit architecture

[SiVittorio]

Hi @ZipCPU!

Thank you for your driver for SDIO, it's very useful! I have a question about the sd_dma_addr. When I compile driver's code on a 64b architecture, I get an error:

 error: 'SDIO_Type' {aka 'struct SDIO_S'} has no member named 'sd_dma_addr'
 1622 |                 dev->d_dev->sd_dma_addr = buf;
      |                           ^~

I don't understand the ifdef sections in SDIO_S struct description. When we have a 64-bit architecture, why is the sd_dma_addr field missing and only the sd_unused field present, instead of the opposite?

sw/sdiodrv.h

typedef	struct SDIO_S {
	volatile uint32_t	sd_cmd, sd_data, sd_fifa, sd_fifb, sd_phy;

#if defined(__SIZEOF_POINTER__) && (__SIZEOF_POINTER__ == 8)
	// 64b architectures
	volatile uint32_t	sd_unused;                               // <- why not the sd_dma_addr?
#else
	// 32b architectures, or default if not __SIZEOF_POINTER__ is undefined
  #if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
	// Little-Endian
	volatile void		*sd_dma_addr;
	volatile uint32_t	sd_unused;
  #else
	// Big-Endian, the default if __BYTE_ORDER__ is undefined
	volatile uint32_t	sd_unused;
	volatile void		*sd_dma_addr;
  #endif
#endif
	volatile uint32_t	sd_dma_length;
} SDIO;

I saw the #23 issue, and @kilicyusuf1 wrote a struct section with the sd_dma_addr field for 64b:

kilicyusuf1 solution

typedef struct SDIO_S {
    volatile uint32_t sd_cmd, sd_data, sd_fifa, sd_fifb, sd_phy;

#if defined(__SIZEOF_POINTER__) && (__SIZEOF_POINTER__ == 4)    
    // 32-bit Architectures
    #if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
        // Little-Endian
        volatile void     *sd_dma_addr;
        volatile uint32_t sd_unused;
    #else
        // Big-Endian
        volatile uint32_t sd_unused;
        volatile void     *sd_dma_addr;
    #endif
#else
    // 64-bit Architectures
    volatile void         *sd_dma_addr;          // <- hi! I'm here! 
#endif
    volatile uint32_t sd_dma_length;
} SDIO;

My question is: am I missing something, or is this just a bug?

[ZipCPU]

My question is: am I missing something, or is this just a bug?

Good question. Did I define the entire 8-byte pointer space in the register set as sd_unused instead of sd_dma_addr?

Yes, that does look like a bug, doesn't it? It should read volatile uint64_t *sd_dma_addr instead of volatile uint32_t sd_unused.

It is unfortunate that I don't have any 64b architectures to test on, so I may need to rely upon you to know if such a fix works.

Dan

[SiVittorio]

@ZipCPU Thank you for your answer!

It is unfortunate that I don't have any 64b architectures to test on, so I may need to rely upon you to know if such a fix works.

OK! I'll integrate this fix into my test code and verify it works.