On Fri, May 01, 2026 at 08:32:30AM +0900, Krzysztof Wilczyński wrote:
> Hello,
>
> > The alternative would be separate llseek callbacks for both the legacy
> > and resource attributes, which we can add if this would be the preference
> > here.
>
> If we were to do this, then it would be as follows:
>
> static loff_t pci_llseek_resource(struct file *filep,
> struct kobject *kobj,
> const struct bin_attribute *attr,
> loff_t offset, int whence)
> {
> struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
> int bar = (unsigned long)attr->private;
>
> return fixed_size_llseek(filep, offset, whence,
> pci_resource_len(pdev, bar));
> }
>
> static loff_t pci_llseek_resource_legacy(struct file *filep,
> struct kobject *kobj __always_unused,
> const struct bin_attribute *attr,
> loff_t offset, int whence)
> {
> return fixed_size_llseek(filep, offset, whence, attr->size);
> }
>
> Each callback would be placed within the corresponding #ifdef block, so one
> for HAVE_PCI_MMAP or ARCH_GENERIC_PCI_MMAP_RESOURCE, and the other for the
> legacy attributes, so behind the HAVE_PCI_LEGACY guard.
>
> Note, the names need to be different, as some architectures offer both
> type of resource files, like PowerPC, which defines both the HAVE_PCI_LEGACY
> and HAVE_PCI_MMAP.
>
> With this split, we can also drop the __maybe_unused annotation.
>
> While I wanted to keep the changes to only what was needed for the
> pci_llseek_resource() to cover both type of resources, it would be
> also fine to have two distinct callbacks, too.
Yes, I think this is much more readable than the "if (attr->size)"
check.