On Wed, Aug 25, 2021 at 09:22:53PM +0000, Krzysztof Wilczyński wrote:
> The pci_dev_resource_attr() macro will be used to declare and define
> each of the PCI resource sysfs objects statically while also reducing
> unnecessary code repetition.
>
> Internally this macro relies on the pci_dev_resource_attr_is_visible()
> helper which should correctly handle different types of PCI BAR address
> space while also providing support for creating either normal and/or
> write-combine attributes as required.
This makes a lot of sense. To make it more approachable, I think we
might extend the commit log to mention that each BAR is independently
visible/hidden based on its existence, and each .is_visible() function
applies to the entire attribute group, which means we need a separate
attribute group for each BAR. Actually I guess we need *two* for each
BAR -- the normal one and the WC one. So 12 attribute groups per
device.
> Signed-off-by: Krzysztof Wilczyński <kw@xxxxxxxxx>
> ---
> drivers/pci/pci-sysfs.c | 47 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index c94ab9830932..6eba5c0887df 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1277,6 +1277,53 @@ static umode_t pci_dev_resource_attr_is_visible(struct kobject *kobj,
>
> return attr->attr.mode;
> }
> +
> +#define pci_dev_resource_attr(_bar) \
> +static struct bin_attribute \
> +pci_dev_resource##_bar##_attr = __BIN_ATTR(resource##_bar, \
> + 0600, NULL, NULL, 0); \
> + \
> +static struct bin_attribute *pci_dev_resource##_bar##_attrs[] = { \
> + &pci_dev_resource##_bar##_attr, \
> + NULL, \
> +}; \
> + \
> +static umode_t \
> +pci_dev_resource##_bar##_attr_is_visible(struct kobject *kobj, \
> + struct bin_attribute *a, \
> + int n) \
> +{ \
> + return pci_dev_resource_attr_is_visible(kobj, a, _bar, false); \
> +}; \
> + \
> +static const struct \
> +attribute_group pci_dev_resource##_bar##_attr_group = { \
> + .bin_attrs = pci_dev_resource##_bar##_attrs, \
> + .is_bin_visible = pci_dev_resource##_bar##_attr_is_visible, \
> +}; \
> + \
> +static struct bin_attribute \
> +pci_dev_resource##_bar##_wc_attr = __BIN_ATTR(resource##_bar##_wc, \
> + 0600, NULL, NULL, 0); \
> + \
> +static struct bin_attribute *pci_dev_resource##_bar##_wc_attrs[] = { \
> + &pci_dev_resource##_bar##_wc_attr, \
> + NULL, \
> +}; \
> + \
> +static umode_t \
> +pci_dev_resource##_bar##_wc_attr_is_visible(struct kobject *kobj, \
> + struct bin_attribute *a, \
> + int n) \
> +{ \
> + return pci_dev_resource_attr_is_visible(kobj, a, _bar, true); \
> +}; \
> + \
> +static const struct \
> +attribute_group pci_dev_resource##_bar##_wc_attr_group = { \
> + .bin_attrs = pci_dev_resource##_bar##_wc_attrs, \
> + .is_bin_visible = pci_dev_resource##_bar##_wc_attr_is_visible, \
> +}
> #else /* !(defined(HAVE_PCI_MMAP) || defined(ARCH_GENERIC_PCI_MMAP_RESOURCE)) */
> int __weak pci_create_resource_files(struct pci_dev *dev) { return 0; }
> void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
> --
> 2.32.0
>
