linux-kernel - Re: [PATCH v3] media: dvb-frontends: add Socionext SC1501A ISDB-S/T demodulator driver

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]

Date:   Wed, 4 Jul 2018 13:58:06 -0300
From:   Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
To:     Katsuhiro Suzuki <suzuki.katsuhiro@...ionext.com>
Cc:     linux-media@...r.kernel.org,
        Masami Hiramatsu <masami.hiramatsu@...aro.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] media: dvb-frontends: add Socionext SC1501A ISDB-S/T
 demodulator driver

Hi Katsuhiro-san,

Em Thu, 21 Jun 2018 12:17:48 +0900
Katsuhiro Suzuki <suzuki.katsuhiro@...ionext.com> escreveu:

> This patch adds a frontend driver for the Socionext SC1501A series
> and Socionext MN88443x ISDB-S/T demodulators.

Sorry for taking so long to review it. We're missing a sub-maintainer
for DVB, with would otherwise speed up reviews of DVB patches.
> 
> The maximum and minimum frequency of Socionext SC1501A comes from
> ISDB-S and ISDB-T so frequency range is the following:
>   - ISDB-S (BS/CS110 IF frequency in kHz, Local freq 10.678GHz)
>     - Min: BS-1: 1032000 => 1032.23MHz
>     - Max: ND24: 2701000 => 2070.25MHz
>   - ISDB-T (in Hz)
>     - Min: ch13: 470000000 => 470.357857MHz
>     - Max: ch62: 770000000 => 769.927857MHz

There is actually an error on that part of the driver. Right now,
the DVB core expects Satellite frequencies (DVB-S, ISDB-S, ...)
in kHz. For all other delivery systems, it is in Hz.

It is this way due to historic reasons. While it won't be hard to
change the core, that would require to touch all Satellite drivers.

As there are very few frontend drivers that accept both Satellite
and Terrestrial standards, what we do, instead, is to setup
two frontends. See, for example, drivers/media/dvb-frontends/helene.c.

...
> +static const struct dvb_frontend_ops sc1501a_ops = {
> +	.delsys = { SYS_ISDBS, SYS_ISDBT },
> +	.info = {
> +		.name          = "Socionext SC1501A",
> +		.frequency_min = 1032000,
> +		.frequency_max = 770000000,
> +		.caps = FE_CAN_INVERSION_AUTO | FE_CAN_FEC_AUTO |
> +			FE_CAN_QAM_AUTO | FE_CAN_TRANSMISSION_MODE_AUTO |
> +			FE_CAN_GUARD_INTERVAL_AUTO | FE_CAN_HIERARCHY_AUTO,
> +	},
> +
> +	.sleep                   = sc1501a_sleep,
> +	.set_frontend            = sc1501a_set_frontend,
> +	.get_tune_settings       = sc1501a_get_tune_settings,
> +	.read_status             = sc1501a_read_status,
> +};

In other words, you'll need to declare two structs here, one for ISDB-T
and another one for ISDB-S.

Yeah, I know that this sucks. If you are in the mood of touching the
DVB core, I'm willing to consider a patch that would fix this, provided
that it won't break backward compatibility with other drivers (or would
convert the other satellite drivers to use the new way).

Thanks,
Mauro