Hello,
This series containing improvements for the Alvium camera driver.
Specifically:
Patches:
- 1 Fix fix alvium_get_fw_version()
- 2 Rename acquisition frame rate enable define into a short define
- 3 Alvium camera by default is in free running mode. Datasheet say that
acquisition frame rate reg can only be used if frame start trigger
mode is set to off. Enable r/w aquisition frame rate and turn off trigger mode.
- 4 Implement enum_frame_size
- 5 Use the right V4L2_CID for the analogue gain
Thanks & Regards,
Tommaso
Tommaso Merciai (5):
media: i2c: alvium: fix alvium_get_fw_version()
media: i2c: alvium: rename acquisition frame rate enable reg
media: i2c: alvium: enable acquisition frame rate
media: i2c: alvium: implement enum_frame_size
media: i2c: alvium: Move V4L2_CID_GAIN to V4L2_CID_ANALOG_GAIN
drivers/media/i2c/alvium-csi2.c | 62 +++++++++++++++++++++++++--------
drivers/media/i2c/alvium-csi2.h | 17 ++++++---
2 files changed, 59 insertions(+), 20 deletions(-)
--
2.34.1
Instead of reading device_fw reg as multiple regs let's read the entire
64bit reg using one i2c read and store this info into alvium_fw_version
union fixing the dev_info formatting output.
Signed-off-by: Tommaso Merciai <[email protected]>
---
drivers/media/i2c/alvium-csi2.c | 20 ++++++++------------
drivers/media/i2c/alvium-csi2.h | 15 +++++++++++----
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index e65702e3f73e..991b3bcc8b80 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -403,21 +403,17 @@ static int alvium_get_bcrm_vers(struct alvium_dev *alvium)
static int alvium_get_fw_version(struct alvium_dev *alvium)
{
struct device *dev = &alvium->i2c_client->dev;
- u64 spec, maj, min, pat;
+ union alvium_fw_version v;
int ret = 0;
- ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_SPEC_VERSION_R,
- &spec, &ret);
- ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MAJOR_VERSION_R,
- &maj, &ret);
- ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MINOR_VERSION_R,
- &min, &ret);
- ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_PATCH_VERSION_R,
- &pat, &ret);
- if (ret)
- return ret;
+ ret = alvium_read(alvium, REG_BCRM_DEVICE_FW,
+ &v.value, &ret);
- dev_info(dev, "fw version: %llu.%llu.%llu.%llu\n", spec, maj, min, pat);
+ dev_info(dev, "fw version: %u.%u.%08x special: %u\n",
+ (u32)v.alvium_fw_ver.major,
+ (u32)v.alvium_fw_ver.minor,
+ v.alvium_fw_ver.patch,
+ (u32)v.alvium_fw_ver.special);
return 0;
}
diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
index 9463f8604fbc..9c4cfb35de8e 100644
--- a/drivers/media/i2c/alvium-csi2.h
+++ b/drivers/media/i2c/alvium-csi2.h
@@ -31,10 +31,7 @@
#define REG_BCRM_REG_ADDR_R CCI_REG16(0x0014)
#define REG_BCRM_FEATURE_INQUIRY_R REG_BCRM_V4L2_64BIT(0x0008)
-#define REG_BCRM_DEVICE_FW_SPEC_VERSION_R REG_BCRM_V4L2_8BIT(0x0010)
-#define REG_BCRM_DEVICE_FW_MAJOR_VERSION_R REG_BCRM_V4L2_8BIT(0x0011)
-#define REG_BCRM_DEVICE_FW_MINOR_VERSION_R REG_BCRM_V4L2_16BIT(0x0012)
-#define REG_BCRM_DEVICE_FW_PATCH_VERSION_R REG_BCRM_V4L2_32BIT(0x0014)
+#define REG_BCRM_DEVICE_FW REG_BCRM_V4L2_64BIT(0x0010)
#define REG_BCRM_WRITE_HANDSHAKE_RW REG_BCRM_V4L2_8BIT(0x0018)
/* Streaming Control Registers */
@@ -276,6 +273,16 @@ enum alvium_av_mipi_bit {
ALVIUM_NUM_SUPP_MIPI_DATA_BIT
};
+union alvium_fw_version {
+ struct {
+ u8 special;
+ u8 major;
+ u16 minor;
+ u32 patch;
+ } alvium_fw_ver;
+ u64 value;
+};
+
struct alvium_avail_feat {
u64 rev_x:1;
u64 rev_y:1;
--
2.34.1
Hi Tommaso,
On Tue, Apr 16, 2024 at 04:19:01PM +0200, Tommaso Merciai wrote:
> Instead of reading device_fw reg as multiple regs let's read the entire
> 64bit reg using one i2c read and store this info into alvium_fw_version
> union fixing the dev_info formatting output.
>
> Signed-off-by: Tommaso Merciai <[email protected]>
> ---
> drivers/media/i2c/alvium-csi2.c | 20 ++++++++------------
> drivers/media/i2c/alvium-csi2.h | 15 +++++++++++----
> 2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
> index e65702e3f73e..991b3bcc8b80 100644
> --- a/drivers/media/i2c/alvium-csi2.c
> +++ b/drivers/media/i2c/alvium-csi2.c
> @@ -403,21 +403,17 @@ static int alvium_get_bcrm_vers(struct alvium_dev *alvium)
> static int alvium_get_fw_version(struct alvium_dev *alvium)
> {
> struct device *dev = &alvium->i2c_client->dev;
> - u64 spec, maj, min, pat;
> + union alvium_fw_version v;
> int ret = 0;
>
> - ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_SPEC_VERSION_R,
> - &spec, &ret);
> - ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MAJOR_VERSION_R,
> - &maj, &ret);
> - ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_MINOR_VERSION_R,
> - &min, &ret);
> - ret = alvium_read(alvium, REG_BCRM_DEVICE_FW_PATCH_VERSION_R,
> - &pat, &ret);
> - if (ret)
> - return ret;
> + ret = alvium_read(alvium, REG_BCRM_DEVICE_FW,
> + &v.value, &ret);
Doesn't this have the same endianness issues that earlier were resolved by
doing the reads separately?
>
> - dev_info(dev, "fw version: %llu.%llu.%llu.%llu\n", spec, maj, min, pat);
> + dev_info(dev, "fw version: %u.%u.%08x special: %u\n",
> + (u32)v.alvium_fw_ver.major,
> + (u32)v.alvium_fw_ver.minor,
> + v.alvium_fw_ver.patch,
> + (u32)v.alvium_fw_ver.special);
>
> return 0;
> }
> diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
> index 9463f8604fbc..9c4cfb35de8e 100644
> --- a/drivers/media/i2c/alvium-csi2.h
> +++ b/drivers/media/i2c/alvium-csi2.h
> @@ -31,10 +31,7 @@
> #define REG_BCRM_REG_ADDR_R CCI_REG16(0x0014)
>
> #define REG_BCRM_FEATURE_INQUIRY_R REG_BCRM_V4L2_64BIT(0x0008)
> -#define REG_BCRM_DEVICE_FW_SPEC_VERSION_R REG_BCRM_V4L2_8BIT(0x0010)
> -#define REG_BCRM_DEVICE_FW_MAJOR_VERSION_R REG_BCRM_V4L2_8BIT(0x0011)
> -#define REG_BCRM_DEVICE_FW_MINOR_VERSION_R REG_BCRM_V4L2_16BIT(0x0012)
> -#define REG_BCRM_DEVICE_FW_PATCH_VERSION_R REG_BCRM_V4L2_32BIT(0x0014)
> +#define REG_BCRM_DEVICE_FW REG_BCRM_V4L2_64BIT(0x0010)
> #define REG_BCRM_WRITE_HANDSHAKE_RW REG_BCRM_V4L2_8BIT(0x0018)
>
> /* Streaming Control Registers */
> @@ -276,6 +273,16 @@ enum alvium_av_mipi_bit {
> ALVIUM_NUM_SUPP_MIPI_DATA_BIT
> };
>
> +union alvium_fw_version {
> + struct {
> + u8 special;
> + u8 major;
> + u16 minor;
> + u32 patch;
> + } alvium_fw_ver;
> + u64 value;
> +};
> +
> struct alvium_avail_feat {
> u64 rev_x:1;
> u64 rev_y:1;
--
Kind regards,
Sakari Ailus