Re: [PATCH v1 04/29] xen/asm-generic: introduce stub header device.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.10.2023 12:42, Julien Grall wrote:
> On 19/10/2023 10:14, Jan Beulich wrote:
>> On 14.09.2023 16:56, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/include/asm-generic/device.h
>>> @@ -0,0 +1,65 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +#ifndef __ASM_GENERIC_DEVICE_H__
>>> +#define __ASM_GENERIC_DEVICE_H__
>>> +
>>> +struct dt_device_node;
>>> +
>>> +enum device_type
>>> +{
>>> +    DEV_DT,
>>> +    DEV_PCI,
>>> +};
>>
>> Are both of these really generic?
> 
> I think can be re-used for RISC-V to have an abstract view a device. 
> This is for instance used in the IOMMU code where both PCI and platform 
> (here called DT) can be assigned to a domain. The driver will need to 
> know the difference, but the common layer doesn't need to.

Question to me is whether DT and PCI can be considered "common", which
is a prereq for being used here.

>>> +struct device {
>>> +    enum device_type type;
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> +    struct dt_device_node *of_node; /* Used by drivers imported from Linux 
>>> */
>>> +#endif
>>> +};
>>> +
>>> +enum device_class
>>> +{
>>> +    DEVICE_SERIAL,
>>> +    DEVICE_IOMMU,
>>> +    DEVICE_GIC,
>>
>> This one certainly is Arm-specific.
> 
> This could be renamed to DEVICE_IC (or INTERRUPT_CONTROLLER)
> 
>>
>>> +    DEVICE_PCI_HOSTBRIDGE,
>>
>> And this one's PCI-specific.
> 
> Are you suggesting to #ifdef it? If so, I don't exactly see the value here.

What to do with it is secondary to me. I was questioning its presence here.

>> Overall same question as before: Are you expecting that RISC-V is going to
>> get away without a customized header? I wouldn't think so.
> 
> I think it can be useful. Most likely you will have multiple drivers for 
> a class and you may want to initialize certain device class early than 
> others. See how it is used in device_init().

I'm afraid I don't see how your reply relates to the question of such a
fallback header being sensible to have, or whether instead RISC-V will
need its own private header anyway.

Jan