[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 02/14] xen/asm-generic: introduce generic device.h
On Fri, 2023-11-24 at 11:01 +0000, Julien Grall wrote: > Hi Oleksii, > > On 17/11/2023 12:24, Oleksii Kurochko wrote: > > Arm, PPC and RISC-V use the same device.h thereby device.h > > was moved to asm-generic. > > I read "was moved" as the patch should also contain some deleted > lines. > But below, I only see the file introduced. Did you intend to also > remove > the version in arm/include/asm? Yes, I just messed up with the patch version. Here is the version of the patch which remove Arm's device.h: https://gitlab.com/xen-project/people/olkur/xen/-/commit/ce845abfb57d27b0c08984f5433085c767550495 > > > Arm's device.h was taken as a base with > > the following changes: > > - #ifdef PCI related things. > > - #ifdef ACPI related things. > > - Rename #ifdef guards. > > - Add SPDX tag. > > - #ifdef CONFIG_HAS_DEVICE_TREE related things. > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx> > > --- > > It is still open question if device.h should be in asm-generic. > > Need more opinions. > > I still think it should. But I guess you want others to answer? If so > it > would be worth to point out from who you seek opinions. > > > --- > > Changes in V3: > > - ifdef device tree related things. > > - update the commit message > > --- > > Changes in V2: > > - take ( as common ) device.h from Arm as PPC and RISC-V > > use it as a base. > > - #ifdef PCI related things. > > - #ifdef ACPI related things. > > - rename DEVICE_GIC to DEVIC_IC. > > - rename #ifdef guards. > > - switch Arm and PPC to generic device.h > > - add SPDX tag > > - update the commit message > > > > --- > > xen/include/asm-generic/device.h | 147 > > +++++++++++++++++++++++++++++++ > > 1 file changed, 147 insertions(+) > > create mode 100644 xen/include/asm-generic/device.h > > > > diff --git a/xen/include/asm-generic/device.h b/xen/include/asm- > > generic/device.h > > new file mode 100644 > > index 0000000000..7ef5aa955a > > --- /dev/null > > +++ b/xen/include/asm-generic/device.h > > @@ -0,0 +1,147 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +#ifndef __ASM_GENERIC_DEVICE_H__ > > +#define __ASM_GENERIC_DEVICE_H__ > > + > > +enum device_type > > +{ > > +#ifdef CONFIG_HAS_DEVICE_TREE > > + DEV_DT, > > +#endif > > + > > +#ifdef HAS_PCI > > + DEV_PCI, > > +#endif > > +}; > > + > > +struct dev_archdata { > > + void *iommu; /* IOMMU private data */ > > +}; > > + > > +/* struct device - The basic device structure */ > > +struct device > > +{ > > + enum device_type type; > > +#ifdef CONFIG_HAS_DEVICE_TREE > > + struct dt_device_node *of_node; /* Used by drivers imported > > from Linux */ > > +#endif > > + struct dev_archdata archdata; > > + struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU > > instance data */ > > +}; > > + > > +typedef struct device device_t; > > + > > +#ifdef CONFIG_HAS_DEVICE_TREE > > +#include <xen/device_tree.h> > > +#endif > > NIT: Could we try to rationalize the number of #ifdef CONFIG_HAS_*? > For > example ... > > > + > > +#ifdef HAS_PCI > > +#define dev_is_pci(dev) ((dev)->type == DEV_PCI) > > +#endif > > + > > +#ifdef CONFIG_HAS_DEVICE_TREE > > +#define dev_is_dt(dev) ((dev)->type == DEV_DT) > > +#endif > > ... this is another definition for Device-Tree only. It could be > easily > moved above the definitnion of dev_is_pci(). The others would be the > DT_DEVICE_*() helpers. Yes, sure. I can combine CONFIG_HAS_DEVICE_TREE related things better. > > > + > > +enum device_class > > +{ > > + DEVICE_SERIAL, > > + DEVICE_IOMMU, > > + DEVICE_IC, > > I guess you mean INTERRUPT_CONTROLLER? If so, can this be spelt out? > (I > don't think shorthand version is worth it here) Sure, I'll change to INTERRUPT_CONTROLLER. ~ Oleksii
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |