linux-kernel - Re: [PATCH v5] Driver for ON Semi AR0521 camera 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: <m335p5lc04.fsf@t19.piap.pl>
Date:   Wed, 13 Oct 2021 07:39:07 +0200
From:   Krzysztof Hałasa <khalasa@...p.pl>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     Jacopo Mondi <jacopo@...ndi.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Matteo Lisi <matteo.lisi@...icam.com>
Subject: Re: [PATCH v5] Driver for ON Semi AR0521 camera sensor

Hi Sakari,

> 	https://hverkuil.home.xs4all.nl/spec/driver-api/camera-sensor.html

Ok:
"8.2.2. Devicetree

The currently preferred way to achieve this is using assigned-clocks,
assigned-clock-parents and assigned-clock-rates properties. See
Documentation/devicetree/bindings/clock/clock-bindings.txt for more
information. The driver then gets the frequency using clk_get_rate()."

Let's see:
Documentation/devicetree/bindings/clock/clock-bindings.txt:

"==Assigned clock parents and rates==

Some platforms may require initial configuration of default parent clocks
and clock frequencies. Such a configuration can be specified in a device tree
node through assigned-clocks, assigned-clock-parents and assigned-clock-rates
properties. The assigned-clock-parents property should contain a list of parent
clocks in the form of a phandle and clock specifier pair and the
assigned-clock-rates property should contain a list of frequencies in Hz. Both
these properties should correspond to the clocks listed in the assigned-clocks
property."

So I'm after "assigned-clock-rates", right?

"Configuring a clock's parent and rate through the device node that consumes
the clock can be done only for clocks that have a single user. Specifying
conflicting parent or rate configuration in multiple consumer nodes for
a shared clock is forbidden."

This sounds a bit problematic, the clock I use is at least potentially
shared by multiple parts of the system, depending on current (run time)
configuration. I am/was getting different frequencies depending of the
particular system (all based on the same i.MX6* SoC, but with different
peripherals used/enabled). I think it's quite a common situation.

> Generally camera sensor drivers that set the clock in drivers themselves
> are (very) old.

Let's have a look... ov9282 is (one of) the newest drivers. It does:
#define OV9282_INCLK_RATE    24000000

        /* Get sensor input clock */
        ov9282->inclk = devm_clk_get(ov9282->dev, NULL);
        if (IS_ERR(ov9282->inclk)) {
                dev_err(ov9282->dev, "could not get inclk");
                return PTR_ERR(ov9282->inclk);
        }

        rate = clk_get_rate(ov9282->inclk);
        if (rate != OV9282_INCLK_RATE) {
                dev_err(ov9282->dev, "inclk frequency mismatch");
                return -EINVAL;
        }

$ git grep -l ov9282
Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml
MAINTAINERS
drivers/media/i2c/Kconfig
drivers/media/i2c/Makefile
drivers/media/i2c/ov9282.c

  clocks:
    description: Clock frequency from 6 to 27MHz

No in-tree DTS exists, but the single frequency (both in the driver -
this one can be fixed - and in the DTS) is rather limiting. Maybe
another:

imx412, imx335, imx334, imx258 - same here.
imx208 is ACPI-based.

Which driver should I consult?
-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa