mbox series

[v4,0/8] Add StarFive Camera Subsystem driver

Message ID 20230413035541.62129-1-jack.zhu@starfivetech.com
Headers show
Series Add StarFive Camera Subsystem driver | expand

Message

Jack Zhu April 13, 2023, 3:55 a.m. UTC
Hi,

This patch series adds support for the StarFive Camera Subsystem
found on StarFive JH7110 SoC.

The driver implements V4L2, Media controller and V4L2 subdev interfaces.
Camera sensor using V4L2 subdev interface in the kernel is supported.

The driver is tested on VisionFive V2 board with IMX219 camera sensor.
GStreamer 1.18.5 with v4l2src plugin is supported.

Changes since v3:
Patch 1:
- Modified port@0 and port@1 properties.
- Extended the port@0 example with appropriate properties.
- Added 'port@0' for 'required'
Patch 2:
- Modified spelling errors.
Patch 3:
- Merged patch 5 into the patch with an explanation for compatible in
  commit msg.
Patch 6:
- Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
  label.
- Modified Code Style for getting sys_rst and p_rst.
- Renamed clk_name to name and modified the relevant code.
Patch 9:
- Added static for stfcamss_get_mem_res function.
- Added static for isp_close function.
- Fixed implicit conversion warning for stf_vin_map_isp_pad function.
- Dropped unused variables.

  v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/

Changes since v2:
- Rebased on v6.3-rc1.
Patch 1:
- Modified spelling errors.
- Added port@0.
- Modified '$ref' of port.
- Added 'ports' to 'required'.
- Dropped 'stfcamss' label in example.
- Added port@0 in example.
- Added MAINTAINERS file.
Patch 2:
- Split this patch into three new patches.
- Modified compatible property.
- Replaced clock names with the existing names.
- Modified 'bus-type' and 'clock-lanes'
- Added port@2 - port@4
- Dropped 'csi2rx' label in example.
Patch 3:
- Updated rst and dot file as three pipelines were deleted.
Patch 4:
- Split this patch into three new patches.
- Dropped .s_power() and .get_fmt().
- Dropped CSI-2 DT support.
- Dropped v4l2_device_register_subdev_nodes().
- Used assigned-clock-rates in DT to set clk value.
- Modified 'compatible' field.
Patch 5:
- Deleted three pipelines. 
- Modified 'stfcamss_clocks'/'stfcamss_resets' struct.
- Dropped stfcamss_find_sensor() function.
- Removed redundant code from stfcamss_of_parse_endpoint_node().
- Modified spelling errors.
- Rewrote stfcamss_reg_media_subdev_node() function.
- Modified stfcamss_subdev_notifier_bound().
- Modified stfcamss_probe() function.
- Dropped stfcamss_suspend() and stfcamss_resume().
- Dropped dev_info() in stfcamss_remove() function.
- Added 'stf_' prefix for enum subdev_type.
- Moved all includes to the top in stf_camss.h file.
- Dropped unused fields in stfcamss struct.
- Replaced Custom logging macros with regular macros.
- Rewrote register read and write functions.
- Used lowercase for all hex constants.
- Used macro to name registers.
- Dropped unused ioctl and stf_isp_ioctl.h file.

  v2: https://lore.kernel.org/all/20230310120553.60586-1-jack.zhu@starfivetech.com/

Changes since v1:
- Deleted starfive,jh7110-mipi-csi2.yaml.
- Converted cdns,csi2rx.txt to cdns,csi2rx.yaml and added ‘resets’
  properties.
- Added ‘cdns,csi2rx.yaml’ in ‘CADENCE MIPI-CSI2 BRIDGES’ entry.
- The following contents were modified in starfive,jh7110-camss.yaml:
  dropped quotes from ’id’ and ‘schema’; dropped ‘|’ for ‘description’;
  corrected the wrong or redundant words: ‘a ISP’, ‘PD ISP’;
  dropped ‘minItems’ for ‘reg’, ‘clocks’, ‘resets’ and ‘interrupts’;
  dropped the '_clk' and 'rst_' prefix about the 'clock-names' and
  'reset-names';
  changed ‘endpoint@1’ to ‘endpoint’; updated examples;
