Hi, Binyamin Sharet <[email protected]> writes: > Feature control mechanism allows addition of dynamic features to > gadgetfs. > > It provides a user-mode driver the ability to control those features, > by querying the supported and enabled features and enable/disable > features in runtime via ioctl on ep0 fd. > --- > drivers/usb/gadget/legacy/inode.c | 67 > +++++++++++++++++++++++++++++++++++++-- > include/uapi/linux/usb/gadgetfs.h | 29 +++++++++++++++++ > 2 files changed, 94 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/legacy/inode.c > b/drivers/usb/gadget/legacy/inode.c > index 16104b5e..f54200d 100644 > --- a/drivers/usb/gadget/legacy/inode.c > +++ b/drivers/usb/gadget/legacy/inode.c > @@ -144,6 +144,8 @@ struct dev_data { > wait_queue_head_t wait; > struct super_block *sb; > struct dentry *dentry; > + struct usb_gadgetfs_features enabled_features; > + struct usb_gadgetfs_features supported_features; > > /* except this scratch i/o buffer for ep0 */ > u8 rbuf [256]; > @@ -1235,14 +1237,75 @@ out: > return mask; > } > > +/* feature control */ > + > +static bool > +is_feature_set(struct usb_gadgetfs_features * features, unsigned int > feature)
patch is line-wrapped. Please fix it. Also, you should be taking u64 as
argument here.
> +{
> + uint64_t feature_bit;
not userspace types in kernel ;-)
> + /* overflow */
> + if(feature >= (sizeof(*features) * 8))
run checkpatch.pl on this patch. Formatting is pretty bad.
> + return false;
> + feature_bit = 1 << (feature % 64);
> + return (features->bitmap[feature / 64] & feature_bit) != 0;
> +}
> +
> +/* clear a feature in a feature bitmap */
> +static bool
> +feature_clear(struct usb_gadgetfs_features * features, unsigned int
> feature)
> +{
> + uint64_t feature_bit;
> + /* overflow */
> + if(feature >= (sizeof(*features) * 8))
> + return false;
> + feature_bit = 1 << (feature % 64);
> + features->bitmap[feature / 64] &= ~feature_bit;
> + return true;
> +}
> +
> +/* set a feature in a feature bitmap */
> +static bool
> +feature_set(struct usb_gadgetfs_features * features, unsigned int feature)
> +{
> + uint64_t feature_bit;
> + /* overflow */
> + if(feature >= (sizeof(*features) * 8))
> + return false;
> + feature_bit = 1 << (feature % 64);
> + features->bitmap[feature / 64] |= feature_bit;
> + return true;
> +}
you can't allow for set/clear/get on "reserved" bits. Also, I had
mentioned that we should hardcode bit 0 to 1 when this is enabled.
> diff --git a/include/uapi/linux/usb/gadgetfs.h
> b/include/uapi/linux/usb/gadgetfs.h
> index 0bb12e0..9d304e0 100644
> --- a/include/uapi/linux/usb/gadgetfs.h
> +++ b/include/uapi/linux/usb/gadgetfs.h
> @@ -85,4 +85,33 @@ struct usb_gadgetfs_event {
> */
> #define GADGETFS_CLEAR_HALT _IO('g', 3)
>
> +
> +
> +struct usb_gadgetfs_features {
> + uint64_t bitmap[4];
this should really be:
uint64_t features0;
/* Keep 24 bytes reserved for possible future usage */
uint8_t RESERVED[24];
--
balbi
signature.asc
Description: PGP signature
