Re: [PATCH v12 1/8] dt-bindings: media: platform: visconti: Add Toshiba Visconti MIPI CSI-2 Receiver

[Krzysztof Kozlowski]

On 17/12/2024 00:57, yuji2.ishikawa@xxxxxxxxxxxxx wrote:
>> On 25/11/2024 10:21, Yuji Ishikawa wrote:
>>> Adds the Device Tree binding documentation that allows to describe the
>>> MIPI CSI-2 Receiver found in Toshiba Visconti SoCs.
>>>
>>> Signed-off-by: Yuji Ishikawa <yuji2.ishikawa@xxxxxxxxxxxxx>
>>> Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx>
>>
>>
>> How newly added patch can have already Rb tag? Was this review really, really
>> performed internally or you just satisfy some internal managers requirements
>> and fake the stats?
>>
> 
> I added this Reviewed-by tag because the patch was reviewed internally.


What issues were identified by internal review, especially in the
context of bindings?

> 
>>> ---
>>>
>>> Changelog v12:
>>> - Newly add bindings for CSI2RX driver
>>>
>>>  .../media/toshiba,visconti5-csi2rx.yaml       | 104
>> ++++++++++++++++++
>>>  1 file changed, 104 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.yam
>>> l
>>> b/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx.yam
>>> l
>>> new file mode 100644
>>> index 000000000000..5488072bc82a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/toshiba,visconti5-csi2rx
>>> +++ .yaml
>>> @@ -0,0 +1,104 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id:
>>> +http://devicetree.org/schemas/media/toshiba,visconti5-csi2rx.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Toshiba Visconti5 SoC MIPI CSI-2 receiver
>>> +
>>> +maintainers:
>>> +  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx>
>>> +
>>> +description: |-
>>
>> Drop |-
>>
> 
> I'll drop "|-"
> 
>>> +  Toshiba Visconti5 SoC MIPI CSI-2 receiver device receives MIPI
>>> + CSI-2 video  stream. Use with VIIF device. T.B.D
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: toshiba,visconti5-csi2rx
>>
>> Why this is called "RX"? Can you have a TX? I had impression that one cannot.
>>
> 
> VIIF has only MIPI CSI2 receiver (CSI2RX). There's no TX for it.

So this device cannot be anything else? Then drop rx.

> Visconti also has VOIF (Video Output Interface) hardware which has MIPI CSI2 (not DSI) transmitter (CSI2TX).

Or this can be something else? Confusing.



Best regards,
Krzysztof