[U-Boot-Users] [Patch 1/9] U-boot-V2:ID: Sync mod_devicetable

[Robert Schwebel]

On Wed, Jun 18, 2008 at 07:33:50AM -0500, Menon, Nishanth wrote:
>  include/linux/mod_devicetable.h |  382 ++++++++++++++++++++++++++++++++++++++++

This is u-boot, not linux. Please let's not introduce more of the
copy-and-paste things we have around from the past.

> +/*
> + * Device tables which are exported to userspace via
> + * scripts/mod/file2alias.c.  You must keep that file in sync with this
> + * header.
> + */

U-Boot doesn't have userspace.

> +
> +#ifndef LINUX_MOD_DEVICETABLE_H
> +#define LINUX_MOD_DEVICETABLE_H
> +
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +typedef unsigned long kernel_ulong_t;
> +#endif

No Linux, no __KERNEL__.

> +#define PCI_ANY_ID (~0)

Until now, there is no PCI support in u-boot-v2. It would be great to
have it, but please let's first add PCI support and then stuff that
depends on it :-)

> +struct pci_device_id {
> +       __u32 vendor, device;           /* Vendor and device ID or PCI_ANY_ID*/
> +       __u32 subvendor, subdevice;     /* Subsystem ID's or PCI_ANY_ID */
> +       __u32 class, class_mask;        /* (class,subclass,prog-if) triplet */
> +       kernel_ulong_t driver_data;     /* Data private to the driver */
> +};

I assume you don't have your controller attached via pci; so before we
have proper PCI support, please don't do this. It might happen that it
turns out that the data model above doesn't fit u-boot's needs in the
end.

Pushing in stuff just because it was there let's people assume that
someone has *thought* about PCI in u-boot-v2. The net one comes and
tries to use it and *bang*. And even worse, if somebody tries to really
add PCI support, he will probably collide with this.

> +#define IEEE1394_MATCH_VENDOR_ID       0x0001
> +#define IEEE1394_MATCH_MODEL_ID                0x0002
> +#define IEEE1394_MATCH_SPECIFIER_ID    0x0004
> +#define IEEE1394_MATCH_VERSION         0x0008

We also don't have 1394.

> + * Device table entry for "new style" table-driven USB drivers.
> + * User mode code can read these tables to choose which modules to load.
> + * Declare the table as a MODULE_DEVICE_TABLE.

Nevertheless, this "new style" model is a good idea and is worth being
considered for the concept of i2c in u-boot-v2.

> +/**
> + * struct usb_device_id - identifies USB devices for probing and hotplugging
> + * @match_flags: Bit mask controlling of the other fields are used to match

If you add header documentation, please use Doygen-style comments.
U-Boot-v2 has a Doygen-generated manual which was layed out carefully,
so please

a) check what the real interfaces of a subsystem are
b) document them
c) integrate them into the main manual, in a way that it is readable and
   can be used as an introduction for somebody who wants to start
   learning about the concepts.

> +/* Some useful macros to use to create struct usb_device_id */

Argh, we don't have USB in u-boot-v2 yet. Same arguments as above with
PCI.

> +/* s390 CCW devices */

Although u-boot-v2 on S390 would surely be a cool idea, I assume that
doesn't fit here.

> +struct acpi_device_id {
> +       __u8 id[ACPI_ID_LEN];
> +       kernel_ulong_t driver_data;
> +};

... and there is also no ACPI support available yet.

> +struct pnp_device_id {
> +       __u8 id[PNP_ID_LEN];
> +       kernel_ulong_t driver_data;
> +};

... and no pnp.

> +struct of_device_id {
> +       char    name[32];
> +       char    type[32];
> +       char    compatible[128];
> +#ifdef __KERNEL__
> +       void    *data;
> +#else
> +       kernel_ulong_t data;
> +#endif
> +};

No of-tree support here. U-Boot has to *provide* an oftree, not to
consume one.

I leave the other subsystems out that are not available yet.

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9