Re: [PATCH] PCI: Fix warning without CONFIG_VIDEO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/18/2025 12:23 PM, Manivannan Sadhasivam wrote:
On Fri, Jul 18, 2025 at 12:06:22PM GMT, Mario Limonciello wrote:
On 7/18/2025 12:00 PM, Manivannan Sadhasivam wrote:
On Fri, Jul 18, 2025 at 08:41:33AM GMT, Mario Limonciello wrote:
From: Mario Limonciello <mario.limonciello@xxxxxxx>

When compiled without CONFIG_VIDEO pci_create_boot_display_file() will
never create a sysfs file for boot_display. Guard the sysfs file
declaration against CONFIG_VIDEO.

Reported-by: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/linux-next/20250718224118.5b3f22b0@xxxxxxxxxxxxxxxx/
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
   drivers/pci/pci-sysfs.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 6b1a0ae254d3a..f6540a72204d3 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -680,12 +680,14 @@ const struct attribute_group *pcibus_groups[] = {
   	NULL,
   };
+#ifdef CONFIG_VIDEO
   static ssize_t boot_display_show(struct device *dev, struct device_attribute *attr,
   				 char *buf)
   {
   	return sysfs_emit(buf, "1\n");
   }
   static DEVICE_ATTR_RO(boot_display);

I failed to give my comment during the offending series itself, but it is never
late than never. Why are we adding non-PCI attributes under bus/pci in the first
place? Though the underlying device uses PCI as a transport, only the PCI bus
specific attrbutes should be placed under bus/pci and the driver/peripheral
specific attrbutes should belong to the respective bus/class/device hierarchy.

Now, if other peripherals (like netdev) start adding these device specific
attributes under bus/pci, it will turn out to be a mess.

- Mani


It was mostly to mirror the location of where boot_vga is, which arguably
has the same issue you raise.


Yes, I agree. But 'boot_vga' has set a bad precedence IMO.

I would be incredibly surprised if there was a proposal to add a
'boot_display' attribute from netdev..

Not 'boot_display' but why not 'boot_network' or something else. I was just
merely pointing out the fact that the other subsystems can start dumping
device/usecase specific attributes under bus/pci.

- Mani


This is a pretty general problem that exists that attributes are first come first served. For example amdgpu adds mem_busy_percent and it has certain semantics. Now PCI core can't add that.

And if nouveau.ko wants to add the same thing they need to follow the same semantics because userspace will look for those.




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux