Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH] cdx: add MSI support for CDX bus | Date | Fri, 12 May 2023 20:15:05 +0200 |
| |
Nipun!
On Fri, May 12 2023 at 19:50, Nipun Gupta wrote: > On 5/11/2023 3:59 AM, Thomas Gleixner wrote: >> CDX is not any different than PCI. The actual "interrupt chip" is not >> part of the bus, it's part of the device and pretending that it is a bus >> specific thing is just running in to the same cul-de-sac sooner than >> later. > > I understand your viewpoint, but would state that CDX bus is somewhat > different than PCI in the sense that firmware is a controller for > all the devices and their configuration. CDX bus controller sends all > the write_msi_msg commands to firmware running on RPU over the RPmsg and > it is the firmware which interfaces with actual devices to pass this > information to devices in a way agreed between firmware and device. The > only way to pass MSI information to device is via firmware and CDX bus > controller is only entity which can communicate with the firmware for > this.
Fair enough, but we wouldn't had this dicussion if the above information would have been part of the changelog. See?
>> IIRC, there is a gap vs. interrupt affinity setting from user space, >> which is irrelevant for I2C, SPI etc. configured interrupt chips as they >> raise interrupt via an SoC interrupt pin and that's the entity which >> does the affinity management w/o requiring I2C/SPI. IIRC I posted a >> patch snippet to that effect in one of those lengthy PCI/MSI/IMS threads >> because that is also required for MSI storage which happens to be in >> queue memory and needs to be synchronized via some command channel. But >> I can't be bothered to search for it as it's a no-brainer to fix that >> up. > > Thanks for this analysis and pointing the hidden crucial issues with the > implementation. These needs to be fixed. > > As per your suggestion, we can add Firmware interaction code in the > irq_bus_sync_xx APIs. Another option is to change the > cdx_mcdi_rpc_async() API to atomic synchronous API.
I'm not a great fan of that. Depending on how long this update takes the CPU will busy wait for it to complete with interrupts disabled and locks held.
> We are evaluating both the solutions and will update the > implementation accordingly.
Thanks,
tglx
| |