diff mbox series

[1/5] media: i2c: alvium: fix alvium_get_fw_version()

Message ID 20240416141905.454253-2-tomm.merciai@gmail.com (mailing list archive)
State New, archived
Headers show
Series Alvium improvements | expand

Commit Message

Tommaso Merciai April 16, 2024, 2:19 p.m. UTC 
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 <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 20 ++++++++------------
 drivers/media/i2c/alvium-csi2.h | 15 +++++++++++----
 2 files changed, 19 insertions(+), 16 deletions(-)

Comments

Sakari Ailus April 24, 2024, 6:10 p.m. UTC | #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 <tomm.merciai@gmail.com>
> ---
>  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;
diff mbox series

Patch

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;