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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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