[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 06/19] xen: Clean up asm-generic/device.h
On Thu Jun 5, 2025 at 4:20 PM CEST, Jan Beulich wrote: > On 05.06.2025 16:15, Alejandro Vallejo wrote: >> On Mon Jun 2, 2025 at 4:24 PM CEST, Jan Beulich wrote: >>> On 02.06.2025 16:19, Alejandro Vallejo wrote: >>>> On Mon Jun 2, 2025 at 9:51 AM CEST, Jan Beulich wrote: >>>>> On 30.05.2025 14:02, Alejandro Vallejo wrote: >>>>>> --- a/xen/include/asm-generic/device.h >>>>>> +++ b/xen/include/asm-generic/device.h >>>>>> @@ -6,9 +6,7 @@ >>>>>> >>>>>> enum device_type >>>>>> { >>>>>> -#ifdef CONFIG_HAS_DEVICE_TREE >>>>>> DEV_DT, >>>>>> -#endif >>>>> >>>>> Why would this enumerator need exposing on a non-DT arch? In fact I would >>>>> have >>>>> hoped for ... >>>> >>>> A non-DT arch would not include this. x86 doesn't. >>> >>> Both here and ... >>> >>>>>> DEV_PCI >>>>> >>>>> ... this to be hidden for arch-es not supporting PCI. >>>>> >>>>> Similar concerns elsewhere in this change. >>>> >>>> This file is exclusively used by arches supporting DT to abstract away >>>> where >>>> the device came from. x86 does not use it at all, and while it wouldn't be >>>> impossible to compile-out DEV_PCI, it would needlessly pollute the >>>> codebase with >>>> no measurable gain, because the abstractions still need to stay. >>> >>> ... here: In "xen/include/asm-generic/device.h" there's nothing at all >>> saying >>> that this file is a DT-only one. Instead there is something in there saying >>> that it's suitable to use in the entirely "generic" case. >>> >>> Jan >> >> Try to use it from x86 and observe the build system catch fire. It could be >> made >> to not go on fire, but it implies heavy refactoring in x86 (particularly >> IOMMU >> code) for no good reason because there's no devices in a DTB to disambiguate. >> >> How about adding this to the top of the header? >> >> ``` >> /* >> * This header helps DTB-based architectures abstract away where a >> particular >> * device comes from; be it the DTB itself or enumerated on a PCI bus. >> */ >> >> [snip] >> >> #ifndef CONFIG_HAS_DEVICE_TREE >> #error "Header meant to be used exclusively by DTB-base architectures." >> #endif >> ``` > > Might be fine, together with giving the file a name somewhat referring to DT. > > Jan That would bring it out of sync with x86's asm/device.h. Both of them define device_t and doing so in differently named headers would just be confusing for everyone. Cheers, Alejandro
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |