Messages in this thread | | | Subject | Re: [PATCH v7 15/15] irqchip: mbigen: Add ACPI support | From | Hanjun Guo <> | Date | Mon, 16 Jan 2017 22:23:16 +0800 |
| |
Hi Lorenzo,
On 2017/1/16 19:38, Lorenzo Pieralisi wrote: > On Sat, Jan 14, 2017 at 10:56:54AM +0800, Hanjun Guo wrote: >> Hi Lorenzo, >> >> On 2017/1/13 18:21, Lorenzo Pieralisi wrote: >>> On Wed, Jan 11, 2017 at 11:06:39PM +0800, Hanjun Guo wrote: >>>> With the preparation of platform msi support and interrupt producer >>>> in DSDT, we can add mbigen ACPI support now. >>>> >>>> We are using _PRS methd to indicate number of irq pins instead >>>> of num_pins in DT to avoid _DSD usage in this case. >>>> >>>> For mbi-gen, >>>> Device(MBI0) { >>>> Name(_HID, "HISI0152") >>>> Name(_UID, Zero) >>>> Name(_CRS, ResourceTemplate() { >>>> Memory32Fixed(ReadWrite, 0xa0080000, 0x10000) >>>> }) >>>> >>>> Name (_PRS, ResourceTemplate() { >>>> Interrupt(ResourceProducer,...) {12,14,....} >>> I still do not understand why you are using _PRS for this, I think >>> the MBIgen configuration is static and if it is so the Interrupt >>> resource should be part of the _CRS unless there is something I am >>> missing here. >> Sorry for not clear in the commit message. MBIgen is an interrupt producer >> which produces irq resource to devices connecting to it, and MBIgen itself >> don't consume wired interrupts. > That's why you mark it as ResourceProducer, but that's not a reason to > put it in the _PRS instead of _CRS.
If using _CRS for the interrupt resource, the irq number represented will be mapped (i.e acpi_register_gsi()), then will conflict with the irq number of devices consuming it (mbigen is producing the interrupts), but I agree with you that let's ask Rafael's point of view.
> > IIUC _PRS is there to provide a way to define the possible resource > settings of a _configurable_ device (ie programmable) so that the actual > resource value you would programme with a call to its _SRS is sane (ie > the OS has a way, through the _PRS, to detect what possible resource > settings are available for the device). > > I think Rafael has more insights into how the _PRS is used on x86 > systems so I would ask his point of view here before merrily merging > this code.
OK, Rafael is traveling now, hope he will have time to take a look.
How about updating this patch set then sending a new version for review with this patch unchanged? if Rafael have comments on this one, I will send a single updated one for this patch (if no other changes).
> >> Also devices connecting MBIgen may not consume all the interrupts produced >> by MBIgen, for example, MBIgen may produce 128 interrupts but only half of >> them are currently used, so _PRS here means "provide interrupt resources >> may consumed by devices connecting to it". > See above.
Thanks Hanjun
| |