Re: [PATCH v4 2/4] dt-bindings: media: add Maxim MAX96714 GMSL2 Deserializer

[Julien Massot <julien.massot@collabora.com>]

Hi Conor,

Thanks for reviewing my patchset.

On 3/7/24 20:21, Conor Dooley wrote:
> On Tue, Mar 05, 2024 at 04:26:06PM +0100, Julien Massot wrote:
>> Add DT bindings for Maxim MAX96714 GMSL2 Deserializer.
>>
>> Signed-off-by: Julien Massot <julien.massot@collabora.com>
>> ---
>> Change since v3:
>>   - Renamed file to maxim,max96714.yaml dropped the 'f' suffix
> 
> Why? The filename should match the compatible, which /does/ have an f.
All the work has been done on MAX96714F variant of this Maxim GMSL2 
deserializer.
The driver and the binding remain suitable for all variants of this 
chipset, since they share the same
register mapping, similar features etc..

MAX96714 exists in different variant: MAX96714 / MAX96714F / MAX96714K 
that will be easy
to add support for this binding and driver later.

The MAX96714 name looks the most suitable.
Please have a look at this discussion on the V3 version
https://lore.kernel.org/lkml/ZdXYpc2csVnhtZH9@valkosipuli.retiisi.eu

> 
>>   - Removed mention to C-PHY since it's not supported by MAX96714 deserializers
>>   - Removed bus-type requirement on CSI endpoint since the device only support D-PHY
>>   - Removed the clock-lanes property in the dt example
>>
>> Change since v2:
>>   - remove reg description
>>   - rename enable gpio to powerdown
>>   - use generic node name: i2c, serializer, deserializer
>> ---
>> ---
>>   .../bindings/media/i2c/maxim,max96714.yaml    | 169 ++++++++++++++++++
>>   1 file changed, 169 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/i2c/maxim,max96714.yaml
> 
>> +properties:
>> +  compatible:
>> +    const: maxim,max96714f
> 
>> +  i2c-gate:
>> +    $ref: /schemas/i2c/i2c-controller.yaml
> 
> There is an i2c-gate binding, you should reference it here instead.
Ok, I will post a new version with a reference to the i2c-gate binding.

> 
>> +    unevaluatedProperties: false
>> +    description: |
> 
> This | is not needed, there's no formatting to preserve.
Ok I will drop the '|'
> 
>> +      The MAX96714 will pass through and forward the I2C requests from the
>> +      incoming I2C bus over the GMSL2 link. Therefore it supports an i2c-gate
>> +      subnode to configure a serializer.

Regards,
-- 
Julien Massot
Senior Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718