Hi Alasdair,
Alasdair G Kergon wrote:
> +/*
> + * Constructs a hardware handler object, takes custom arguments
> + */
> +typedef int (*hwh_ctr_fn) (struct hw_handler *hwh, unsigned arc, char
> **argv); +typedef void (*hwh_dtr_fn) (struct hw_handler *hwh);
> +
> +typedef void (*hwh_pg_init_fn) (struct hw_handler *hwh, unsigned bypassed,
> + struct path *path);
> +typedef unsigned (*hwh_err_fn) (struct hw_handler *hwh, struct bio *bio);
> +typedef int (*hwh_status_fn) (struct hw_handler *hwh,
> + status_type_t type,
> + char *result, unsigned int maxlen);
> +
> +/* Information about a hardware handler type */
> +struct hw_handler_type {
> + char *name;
> + struct module *module;
> +
> + hwh_ctr_fn ctr;
> + hwh_dtr_fn dtr;
> +
> + hwh_pg_init_fn pg_init;
> + hwh_err_fn err;
> + hwh_status_fn status;
> +};
Please loose the prototypes, don't use prefixes/suffixes and use more
descriptive names. Reasons are in Documentation/CodingStyle, Chapter 4.
So I suggest declaring it like this:
struct hardware_handler_operations {
char *name;
struct module *module;
int (*create) (struct hw_handler *handler, unsigned int argc, char **argv);
void (*destroy) (struct hw_handler *handler);
void (*pg_init) (struct hw_handler *handler, unsigned int bypassed,
struct path *path);
unsigned (*error) (struct hw_handler *hwh, struct bio *bio);
int (*status) (struct hw_handler *hwh, status_type_t type, char *result,
unsigned int maxlen);
};
But you might want to loose status_type_t, too. Also hw_foo is a bit generic,
isn't it? We are all dealing with "hardware" in any driver (which is
basically another word for "hardware handler"). So please be a bit more
creative on WHAT you drive.
Regards
Ingo Oeser
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