- Updated Subject for some patches.
- Merged patch 6, 7, 8, 9, 10, 11 into one patch.

Jack Zhu (8):
  media: dt-bindings: cadence-csi2rx: Convert to DT schema
  media: dt-bindings: cadence-csi2rx: Add resets property
  media: cadence: Add operation on reset
  media: cadence: Add support for external dphy
  media: cadence: Add support for JH7110 SoC
  media: dt-bindings: Add bindings for JH7110 Camera Subsystem
  media: admin-guide: Add starfive_camss.rst for Starfive Camera
    Subsystem
  media: starfive: Add Starfive Camera Subsystem driver

 .../admin-guide/media/starfive_camss.rst      |   57 +
 .../media/starfive_camss_graph.dot            |   16 +
 .../admin-guide/media/v4l-drivers.rst         |    1 +
 .../devicetree/bindings/media/cdns,csi2rx.txt |  100 --
 .../bindings/media/cdns,csi2rx.yaml           |  201 +++
 .../bindings/media/starfive,jh7110-camss.yaml |  164 +++
 MAINTAINERS                                   |   10 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/cadence/cdns-csi2rx.c  |  107 +-
 drivers/media/platform/starfive/Kconfig       |   18 +
 drivers/media/platform/starfive/Makefile      |   14 +
 drivers/media/platform/starfive/stf_camss.c   |  477 +++++++
 drivers/media/platform/starfive/stf_camss.h   |  150 +++
 drivers/media/platform/starfive/stf_common.h  |   18 +
 drivers/media/platform/starfive/stf_isp.c     |  737 +++++++++++
 drivers/media/platform/starfive/stf_isp.h     |  999 +++++++++++++++
 .../media/platform/starfive/stf_isp_hw_ops.c  |  715 +++++++++++
 drivers/media/platform/starfive/stf_video.c   |  989 ++++++++++++++
 drivers/media/platform/starfive/stf_video.h   |   89 ++
 drivers/media/platform/starfive/stf_vin.c     | 1138 +++++++++++++++++
 drivers/media/platform/starfive/stf_vin.h     |  174 +++
 .../media/platform/starfive/stf_vin_hw_ops.c  |  211 +++
 23 files changed, 6272 insertions(+), 115 deletions(-)
 create mode 100644 Documentation/admin-guide/media/starfive_camss.rst
 create mode 100644 Documentation/admin-guide/media/starfive_camss_graph.dot
 delete mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.txt
 create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
 create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
 create mode 100644 drivers/media/platform/starfive/Kconfig
 create mode 100644 drivers/media/platform/starfive/Makefile
 create mode 100644 drivers/media/platform/starfive/stf_camss.c
 create mode 100644 drivers/media/platform/starfive/stf_camss.h
 create mode 100644 drivers/media/platform/starfive/stf_common.h
 create mode 100644 drivers/media/platform/starfive/stf_isp.c
 create mode 100644 drivers/media/platform/starfive/stf_isp.h
 create mode 100644 drivers/media/platform/starfive/stf_isp_hw_ops.c
 create mode 100644 drivers/media/platform/starfive/stf_video.c
 create mode 100644 drivers/media/platform/starfive/stf_video.h
 create mode 100644 drivers/media/platform/starfive/stf_vin.c
 create mode 100644 drivers/media/platform/starfive/stf_vin.h
 create mode 100644 drivers/media/platform/starfive/stf_vin_hw_ops.c

Comments

