[PATCH 0/1] media: i2c: alvium: add camera sysfs attributes

[PATCH 0/1] media: i2c: alvium: add camera sysfs attributes

[Tommaso Merciai]

Hi All,
With this patch I'm going to add some sysfs attributes to the alvium-csi2 driver.
This patch adds the following sysfs attributes:

 - cci_register_layout_version:
   Shows current cci regs layout version of the camera.

 - csi_clock_mhz:
   Shows current alvium camera csi2 freq.

 - device_capabilities:
   Show the capabilities of the current camera.

 - device_guid:
   Shows the current device guid as programmed by the vendor during production.

 - device_version:
   Shows the version of the alvium hardware.

 - family_name:
   Shows the Alvium family name, like Alvium CSI-2, GM2, FP3, ...

 - genio:
   Generic camera input/output xfer for using user programmable part of the flash.
   Reading and writing camera's user programmable flash memory.

 - lane_count:
   Shows device current CSI2 camera's lanes number.

 - manufacturer_info:
   Shows manufacturer info as programmed by the vendor during production.

 - manufacturer_name:
   Shows manufacturer name as programmed by the vendor during production.

 - mode:
   As stated by the BCRM Ref Manual camera can work in 2 modes BCM/GENCP.
   This attribute is responsible for switching the camera mode and check the current mode.

 - model_name:
   Shows model name as programmed by the vendor during production.

 - serial_number:
   Shows camera serial number as programmed by the vendor during production.

 - user_defined_name:
   Shows camera user defined name as programmed by the vendor during production.

Thanks & Regards,
Tommaso

Tommaso Merciai (1):
  media: i2c: alvium: add camera sysfs attributes

 drivers/media/i2c/alvium-csi2.c | 484 ++++++++++++++++++++++++++++++++
 drivers/media/i2c/alvium-csi2.h |  25 ++
 2 files changed, 509 insertions(+)

-- 
2.34.1

[PATCH 1/1] media: i2c: alvium: add camera sysfs attributes

[Tommaso Merciai]

Add the following sysfs attributes:

 - cci_register_layout_version: dump current cci regs layout version
 - csi_clock_mhz: dump current csi2 freq
 - device_capabilities: dump alvium device capabilities
 - device_guid: dump alvium device guid
 - device_version: dump device version
 - family_name: dump alvium device family name
 - genio: generic camera input/output xfer
 - lane_count: dump alvium device data lanes number
 - manufacturer_info: custom manufacturer info
 - manufacturer_name: manufacturer info
 - mode: show/set bcm/gencp mode
 - model_name: dump alvium device model name
 - serial_number: dump alvium device serial number
 - user_defined_name: show user defined name

 alvium_sysfs_add_dev_info() create alvium sysfs attributes.
 alvium_sysfs_rm_dev_info() remove alvium attributes.

Signed-off-by: Tommaso Merciai <tomm.merciai@gmail.com>
---
 drivers/media/i2c/alvium-csi2.c | 484 ++++++++++++++++++++++++++++++++
 drivers/media/i2c/alvium-csi2.h |  25 ++
 2 files changed, 509 insertions(+)

diff --git a/drivers/media/i2c/alvium-csi2.c b/drivers/media/i2c/alvium-csi2.c
index 5ddfd3dcb188..11bfeb3bb1fd 100644
--- a/drivers/media/i2c/alvium-csi2.c
+++ b/drivers/media/i2c/alvium-csi2.c
@@ -1653,6 +1653,482 @@ static int alvium_hw_init(struct alvium_dev *alvium)
 	return 0;
 }
 
