linux-kernel - Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP 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: <fd2e91a2-35e3-3ba8-d3c6-8963b504dd65@gmail.com>
Date:   Wed, 14 Dec 2016 17:52:59 +0200
From:   Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>
To:     Pali Rohár <pali.rohar@...il.com>,
        Pavel Machek <pavel@....cz>
Cc:     Sakari Ailus <sakari.ailus@....fi>, sre@...nel.org,
        linux-media@...r.kernel.org, galak@...eaurora.org,
        mchehab@....samsung.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP sensor

Hi

On 14.12.2016 15:03, Pali Rohár wrote:
> Hi! See inlined some my notes.
>

>> +
>> +#ifdef USE_CRC
>> +	rval = et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, 0x1263, &val);
>> +	if (rval)
>> +		goto out;
>> +#if USE_CRC /* TODO get crc setting from DT */
>> +	val |= BIT(4);
>> +#else
>> +	val &= ~BIT(4);
>> +#endif
>> +	rval = et8ek8_i2c_write_reg(client, ET8EK8_REG_8BIT, 0x1263, val);
>> +	if (rval)
>> +		goto out;
>> +#endif
>
> USE_CRC is defined to 1. Do we need that #ifdef check at all?
>
> What with above TODO?
>
>> +

It becomes a bit more complicated :) - on n900, front camera doesn't use 
CRC, while back camera does. Right now there is no way, even if we use 
video bus switch driver, to tell ISP to have 2 ports with different 
settings, not only for CRC, but for strobe, etc. Look at that commit 
https://github.com/freemangordon/linux-n900/commit/e5582fa56bbc0161d6c567157df42433829ee4de. 
What I think here is that ISP should take settings from the remote 
endpoints rather from it's local port. So far it does not.

So, until there is a way for ISP to have more than one CCP channel with 
different settings, I can't think of anything we can do here but to 
place TODO.

Ivo