Krzysztof Kozlowski April 15, 2023, 8:56 a.m. UTC | #1
On 13/04/2023 05:55, Jack Zhu wrote:
> Convert DT bindings document for Cadence MIPI-CSI2 RX controller to
> DT schema format.
> 
> For compatible, new compatibles should not be messed with conversion,
> but the original binding did not specify any SoC-specific compatible
> string, so add the StarFive compatible string.
> 
> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
> ---
>  .../devicetree/bindings/media/cdns,csi2rx.txt | 100 ----------
>  .../bindings/media/cdns,csi2rx.yaml           | 177 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Krzysztof Kozlowski April 15, 2023, 8:59 a.m. UTC | #2
On 13/04/2023 05:55, Jack Zhu wrote:
> Add the bindings documentation for Starfive JH7110 Camera Subsystem
> which is used for handing image sensor data.
> 
> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>

Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.

> ---
>  .../bindings/media/starfive,jh7110-camss.yaml | 164 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> new file mode 100644
> index 000000000000..4cd144f1b845
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> @@ -0,0 +1,164 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +

No need for blank line.

With two fixes above:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Jack Zhu April 17, 2023, 9:21 a.m. UTC | #3
On 2023/4/15 16:59, Krzysztof Kozlowski wrote:
> On 13/04/2023 05:55, Jack Zhu wrote:
>> Add the bindings documentation for Starfive JH7110 Camera Subsystem
>> which is used for handing image sensor data.
>> 
>> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
> 
> Subject: drop second/last, redundant "bindings for". The "dt-bindings"
> prefix is already stating that these are bindings.
> 

OK, I will fix it. Thank you.

>> ---
>>  .../bindings/media/starfive,jh7110-camss.yaml | 164 ++++++++++++++++++
>>  MAINTAINERS                                   |   7 +
>>  2 files changed, 171 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> new file mode 100644
>> index 000000000000..4cd144f1b845
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> @@ -0,0 +1,164 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +
> 
> No need for blank line.
> 

OK, I will delete the blank line.

> With two fixes above:
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Best regards,
> Krzysztof
>
Jack Zhu May 5, 2023, 5:57 a.m. UTC | #4
On 2023/4/24 19:19, Jack Zhu wrote:
> 
> 
> On 2023/4/13 11:55, Jack Zhu wrote:
>> Hi,
>> 
>> This patch series adds support for the StarFive Camera Subsystem
>> found on StarFive JH7110 SoC.
>> 
>> The driver implements V4L2, Media controller and V4L2 subdev interfaces.
>> Camera sensor using V4L2 subdev interface in the kernel is supported.
>> 
>> The driver is tested on VisionFive V2 board with IMX219 camera sensor.
>> GStreamer 1.18.5 with v4l2src plugin is supported.
>> 
>> Changes since v3:
>> Patch 1:
>> - Modified port@0 and port@1 properties.
>> - Extended the port@0 example with appropriate properties.
>> - Added 'port@0' for 'required'
>> Patch 2:
>> - Modified spelling errors.
>> Patch 3:
>> - Merged patch 5 into the patch with an explanation for compatible in
>>   commit msg.
>> Patch 6:
>> - Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
>>   label.
>> - Modified Code Style for getting sys_rst and p_rst.
>> - Renamed clk_name to name and modified the relevant code.
>> Patch 9:
>> - Added static for stfcamss_get_mem_res function.
>> - Added static for isp_close function.
>> - Fixed implicit conversion warning for stf_vin_map_isp_pad function.
>> - Dropped unused variables.
>> 
>>   v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/
>> 
> 
> Hello everyone,
> 
> From the current review status, the patches related to the CSI module
> have 'reviewed-by' tags. I would like to know if it is okay to add
> patches 1-5 from this series to a PR first.
> 
> Thank you!
> 
> Jack
> 

Hello Mauro, Laurent, Maxime, Rob, Krzysztof, Robert, Todor and Philipp,

Can you give me some suggestions and comments on the previous request
to commit CSI related patches first? Thank you for your time.

Jack

