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

[Sakari Ailus]

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.

> 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.

-- 
Kind regards,

Kind regards,

Sakari Ailus