Re: [Qemu-devel] [RFC v2] hw/sd/aspeed_sdhci: New device

[Eddie James <eajames@linux.ibm.com>]

On 8/15/19 3:13 PM, Eddie James wrote:
>
> On 8/15/19 3:05 AM, Cédric Le Goater wrote:
>> Hello Eddie,
>>
>> On 14/08/2019 22:27, Eddie James wrote:
>>>
>>> +        sdhci->slots[0].capareg = (uint64_t)(uint32_t)val;
>>> +        break;
>>> +    case ASPEED_SDHCI_SDIO_148:
>>> +        sdhci->slots[0].maxcurr = (uint64_t)(uint32_t)val;
>>> +        break;
>>> +    case ASPEED_SDHCI_SDIO_240:
>>> +        sdhci->slots[1].capareg = (uint64_t)(uint32_t)val;
>>> +        break;
>>> +    case ASPEED_SDHCI_SDIO_248:
>>> +        sdhci->slots[1].maxcurr = (uint64_t)(uint32_t)val;
>>> +        break;
>> I think these regs are readonly.
>
>
> Well the actual regs at slot + 0x40/0x48 are indeed, but not the 
> Aspeed-specific ones that mirror there. I think the idea is that 
> Aspeed-specific code can set it's capabilities differently if desired. 
> This may prevent the use of alias regions here.


Actually I could be wrong after reading the specs again. It's a little 
confusing. I'm fine with making it read-only anyway, I doubt there will 
be any code that needs to write it.


>
>
>>
>>> +    default:
>>> +        if (addr < ASPEED_SDHCI_REG_SIZE) {
>>> +            sdhci->regs[TO_REG(addr)] = (uint32_t)val;
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps aspeed_sdhci_ops = {
>>> +    .read = aspeed_sdhci_read,
>>> +    .write = aspeed_sdhci_write,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .valid.min_access_size = 4,