>> Changes since v2:
>> - Rebased on v6.3-rc1.
>> Patch 1:
>> - Modified spelling errors.
>> - Added port@0.
>> - Modified '$ref' of port.
>> - Added 'ports' to 'required'.
>> - Dropped 'stfcamss' label in example.
>> - Added port@0 in example.
>> - Added MAINTAINERS file.
>> Patch 2:
>> - Split this patch into three new patches.
>> - Modified compatible property.
>> - Replaced clock names with the existing names.
>> - Modified 'bus-type' and 'clock-lanes'
>> - Added port@2 - port@4
>> - Dropped 'csi2rx' label in example.
>> Patch 3:
>> - Updated rst and dot file as three pipelines were deleted.
>> Patch 4:
>> - Split this patch into three new patches.
>> - Dropped .s_power() and .get_fmt().
>> - Dropped CSI-2 DT support.
>> - Dropped v4l2_device_register_subdev_nodes().
>> - Used assigned-clock-rates in DT to set clk value.
>> - Modified 'compatible' field.
>> Patch 5:
>> - Deleted three pipelines. 
>> - Modified 'stfcamss_clocks'/'stfcamss_resets' struct.
>> - Dropped stfcamss_find_sensor() function.
>> - Removed redundant code from stfcamss_of_parse_endpoint_node().
>> - Modified spelling errors.
>> - Rewrote stfcamss_reg_media_subdev_node() function.
>> - Modified stfcamss_subdev_notifier_bound().
>> - Modified stfcamss_probe() function.
>> - Dropped stfcamss_suspend() and stfcamss_resume().
>> - Dropped dev_info() in stfcamss_remove() function.
>> - Added 'stf_' prefix for enum subdev_type.
>> - Moved all includes to the top in stf_camss.h file.
>> - Dropped unused fields in stfcamss struct.
>> - Replaced Custom logging macros with regular macros.
>> - Rewrote register read and write functions.
>> - Used lowercase for all hex constants.
>> - Used macro to name registers.
>> - Dropped unused ioctl and stf_isp_ioctl.h file.
>> 
>>   v2: https://lore.kernel.org/all/20230310120553.60586-1-jack.zhu@starfivetech.com/
>> 
>> Changes since v1:
>> - Deleted starfive,jh7110-mipi-csi2.yaml.
>> - Converted cdns,csi2rx.txt to cdns,csi2rx.yaml and added ‘resets’
>>   properties.
>> - Added ‘cdns,csi2rx.yaml’ in ‘CADENCE MIPI-CSI2 BRIDGES’ entry.
>> - The following contents were modified in starfive,jh7110-camss.yaml:
>>   dropped quotes from ’id’ and ‘schema’; dropped ‘|’ for ‘description’;
>>   corrected the wrong or redundant words: ‘a ISP’, ‘PD ISP’;
>>   dropped ‘minItems’ for ‘reg’, ‘clocks’, ‘resets’ and ‘interrupts’;
>>   dropped the '_clk' and 'rst_' prefix about the 'clock-names' and
>>   'reset-names';
>>   changed ‘endpoint@1’ to ‘endpoint’; updated examples;
>> - Updated Subject for some patches.
>> - Merged patch 6, 7, 8, 9, 10, 11 into one patch.
>> 
>> Jack Zhu (8):
>>   media: dt-bindings: cadence-csi2rx: Convert to DT schema
>>   media: dt-bindings: cadence-csi2rx: Add resets property
>>   media: cadence: Add operation on reset
>>   media: cadence: Add support for external dphy
>>   media: cadence: Add support for JH7110 SoC
>>   media: dt-bindings: Add bindings for JH7110 Camera Subsystem
>>   media: admin-guide: Add starfive_camss.rst for Starfive Camera
>>     Subsystem
>>   media: starfive: Add Starfive Camera Subsystem driver
>> 
>>  .../admin-guide/media/starfive_camss.rst      |   57 +
>>  .../media/starfive_camss_graph.dot            |   16 +
>>  .../admin-guide/media/v4l-drivers.rst         |    1 +
>>  .../devicetree/bindings/media/cdns,csi2rx.txt |  100 --
>>  .../bindings/media/cdns,csi2rx.yaml           |  201 +++
>>  .../bindings/media/starfive,jh7110-camss.yaml |  164 +++
>>  MAINTAINERS                                   |   10 +
>>  drivers/media/platform/Kconfig                |    1 +
>>  drivers/media/platform/Makefile               |    1 +
>>  drivers/media/platform/cadence/cdns-csi2rx.c  |  107 +-
>>  drivers/media/platform/starfive/Kconfig       |   18 +
>>  drivers/media/platform/starfive/Makefile      |   14 +
>>  drivers/media/platform/starfive/stf_camss.c   |  477 +++++++
>>  drivers/media/platform/starfive/stf_camss.h   |  150 +++
>>  drivers/media/platform/starfive/stf_common.h  |   18 +
>>  drivers/media/platform/starfive/stf_isp.c     |  737 +++++++++++
>>  drivers/media/platform/starfive/stf_isp.h     |  999 +++++++++++++++
>>  .../media/platform/starfive/stf_isp_hw_ops.c  |  715 +++++++++++
>>  drivers/media/platform/starfive/stf_video.c   |  989 ++++++++++++++
>>  drivers/media/platform/starfive/stf_video.h   |   89 ++
>>  drivers/media/platform/starfive/stf_vin.c     | 1138 +++++++++++++++++
>>  drivers/media/platform/starfive/stf_vin.h     |  174 +++
>>  .../media/platform/starfive/stf_vin_hw_ops.c  |  211 +++
>>  23 files changed, 6272 insertions(+), 115 deletions(-)
>>  create mode 100644 Documentation/admin-guide/media/starfive_camss.rst
>>  create mode 100644 Documentation/admin-guide/media/starfive_camss_graph.dot
>>  delete mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.txt
>>  create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
>>  create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>>  create mode 100644 drivers/media/platform/starfive/Kconfig
>>  create mode 100644 drivers/media/platform/starfive/Makefile
>>  create mode 100644 drivers/media/platform/starfive/stf_camss.c
>>  create mode 100644 drivers/media/platform/starfive/stf_camss.h
>>  create mode 100644 drivers/media/platform/starfive/stf_common.h
>>  create mode 100644 drivers/media/platform/starfive/stf_isp.c
>>  create mode 100644 drivers/media/platform/starfive/stf_isp.h
>>  create mode 100644 drivers/media/platform/starfive/stf_isp_hw_ops.c
>>  create mode 100644 drivers/media/platform/starfive/stf_video.c
>>  create mode 100644 drivers/media/platform/starfive/stf_video.h
>>  create mode 100644 drivers/media/platform/starfive/stf_vin.c
>>  create mode 100644 drivers/media/platform/starfive/stf_vin.h
>>  create mode 100644 drivers/media/platform/starfive/stf_vin_hw_ops.c
>>
Krzysztof Kozlowski May 5, 2023, 6:40 a.m. UTC | #5
On 05/05/2023 07:57, Jack Zhu wrote:
> 
> 
> On 2023/4/24 19:19, Jack Zhu wrote:
>>
>>
>> On 2023/4/13 11:55, Jack Zhu wrote:
>>> Hi,
>>>
>>> This patch series adds support for the StarFive Camera Subsystem
>>> found on StarFive JH7110 SoC.
>>>
>>> The driver implements V4L2, Media controller and V4L2 subdev interfaces.
>>> Camera sensor using V4L2 subdev interface in the kernel is supported.
>>>
>>> The driver is tested on VisionFive V2 board with IMX219 camera sensor.
>>> GStreamer 1.18.5 with v4l2src plugin is supported.
>>>
>>> Changes since v3:
>>> Patch 1:
>>> - Modified port@0 and port@1 properties.
>>> - Extended the port@0 example with appropriate properties.
>>> - Added 'port@0' for 'required'
>>> Patch 2:
>>> - Modified spelling errors.
>>> Patch 3:
>>> - Merged patch 5 into the patch with an explanation for compatible in
>>>   commit msg.
>>> Patch 6:
>>> - Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
>>>   label.
>>> - Modified Code Style for getting sys_rst and p_rst.
>>> - Renamed clk_name to name and modified the relevant code.
>>> Patch 9:
>>> - Added static for stfcamss_get_mem_res function.
>>> - Added static for isp_close function.
>>> - Fixed implicit conversion warning for stf_vin_map_isp_pad function.
>>> - Dropped unused variables.
>>>
>>>   v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/
>>>
>>
>> Hello everyone,
>>
>> From the current review status, the patches related to the CSI module
>> have 'reviewed-by' tags. I would like to know if it is okay to add
>> patches 1-5 from this series to a PR first.
>>
>> Thank you!
>>
>> Jack
>>
> 
> Hello Mauro, Laurent, Maxime, Rob, Krzysztof, Robert, Todor and Philipp,
> 
> Can you give me some suggestions and comments on the previous request
> to commit CSI related patches first? Thank you for your time.