+/* --------------- Sysfs attributes --------------- */
+static ssize_t cci_register_layout_version_show(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 min, maj, ver;
+	int ret = 0;
+
+	ret = alvium_read(alvium, REG_BCRM_MINOR_VERSION_R, &min, &ret);
+	ret = alvium_read(alvium, REG_BCRM_MAJOR_VERSION_R, &maj, &ret);
+	if (ret)
+		return ret;
+
+	ver = (maj << 16) | min;
+
+	return sysfs_emit(buf, "%u\n", (u32)ver);
+}
+static DEVICE_ATTR_RO(cci_register_layout_version);
+
+static ssize_t csi_clock_mhz_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val;
+	int ret;
+
+	ret = alvium_read(alvium, REG_BCRM_CSI2_CLOCK_RW, &val, NULL);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", (u32)val);
+}
+static DEVICE_ATTR_RO(csi_clock_mhz);
+
+static ssize_t device_capabilities_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val;
+	int ret;
+
+	ret = alvium_read(alvium, REG_CCI_DEV_CAP_RW, &val, NULL);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "0x%llx\n", val);
+}
+static DEVICE_ATTR_RO(device_capabilities);
+
+static ssize_t device_guid_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_DEVICE_GUID_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(device_guid);
+
+static ssize_t device_version_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_DEVICE_VERSION_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(device_version);
+
+static ssize_t family_name_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_DEVICE_FAMILY_NAME_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(family_name);
+
+static void alvium_genio_read_prepare(struct alvium_dev *alvium,
+				      struct alvium_genio_proto *proto)
+{
+	alvium->req_data.reg = proto->reg;
+	alvium->req_data.count = proto->count;
+}
+
+static ssize_t alvium_genio_xfer(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	struct alvium_genio_proto *proto;
+	char *kbuf;
+	char *data;
+	int ret = 0;
+
+	switch (alvium->genio_op) {
+	case ALVIUM_GENIO_READ:
+		kbuf = kzalloc(alvium->req_data.count, GFP_KERNEL);
+		if (!kbuf)
+			return -ENOMEM;
+
+		ret = regmap_bulk_read(alvium->regmap, alvium->req_data.reg,
+				       (char *)kbuf, alvium->req_data.count);
+		if (ret)
+			goto genio_xfer_done;
+
+		memcpy((char *)buf, kbuf, alvium->req_data.count);
+
+		ret = alvium->req_data.count;
+		break;
+
+	case ALVIUM_GENIO_WRITE:
+		proto = (struct alvium_genio_proto *)buf;
+		data = (char *)(proto + 1);
+
+		kbuf = kzalloc(proto->count, GFP_KERNEL);
+		if (!kbuf)
+			return -ENOMEM;
+
+		memcpy(kbuf, data, proto->count);
+
+		ret = regmap_bulk_write(alvium->regmap, proto->reg,
+					kbuf, proto->count);
+		if (ret)
+			goto genio_xfer_done;
+
+		ret = proto->count;
+		break;
+	}
+
+genio_xfer_done:
+	kfree(kbuf);
+	return ret;
+}
+
+static ssize_t genio_show(struct device *dev,
+			  struct device_attribute *attr,
+			  char *buf)
+{
+	return alvium_genio_xfer(dev, attr, buf, 0);
+}
+
+static ssize_t genio_store(struct device *dev,
+			   struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	struct alvium_genio_proto *proto = (struct alvium_genio_proto *)buf;
+	int ret = 0;
+
+	if (proto->op) {
+		alvium->genio_op = ALVIUM_GENIO_READ;
+		alvium_genio_read_prepare(alvium, proto);
+		return ret;
+	}
+
+	alvium->genio_op = ALVIUM_GENIO_WRITE;
+
+	return alvium_genio_xfer(dev, attr, buf, count);
+}
+static DEVICE_ATTR_RW(genio);
+
+static ssize_t lane_count_show(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+
+	return sysfs_emit(buf, "%d\n", alvium->h_sup_csi_lanes);
+}
+static DEVICE_ATTR_RO(lane_count);
+
+static ssize_t manufacturer_info_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_MANUFACTURER_INFO_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(manufacturer_info);
+
+static ssize_t manufacturer_name_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_MANUFACTURER_NAME_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(manufacturer_name);
+
+static ssize_t mode_store(struct device *dev,
+			  struct device_attribute *attr,
+			  const char *buf, size_t len)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val;
+	int ret = 0;
+
+	if (strncmp(buf, "bcm", strlen(buf) - 1) == 0)
+		alvium->bcrm_mode = ALVIUM_BCM_MODE;
+	else if (strncmp(buf, "gencp", strlen(buf) - 1) == 0)
+		alvium->bcrm_mode = ALVIUM_GENCP_MODE;
+	else
+		return -EINVAL;
+
+	alvium_write(alvium, REG_GENCP_CHANGEMODE_W,
+		     (u8)alvium->bcrm_mode, &ret);
+	if (ret)
+		return ret;
+
+	read_poll_timeout(alvium_read, val,
+			  (val == alvium->bcrm_mode),
+			  15000, 45000, true,
+			  alvium, REG_GENCP_CURRENTMODE_R,
+			  &val, &ret);
+	if (ret) {
+		dev_err(dev, "poll curr mode fail\n");
+		return ret;
+	}
+
+	return ret ? ret : len;
+}
+
+static ssize_t mode_show(struct device *dev,
+			 struct device_attribute *attr,
+			 char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 mode;
+	int ret = 0;
+
+	ret = alvium_read(alvium, REG_GENCP_CURRENTMODE_R, &mode, NULL);
+	if (ret)
+		return ret;
+
+	switch (mode) {
+	case ALVIUM_BCM_MODE:
+		return sysfs_emit(buf, "bcm\n");
+	case ALVIUM_GENCP_MODE:
+		return sysfs_emit(buf, "gencp\n");
+	}
+
+	return sysfs_emit(buf, "Alvium is in invalid Mode: 0x%x\n", (u8)mode);
+}
+static DEVICE_ATTR_RW(mode);
+
+static ssize_t model_name_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_DEVICE_MODEL_NAME_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(model_name);
+
+static ssize_t serial_number_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_DEVICE_SERIAL_NUMBER_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(serial_number);
+
+static ssize_t user_defined_name_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct alvium_dev *alvium = sd_to_alvium(sd);
+	u64 val[8];
+	int i, ret = 0;
+
+	/* 64 bytes length */
+	for (i = 0; i < 8; i++)
+		alvium_read(alvium, REG_CCI_USER_DEFINED_NAME_R + 8 * i,
+			    &val[i], &ret);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%s\n", (char *)&val);
+}
+static DEVICE_ATTR_RO(user_defined_name);
+
+static int alvium_sysfs_add_dev_info(struct alvium_dev *alvium)
+{
+	struct device *dev = &alvium->i2c_client->dev;
+	int ret;
+
+	ret = device_create_file(dev, &dev_attr_cci_register_layout_version);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_cci_register_layout_version);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_csi_clock_mhz);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_csi_clock_mhz);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_device_capabilities);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_device_capabilities);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_device_guid);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_device_guid);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_device_version);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_device_version);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_family_name);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_family_name);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_genio);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_genio);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_lane_count);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_lane_count);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_manufacturer_info);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_manufacturer_info);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_manufacturer_name);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_manufacturer_name);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_serial_number);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_serial_number);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_mode);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_mode);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_model_name);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_model_name);
+		return ret;
+	}
+
+	ret = device_create_file(dev, &dev_attr_user_defined_name);
+	if (ret) {
+		device_remove_file(dev, &dev_attr_user_defined_name);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void alvium_sysfs_rm_dev_info(struct alvium_dev *alvium)
+{
+	struct device *dev = &alvium->i2c_client->dev;
+
+	device_remove_file(dev, &dev_attr_cci_register_layout_version);
+	device_remove_file(dev, &dev_attr_csi_clock_mhz);
+	device_remove_file(dev, &dev_attr_device_capabilities);
+	device_remove_file(dev, &dev_attr_device_guid);
+	device_remove_file(dev, &dev_attr_device_version);
+	device_remove_file(dev, &dev_attr_family_name);
+	device_remove_file(dev, &dev_attr_genio);
+	device_remove_file(dev, &dev_attr_lane_count);
+	device_remove_file(dev, &dev_attr_manufacturer_info);
+	device_remove_file(dev, &dev_attr_manufacturer_name);
+	device_remove_file(dev, &dev_attr_mode);
+	device_remove_file(dev, &dev_attr_model_name);
+	device_remove_file(dev, &dev_attr_serial_number);
+	device_remove_file(dev, &dev_attr_user_defined_name);
+}
+
 /* --------------- Subdev Operations --------------- */
 static int alvium_s_frame_interval(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *sd_state,
@@ -2469,6 +2945,12 @@ static int alvium_probe(struct i2c_client *client)
 		goto err_powerdown;
 	}
 
+	ret = alvium_sysfs_add_dev_info(alvium);
+	if (ret) {
+		dev_err_probe(dev, ret, "sysfs_add_dev_info fail\n");
+		goto err_powerdown;
+	}
+
 	/*
 	 * Enable runtime PM without autosuspend:
 	 *
@@ -2499,6 +2981,7 @@ static int alvium_probe(struct i2c_client *client)
 err_pm:
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
+	alvium_sysfs_rm_dev_info(alvium);
 	kfree(alvium->alvium_csi2_fmt);
 err_powerdown:
 	alvium_set_power(alvium, false);
@@ -2513,6 +2996,7 @@ static void alvium_remove(struct i2c_client *client)
 	struct device *dev = &alvium->i2c_client->dev;
 
 	v4l2_async_unregister_subdev(sd);
+	alvium_sysfs_rm_dev_info(alvium);
 	alvium_subdev_cleanup(alvium);
 	kfree(alvium->alvium_csi2_fmt);
 	/*
diff --git a/drivers/media/i2c/alvium-csi2.h b/drivers/media/i2c/alvium-csi2.h
index 978af44f76c7..8567898d39ed 100644
--- a/drivers/media/i2c/alvium-csi2.h
+++ b/drivers/media/i2c/alvium-csi2.h
@@ -28,7 +28,16 @@
 /* Basic Control Register Map register offsets (BCRM) */
 #define REG_BCRM_MINOR_VERSION_R			CCI_REG16(0x0000)
 #define REG_BCRM_MAJOR_VERSION_R			CCI_REG16(0x0002)
+#define REG_CCI_DEV_CAP_RW				CCI_REG64(0x0008)
 #define REG_BCRM_REG_ADDR_R				CCI_REG16(0x0014)
+#define REG_CCI_DEVICE_GUID_R				CCI_REG64_LE(0x0018)
+#define REG_CCI_MANUFACTURER_NAME_R			CCI_REG64_LE(0x0058)
+#define REG_CCI_DEVICE_MODEL_NAME_R			CCI_REG64_LE(0x0098)
+#define REG_CCI_DEVICE_FAMILY_NAME_R			CCI_REG64_LE(0x00d8)
+#define REG_CCI_DEVICE_VERSION_R			CCI_REG64_LE(0x0118)
+#define REG_CCI_MANUFACTURER_INFO_R			CCI_REG64_LE(0x0158)
+#define REG_CCI_DEVICE_SERIAL_NUMBER_R			CCI_REG64_LE(0x0198)
+#define REG_CCI_USER_DEFINED_NAME_R			CCI_REG64_LE(0x01d8)
 
 #define REG_BCRM_FEATURE_INQUIRY_R			REG_BCRM_V4L2_64BIT(0x0008)
 #define REG_BCRM_DEVICE_FW				REG_BCRM_V4L2_64BIT(0x0010)
@@ -211,6 +220,19 @@
 #define BCRM_DEVICE_FW_SPEC_MASK			GENMASK_ULL(7, 0)
 #define BCRM_DEVICE_FW_SPEC_SHIFT			0
 
+enum alvium_fw_ops {
+	ALVIUM_GENIO_WRITE,
+	ALVIUM_GENIO_READ,
+	ALVIUM_GENIO_NUM_OPS
+};
+
+struct alvium_genio_proto {
+	u16 reg;
+	u16 count;
+	u8 op;
+	u8 reserved[3];
+};
+
 enum alvium_bcrm_mode {
 	ALVIUM_BCM_MODE,
 	ALVIUM_GENCP_MODE,
@@ -461,6 +483,9 @@ struct alvium_dev {
 
 	u8 streaming;
 	u8 apply_fiv;
+
+	u8 genio_op;
+	struct alvium_genio_proto req_data;
 };
 
 static inline struct alvium_dev *sd_to_alvium(struct v4l2_subdev *sd)
-- 
2.34.1

Re: [PATCH 0/1] media: i2c: alvium: add camera sysfs attributes

[Laurent Pinchart]

Hi Tommaso,

On Mon, Sep 09, 2024 at 12:58:29PM +0200, Tommaso Merciai wrote:
> Hi All,
> With this patch I'm going to add some sysfs attributes to the alvium-csi2 driver.

Why ? You need to convince us that this shouldn't be done with existing
V4L2 mechanisms instead, like controls, or VIDIOC_LOG_STATUS. I expect
the answer to depend on the attributes.

> This patch adds the following sysfs attributes:
> 
>  - cci_register_layout_version:
>    Shows current cci regs layout version of the camera.
> 
>  - csi_clock_mhz:
>    Shows current alvium camera csi2 freq.
> 
>  - device_capabilities:
>    Show the capabilities of the current camera.
> 
>  - device_guid:
>    Shows the current device guid as programmed by the vendor during production.
> 
>  - device_version:
>    Shows the version of the alvium hardware.
> 
>  - family_name:
>    Shows the Alvium family name, like Alvium CSI-2, GM2, FP3, ...
> 
>  - genio:
>    Generic camera input/output xfer for using user programmable part of the flash.
>    Reading and writing camera's user programmable flash memory.

Complete and absolute NACK. We don't expose direct I2C access to
registers.

>  - lane_count:
>    Shows device current CSI2 camera's lanes number.
> 
>  - manufacturer_info:
>    Shows manufacturer info as programmed by the vendor during production.
> 
>  - manufacturer_name:
>    Shows manufacturer name as programmed by the vendor during production.
> 
>  - mode:
>    As stated by the BCRM Ref Manual camera can work in 2 modes BCM/GENCP.
>    This attribute is responsible for switching the camera mode and check the current mode.

You will need to be very, very convincing to get a writable sysfs
attribute accepted.

>  - model_name:
>    Shows model name as programmed by the vendor during production.
> 
>  - serial_number:
>    Shows camera serial number as programmed by the vendor during production.
> 
>  - user_defined_name:
>    Shows camera user defined name as programmed by the vendor during production.
> 
> Thanks & Regards,
> Tommaso
> 
> Tommaso Merciai (1):
>   media: i2c: alvium: add camera sysfs attributes
> 
>  drivers/media/i2c/alvium-csi2.c | 484 ++++++++++++++++++++++++++++++++
>  drivers/media/i2c/alvium-csi2.h |  25 ++
>  2 files changed, 509 insertions(+)

All sysfs attributes are ABIs, and need to be documented in
Documentation/ABI/. I expect you'll need one patch per attribute. Don't
forget to CC linux-doc@vger.kernel.org on the patches.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/1] media: i2c: alvium: add camera sysfs attributes

[Sakari Ailus]

Hi Tommaso,

On Mon, Sep 09, 2024 at 12:58:29PM +0200, Tommaso Merciai wrote:
> Hi All,
> With this patch I'm going to add some sysfs attributes to the alvium-csi2 driver.
> This patch adds the following sysfs attributes:
> 
>  - cci_register_layout_version:
>    Shows current cci regs layout version of the camera.
> 
>  - csi_clock_mhz:
>    Shows current alvium camera csi2 freq.
> 
>  - device_capabilities:
>    Show the capabilities of the current camera.
> 
>  - device_guid:
>    Shows the current device guid as programmed by the vendor during production.
> 
>  - device_version:
>    Shows the version of the alvium hardware.
> 
>  - family_name:
>    Shows the Alvium family name, like Alvium CSI-2, GM2, FP3, ...
> 
>  - genio:
>    Generic camera input/output xfer for using user programmable part of the flash.
>    Reading and writing camera's user programmable flash memory.
> 
>  - lane_count:
>    Shows device current CSI2 camera's lanes number.
> 
>  - manufacturer_info:
>    Shows manufacturer info as programmed by the vendor during production.
> 
>  - manufacturer_name:
>    Shows manufacturer name as programmed by the vendor during production.
> 
>  - mode:
>    As stated by the BCRM Ref Manual camera can work in 2 modes BCM/GENCP.
>    This attribute is responsible for switching the camera mode and check the current mode.
> 
>  - model_name:
>    Shows model name as programmed by the vendor during production.
> 
>  - serial_number:
>    Shows camera serial number as programmed by the vendor during production.
> 
>  - user_defined_name:
>    Shows camera user defined name as programmed by the vendor during production.

I think most these would be better implemented as V4L2 controls. Some
appear to be internal to the driver and not matter from actual user space
implementation point of view, such as CCI register layout version and
possibly device capabilities to some extent. What would be the reason to
expose these to the user space?

What are the BCM and GENCP modes?

If there's a need to program the device's memory, the NVM interface would
seem like a better fit for that. Presumably this would be only accessible
for the root?

The lane count control should probably be standardised, there are other
devices that could benefit from it. The LINK_FREQ control already exists,
it conveys the (CSI-2) link frequency to the user.

-- 
Kind regards,

Sakari Ailus