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

[Eddie James]

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,