You received very specific feedback, so know you decided to ignore it?

No, implement what you were asked for.

Best regards,
Krzysztof
Jack Zhu May 5, 2023, 8:14 a.m. UTC | #6
On 2023/5/5 14:40, Krzysztof Kozlowski wrote:
> On 05/05/2023 07:57, Jack Zhu wrote:
>> 
>> 
>> On 2023/4/24 19:19, Jack Zhu wrote:
>>>
>>>
>>> On 2023/4/13 11:55, Jack Zhu wrote:
>>>> Hi,
>>>>
>>>> This patch series adds support for the StarFive Camera Subsystem
>>>> found on StarFive JH7110 SoC.
>>>>
>>>> The driver implements V4L2, Media controller and V4L2 subdev interfaces.
>>>> Camera sensor using V4L2 subdev interface in the kernel is supported.
>>>>
>>>> The driver is tested on VisionFive V2 board with IMX219 camera sensor.
>>>> GStreamer 1.18.5 with v4l2src plugin is supported.
>>>>
>>>> Changes since v3:
>>>> Patch 1:
>>>> - Modified port@0 and port@1 properties.
>>>> - Extended the port@0 example with appropriate properties.
>>>> - Added 'port@0' for 'required'
>>>> Patch 2:
>>>> - Modified spelling errors.
>>>> Patch 3:
>>>> - Merged patch 5 into the patch with an explanation for compatible in
>>>>   commit msg.
>>>> Patch 6:
>>>> - Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
>>>>   label.
>>>> - Modified Code Style for getting sys_rst and p_rst.
>>>> - Renamed clk_name to name and modified the relevant code.
>>>> Patch 9:
>>>> - Added static for stfcamss_get_mem_res function.
>>>> - Added static for isp_close function.
>>>> - Fixed implicit conversion warning for stf_vin_map_isp_pad function.
>>>> - Dropped unused variables.
>>>>
>>>>   v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/
>>>>
>>>
>>> Hello everyone,
>>>
>>> From the current review status, the patches related to the CSI module
>>> have 'reviewed-by' tags. I would like to know if it is okay to add
>>> patches 1-5 from this series to a PR first.
>>>
>>> Thank you!
>>>
>>> Jack
>>>
>> 
>> Hello Mauro, Laurent, Maxime, Rob, Krzysztof, Robert, Todor and Philipp,
>> 
>> Can you give me some suggestions and comments on the previous request
>> to commit CSI related patches first? Thank you for your time.
> 
> You received very specific feedback, so know you decided to ignore it?
> 
> No, implement what you were asked for.
> 

