linux-kernel - Re: [PATCH 5/5] [media] cxusb: add analog mode support for Medion MD95700

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] [day] [month] [year] [list]

Message-ID: <ed5f3e5f-a57d-6ee6-33f5-187ba49b9aa6@maciej.szmigiero.name>
Date:   Fri, 1 Sep 2017 00:29:04 +0200
From:   "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     Michael Krufky <mkrufky@...uxtv.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Andy Walls <awalls@...metrocast.net>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-media@...r.kernel.org
Subject: Re: [PATCH 5/5] [media] cxusb: add analog mode support for Medion
 MD95700

Hi Hans,

On 21.08.2017 15:23, Hans Verkuil wrote:
> Hi Maciej,
> 
> On 08/10/2017 11:53 PM, Maciej S. Szmigiero wrote:
>> This patch adds support for analog part of Medion 95700 in the cxusb
>> driver.
>>
>> What works:
>> * Video capture at various sizes with sequential fields,
>> * Input switching (TV Tuner, Composite, S-Video),
>> * TV and radio tuning,
>> * Video standard switching and auto detection,
>> * Radio mode switching (stereo / mono),
>> * Unplugging while capturing,
>> * DVB / analog coexistence,
>> * Raw BT.656 stream support.
> 
> Another scary patch :-)

Although this isn't a single-liner, most of the code in cxusb-analog.c are
simple implementations of v4l2 and videobuf2 callbacks - only the buffer
management code is a bit more complicated.

> A high-level question first: is any of the code in cxusb-analog medion
> specific? There are a lot of cxusb_medion_ prefixes, but I wonder if that
> shouldn't be cxusb_analog_.

>From all the devices cxusb driver supports it looks like only Medion 95700
and FusionHDTV5 USB have an analog part.

However, FusionHDTV5 USB has a different tuner and a different digital
frontend, doesn't support power off command, has a different USB interface
number for digital mode than Medion and requires upload of a firmware upon
plugging in (while Medion doesn't), so it's unlikely the current code for
Medion would work for it without significant changes.

> There are some obvious code cleanups that need to take place first, such
> as the huge functions with too many indentations. I would also split off
> cxusb-analog.c as a separate patch.

For example like splitting this part into two:
1) one that adds required analog support to cxusb with functions that are
provided by cxusb-analog.c (analog init, register, unregister) replaced
with stubs,

2) the second one that actually provides correct implementations via
cxusb-analog.c?

> 
> Regards,
> 
> 	Hans

Best regards,
Maciej