Re: [patch V2 18/36] genirq/msi: Add msi_device_data::properties

Cédric Le Goater
2021-12-07T09:22:35Z

Hello Thomas,

On 12/6/21 23:39, Thomas Gleixner wrote:
> Add a properties field which allows core code to store information for easy
> retrieval in order to replace MSI descriptor fiddling.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> V2: Add a setter function to prepare for future changes
> ---
> include/linux/msi.h | 17 +++++++++++++++++
> kernel/irq/msi.c | 24 ++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -4,6 +4,7 @@
>
> #include <linux/cpumask.h>
> #include <linux/list.h>
> +#include <linux/bits.h>
> #include <asm/msi.h>
>
> /* Dummy shadow structures if an architecture does not define them */
> @@ -153,6 +154,22 @@ struct msi_device_data {
>
> int msi_setup_device_data(struct device *dev);
>
> +/* MSI device properties */
> +#define MSI_PROP_PCI_MSI BIT(0)
> +#define MSI_PROP_PCI_MSIX BIT(1)
> +#define MSI_PROP_64BIT BIT(2)
> +
> +#ifdef CONFIG_GENERIC_MSI_IRQ
> +bool msi_device_has_property(struct device *dev, unsigned long prop);
> +void msi_device_set_properties(struct device *dev, unsigned long prop);
> +#else
> +static inline bool msi_device_has_property(struct device *dev, unsigned long prop)
> +{
> + return false;
> +}
> +static inline void msi_device_set_properties(struct device *dev, unsigned long prop) { }
> +#endif
> +
> /* Helpers to hide struct msi_desc implementation details */
> #define msi_desc_to_dev(desc) ((desc)->dev)
> #define dev_to_msi_list(dev) (&(dev)->msi_list)
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -60,6 +60,30 @@ void free_msi_entry(struct msi_desc *ent
> kfree(entry);
> }
>
> +/**
> + * msi_device_set_properties - Set device specific MSI properties
> + * @dev: Pointer to the device which is queried
> + * @prop: Properties to set
> + */
> +void msi_device_set_properties(struct device *dev, unsigned long prop)
> +{
> + if (WARN_ON_ONCE(!dev->msi.data))
> + return ;
> + dev->msi.data->properties = 0;
It would work better if the prop variable was used instead of 0.

With that fixed,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> +}
> +
> +/**
> + * msi_device_has_property - Check whether a device has a specific MSI property
> + * @dev: Pointer to the device which is queried
> + * @prop: Property to check for
> + */
> +bool msi_device_has_property(struct device *dev, unsigned long prop)
> +{
> + if (!dev->msi.data)
> + return false;
> + return !!(dev->msi.data->properties & prop);
> +}
> +
> void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> {
> *msg = entry->msg;
>