On Thu, Jul 03, 2025 at 07:56:52PM +0530, Anup Patel wrote:
> On Thu, Jul 3, 2025 at 7:24 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Thu, Jul 03, 2025 at 04:24:18PM +0530, Sunil V L wrote:
> > > On Wed, Jul 02, 2025 at 03:28:45PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Jul 02, 2025 at 10:43:42AM +0530, Anup Patel wrote:
...
> > > > > - if (dev_of_node(dev))
> > > > > + if (is_of_node(fwnode)) {
> > > > > of_msi_configure(dev, dev_of_node(dev));
> > > > > + } else if (is_acpi_device_node(fwnode)) {
> > > > > + msi_domain = irq_find_matching_fwnode(imsic_acpi_get_fwnode(dev),
> > > > > + DOMAIN_BUS_PLATFORM_MSI);
> > > > > + dev_set_msi_domain(dev, msi_domain);
> > > > > + }
> > > >
> > > > Actually you don't need to have the if-else-if if I am not mistaken.
> > > > The OF does almost the same as it's done in the second branch for ACPI case.
> > > > How many MSI parents this may have?
> > > >
> > > OF already has a well defined interface to configure the MSI domain. The
> > > mechanisms existing today are different for DT vs ACPI to find out the
> > > fwnode of the MSI controller. So, it is done differently.
> >
> > I don't see how. The only difference I see is that OF iterates over all listed
> > parents, if any, ACPI tries only one.
> >
> > So, perhaps it's a time to have a common API somewhere for this to be agnostic?
> > Something like fwnode_msi_configure() in somewhere of IRQ MSI core?
>
> There is an issue/gap in the DD framework which is being work-around
> here. This issue manifest mostly in RISC-V land because in RISC-V both
> MSI controller driver and drivers using MSI are regular platform drivers
> while the probe ordering is ensured by dev_link support of DD framework
> or the frameworks (like ACPI) creating the device.
>
> As-per this issue, when platform devices (DT or ACPI) are created the
> MSI domain instance is not available and hence set to NULL. The MSI
> domain instance is only available after MSI controller driver is probed
> so currently we explicitly do of_msi_configure() or dev_set_msi_domain()
> in the driver using MSI as a work-around. Adding a common
> fwnode_msi_configure() is only going to be an improvement to the
> existing work-around which we should not have in the first place
> hence not the right approach IMO.
>
> In the long run, we need a clean fix for the above issue in the DD
> framework such that platform drivers using MSI don't have to explicitly
> do of_msi_configure() or dev_set_msi_domain().
I see, thanks a lot for this explanation. Can you add a summary as a comment on
top of the if-else-if in case it's not there already?
--
With Best Regards,
Andy Shevchenko
