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

[Mauro Carvalho Chehab]

Hi Katsuhiro-san,

Em Thu, 21 Jun 2018 12:17:48 +0900
Katsuhiro Suzuki <suzuki.katsuhiro@xxxxxxxxxxxxx> 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