| Message ID | 1420736772-11088-2-git-send-email-marc.zyngier@arm.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > As MSI-type features are creeping into non-PCI devices, it is > starting to make sense to give our struct device some form of > support for this, by allowing a pointer to an MSI irq domain to > be set/retrieved. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > include/linux/device.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/device.h b/include/linux/device.h > index fb50673..ec4cee5 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -690,6 +690,7 @@ struct acpi_dev_node { > * along with subsystem-level and driver-level callbacks. > * @pins: For device pin management. > * See Documentation/pinctrl.txt for details. > + * @msi_domain: The generic MSI domain this device is using. > * @numa_node: NUMA node this device is close to. > * @dma_mask: Dma mask (if dma'ble device). > * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all > @@ -750,6 +751,9 @@ struct device { > struct dev_pm_info power; > struct dev_pm_domain *pm_domain; > > +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN > + struct irq_domain *msi_domain; /* MSI domain device uses */ > +#endif This is not a comment on this patch specifically, but a question about other MSI specific fields that might be needed in struct device. Currently the generic MSI domain handling has hardcoded assumptions that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: #define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list) #define for_each_msi_entry(desc, dev) \ list_for_each_entry((desc), dev_to_msi_list((dev)), list) One approach would be to move the msi_list out of pci_dev and put it in struct device, so all devices can have an msi_list. The other approach would be to keep msi_list in a bus specific device struct, and then dev_to_msi_list() would need to be implemented as a bus specific callback of some kind. The above hardcoded PCI assumption isn't going to work. Wanted to see if there is any advice in which direction to go. Thanks, Stuart Yoder -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Stuart, On 15/01/15 20:35, Stuart Yoder wrote: > On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> As MSI-type features are creeping into non-PCI devices, it is >> starting to make sense to give our struct device some form of >> support for this, by allowing a pointer to an MSI irq domain to >> be set/retrieved. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> include/linux/device.h | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/include/linux/device.h b/include/linux/device.h >> index fb50673..ec4cee5 100644 >> --- a/include/linux/device.h >> +++ b/include/linux/device.h >> @@ -690,6 +690,7 @@ struct acpi_dev_node { >> * along with subsystem-level and driver-level callbacks. >> * @pins: For device pin management. >> * See Documentation/pinctrl.txt for details. >> + * @msi_domain: The generic MSI domain this device is using. >> * @numa_node: NUMA node this device is close to. >> * @dma_mask: Dma mask (if dma'ble device). >> * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all >> @@ -750,6 +751,9 @@ struct device { >> struct dev_pm_info power; >> struct dev_pm_domain *pm_domain; >> >> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN >> + struct irq_domain *msi_domain; /* MSI domain device uses */ >> +#endif > > This is not a comment on this patch specifically, but a question about other > MSI specific fields that might be needed in struct device. > > Currently the generic MSI domain handling has hardcoded assumptions > that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: > > #define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list) > > #define for_each_msi_entry(desc, dev) \ > list_for_each_entry((desc), dev_to_msi_list((dev)), list) > > One approach would be to move the msi_list out of pci_dev and put > it in struct device, so all devices can have an msi_list. > > The other approach would be to keep msi_list in a bus specific > device struct, and then dev_to_msi_list() would need to be > implemented as a bus specific callback of some kind. > > The above hardcoded PCI assumption isn't going to work. Wanted to > see if there is any advice in which direction to go. The question is: can we define a generic msi_desc? If yes, then your first proposal make sense. If not, then it is the second one. My hunch is that we'll have to move to a model that would look like this: struct mybus_msi_desc { struct msi_desc desc; struct mybus_stuff stuff; }; and move the PCI-specific stuff out of msi_desc. Thoughts? M.
On 2015/1/16 4:35, Stuart Yoder wrote: > On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> As MSI-type features are creeping into non-PCI devices, it is >> starting to make sense to give our struct device some form of >> support for this, by allowing a pointer to an MSI irq domain to >> be set/retrieved. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> include/linux/device.h | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/include/linux/device.h b/include/linux/device.h >> index fb50673..ec4cee5 100644 >> --- a/include/linux/device.h >> +++ b/include/linux/device.h >> @@ -690,6 +690,7 @@ struct acpi_dev_node { >> * along with subsystem-level and driver-level callbacks. >> * @pins: For device pin management. >> * See Documentation/pinctrl.txt for details. >> + * @msi_domain: The generic MSI domain this device is using. >> * @numa_node: NUMA node this device is close to. >> * @dma_mask: Dma mask (if dma'ble device). >> * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all >> @@ -750,6 +751,9 @@ struct device { >> struct dev_pm_info power; >> struct dev_pm_domain *pm_domain; >> >> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN >> + struct irq_domain *msi_domain; /* MSI domain device uses */ >> +#endif > > This is not a comment on this patch specifically, but a question about other > MSI specific fields that might be needed in struct device. > > Currently the generic MSI domain handling has hardcoded assumptions > that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: > > #define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list) > > #define for_each_msi_entry(desc, dev) \ > list_for_each_entry((desc), dev_to_msi_list((dev)), list) > > One approach would be to move the msi_list out of pci_dev and put > it in struct device, so all devices can have an msi_list. > > The other approach would be to keep msi_list in a bus specific > device struct, and then dev_to_msi_list() would need to be > implemented as a bus specific callback of some kind. > > The above hardcoded PCI assumption isn't going to work. Wanted to > see if there is any advice in which direction to go. Hi Stuart, I already have some a patch set to go that direction waiting send out for review:) Thanks! Gerry > > Thanks, > Stuart Yoder > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Gerry, So which direction did you take in your patch set-- a) common, generic msi_desc, or b) bus-specific msi_desc like Marc showed (mybus_msi_desc)? Thanks, Stuart On Sun, Jan 18, 2015 at 8:10 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: > > > On 2015/1/16 4:35, Stuart Yoder wrote: >> On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>> As MSI-type features are creeping into non-PCI devices, it is >>> starting to make sense to give our struct device some form of >>> support for this, by allowing a pointer to an MSI irq domain to >>> be set/retrieved. >>> >>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>> --- >>> include/linux/device.h | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/include/linux/device.h b/include/linux/device.h >>> index fb50673..ec4cee5 100644 >>> --- a/include/linux/device.h >>> +++ b/include/linux/device.h >>> @@ -690,6 +690,7 @@ struct acpi_dev_node { >>> * along with subsystem-level and driver-level callbacks. >>> * @pins: For device pin management. >>> * See Documentation/pinctrl.txt for details. >>> + * @msi_domain: The generic MSI domain this device is using. >>> * @numa_node: NUMA node this device is close to. >>> * @dma_mask: Dma mask (if dma'ble device). >>> * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all >>> @@ -750,6 +751,9 @@ struct device { >>> struct dev_pm_info power; >>> struct dev_pm_domain *pm_domain; >>> >>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN >>> + struct irq_domain *msi_domain; /* MSI domain device uses */ >>> +#endif >> >> This is not a comment on this patch specifically, but a question about other >> MSI specific fields that might be needed in struct device. >> >> Currently the generic MSI domain handling has hardcoded assumptions >> that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: >> >> #define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list) >> >> #define for_each_msi_entry(desc, dev) \ >> list_for_each_entry((desc), dev_to_msi_list((dev)), list) >> >> One approach would be to move the msi_list out of pci_dev and put >> it in struct device, so all devices can have an msi_list. >> >> The other approach would be to keep msi_list in a bus specific >> device struct, and then dev_to_msi_list() would need to be >> implemented as a bus specific callback of some kind. >> >> The above hardcoded PCI assumption isn't going to work. Wanted to >> see if there is any advice in which direction to go. > Hi Stuart, > I already have some a patch set to go that direction waiting > send out for review:) > Thanks! > Gerry > >> >> Thanks, >> Stuart Yoder >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2015/1/21 1:17, Stuart Yoder wrote: > Gerry, > > So which direction did you take in your patch set-- a) common, > generic msi_desc, or b) bus-specific msi_desc like Marc showed > (mybus_msi_desc)? Hi Stuart, Currently I'm trying to go the former way as below. Regards, Gerry ----------------------------------------------------------------------- struct msi_desc { struct list_head list; unsigned int irq; unsigned int nvec_used; /* number of messages */ struct device * dev; struct msi_msg msg; /* Last set MSI message */ #ifdef CONFIG_PCI_MSI union { struct { /* For PCI MSI/MSI-X */ u32 masked; /* mask bits */ struct { __u8 is_msix : 1; __u8 multiple: 3; /* log2 num of messages allocated */ __u8 multi_cap : 3; /* log2 num of messages supported */ __u8 maskbit : 1; /* mask-pending bit supported ? */ __u8 is_64 : 1; /* Address size: 0=32bit 1=64bit */ __u16 entry_nr; /* specific enabled entry */ unsigned default_irq; /* default pre-assigned irq */ } msi_attrib; union { u8 mask_pos; void __iomem *mask_base; }; }; }; #endif /* CONFIG_PCI_MSI */ }; > > Thanks, > Stuart > > On Sun, Jan 18, 2015 at 8:10 PM, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> >> >> On 2015/1/16 4:35, Stuart Yoder wrote: >>> On Thu, Jan 8, 2015 at 11:06 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>>> As MSI-type features are creeping into non-PCI devices, it is >>>> starting to make sense to give our struct device some form of >>>> support for this, by allowing a pointer to an MSI irq domain to >>>> be set/retrieved. >>>> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> --- >>>> include/linux/device.h | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/include/linux/device.h b/include/linux/device.h >>>> index fb50673..ec4cee5 100644 >>>> --- a/include/linux/device.h >>>> +++ b/include/linux/device.h >>>> @@ -690,6 +690,7 @@ struct acpi_dev_node { >>>> * along with subsystem-level and driver-level callbacks. >>>> * @pins: For device pin management. >>>> * See Documentation/pinctrl.txt for details. >>>> + * @msi_domain: The generic MSI domain this device is using. >>>> * @numa_node: NUMA node this device is close to. >>>> * @dma_mask: Dma mask (if dma'ble device). >>>> * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all >>>> @@ -750,6 +751,9 @@ struct device { >>>> struct dev_pm_info power; >>>> struct dev_pm_domain *pm_domain; >>>> >>>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN >>>> + struct irq_domain *msi_domain; /* MSI domain device uses */ >>>> +#endif >>> >>> This is not a comment on this patch specifically, but a question about other >>> MSI specific fields that might be needed in struct device. >>> >>> Currently the generic MSI domain handling has hardcoded assumptions >>> that devices are PCI-- see the for_each_msi_entry() iterator in msi.h: >>> >>> #define dev_to_msi_list(dev) (&to_pci_dev((dev))->msi_list) >>> >>> #define for_each_msi_entry(desc, dev) \ >>> list_for_each_entry((desc), dev_to_msi_list((dev)), list) >>> >>> One approach would be to move the msi_list out of pci_dev and put >>> it in struct device, so all devices can have an msi_list. >>> >>> The other approach would be to keep msi_list in a bus specific >>> device struct, and then dev_to_msi_list() would need to be >>> implemented as a bus specific callback of some kind. >>> >>> The above hardcoded PCI assumption isn't going to work. Wanted to >>> see if there is any advice in which direction to go. >> Hi Stuart, >> I already have some a patch set to go that direction waiting >> send out for review:) >> Thanks! >> Gerry >> >>> >>> Thanks, >>> Stuart Yoder >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/device.h b/include/linux/device.h index fb50673..ec4cee5 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -690,6 +690,7 @@ struct acpi_dev_node { * along with subsystem-level and driver-level callbacks. * @pins: For device pin management. * See Documentation/pinctrl.txt for details. + * @msi_domain: The generic MSI domain this device is using. * @numa_node: NUMA node this device is close to. * @dma_mask: Dma mask (if dma'ble device). * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all @@ -750,6 +751,9 @@ struct device { struct dev_pm_info power; struct dev_pm_domain *pm_domain; +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + struct irq_domain *msi_domain; /* MSI domain device uses */ +#endif #ifdef CONFIG_PINCTRL struct dev_pin_info *pins; #endif @@ -837,6 +841,22 @@ static inline void set_dev_node(struct device *dev, int node) } #endif +static inline struct irq_domain *dev_get_msi_domain(const struct device *dev) +{ +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + return dev->msi_domain; +#else + return NULL; +#endif +} + +static inline void dev_set_msi_domain(struct device *dev, struct irq_domain *d) +{ +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN + dev->msi_domain = d; +#endif +} + static inline void *dev_get_drvdata(const struct device *dev) { return dev->driver_data;
As MSI-type features are creeping into non-PCI devices, it is starting to make sense to give our struct device some form of support for this, by allowing a pointer to an MSI irq domain to be set/retrieved. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- include/linux/device.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)