On Wed, Nov 27, 2019 at 12:07 PM Haren Myneni <haren@xxxxxxxxxxxxxxxxxx> wrote:
>
> *snip*
>
> @@ -36,7 +62,18 @@ static int init_vas_instance(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - if (pdev->num_resources != 4) {
> + rc = of_property_read_u64(dn, "ibm,vas-port", &port);
> + if (rc) {
> + pr_err("No ibm,vas-port property for %s?\n", pdev->name);
> + /* No interrupts property */
> + nresources = 4;
> + }
> +
> + /*
> + * interrupts property is available with 'ibm,vas-port' property.
> + * 4 Resources and 1 IRQ if interrupts property is available.
> + */
> + if (pdev->num_resources != nresources) {
> pr_err("Unexpected DT configuration for [%s, %d]\n",
> pdev->name, vasid);
> return -ENODEV;
Right, so adding the IRQ in firmware will break the VAS driver in
existing kernels since it changes the resource count. This is IMO a
bug in the VAS driver that you should fix, but it does mean we need to
think twice about having firmware assign an interrupt at boot.
I had a closer look at this series and I'm not convinced that any
firmware changes are actually required either. We already have OPAL
calls for allocating an hwirq for the kernel to use and for getting
the IRQ's XIVE trigger port (see pnv_ocxl_alloc_xive_irq() for an
example). Why not use those here too? Doing so would allow us to
assign interrupts to individual windows too which might be useful for
the windows used by the kernel.
