linux-kernel - Re: [PATCH 13/18] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

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: <797dda65-aedd-6a83-3f36-0cba457e4570@gmail.com>
Date:   Thu, 17 Dec 2020 14:17:12 +0000
From:   Daniel Scally <djrscally@...il.com>
To:     Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     Sakari Ailus <sakari.ailus@....fi>, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-i2c@...r.kernel.org, linux-media@...r.kernel.org,
        devel@...ica.org, rjw@...ysocki.net, lenb@...nel.org,
        gregkh@...uxfoundation.org, mika.westerberg@...ux.intel.com,
        andriy.shevchenko@...ux.intel.com, linus.walleij@...aro.org,
        bgolaszewski@...libre.com, wsa@...nel.org, yong.zhi@...el.com,
        bingbu.cao@...el.com, tian.shu.qiu@...el.com, mchehab@...nel.org,
        robert.moore@...el.com, erik.kaneda@...el.com, pmladek@...e.com,
        rostedt@...dmis.org, sergey.senozhatsky@...il.com,
        linux@...musvillemoes.dk, kieran.bingham+renesas@...asonboard.com,
        jacopo+renesas@...ndi.org,
        laurent.pinchart+renesas@...asonboard.com,
        jorhand@...ux.microsoft.com, kitakar@...il.com,
        heikki.krogerus@...ux.intel.com
Subject: Re: [PATCH 13/18] ipu3-cio2: Add functionality allowing software_node
 connections to sensors on platforms designed for Windows

Hi Sakari - sorry for delayed reply. I didn't get this email actually,
just spotted it on the newsgroup by chance.

On 15/12/2020 22:02, Sakari Ailus wrote:
> Hi Daniel,
> 
> On Tue, Dec 15, 2020 at 10:28:59AM +0000, Daniel Scally wrote:
>> Morning Sakari
>>
>> On 30/11/2020 20:35, Sakari Ailus wrote:
>>>> +/*
>>>> + * Extend this array with ACPI Hardware ID's of devices known to be working.
>>>> + * Do not add a HID for a sensor that is not actually supported.
>>>> + */
>>>> +static const char * const cio2_supported_devices[] = {
>>>> +	"INT33BE",
>>>> +	"OVTI2680",
>>>
>>> I guess we don't have the known-good frequencies for the CSI-2 bus in
>>> firmware?
>>>
>>> One option would be to put there what the drivers currently use. This
>>> assumes the support for these devices is, well, somewhat opportunistic but
>>> I guess there's no way around that right now at least.
>>>
>>> As the systems are laptops, they're likely somewhat less prone to EMI
>>> issues to begin with than mobile phones anyway.
>>
>> Just looking at this; we're currently using this with the ov2680 driver
>> that's in mainline currently (with very minor tweaks) plus a
>> hacked-into-roughly-working version of the atomisp-ov5693 driver (ACPI
>> ID INT33BE = ov5693 physical device). Neither of those drivers lists any
>> link frequencies, nor provides a link frequency control for v4l2 to work
>> with.
>>
>> On the other hand, the ov5648 [1] and ov8865 [2] drivers which Paul has
>> submitted recently, which we also want to be able to support, _do_
>> include that. I can register the frequencies Paul's defined there as a
>> link-frequencies property but this gives rise to two questions:
>>
>>
>> 1. Is this _mandatory_? Do I need to be finding the link-frequencies for
>> the OV2680 and OV5693 drivers too? Or can I skip that property where the
>> driver doesn't handle it anyway. Seems to be working fine without
>> controlling it in driver.
> 
> Receiver drivers generally need the information to program the receiver
> timing. It may work for you without using the correct frequency, but the
> risk of failure on another unit increases.

Hmm, ok. I'll see if I can find the correct values then to add to the
existing drivers.

>> 2. Can I trust all the values in the drivers to be available on each
>> platform? For example for the ov5648 Paul lists these as available:
>>
>>  938static const s64 ov5648_link_freq_menu[] = {
>>
>>
>>  939        210000000,
>>
>>
>>  940        168000000,
>>
>>
>>  941};
>>
>> But can I safely register a link-frequencies property for both of those
>> and trust that that'll work on all IPU3 platforms with an ov5648 in them?
> 
> Ideally we'd know which frequency Windows uses, and use the same.
> 
> Using another frequency may have adverse effects elsewhere in the system.
> AFAIU mostly this concerns radios of all sorts.
> 
> Now that this is in the kernel in any case, it can be fixed later on so I'm
> not too worried about it. Having still a comment there that the
> configuration is opportunistic would be nice.
> 

Understood - I'll add in the ability to add the link-frequencies plus a
comment explaining.

Thanks
Dan