Messages in this thread |  | | | Date | Mon, 25 Oct 2004 17:28:54 +0900 | | From | Tejun Heo <> | | Subject | Re: [RFC/PATCH] Per-device parameter support (13/16) |
| |
On Mon, Oct 25, 2004 at 04:08:10PM +1000, Rusty Russell wrote: > You need __attribute__((aligned(sizeof(void *))): see moduleparam.h. > The padding between structures is arch-dependent (x86-64 found out the > hard way when we changed kernel_param previously). Not sure why you're > using a separate section here anyway.
As the defs array of struct device_paramset_def is assembled by putting struct device_params continuosly, there shouldn't be any other intervening data while defining the array. So, the wrapping arguments (struct kparam_array, struct kparam_flag) should stay out of line. That's why they're put in different subsections.
> > +/* Bit flag */ > > +#define __DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, Inv, Perm, Desc) \ > > + param_check_uint(__devparam_data(Name), \ > > + &(((__devparam_type *)0)->Field)); \ > > + static struct kparam_flag __devparam_data(__param_flag_##Name) \ > > + __devparam_extra_section = { 0, Flag, Inv }; \ > > + __DEVICE_PARAM_CALL_RANGED(Name, param_set_flag, param_get_flag,\ > > + &__devparam_data(__param_flag_##Name), \ > > + sizeof(struct kparam_flag), 1, 0, Dfl, 0, Perm, Desc, \ > > + offsetof(struct kparam_flag, pflags), \ > > + offsetof(__devparam_type, Field), -1, -1) > > + > > +#define DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, Perm, Desc) \ > > + __DEVICE_PARAM_FLAG(Name, Field, Flag, Dfl, 0, Perm, Desc) > > Haven't read closely, but why is FLAG special?
Because flag requires wrapping struct kparam_flag structure. As I've explained in another mail, wrapping structures need to be copied and fixed up before being used. So, the sizeof and offsetof's.
> I think your macros might be better off always having the range, which > would reduce the number of functions.. I like including the description > in the macros: that's an improvement. > > I always disliked the _NAMED versions (if it's a good name for users, > it's usually a good name for the variable and vice versa), but you might > want to consider always having it if that's the common case.
Again, I just think it's better to be a bit rich in module/device parameter handling and provide convenient interface as it can reduce code in many places. But that's me. :-)
As this is the last mail of this series of replies, I want to add some more stuff to discuss about.
1. What do you think about merging bus_paramset_def, dev_paramset_def and aux_paramset_defs into single paramset_defs array. The only functional difference is that, with the change, a driver should explicitly specify the corresponding bus paramset_def and pass the resulting paramset to the bus registration function explicitly if the bus wants to accept its own paramset.
I think it's better because it's simpler and more consistent, and it gives drivers a chance to peek or mangle bus parameters as needed.
2. What do you think about adding support for user-controllable device - driver association? We'll be able to tell the kernel to 'associate device A to driver B with these parameters'. With proper user-space programs we'll be able to have fine control over device drivers. Also, user-space tool will be able to set the kernel device name as it likes from the start so discrepancies between kernel device names and user-space device names can be removed (udev).
What I was thinking was
- adding a sysctl variable which enables the feature and when it's enabled the kernel stops automatically associating devices to matching drivers. - User space tool can write to a driver sysfs node (maybe named attach) to attach a specific device to the driver. - A device - driver association is represented by a child kobject of the associated driver or device. - Detaching can be done by deleting the file representing the association. Or writing to a detach sysfs file.
As there's no kobject which represents device - driver association, devparam currently makes a child kobject of the corresponding device node itself (the params subdirectory).
3. How about adding a mechanism to automatically add min, max and default value to @Desc. Though I'm not so sure about this one.
Thanks a lot. :-)
-- tejun
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |