linux-kernel - Re: [PATCH v4 1/2] dt-bindings: media: i2c: Add ov2735 sensor

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: <8cdea7c7-739a-4101-9e2f-1bb36632ff5a@kernel.org>
Date: Thu, 17 Jul 2025 15:00:52 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>
Cc: "sakari.ailus@...ux.intel.com" <sakari.ailus@...ux.intel.com>,
 "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
 "laurent.pinchart@...asonboard.com" <laurent.pinchart@...asonboard.com>,
 "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
 Himanshu Bhavani <himanshu.bhavani@...iconsignals.io>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Hans Verkuil <hverkuil@...all.nl>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 André Apitzsch <git@...tzsch.eu>,
 Ricardo Ribalda <ribalda@...omium.org>,
 Heimir Thor Sverrisson <heimir.sverrisson@...il.com>,
 Matthias Fend <matthias.fend@...end.at>,
 Sylvain Petinot <sylvain.petinot@...s.st.com>,
 Jingjing Xiong <jingjing.xiong@...el.com>,
 Benjamin Mugnier <benjamin.mugnier@...s.st.com>,
 Dongcheng Yan <dongcheng.yan@...el.com>, Arnd Bergmann <arnd@...db.de>,
 Hans de Goede <hansg@...nel.org>,
 "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] dt-bindings: media: i2c: Add ov2735 sensor

On 17/07/2025 13:55, Hardevsinh Palaniya wrote:
>> On 17/07/2025 09:28, Hardevsinh Palaniya wrote:
>>>  Hi Krzysztof,
>>>
>>>> On Wed, Jul 16, 2025 at 07:14:16PM +0530, Hardevsinh Palaniya wrote:
>>>>> +        properties:
>>>>> +          data-lanes:
>>>>> +            items:
>>>>> +              - const: 1
>>>>> +              - const: 2
>>>>> +          link-frequencies: true
>>>>
>>>> Drop
>>>>
>>>> I don't understand why this appeared. I don't think anyone asked for it?
>>>
>>> Laurent suggested validating the link frequency in the Device Tree.
>>>
>>> Link[1]: https://lore.kernel.org/linux-media/20250710212131.GG22436@pendragon.ideasonboard.com/
>>
>> ... and I do not see here validation "that the link frequencies
>> specified in DT match".
>>
>> How do you validate that 1111 Hz is not / is a valid link frequency? How
>> did you exactly resolve the comment about validating?
> 
> In the ov2735_parse_endpoint() function, the driver validates the link frequency
> from DT using `v4l2_link_freq_to_bitmap()`. If an unsupported value like 1111 Hz is
> provided, it returns an error: 


But that's driver. You said that you added this property based on
feedback on validation. There is no validation here, thus the reason of
this property is not fulfilled and that's why I was asking. I know now
the answer...



> 
> ret = v4l2_link_freq_to_bitmap(ov2735->dev, bus_cfg.link_frequencies,
>                                bus_cfg.nr_of_link_frequencies,
>                                link_freq_menu_items,
>                                ARRAY_SIZE(link_freq_menu_items),
>                                &link_freq_bitmap);
> if (ret) {
>         ret = dev_err_probe(ov2735->dev,
>                             -EINVAL, "only 420MHz frequency is available\n");
>         goto error_out;
> }
> 
> So, validation is already done in the driver.
> 
> I understand that `link-frequencies: true` in the schema only declares the property
> without restricting values. Would you prefer the schema to explicitly allow only
> 420MHz like this?


You missed the last paragraph of my last email. I don't want to have
here what the driver is doing, but what the hardware is/needs/requires.
It can also be missing completely, because we don't enforce such rules
on other bindings.


Best regards,
Krzysztof