Messages in this thread | | | Date | Wed, 06 May 2015 19:15:27 +0300 | From | Daniel Baluta <> | Subject | Re: [PATCH v5 2/4] iio: core: Introduce IIO configfs support |
| |
On 05/04/2015 10:59 PM, Lars-Peter Clausen wrote: > On 05/04/2015 12:50 PM, Daniel Baluta wrote: >> This creates an IIO configfs subystem named "iio", with a default >> "triggers" >> group. >> >> Triggers group is used for handling software triggers. To create a new >> software >> trigger one must create a directory inside the trigger directory. >> >> Software trigger name MUST follow the following convention: >> * <trigger-type>-<trigger-name> >> Where: >> * <trigger_type>, specifies the interrupt source (e.g: hrtimer) >> * <trigger-name>, specifies the IIO device trigger name >> >> Failing to follow this convention will result in an directory creation >> error. >> >> E.g, assuming that hrtimer trigger type is registered with IIO software >> trigger core: >> >> $ mkdir /config/iio/triggers/hrtimer-instance1 >> > > Nice, short and clean. Looks pretty good. It's a bit of a shame that we > can't have a per type directory, but if that's how configfs works I > guess there is not much choice. > > [...] >> +static struct config_group *trigger_make_group(struct config_group >> *group, >> + const char *name) >> +{ >> + char *type_name; >> + char *trigger_name; >> + char buf[MAX_NAME_LEN]; >> + struct iio_sw_trigger *t; >> + >> + snprintf(buf, MAX_NAME_LEN, "%s", name); >> + >> + /* group name should have the form <trigger-type>-<trigger-name> */ >> + type_name = buf; >> + trigger_name = strchr(buf, '-'); >> + if (!trigger_name) { >> + pr_err("Unable to locate '-' in %s. Use <type>-<name>.\n", buf); > > Do we want to print this side channel message? Makes it pretty easy to > spam the kernel log with a rouge application.
I think this is useful, people can get crazy for random -EINVAL errors :). Also the same is done in drivers/usb/gadget/configfs.c.
> >> + return ERR_PTR(-EINVAL); >> + } >> + >> + /* replace - with \0, this nicely separates the two strings */ >> + *trigger_name = '\0'; >> + trigger_name++; >> + >> + t = iio_sw_trigger_create(type_name, trigger_name); >> + if (IS_ERR(t)) >> + return ERR_CAST(t); >> + >> + config_item_set_name(&t->group.cg_item, name); >> + >> + return &t->group; >> +} >> + >> +static void trigger_drop_group(struct config_group *group, >> + struct config_item *item) >> +{ >> + struct iio_sw_trigger *t = to_iio_sw_trigger(item); >> + >> + if (t) > > t will never be NULL.
Agree, will fix in v6. > >> + iio_sw_trigger_destroy(t); >> + config_item_put(item); >> +} >
| |