Fetch the rotation from the fwnode and map it into a control.
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 22 +++++++++++++--
drivers/media/usb/uvc/uvc_swentity.c | 55 ++++++++++++++++++++++++++++++++----
drivers/media/usb/uvc/uvcvideo.h | 5 ++++
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 59be62ae24a4219fa9d7aacf2ae7382c95362178..5788f0c0f6604da06a7bca1b9999d0957817e75e 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -378,11 +378,18 @@ static const struct uvc_control_info uvc_ctrls[] = {
},
{
.entity = UVC_GUID_SWENTITY,
- .selector = 0,
- .index = 0,
+ .selector = UVC_SWENTITY_ORIENTATION,
+ .index = UVC_SWENTITY_ORIENTATION,
.size = 1,
.flags = UVC_CTRL_FLAG_GET_CUR,
},
+ {
+ .entity = UVC_GUID_SWENTITY,
+ .selector = UVC_SWENTITY_ROTATION,
+ .index = UVC_SWENTITY_ROTATION,
+ .size = 2,
+ .flags = UVC_CTRL_FLAG_GET_RANGE,
+ },
};
static const u32 uvc_control_classes[] = {
@@ -1025,7 +1032,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
{
.id = V4L2_CID_CAMERA_ORIENTATION,
.entity = UVC_GUID_SWENTITY,
- .selector = 0,
+ .selector = UVC_SWENTITY_ORIENTATION,
.size = 8,
.offset = 0,
.v4l2_type = V4L2_CTRL_TYPE_MENU,
@@ -1033,6 +1040,15 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
.menu_mask = GENMASK(V4L2_CAMERA_ORIENTATION_EXTERNAL,
V4L2_CAMERA_ORIENTATION_FRONT),
},
+ {
+ .id = V4L2_CID_CAMERA_SENSOR_ROTATION,
+ .entity = UVC_GUID_SWENTITY,
+ .selector = UVC_SWENTITY_ROTATION,
+ .size = 16,
+ .offset = 0,
+ .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
+ .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
+ },
};
/* ------------------------------------------------------------------------
diff --git a/drivers/media/usb/uvc/uvc_swentity.c b/drivers/media/usb/uvc/uvc_swentity.c
index 702a2c26e029a0655dade177ed2a9b88d7a4136d..60f3166addbeb7d2e431d107b23034d2d11a1812 100644
--- a/drivers/media/usb/uvc/uvc_swentity.c
+++ b/drivers/media/usb/uvc/uvc_swentity.c
@@ -10,10 +10,11 @@
#include <media/v4l2-fwnode.h>
#include "uvcvideo.h"
-static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
- u8 cs, void *data, u16 size)
+static int uvc_swentity_get_orientation(struct uvc_device *dev,
+ struct uvc_entity *entity, u8 cs,
+ void *data, u16 size)
{
- if (size < 1)
+ if (cs != UVC_SWENTITY_ORIENTATION || size != 1)
return -EINVAL;
switch (entity->swentity.props.orientation) {
@@ -30,6 +31,31 @@ static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entit
return 0;
}
+static int uvc_swentity_get_rotation(struct uvc_device *dev,
+ struct uvc_entity *entity, u8 cs, void *data,
+ u16 size)
+{
+ if (cs != UVC_SWENTITY_ROTATION || size != 2)
+ return -EINVAL;
+
+ ((u8 *)data)[0] = entity->swentity.props.rotation;
+ ((u8 *)data)[1] = entity->swentity.props.rotation >> 8;
+
+ return 0;
+}
+
+static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
+ u8 cs, void *data, u16 size)
+{
+ switch (cs) {
+ case UVC_SWENTITY_ORIENTATION:
+ return uvc_swentity_get_orientation(dev, entity, cs, data, size);
+ case UVC_SWENTITY_ROTATION:
+ return uvc_swentity_get_rotation(dev, entity, cs, data, size);
+ }
+ return -EINVAL;
+}
+
static int uvc_swentity_get_info(struct uvc_device *dev,
struct uvc_entity *entity, u8 cs, u8 *caps)
{
@@ -37,11 +63,22 @@ static int uvc_swentity_get_info(struct uvc_device *dev,
return 0;
}
+static int uvc_swentity_get_res(struct uvc_device *dev, struct uvc_entity *entity,
+ u8 cs, void *res, u16 size)
+{
+ if (size == 0)
+ return -EINVAL;
+ ((u8 *)res)[0] = 1;
+ memset(res + 1, 0, size - 1);
+ return 0;
+}
+
int uvc_swentity_init(struct uvc_device *dev)
{
static const u8 uvc_swentity_guid[] = UVC_GUID_SWENTITY;
struct v4l2_fwnode_device_properties props;
struct uvc_entity *unit;
+ u8 controls = 0;
int ret;
ret = v4l2_fwnode_device_parse(&dev->udev->dev, &props);
@@ -49,7 +86,11 @@ int uvc_swentity_init(struct uvc_device *dev)
return dev_err_probe(&dev->intf->dev, ret,
"Can't parse fwnode\n");
- if (props.orientation == V4L2_FWNODE_PROPERTY_UNSET)
+ if (props.orientation != V4L2_FWNODE_PROPERTY_UNSET)
+ controls |= BIT(UVC_SWENTITY_ORIENTATION);
+ if (props.rotation != V4L2_FWNODE_PROPERTY_UNSET)
+ controls |= BIT(UVC_SWENTITY_ROTATION);
+ if (!controls)
return 0;
unit = uvc_alloc_entity(UVC_SWENTITY_UNIT, UVC_SWENTITY_UNIT_ID, 0, 1);
@@ -60,9 +101,13 @@ int uvc_swentity_init(struct uvc_device *dev)
unit->swentity.props = props;
unit->swentity.bControlSize = 1;
unit->swentity.bmControls = (u8 *)unit + sizeof(*unit);
- unit->swentity.bmControls[0] = 1;
+ unit->swentity.bmControls[0] = controls;
unit->get_cur = uvc_swentity_get_cur;
unit->get_info = uvc_swentity_get_info;
+ unit->get_res = uvc_swentity_get_res;
+ unit->get_def = uvc_swentity_get_rotation;
+ unit->get_min = uvc_swentity_get_rotation;
+ unit->get_max = uvc_swentity_get_rotation;
strscpy(unit->name, "SWENTITY", sizeof(unit->name));
list_add_tail(&unit->list, &dev->entities);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index d6da8ed3ad4cf3377df49923e051fe04d83d2e38..7cca0dc75d11f6a13bc4f09676a5a00e80cb38f7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -45,6 +45,11 @@
#define UVC_SWENTITY_UNIT 0x7ffd
#define UVC_SWENTITY_UNIT_ID 0x101
+enum {
+ UVC_SWENTITY_ORIENTATION,
+ UVC_SWENTITY_ROTATION
+};
+
/* ------------------------------------------------------------------------
* Driver specific constants.
*/
--
2.50.0.rc0.642.g800a2b2222-googOn Thu, Jun 05, 2025 at 05:53:04PM +0000, Ricardo Ribalda wrote:
> Fetch the rotation from the fwnode and map it into a control.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 22 +++++++++++++--
> drivers/media/usb/uvc/uvc_swentity.c | 55 ++++++++++++++++++++++++++++++++----
> drivers/media/usb/uvc/uvcvideo.h | 5 ++++
> 3 files changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 59be62ae24a4219fa9d7aacf2ae7382c95362178..5788f0c0f6604da06a7bca1b9999d0957817e75e 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -378,11 +378,18 @@ static const struct uvc_control_info uvc_ctrls[] = {
> },
> {
> .entity = UVC_GUID_SWENTITY,
> - .selector = 0,
> - .index = 0,
> + .selector = UVC_SWENTITY_ORIENTATION,
> + .index = UVC_SWENTITY_ORIENTATION,
> .size = 1,
> .flags = UVC_CTRL_FLAG_GET_CUR,
> },
> + {
> + .entity = UVC_GUID_SWENTITY,
> + .selector = UVC_SWENTITY_ROTATION,
> + .index = UVC_SWENTITY_ROTATION,
> + .size = 2,
> + .flags = UVC_CTRL_FLAG_GET_RANGE,
> + },
> };
>
> static const u32 uvc_control_classes[] = {
> @@ -1025,7 +1032,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> {
> .id = V4L2_CID_CAMERA_ORIENTATION,
> .entity = UVC_GUID_SWENTITY,
> - .selector = 0,
> + .selector = UVC_SWENTITY_ORIENTATION,
> .size = 8,
> .offset = 0,
> .v4l2_type = V4L2_CTRL_TYPE_MENU,
> @@ -1033,6 +1040,15 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> .menu_mask = GENMASK(V4L2_CAMERA_ORIENTATION_EXTERNAL,
> V4L2_CAMERA_ORIENTATION_FRONT),
> },
> + {
> + .id = V4L2_CID_CAMERA_SENSOR_ROTATION,
> + .entity = UVC_GUID_SWENTITY,
> + .selector = UVC_SWENTITY_ROTATION,
> + .size = 16,
> + .offset = 0,
> + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> + },
> };
>
> /* ------------------------------------------------------------------------
> diff --git a/drivers/media/usb/uvc/uvc_swentity.c b/drivers/media/usb/uvc/uvc_swentity.c
> index 702a2c26e029a0655dade177ed2a9b88d7a4136d..60f3166addbeb7d2e431d107b23034d2d11a1812 100644
> --- a/drivers/media/usb/uvc/uvc_swentity.c
> +++ b/drivers/media/usb/uvc/uvc_swentity.c
> @@ -10,10 +10,11 @@
> #include <media/v4l2-fwnode.h>
> #include "uvcvideo.h"
>
> -static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> - u8 cs, void *data, u16 size)
> +static int uvc_swentity_get_orientation(struct uvc_device *dev,
> + struct uvc_entity *entity, u8 cs,
> + void *data, u16 size)
> {
> - if (size < 1)
> + if (cs != UVC_SWENTITY_ORIENTATION || size != 1)
> return -EINVAL;
>
> switch (entity->swentity.props.orientation) {
> @@ -30,6 +31,31 @@ static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entit
> return 0;
> }
>
> +static int uvc_swentity_get_rotation(struct uvc_device *dev,
> + struct uvc_entity *entity, u8 cs, void *data,
> + u16 size)
> +{
> + if (cs != UVC_SWENTITY_ROTATION || size != 2)
> + return -EINVAL;
> +
> + ((u8 *)data)[0] = entity->swentity.props.rotation;
> + ((u8 *)data)[1] = entity->swentity.props.rotation >> 8;
> +
> + return 0;
> +}
> +
> +static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> + u8 cs, void *data, u16 size)
> +{
> + switch (cs) {
> + case UVC_SWENTITY_ORIENTATION:
> + return uvc_swentity_get_orientation(dev, entity, cs, data, size);
> + case UVC_SWENTITY_ROTATION:
> + return uvc_swentity_get_rotation(dev, entity, cs, data, size);
> + }
> + return -EINVAL;
> +}
> +
> static int uvc_swentity_get_info(struct uvc_device *dev,
> struct uvc_entity *entity, u8 cs, u8 *caps)
> {
> @@ -37,11 +63,22 @@ static int uvc_swentity_get_info(struct uvc_device *dev,
> return 0;
> }
>
> +static int uvc_swentity_get_res(struct uvc_device *dev, struct uvc_entity *entity,
> + u8 cs, void *res, u16 size)
> +{
> + if (size == 0)
> + return -EINVAL;
The get_cur functions return an error if the size doesn't match the
expected size. I think you can return -EINVAL if size != 1.
> + ((u8 *)res)[0] = 1;
> + memset(res + 1, 0, size - 1);
And drop the memset.
> + return 0;
> +}
> +
> int uvc_swentity_init(struct uvc_device *dev)
> {
> static const u8 uvc_swentity_guid[] = UVC_GUID_SWENTITY;
> struct v4l2_fwnode_device_properties props;
> struct uvc_entity *unit;
> + u8 controls = 0;
> int ret;
>
> ret = v4l2_fwnode_device_parse(&dev->udev->dev, &props);
> @@ -49,7 +86,11 @@ int uvc_swentity_init(struct uvc_device *dev)
> return dev_err_probe(&dev->intf->dev, ret,
> "Can't parse fwnode\n");
>
> - if (props.orientation == V4L2_FWNODE_PROPERTY_UNSET)
> + if (props.orientation != V4L2_FWNODE_PROPERTY_UNSET)
> + controls |= BIT(UVC_SWENTITY_ORIENTATION);
> + if (props.rotation != V4L2_FWNODE_PROPERTY_UNSET)
> + controls |= BIT(UVC_SWENTITY_ROTATION);
> + if (!controls)
> return 0;
>
> unit = uvc_alloc_entity(UVC_SWENTITY_UNIT, UVC_SWENTITY_UNIT_ID, 0, 1);
> @@ -60,9 +101,13 @@ int uvc_swentity_init(struct uvc_device *dev)
> unit->swentity.props = props;
> unit->swentity.bControlSize = 1;
> unit->swentity.bmControls = (u8 *)unit + sizeof(*unit);
> - unit->swentity.bmControls[0] = 1;
> + unit->swentity.bmControls[0] = controls;
> unit->get_cur = uvc_swentity_get_cur;
> unit->get_info = uvc_swentity_get_info;
> + unit->get_res = uvc_swentity_get_res;
> + unit->get_def = uvc_swentity_get_rotation;
> + unit->get_min = uvc_swentity_get_rotation;
> + unit->get_max = uvc_swentity_get_rotation;
Why do you support GET_DEF, GET_MIN and GET_MAX for rotation only ?
> strscpy(unit->name, "SWENTITY", sizeof(unit->name));
>
> list_add_tail(&unit->list, &dev->entities);
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index d6da8ed3ad4cf3377df49923e051fe04d83d2e38..7cca0dc75d11f6a13bc4f09676a5a00e80cb38f7 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -45,6 +45,11 @@
> #define UVC_SWENTITY_UNIT 0x7ffd
> #define UVC_SWENTITY_UNIT_ID 0x101
>
> +enum {
> + UVC_SWENTITY_ORIENTATION,
> + UVC_SWENTITY_ROTATION
> +};
> +
> /* ------------------------------------------------------------------------
> * Driver specific constants.
> */
--
Regards,
Laurent Pinchart
On Sun, 29 Jun 2025 at 20:15, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Thu, Jun 05, 2025 at 05:53:04PM +0000, Ricardo Ribalda wrote:
> > Fetch the rotation from the fwnode and map it into a control.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 22 +++++++++++++--
> > drivers/media/usb/uvc/uvc_swentity.c | 55 ++++++++++++++++++++++++++++++++----
> > drivers/media/usb/uvc/uvcvideo.h | 5 ++++
> > 3 files changed, 74 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 59be62ae24a4219fa9d7aacf2ae7382c95362178..5788f0c0f6604da06a7bca1b9999d0957817e75e 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -378,11 +378,18 @@ static const struct uvc_control_info uvc_ctrls[] = {
> > },
> > {
> > .entity = UVC_GUID_SWENTITY,
> > - .selector = 0,
> > - .index = 0,
> > + .selector = UVC_SWENTITY_ORIENTATION,
> > + .index = UVC_SWENTITY_ORIENTATION,
> > .size = 1,
> > .flags = UVC_CTRL_FLAG_GET_CUR,
> > },
> > + {
> > + .entity = UVC_GUID_SWENTITY,
> > + .selector = UVC_SWENTITY_ROTATION,
> > + .index = UVC_SWENTITY_ROTATION,
> > + .size = 2,
> > + .flags = UVC_CTRL_FLAG_GET_RANGE,
> > + },
> > };
> >
> > static const u32 uvc_control_classes[] = {
> > @@ -1025,7 +1032,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > {
> > .id = V4L2_CID_CAMERA_ORIENTATION,
> > .entity = UVC_GUID_SWENTITY,
> > - .selector = 0,
> > + .selector = UVC_SWENTITY_ORIENTATION,
> > .size = 8,
> > .offset = 0,
> > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > @@ -1033,6 +1040,15 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > .menu_mask = GENMASK(V4L2_CAMERA_ORIENTATION_EXTERNAL,
> > V4L2_CAMERA_ORIENTATION_FRONT),
> > },
> > + {
> > + .id = V4L2_CID_CAMERA_SENSOR_ROTATION,
> > + .entity = UVC_GUID_SWENTITY,
> > + .selector = UVC_SWENTITY_ROTATION,
> > + .size = 16,
> > + .offset = 0,
> > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > + },
> > };
> >
> > /* ------------------------------------------------------------------------
> > diff --git a/drivers/media/usb/uvc/uvc_swentity.c b/drivers/media/usb/uvc/uvc_swentity.c
> > index 702a2c26e029a0655dade177ed2a9b88d7a4136d..60f3166addbeb7d2e431d107b23034d2d11a1812 100644
> > --- a/drivers/media/usb/uvc/uvc_swentity.c
> > +++ b/drivers/media/usb/uvc/uvc_swentity.c
> > @@ -10,10 +10,11 @@
> > #include <media/v4l2-fwnode.h>
> > #include "uvcvideo.h"
> >
> > -static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > - u8 cs, void *data, u16 size)
> > +static int uvc_swentity_get_orientation(struct uvc_device *dev,
> > + struct uvc_entity *entity, u8 cs,
> > + void *data, u16 size)
> > {
> > - if (size < 1)
> > + if (cs != UVC_SWENTITY_ORIENTATION || size != 1)
> > return -EINVAL;
> >
> > switch (entity->swentity.props.orientation) {
> > @@ -30,6 +31,31 @@ static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entit
> > return 0;
> > }
> >
> > +static int uvc_swentity_get_rotation(struct uvc_device *dev,
> > + struct uvc_entity *entity, u8 cs, void *data,
> > + u16 size)
> > +{
> > + if (cs != UVC_SWENTITY_ROTATION || size != 2)
> > + return -EINVAL;
> > +
> > + ((u8 *)data)[0] = entity->swentity.props.rotation;
> > + ((u8 *)data)[1] = entity->swentity.props.rotation >> 8;
> > +
> > + return 0;
> > +}
> > +
> > +static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > + u8 cs, void *data, u16 size)
> > +{
> > + switch (cs) {
> > + case UVC_SWENTITY_ORIENTATION:
> > + return uvc_swentity_get_orientation(dev, entity, cs, data, size);
> > + case UVC_SWENTITY_ROTATION:
> > + return uvc_swentity_get_rotation(dev, entity, cs, data, size);
> > + }
> > + return -EINVAL;
> > +}
> > +
> > static int uvc_swentity_get_info(struct uvc_device *dev,
> > struct uvc_entity *entity, u8 cs, u8 *caps)
> > {
> > @@ -37,11 +63,22 @@ static int uvc_swentity_get_info(struct uvc_device *dev,
> > return 0;
> > }
> >
> > +static int uvc_swentity_get_res(struct uvc_device *dev, struct uvc_entity *entity,
> > + u8 cs, void *res, u16 size)
> > +{
> > + if (size == 0)
> > + return -EINVAL;
>
> The get_cur functions return an error if the size doesn't match the
> expected size. I think you can return -EINVAL if size != 1.
>
> > + ((u8 *)res)[0] = 1;
> > + memset(res + 1, 0, size - 1);
>
> And drop the memset.
>
> > + return 0;
> > +}
> > +
> > int uvc_swentity_init(struct uvc_device *dev)
> > {
> > static const u8 uvc_swentity_guid[] = UVC_GUID_SWENTITY;
> > struct v4l2_fwnode_device_properties props;
> > struct uvc_entity *unit;
> > + u8 controls = 0;
> > int ret;
> >
> > ret = v4l2_fwnode_device_parse(&dev->udev->dev, &props);
> > @@ -49,7 +86,11 @@ int uvc_swentity_init(struct uvc_device *dev)
> > return dev_err_probe(&dev->intf->dev, ret,
> > "Can't parse fwnode\n");
> >
> > - if (props.orientation == V4L2_FWNODE_PROPERTY_UNSET)
> > + if (props.orientation != V4L2_FWNODE_PROPERTY_UNSET)
> > + controls |= BIT(UVC_SWENTITY_ORIENTATION);
> > + if (props.rotation != V4L2_FWNODE_PROPERTY_UNSET)
> > + controls |= BIT(UVC_SWENTITY_ROTATION);
> > + if (!controls)
> > return 0;
> >
> > unit = uvc_alloc_entity(UVC_SWENTITY_UNIT, UVC_SWENTITY_UNIT_ID, 0, 1);
> > @@ -60,9 +101,13 @@ int uvc_swentity_init(struct uvc_device *dev)
> > unit->swentity.props = props;
> > unit->swentity.bControlSize = 1;
> > unit->swentity.bmControls = (u8 *)unit + sizeof(*unit);
> > - unit->swentity.bmControls[0] = 1;
> > + unit->swentity.bmControls[0] = controls;
> > unit->get_cur = uvc_swentity_get_cur;
> > unit->get_info = uvc_swentity_get_info;
> > + unit->get_res = uvc_swentity_get_res;
> > + unit->get_def = uvc_swentity_get_rotation;
> > + unit->get_min = uvc_swentity_get_rotation;
> > + unit->get_max = uvc_swentity_get_rotation;
>
> Why do you support GET_DEF, GET_MIN and GET_MAX for rotation only ?
Orientation has enum type. It does not require min or max.
For get_def I could use get_cur, but 0 is as good as any other value
within range.
>
> > strscpy(unit->name, "SWENTITY", sizeof(unit->name));
> >
> > list_add_tail(&unit->list, &dev->entities);
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index d6da8ed3ad4cf3377df49923e051fe04d83d2e38..7cca0dc75d11f6a13bc4f09676a5a00e80cb38f7 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -45,6 +45,11 @@
> > #define UVC_SWENTITY_UNIT 0x7ffd
> > #define UVC_SWENTITY_UNIT_ID 0x101
> >
> > +enum {
> > + UVC_SWENTITY_ORIENTATION,
> > + UVC_SWENTITY_ROTATION
> > +};
> > +
> > /* ------------------------------------------------------------------------
> > * Driver specific constants.
> > */
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi Ricardo,
On Tue, Jul 01, 2025 at 01:26:51PM +0200, Ricardo Ribalda wrote:
> On Sun, 29 Jun 2025 at 20:15, Laurent Pinchart wrote:
> > On Thu, Jun 05, 2025 at 05:53:04PM +0000, Ricardo Ribalda wrote:
> > > Fetch the rotation from the fwnode and map it into a control.
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > drivers/media/usb/uvc/uvc_ctrl.c | 22 +++++++++++++--
> > > drivers/media/usb/uvc/uvc_swentity.c | 55 ++++++++++++++++++++++++++++++++----
> > > drivers/media/usb/uvc/uvcvideo.h | 5 ++++
> > > 3 files changed, 74 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index 59be62ae24a4219fa9d7aacf2ae7382c95362178..5788f0c0f6604da06a7bca1b9999d0957817e75e 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -378,11 +378,18 @@ static const struct uvc_control_info uvc_ctrls[] = {
> > > },
> > > {
> > > .entity = UVC_GUID_SWENTITY,
> > > - .selector = 0,
> > > - .index = 0,
> > > + .selector = UVC_SWENTITY_ORIENTATION,
> > > + .index = UVC_SWENTITY_ORIENTATION,
> > > .size = 1,
> > > .flags = UVC_CTRL_FLAG_GET_CUR,
> > > },
> > > + {
> > > + .entity = UVC_GUID_SWENTITY,
> > > + .selector = UVC_SWENTITY_ROTATION,
> > > + .index = UVC_SWENTITY_ROTATION,
> > > + .size = 2,
> > > + .flags = UVC_CTRL_FLAG_GET_RANGE,
> > > + },
> > > };
> > >
> > > static const u32 uvc_control_classes[] = {
> > > @@ -1025,7 +1032,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > {
> > > .id = V4L2_CID_CAMERA_ORIENTATION,
> > > .entity = UVC_GUID_SWENTITY,
> > > - .selector = 0,
> > > + .selector = UVC_SWENTITY_ORIENTATION,
> > > .size = 8,
> > > .offset = 0,
> > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > @@ -1033,6 +1040,15 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > .menu_mask = GENMASK(V4L2_CAMERA_ORIENTATION_EXTERNAL,
> > > V4L2_CAMERA_ORIENTATION_FRONT),
> > > },
> > > + {
> > > + .id = V4L2_CID_CAMERA_SENSOR_ROTATION,
> > > + .entity = UVC_GUID_SWENTITY,
> > > + .selector = UVC_SWENTITY_ROTATION,
> > > + .size = 16,
> > > + .offset = 0,
> > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > + },
> > > };
> > >
> > > /* ------------------------------------------------------------------------
> > > diff --git a/drivers/media/usb/uvc/uvc_swentity.c b/drivers/media/usb/uvc/uvc_swentity.c
> > > index 702a2c26e029a0655dade177ed2a9b88d7a4136d..60f3166addbeb7d2e431d107b23034d2d11a1812 100644
> > > --- a/drivers/media/usb/uvc/uvc_swentity.c
> > > +++ b/drivers/media/usb/uvc/uvc_swentity.c
> > > @@ -10,10 +10,11 @@
> > > #include <media/v4l2-fwnode.h>
> > > #include "uvcvideo.h"
> > >
> > > -static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > > - u8 cs, void *data, u16 size)
> > > +static int uvc_swentity_get_orientation(struct uvc_device *dev,
> > > + struct uvc_entity *entity, u8 cs,
> > > + void *data, u16 size)
> > > {
> > > - if (size < 1)
> > > + if (cs != UVC_SWENTITY_ORIENTATION || size != 1)
> > > return -EINVAL;
> > >
> > > switch (entity->swentity.props.orientation) {
> > > @@ -30,6 +31,31 @@ static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entit
> > > return 0;
> > > }
> > >
> > > +static int uvc_swentity_get_rotation(struct uvc_device *dev,
> > > + struct uvc_entity *entity, u8 cs, void *data,
> > > + u16 size)
> > > +{
> > > + if (cs != UVC_SWENTITY_ROTATION || size != 2)
> > > + return -EINVAL;
> > > +
> > > + ((u8 *)data)[0] = entity->swentity.props.rotation;
> > > + ((u8 *)data)[1] = entity->swentity.props.rotation >> 8;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > > + u8 cs, void *data, u16 size)
> > > +{
> > > + switch (cs) {
> > > + case UVC_SWENTITY_ORIENTATION:
> > > + return uvc_swentity_get_orientation(dev, entity, cs, data, size);
> > > + case UVC_SWENTITY_ROTATION:
> > > + return uvc_swentity_get_rotation(dev, entity, cs, data, size);
> > > + }
> > > + return -EINVAL;
> > > +}
> > > +
> > > static int uvc_swentity_get_info(struct uvc_device *dev,
> > > struct uvc_entity *entity, u8 cs, u8 *caps)
> > > {
> > > @@ -37,11 +63,22 @@ static int uvc_swentity_get_info(struct uvc_device *dev,
> > > return 0;
> > > }
> > >
> > > +static int uvc_swentity_get_res(struct uvc_device *dev, struct uvc_entity *entity,
> > > + u8 cs, void *res, u16 size)
> > > +{
> > > + if (size == 0)
> > > + return -EINVAL;
> >
> > The get_cur functions return an error if the size doesn't match the
> > expected size. I think you can return -EINVAL if size != 1.
> >
> > > + ((u8 *)res)[0] = 1;
> > > + memset(res + 1, 0, size - 1);
> >
> > And drop the memset.
> >
> > > + return 0;
> > > +}
> > > +
> > > int uvc_swentity_init(struct uvc_device *dev)
> > > {
> > > static const u8 uvc_swentity_guid[] = UVC_GUID_SWENTITY;
> > > struct v4l2_fwnode_device_properties props;
> > > struct uvc_entity *unit;
> > > + u8 controls = 0;
> > > int ret;
> > >
> > > ret = v4l2_fwnode_device_parse(&dev->udev->dev, &props);
> > > @@ -49,7 +86,11 @@ int uvc_swentity_init(struct uvc_device *dev)
> > > return dev_err_probe(&dev->intf->dev, ret,
> > > "Can't parse fwnode\n");
> > >
> > > - if (props.orientation == V4L2_FWNODE_PROPERTY_UNSET)
> > > + if (props.orientation != V4L2_FWNODE_PROPERTY_UNSET)
> > > + controls |= BIT(UVC_SWENTITY_ORIENTATION);
> > > + if (props.rotation != V4L2_FWNODE_PROPERTY_UNSET)
> > > + controls |= BIT(UVC_SWENTITY_ROTATION);
> > > + if (!controls)
> > > return 0;
> > >
> > > unit = uvc_alloc_entity(UVC_SWENTITY_UNIT, UVC_SWENTITY_UNIT_ID, 0, 1);
> > > @@ -60,9 +101,13 @@ int uvc_swentity_init(struct uvc_device *dev)
> > > unit->swentity.props = props;
> > > unit->swentity.bControlSize = 1;
> > > unit->swentity.bmControls = (u8 *)unit + sizeof(*unit);
> > > - unit->swentity.bmControls[0] = 1;
> > > + unit->swentity.bmControls[0] = controls;
> > > unit->get_cur = uvc_swentity_get_cur;
> > > unit->get_info = uvc_swentity_get_info;
> > > + unit->get_res = uvc_swentity_get_res;
> > > + unit->get_def = uvc_swentity_get_rotation;
> > > + unit->get_min = uvc_swentity_get_rotation;
> > > + unit->get_max = uvc_swentity_get_rotation;
> >
> > Why do you support GET_DEF, GET_MIN and GET_MAX for rotation only ?
>
> Orientation has enum type. It does not require min or max.
>
> For get_def I could use get_cur, but 0 is as good as any other value
> within range.
Both the orientation and rotation are read-only, and should report min
== max == def == cur. What am I missing ?
> > > strscpy(unit->name, "SWENTITY", sizeof(unit->name));
> > >
> > > list_add_tail(&unit->list, &dev->entities);
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index d6da8ed3ad4cf3377df49923e051fe04d83d2e38..7cca0dc75d11f6a13bc4f09676a5a00e80cb38f7 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -45,6 +45,11 @@
> > > #define UVC_SWENTITY_UNIT 0x7ffd
> > > #define UVC_SWENTITY_UNIT_ID 0x101
> > >
> > > +enum {
> > > + UVC_SWENTITY_ORIENTATION,
> > > + UVC_SWENTITY_ROTATION
> > > +};
> > > +
> > > /* ------------------------------------------------------------------------
> > > * Driver specific constants.
> > > */
--
Regards,
Laurent Pinchart
On Mon, 14 Jul 2025 at 16:31, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> On Tue, Jul 01, 2025 at 01:26:51PM +0200, Ricardo Ribalda wrote:
> > On Sun, 29 Jun 2025 at 20:15, Laurent Pinchart wrote:
> > > On Thu, Jun 05, 2025 at 05:53:04PM +0000, Ricardo Ribalda wrote:
> > > > Fetch the rotation from the fwnode and map it into a control.
> > > >
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > > drivers/media/usb/uvc/uvc_ctrl.c | 22 +++++++++++++--
> > > > drivers/media/usb/uvc/uvc_swentity.c | 55 ++++++++++++++++++++++++++++++++----
> > > > drivers/media/usb/uvc/uvcvideo.h | 5 ++++
> > > > 3 files changed, 74 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > index 59be62ae24a4219fa9d7aacf2ae7382c95362178..5788f0c0f6604da06a7bca1b9999d0957817e75e 100644
> > > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > @@ -378,11 +378,18 @@ static const struct uvc_control_info uvc_ctrls[] = {
> > > > },
> > > > {
> > > > .entity = UVC_GUID_SWENTITY,
> > > > - .selector = 0,
> > > > - .index = 0,
> > > > + .selector = UVC_SWENTITY_ORIENTATION,
> > > > + .index = UVC_SWENTITY_ORIENTATION,
> > > > .size = 1,
> > > > .flags = UVC_CTRL_FLAG_GET_CUR,
> > > > },
> > > > + {
> > > > + .entity = UVC_GUID_SWENTITY,
> > > > + .selector = UVC_SWENTITY_ROTATION,
> > > > + .index = UVC_SWENTITY_ROTATION,
> > > > + .size = 2,
> > > > + .flags = UVC_CTRL_FLAG_GET_RANGE,
> > > > + },
> > > > };
> > > >
> > > > static const u32 uvc_control_classes[] = {
> > > > @@ -1025,7 +1032,7 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > > {
> > > > .id = V4L2_CID_CAMERA_ORIENTATION,
> > > > .entity = UVC_GUID_SWENTITY,
> > > > - .selector = 0,
> > > > + .selector = UVC_SWENTITY_ORIENTATION,
> > > > .size = 8,
> > > > .offset = 0,
> > > > .v4l2_type = V4L2_CTRL_TYPE_MENU,
> > > > @@ -1033,6 +1040,15 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > > > .menu_mask = GENMASK(V4L2_CAMERA_ORIENTATION_EXTERNAL,
> > > > V4L2_CAMERA_ORIENTATION_FRONT),
> > > > },
> > > > + {
> > > > + .id = V4L2_CID_CAMERA_SENSOR_ROTATION,
> > > > + .entity = UVC_GUID_SWENTITY,
> > > > + .selector = UVC_SWENTITY_ROTATION,
> > > > + .size = 16,
> > > > + .offset = 0,
> > > > + .v4l2_type = V4L2_CTRL_TYPE_INTEGER,
> > > > + .data_type = UVC_CTRL_DATA_TYPE_UNSIGNED,
> > > > + },
> > > > };
> > > >
> > > > /* ------------------------------------------------------------------------
> > > > diff --git a/drivers/media/usb/uvc/uvc_swentity.c b/drivers/media/usb/uvc/uvc_swentity.c
> > > > index 702a2c26e029a0655dade177ed2a9b88d7a4136d..60f3166addbeb7d2e431d107b23034d2d11a1812 100644
> > > > --- a/drivers/media/usb/uvc/uvc_swentity.c
> > > > +++ b/drivers/media/usb/uvc/uvc_swentity.c
> > > > @@ -10,10 +10,11 @@
> > > > #include <media/v4l2-fwnode.h>
> > > > #include "uvcvideo.h"
> > > >
> > > > -static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > > > - u8 cs, void *data, u16 size)
> > > > +static int uvc_swentity_get_orientation(struct uvc_device *dev,
> > > > + struct uvc_entity *entity, u8 cs,
> > > > + void *data, u16 size)
> > > > {
> > > > - if (size < 1)
> > > > + if (cs != UVC_SWENTITY_ORIENTATION || size != 1)
> > > > return -EINVAL;
> > > >
> > > > switch (entity->swentity.props.orientation) {
> > > > @@ -30,6 +31,31 @@ static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entit
> > > > return 0;
> > > > }
> > > >
> > > > +static int uvc_swentity_get_rotation(struct uvc_device *dev,
> > > > + struct uvc_entity *entity, u8 cs, void *data,
> > > > + u16 size)
> > > > +{
> > > > + if (cs != UVC_SWENTITY_ROTATION || size != 2)
> > > > + return -EINVAL;
> > > > +
> > > > + ((u8 *)data)[0] = entity->swentity.props.rotation;
> > > > + ((u8 *)data)[1] = entity->swentity.props.rotation >> 8;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int uvc_swentity_get_cur(struct uvc_device *dev, struct uvc_entity *entity,
> > > > + u8 cs, void *data, u16 size)
> > > > +{
> > > > + switch (cs) {
> > > > + case UVC_SWENTITY_ORIENTATION:
> > > > + return uvc_swentity_get_orientation(dev, entity, cs, data, size);
> > > > + case UVC_SWENTITY_ROTATION:
> > > > + return uvc_swentity_get_rotation(dev, entity, cs, data, size);
> > > > + }
> > > > + return -EINVAL;
> > > > +}
> > > > +
> > > > static int uvc_swentity_get_info(struct uvc_device *dev,
> > > > struct uvc_entity *entity, u8 cs, u8 *caps)
> > > > {
> > > > @@ -37,11 +63,22 @@ static int uvc_swentity_get_info(struct uvc_device *dev,
> > > > return 0;
> > > > }
> > > >
> > > > +static int uvc_swentity_get_res(struct uvc_device *dev, struct uvc_entity *entity,
> > > > + u8 cs, void *res, u16 size)
> > > > +{
> > > > + if (size == 0)
> > > > + return -EINVAL;
> > >
> > > The get_cur functions return an error if the size doesn't match the
> > > expected size. I think you can return -EINVAL if size != 1.
> > >
> > > > + ((u8 *)res)[0] = 1;
> > > > + memset(res + 1, 0, size - 1);
> > >
> > > And drop the memset.
> > >
> > > > + return 0;
> > > > +}
> > > > +
> > > > int uvc_swentity_init(struct uvc_device *dev)
> > > > {
> > > > static const u8 uvc_swentity_guid[] = UVC_GUID_SWENTITY;
> > > > struct v4l2_fwnode_device_properties props;
> > > > struct uvc_entity *unit;
> > > > + u8 controls = 0;
> > > > int ret;
> > > >
> > > > ret = v4l2_fwnode_device_parse(&dev->udev->dev, &props);
> > > > @@ -49,7 +86,11 @@ int uvc_swentity_init(struct uvc_device *dev)
> > > > return dev_err_probe(&dev->intf->dev, ret,
> > > > "Can't parse fwnode\n");
> > > >
> > > > - if (props.orientation == V4L2_FWNODE_PROPERTY_UNSET)
> > > > + if (props.orientation != V4L2_FWNODE_PROPERTY_UNSET)
> > > > + controls |= BIT(UVC_SWENTITY_ORIENTATION);
> > > > + if (props.rotation != V4L2_FWNODE_PROPERTY_UNSET)
> > > > + controls |= BIT(UVC_SWENTITY_ROTATION);
> > > > + if (!controls)
> > > > return 0;
> > > >
> > > > unit = uvc_alloc_entity(UVC_SWENTITY_UNIT, UVC_SWENTITY_UNIT_ID, 0, 1);
> > > > @@ -60,9 +101,13 @@ int uvc_swentity_init(struct uvc_device *dev)
> > > > unit->swentity.props = props;
> > > > unit->swentity.bControlSize = 1;
> > > > unit->swentity.bmControls = (u8 *)unit + sizeof(*unit);
> > > > - unit->swentity.bmControls[0] = 1;
> > > > + unit->swentity.bmControls[0] = controls;
> > > > unit->get_cur = uvc_swentity_get_cur;
> > > > unit->get_info = uvc_swentity_get_info;
> > > > + unit->get_res = uvc_swentity_get_res;
> > > > + unit->get_def = uvc_swentity_get_rotation;
> > > > + unit->get_min = uvc_swentity_get_rotation;
> > > > + unit->get_max = uvc_swentity_get_rotation;
> > >
> > > Why do you support GET_DEF, GET_MIN and GET_MAX for rotation only ?
> >
> > Orientation has enum type. It does not require min or max.
> >
> > For get_def I could use get_cur, but 0 is as good as any other value
> > within range.
>
> Both the orientation and rotation are read-only, and should report min
> == max == def == cur. What am I missing ?
V4L2_CID_CAMERA_ORIENTATION has type V4L2_CTRL_TYPE_MENU
In _uvc_queryctrl_boundaries(), min and max are auto calculated, they
are not based on information from the device.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/uvc/uvc_ctrl.c#n1504
I can change unit->get_(def|min|max)=uvc_swentity_get_cur if you
think that it is cleaner...
Regards!
>
> > > > strscpy(unit->name, "SWENTITY", sizeof(unit->name));
> > > >
> > > > list_add_tail(&unit->list, &dev->entities);
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index d6da8ed3ad4cf3377df49923e051fe04d83d2e38..7cca0dc75d11f6a13bc4f09676a5a00e80cb38f7 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -45,6 +45,11 @@
> > > > #define UVC_SWENTITY_UNIT 0x7ffd
> > > > #define UVC_SWENTITY_UNIT_ID 0x101
> > > >
> > > > +enum {
> > > > + UVC_SWENTITY_ORIENTATION,
> > > > + UVC_SWENTITY_ROTATION
> > > > +};
> > > > +
> > > > /* ------------------------------------------------------------------------
> > > > * Driver specific constants.
> > > > */
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
© 2016 - 2025 Red Hat, Inc.