Re: [PATCH 2/4] PCI/sysfs: Add pci_dev_resource_attr() macro

[Bjorn Helgaas <helgaas@kernel.org>]

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@linux.com>
> ---
>  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
>