linux-kernel - Re: [PATCH v7 6/6] media: starfive: camss: Add VIN driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <73222603-445e-fdb0-e831-219bac1d5865@starfivetech.com>
Date:   Thu, 3 Aug 2023 10:44:50 +0800
From:   Jack Zhu <jack.zhu@...rfivetech.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
CC:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Robert Foss <rfoss@...nel.org>,
        Todor Tomov <todor.too@...il.com>,
        <bryan.odonoghue@...aro.org>, Rob Herring <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Eugen Hristev <eugen.hristev@...labora.com>,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        <linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <changhuang.liang@...rfivetech.com>
Subject: Re: [PATCH v7 6/6] media: starfive: camss: Add VIN driver

Hi Laurent,

On 2023/8/2 18:38, Laurent Pinchart wrote:
> Hi Jack,
> 
> On Wed, Aug 02, 2023 at 05:58:26PM +0800, Jack Zhu wrote:
>> On 2023/7/28 4:49, Laurent Pinchart wrote:
>> > On Mon, Jun 19, 2023 at 07:28:38PM +0800, Jack Zhu wrote:
>> >> Add Video In Controller driver for StarFive Camera Subsystem.
>> > 
>> > I haven't reviewed this patch in details, as I have a high-level
>> > question: why do you need VIN subdevs ? They don't seem to represent any
>> > particular piece of hardware, their input and output formats are always
>> > identical, and they're not used to configure registers. The contents of
>> > this patch seems to belong to the video device, I think you can drop the
>> > VIN subdevs.
>> 
>> The VIN module corresponds to a hardware module, which is mainly responsible
>> for data routing and partial interrupt management (when the image data does
>> not pass through the isp but directly goes to the ddr), the relevant registers
>> need to be configured.
> 
> That's fine, but I don't think you need a subdev for it. As far as I
> understand, the VIn modules are (more or less) DMA engines. You can just
> model them as video devices, connected directly to the CSI-2 RX and ISP
> source pads.
> 

The VIN hardware can also route input data, it can decide whether DVP sensor
or MIPI sensor is used as input data.

> Does the "vin0_wr" have the ability to capture raw data from the DVP
> interface as well, or only from the CSI-2 RX ?
> 

Yes, the "vin0_wr" has the ability to capture raw data from the DVP
interface.

>> >> Signed-off-by: Jack Zhu <jack.zhu@...rfivetech.com>
>> >> ---
>> >>  .../media/platform/starfive/camss/Makefile    |    4 +-
>> >>  .../media/platform/starfive/camss/stf_camss.c |   42 +-
>> >>  .../media/platform/starfive/camss/stf_camss.h |    2 +
>> >>  .../media/platform/starfive/camss/stf_vin.c   | 1069 +++++++++++++++++
>> >>  .../media/platform/starfive/camss/stf_vin.h   |  173 +++
>> >>  .../platform/starfive/camss/stf_vin_hw_ops.c  |  192 +++
>> >>  6 files changed, 1478 insertions(+), 4 deletions(-)
>> >>  create mode 100644 drivers/media/platform/starfive/camss/stf_vin.c
>> >>  create mode 100644 drivers/media/platform/starfive/camss/stf_vin.h
>> >>  create mode 100644 drivers/media/platform/starfive/camss/stf_vin_hw_ops.c
>> >> 
>> >> diff --git a/drivers/media/platform/starfive/camss/Makefile b/drivers/media/platform/starfive/camss/Makefile
>> >> index cdf57e8c9546..ef574e01ca47 100644
>> >> --- a/drivers/media/platform/starfive/camss/Makefile
>> >> +++ b/drivers/media/platform/starfive/camss/Makefile
>> >> @@ -7,6 +7,8 @@ starfive-camss-objs += \
>> >>  		stf_camss.o \
>> >>  		stf_isp.o \
>> >>  		stf_isp_hw_ops.o \
>> >> -		stf_video.o
>> >> +		stf_video.o \
>> >> +		stf_vin.o \
>> >> +		stf_vin_hw_ops.o
>> >>  
>> >>  obj-$(CONFIG_VIDEO_STARFIVE_CAMSS) += starfive-camss.o
>> >> diff --git a/drivers/media/platform/starfive/camss/stf_camss.c b/drivers/media/platform/starfive/camss/stf_camss.c
>> >> index 6f56b45f57db..834ea63eb833 100644
>> >> --- a/drivers/media/platform/starfive/camss/stf_camss.c
>> >> +++ b/drivers/media/platform/starfive/camss/stf_camss.c
>> >> @@ -131,27 +131,61 @@ static int stfcamss_init_subdevices(struct stfcamss *stfcamss)
>> >>  		return ret;
>> >>  	}
>> >>  

Regards,

Jack Zhu