Message ID | 1395225803-25037-2-git-send-email-richard@nod.at |
---|---|
State | Rejected |
Headers | show |
Hi Richard, On Mar 19, Richard Weinberger wrote: > The ->get() function is not optional. > [..] > static struct kernel_param_ops ubiblock_param_ops = { > .set = ubiblock_set_param, > + .get = param_get_charp, > }; > module_param_cb(block, &ubiblock_param_ops, NULL, 0); The comment for the function says they are both optional: /** * module_param_cb - general callback for a module/cmdline parameter * @name: a valid C identifier which is the parameter name. * @ops: the set & get operations for this parameter. * @perm: visibility in sysfs. * * The ops can have NULL set or get functions. */ This has no visibility in sysfs, so I can't see how having a NULL get() is a problem. What's the issue you're trying to fix here? Maybe I'm missing something?
Am 19.03.2014 14:15, schrieb Ezequiel Garcia: > Hi Richard, > > On Mar 19, Richard Weinberger wrote: >> The ->get() function is not optional. >> > > [..] >> static struct kernel_param_ops ubiblock_param_ops = { >> .set = ubiblock_set_param, >> + .get = param_get_charp, >> }; >> module_param_cb(block, &ubiblock_param_ops, NULL, 0); > > The comment for the function says they are both optional: > > /** > * module_param_cb - general callback for a module/cmdline parameter > * @name: a valid C identifier which is the parameter name. > * @ops: the set & get operations for this parameter. > * @perm: visibility in sysfs. > * > * The ops can have NULL set or get functions. > */ > > This has no visibility in sysfs, so I can't see how having a NULL > get() is a problem. What's the issue you're trying to fix here? > > Maybe I'm missing something? > Maybe I've misread the struct. struct kernel_param_ops { /* How the ops should behave */ unsigned int flags; /* Returns 0, or -errno. arg is in kp->arg. */ int (*set)(const char *val, const struct kernel_param *kp); /* Returns length written or -errno. Buffer is 4k (ie. be short!) */ int (*get)(char *buffer, const struct kernel_param *kp); /* Optional function to free kp->arg when module unloaded. */ void (*free)(void *arg); }; To me it looks like only ->free is optional. Thanks, //richard
Am 19.03.2014 14:15, schrieb Ezequiel Garcia: > Hi Richard, > > On Mar 19, Richard Weinberger wrote: >> The ->get() function is not optional. >> > > [..] >> static struct kernel_param_ops ubiblock_param_ops = { >> .set = ubiblock_set_param, >> + .get = param_get_charp, >> }; >> module_param_cb(block, &ubiblock_param_ops, NULL, 0); > > The comment for the function says they are both optional: > > /** > * module_param_cb - general callback for a module/cmdline parameter > * @name: a valid C identifier which is the parameter name. > * @ops: the set & get operations for this parameter. > * @perm: visibility in sysfs. > * > * The ops can have NULL set or get functions. > */ > > This has no visibility in sysfs, so I can't see how having a NULL > get() is a problem. What's the issue you're trying to fix here? > > Maybe I'm missing something? > BTW: I think we can omit ->get() as some other drivers also only have ->set(). Who want the credit for a documentation fix? ;) Thanks, //richard
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c index 7ff473c..9ef7df7 100644 --- a/drivers/mtd/ubi/block.c +++ b/drivers/mtd/ubi/block.c @@ -158,6 +158,7 @@ static int __init ubiblock_set_param(const char *val, static struct kernel_param_ops ubiblock_param_ops = { .set = ubiblock_set_param, + .get = param_get_charp, }; module_param_cb(block, &ubiblock_param_ops, NULL, 0); MODULE_PARM_DESC(block, "Attach block devices to UBI volumes. Parameter format: block=<path|dev,num|dev,name>.\n"
The ->get() function is not optional. Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/block.c | 1 + 1 file changed, 1 insertion(+)