Hi Krzysztof,

Thank you for your comments.

I am talking about CSI-related patches 1-5, not including the patches
6-8. The CSI module is a relatively functionally independent hardware
module. The CSI-related patches 1-5 already have 'reviewed-by' tags,
and there are no unprocessed comments left. So, made the previous
request. Please let me know if I understand something wrong.

I don't want to ignore any comments, I will continue to modify the isp
patches 6-8 in subsequent versions according to your comments. The
ISP-related patches are being prepared.

Jack

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski May 5, 2023, 12:24 p.m. UTC | #7
On 05/05/2023 10:14, Jack Zhu wrote:
> 
> 
> On 2023/5/5 14:40, Krzysztof Kozlowski wrote:
>> On 05/05/2023 07:57, Jack Zhu wrote:
>>>
>>>
>>> On 2023/4/24 19:19, Jack Zhu wrote:
>>>>
>>>>
>>>> On 2023/4/13 11:55, Jack Zhu wrote:
>>>>> Hi,
>>>>>
>>>>> This patch series adds support for the StarFive Camera Subsystem
>>>>> found on StarFive JH7110 SoC.
>>>>>
>>>>> The driver implements V4L2, Media controller and V4L2 subdev interfaces.
>>>>> Camera sensor using V4L2 subdev interface in the kernel is supported.
>>>>>
>>>>> The driver is tested on VisionFive V2 board with IMX219 camera sensor.
>>>>> GStreamer 1.18.5 with v4l2src plugin is supported.
>>>>>
>>>>> Changes since v3:
>>>>> Patch 1:
>>>>> - Modified port@0 and port@1 properties.
>>>>> - Extended the port@0 example with appropriate properties.
>>>>> - Added 'port@0' for 'required'
>>>>> Patch 2:
>>>>> - Modified spelling errors.
>>>>> Patch 3:
>>>>> - Merged patch 5 into the patch with an explanation for compatible in
>>>>>   commit msg.
>>>>> Patch 6:
>>>>> - Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
>>>>>   label.
>>>>> - Modified Code Style for getting sys_rst and p_rst.
>>>>> - Renamed clk_name to name and modified the relevant code.
>>>>> Patch 9:
>>>>> - Added static for stfcamss_get_mem_res function.
>>>>> - Added static for isp_close function.
>>>>> - Fixed implicit conversion warning for stf_vin_map_isp_pad function.
>>>>> - Dropped unused variables.
>>>>>
>>>>>   v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/
>>>>>
>>>>
>>>> Hello everyone,
>>>>
>>>> From the current review status, the patches related to the CSI module
>>>> have 'reviewed-by' tags. I would like to know if it is okay to add
>>>> patches 1-5 from this series to a PR first.
>>>>
>>>> Thank you!
>>>>
>>>> Jack
>>>>
>>>
>>> Hello Mauro, Laurent, Maxime, Rob, Krzysztof, Robert, Todor and Philipp,
>>>
>>> Can you give me some suggestions and comments on the previous request
>>> to commit CSI related patches first? Thank you for your time.
>>
>> You received very specific feedback, so know you decided to ignore it?
>>
>> No, implement what you were asked for.
>>
> 
> Hi Krzysztof,
> 
> Thank you for your comments.
> 
> I am talking about CSI-related patches 1-5, not including the patches
> 6-8. The CSI module is a relatively functionally independent hardware
> module. The CSI-related patches 1-5 already have 'reviewed-by' tags,
> and there are no unprocessed comments left. So, made the previous
> request. Please let me know if I understand something wrong.

