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:
 <PN3P287MB351999B4DB533CCA9C6E6567FF51A@PN3P287MB3519.INDP287.PROD.OUTLOOK.COM>
Date: Thu, 17 Jul 2025 13:30:45 +0000
From: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>
To: Krzysztof Kozlowski <krzk@...nel.org>
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...

What I thought was: since the driver is validating the link-frequencies
property from DT, the DT must contain this property,so I added it to the
schema.

Also, I noticed that other camera dt schema follow a similar approach. 
 
> >
> > 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.

However, the sensor itself supports multiple link frequencies, which is 
why I didn’t restrict it to just 420 MHz in the schema.

Currently, the driver only supports 420 MHz, so we handle validation 
there, not in the DT schema.

Please let me know what you suggest. Should I remove the link-frequencies 
property from the schema, or would you prefer a different approach?

Best regards,
Hardev