| Message ID | 20240416141905.454253-2-tomm.merciai@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] media: i2c: alvium: fix alvium_get_fw_version() | expand |
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 --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;
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(-)