You pinged also me, so we talk about bindings. You got comments to fix,
so if you are not going to fix them, the patches will not get accepted.

> I don't want to ignore any comments, I will continue to modify the isp
> patches 6-8 in subsequent versions according to your comments. The
> ISP-related patches are being prepared.

And how are these patches related to me and Rob?

Best regards,
Krzysztof
Jack Zhu May 6, 2023, 2:25 a.m. UTC | #8
On 2023/5/5 20:24, Krzysztof Kozlowski wrote:
> On 05/05/2023 10:14, Jack Zhu wrote:
>> 
>> 
>> On 2023/5/5 14:40, Krzysztof Kozlowski wrote:
>>> On 05/05/2023 07:57, Jack Zhu wrote:
>>>>
>>>>
>>>> On 2023/4/24 19:19, Jack Zhu wrote:
>>>>>
>>>>>
>>>>> On 2023/4/13 11:55, Jack Zhu wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This patch series adds support for the StarFive Camera Subsystem
>>>>>> found on StarFive JH7110 SoC.
>>>>>>
>>>>>> The driver implements V4L2, Media controller and V4L2 subdev interfaces.
>>>>>> Camera sensor using V4L2 subdev interface in the kernel is supported.
>>>>>>
>>>>>> The driver is tested on VisionFive V2 board with IMX219 camera sensor.
>>>>>> GStreamer 1.18.5 with v4l2src plugin is supported.
>>>>>>
>>>>>> Changes since v3:
>>>>>> Patch 1:
>>>>>> - Modified port@0 and port@1 properties.
>>>>>> - Extended the port@0 example with appropriate properties.
>>>>>> - Added 'port@0' for 'required'
>>>>>> Patch 2:
>>>>>> - Modified spelling errors.
>>>>>> Patch 3:
>>>>>> - Merged patch 5 into the patch with an explanation for compatible in
>>>>>>   commit msg.
>>>>>> Patch 6:
>>>>>> - Asserted pixel_rst[i] reset in the loop after the err_disable_pixclk
>>>>>>   label.
>>>>>> - Modified Code Style for getting sys_rst and p_rst.
>>>>>> - Renamed clk_name to name and modified the relevant code.
>>>>>> Patch 9:
>>>>>> - Added static for stfcamss_get_mem_res function.
>>>>>> - Added static for isp_close function.
>>>>>> - Fixed implicit conversion warning for stf_vin_map_isp_pad function.
>>>>>> - Dropped unused variables.
>>>>>>
>>>>>>   v3: https://lore.kernel.org/all/20230331121826.96973-1-jack.zhu@starfivetech.com/
>>>>>>
>>>>>
>>>>> Hello everyone,
>>>>>
>>>>> From the current review status, the patches related to the CSI module
>>>>> have 'reviewed-by' tags. I would like to know if it is okay to add
>>>>> patches 1-5 from this series to a PR first.
>>>>>
>>>>> Thank you!
>>>>>
>>>>> Jack
>>>>>
>>>>
>>>> Hello Mauro, Laurent, Maxime, Rob, Krzysztof, Robert, Todor and Philipp,
>>>>
>>>> Can you give me some suggestions and comments on the previous request
>>>> to commit CSI related patches first? Thank you for your time.
>>>
>>> You received very specific feedback, so know you decided to ignore it?
>>>
>>> No, implement what you were asked for.
>>>
>> 
>> Hi Krzysztof,
>> 
>> Thank you for your comments.
>> 
>> I am talking about CSI-related patches 1-5, not including the patches
>> 6-8. The CSI module is a relatively functionally independent hardware
>> module. The CSI-related patches 1-5 already have 'reviewed-by' tags,
>> and there are no unprocessed comments left. So, made the previous
>> request. Please let me know if I understand something wrong.
> 
> You pinged also me, so we talk about bindings. You got comments to fix,

Hello Krzysztof,

Sorry, I'm not sure, the 'comments to fix' refers to your comments in
patch 6?

> so if you are not going to fix them, the patches will not get accepted.
> 
>> I don't want to ignore any comments, I will continue to modify the isp
>> patches 6-8 in subsequent versions according to your comments. The
>> ISP-related patches are being prepared.
> 
> And how are these patches related to me and Rob?
> 

Yes, there are bindings files related to ISP.

Maybe I need to split the series into two separate series, a CSI
series(patches 1-5) and an ISP series(patches 6-8). Does this make it
easier for the CSI patches 1-5 to integrate into the main line?

> Best regards,
> Krzysztof
>