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.

Reply via email to