On Tue, Sep 06, 2016 at 04:42:58PM +0200, Cédric Le Goater wrote: > On 09/06/2016 02:48 AM, David Gibson wrote: > > On Mon, Sep 05, 2016 at 05:11:53PM +1000, Benjamin Herrenschmidt wrote: > >> On Mon, 2016-09-05 at 13:39 +1000, David Gibson wrote: > >>>> +static XScomDevice *xscom_find_target(XScomState *s, uint32_t > >>> pcb_addr, > >>>> + uint32_t *range) > >>>> +{ > >>>> + BusChild *bc; > >>>> + > >>>> + QTAILQ_FOREACH(bc, &s->bus->bus.children, sibling) { > >>>> + DeviceState *qd = bc->child; > >>>> + XScomDevice *xd = XSCOM_DEVICE(qd); > >>>> + unsigned int i; > >>>> + > >>>> + for (i = 0; i < MAX_XSCOM_RANGES; i++) { > >>>> + if (xd->ranges[i].addr <= pcb_addr && > >>>> + (xd->ranges[i].addr + xd->ranges[i].size) > > >>> pcb_addr) { > >>>> + *range = i; > >>>> + return xd; > >>>> + } > >>>> + } > >>>> + } > >>> > >>> Hmm.. you could set up a SCOM local address space using the > >>> infrastructure in memory.c, rather than doing your own dispatch. > >> > >> There are pros and cons to this approach. The memory.c stuff comes with > >> quite a lot of baggage, not all of it very shinny to be honest ;-) I > >> still *hate* how it forces upon us a whole 128-bit integer arithmetic > >> library just so that it can represent 1_0000_0000_0000_0000 ... > > > > Ugh, yeah. I tried to argue against this when it first came in, but > > was overruled. > > > >> It would be make more sense to use inclusive start/end instead and > >> stick to 64-bits. > >> > >> That being said, we could do that. We'd have to shift the XSCOM > >> addresses left by 3 since each address is an 8 bytes reigster and > >> forbid non-8-bytes accesses. > > > > Ok. I'm not particularly fussed either way. > > > The change does seem too invasive. I can give it a try in next > version. > > When a memory region is triggered, the impacted device will have > to convert the address with xscom_to_pcb_addr(). There is some > dependency on the chip model because the translation is different. > That would be an extra op in the xscom device model I guess.
Actually, I was still thinking of having an MR for the scom interface unit, which does the xscom_to_pcb_addr() then re-issues the access in the PCB address space. But your suggestion might work too. > Also, the main purpose of the XscomBus is to loop on the devices > to populate the device tree. I am wondering if we could just use > a simple list under the chip for that purpose. > > Thanks, > > C. > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature