linux-kernel - Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining the clock producer

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2egx7hakxleahk5o3ngydrcgtlnpgpdj4kgcijfij2bmss7u4d@2yho4udpzjtm>
Date: Thu, 15 May 2025 15:51:33 +0200
From: Mehdi Djait <mehdi.djait@...ux.intel.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Hans de Goede <hdegoede@...hat.com>, sakari.ailus@...ux.intel.com, 
	tomi.valkeinen@...asonboard.com, jacopo.mondi@...asonboard.com, hverkuil@...all.nl, 
	kieran.bingham@...asonboard.com, naush@...pberrypi.com, mchehab@...nel.org, 
	dave.stevenson@...pberrypi.com, linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v4] media: v4l2-common: Add a helper for obtaining
 the clock producer

Hi Laurent,

On Thu, May 15, 2025 at 02:40:50PM +0200, Laurent Pinchart wrote:
> On Thu, May 15, 2025 at 11:17:37AM +0200, Mehdi Djait wrote:
> > On Thu, May 15, 2025 at 10:44:03AM +0200, Laurent Pinchart wrote:
> > > On Sat, May 10, 2025 at 04:21:09PM +0200, Hans de Goede wrote:
> > > > On 21-Mar-25 2:03 PM, Mehdi Djait wrote:
> > > > > Introduce a helper for v4l2 sensor drivers on both DT- and ACPI-based
> > > > > platforms to retrieve a reference to the clock producer from firmware.
> > > > > 
> > > > > This helper behaves the same as clk_get_optional() except where there is
> > > > > no clock producer like in ACPI-based platforms.
> > > > > 
> > > > > For ACPI-based platforms the function will read the "clock-frequency"
> > > > > ACPI _DSD property and register a fixed frequency clock with the frequency
> > > > > indicated in the property.
> > > > > 
> > > > > Signed-off-by: Mehdi Djait <mehdi.djait@...ux.intel.com>
> > > > 
> > > > This certainly looks quite useful, thank you for working
> > > > on this.
> > > > 
> > > > Note on some IPU3 platforms where the clk is provided by
> > > > a clk-generator which is part of a special sensor-PMIC
> > > > the situation is a bit more complicated.
> > > > 
> > > > Basically if there is both a clk provider and a clock-frequency
> > > > property then the clock-frequency value should be set as freq
> > > > to the clk-provider, see:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov8865.c#n3020
> > > > 
> > > > for an example of a driver which handles this case.
> > > 
> > > On a side note, the DT bindings for the OV8865 doesn't specify the
> > > clock-frequency property...
> > 
> > Is this wrong ?
> > 
> > The OV8865 driver was introduced for DT-based systems, where you will
> > get a reference to the "struct clk corresponding to the clock producer"
> > and then get the clock-rate/frequency with a call to:
> > 
> > 	rate = clk_get_rate(sensor->extclk);
> > 
> > The patch "73dcffeb2ff9 media: i2c: Support 19.2MHz input clock in ov8865"
> > adding support for clock-frequency came later to support ACPI-based
> > systems (IPU3 here)
> 
> I'd expect all device properties to be documented in DT bindings. Is
> that an incorrect assumption ?
> 

I am actually genuinely asking, is the clock-frequency a device property
of the ov8865 camera sensor or the clock source, which is a separate device ?

Example the imx258 with a fixed-clock, which has its own compatible
and DT bindings under bindings/clock/fixed-clock.yaml

So when adding support for ACPI-based systems, the DT bindings should
not be changed because getting the clock-frequency from the ACPI _DSD
property is a workaround only needed on ACPI-based systems.

    i2c {
        #address-cells = <1>;
        #size-cells = <0>;

        sensor@6c {
            compatible = "sony,imx258";
            reg = <0x6c>;
            clocks = <&imx258_clk>;

            port {
                endpoint {
                    remote-endpoint = <&csi1_ep>;
                    data-lanes = <1 2 3 4>;
                    link-frequencies = /bits/ 64 <320000000>;
                };
            };
        };
    };

    /* Oscillator on the camera board */
    imx258_clk: clk {
        compatible = "fixed-clock";
        #clock-cells = <0>;
        clock-frequency = <19200000>;
    };

--
Kind Regards
Mehdi Djait