On 09/06/2016 11:47 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2016-09-06 at 16:42 +0200, Cédric Le Goater wrote:
>>
>> 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.
>
> No. If you split the XSCOM bus from the MMIO -> XSCOM bridge (the ADU)
> then the conversion only happens in the former. You don't directly
> route the MMIOs over ! You intercept the MMIOs and use use the
> address_space_rw to "generate" the XSCOM accesses on the other side,
> like I do for the LPC bus.
Yes. That is what I have been experimenting with. The mmio read/write
ops and the current xscom read/write ops are quite compatible so It
did cost too much to do so :
+static uint64_t pnv_lpc_xscom_mr_read(void *opaque, hwaddr addr, unsigned size)
+{
+ XScomDevice *xd = XSCOM_DEVICE(opaque);
+ uint64_t val = 0;
+
+ pnv_lpc_xscom_read(xd, 0, xscom_to_pcb_addr(xd->chip_type, addr), &val);
+ return val;
+}
+
+static void pnv_lpc_xscom_mr_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+ XScomDevice *xd = XSCOM_DEVICE(opaque);
+ pnv_lpc_xscom_write(xd, 0, xscom_to_pcb_addr(xd->chip_type, addr), val);
+}
+
+static const MemoryRegionOps lpc_xscom_mr_ops = {
+ .read = pnv_lpc_xscom_mr_read,
+ .write = pnv_lpc_xscom_mr_write,
+ .valid.min_access_size = 8,
+ .valid.max_access_size = 8,
+ .impl.min_access_size = 8,
+ .impl.max_access_size = 8,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
static void pnv_lpc_realize(DeviceState *dev, Error **errp)
{
PnvLpcController *lpc = PNV_LPC_CONTROLLER(dev);
+ XScomBus *bus = XSCOM_BUS(qdev_get_parent_bus(dev));
/* LPC XSCOM address is fixed */
+ memory_region_init_io(&lpc->xd.xscom_mr, OBJECT(dev), &lpc_xscom_mr_ops,
+ lpc, "lpc-xscom", 4 * 8);
+ memory_region_add_subregion(bus->xscom_mr, 0xb00200, &lpc->xd.xscom_mr);
+
lpc->xd.ranges[0].addr = 0xb0020;
lpc->xd.ranges[0].size = 4;
To hack my way through, I have put the address space and the backend
region under the XscomBus, bc it's easy to capture from the device.
So that might be a reason to keep this bus/device model.
The xscom_to_pcb_addr() translation should probably done at the upper
level, at the bridge/ADU level. I think that is what you are asking
for above.
As for the mapping, I don't think it should be here. It should be done
at the chip/bus level which controls the address space, but not in the
devices.
> We need that anyway because of the way XSCOMs can manipulate the HMER
> etc...
ah. Another thing I need to look at !
Thanks,
C.
>> 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.
>
>
