[v2,4/5] tc90522: add driver for Toshiba TC90522 quad demodulator

Message ID 1409153356-1887-5-git-send-email-tskd08@gmail.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Akihiro TSUKADA Aug. 27, 2014, 3:29 p.m. UTC
  From: Akihiro Tsukada <tskd08@gmail.com>

This patch adds driver for tc90522 demodulator chips.
The chip contains 4 demod modules that run in parallel and are independently
controllable via separate I2C addresses.
Two of the modules are for ISDB-T and the rest for ISDB-S.
It is used in earthsoft pt3 cards.

Note that this driver does not init the chip,
because the initilization sequence / register setting is not disclosed.
Thus, the driver assumes that the chips are initilized externally
by its parent board driver before fe->ops->init() are called.
Earthsoft PT3 PCIe card, for example, contains the init sequence
in its private memory and provides a command to trigger the sequence.

Signed-off-by: Akihiro Tsukada <tskd08@gmail.com>
---
Changes in v2:
- renamed badly named variables
- moved static const tables out of function scope
- calculate symbol rate per output TS
- improve in _init() to support suspend/resume

 drivers/media/dvb-frontends/Kconfig   |   8 +
 drivers/media/dvb-frontends/Makefile  |   1 +
 drivers/media/dvb-frontends/tc90522.c | 857 ++++++++++++++++++++++++++++++++++
 drivers/media/dvb-frontends/tc90522.h |  63 +++
 4 files changed, 929 insertions(+)
  

Comments

Matthias Schwarzott Aug. 31, 2014, 10:29 a.m. UTC | #1
On 27.08.2014 17:29, tskd08@gmail.com wrote:
> From: Akihiro Tsukada <tskd08@gmail.com>
> 
Hi Akihiro,

> This patch adds driver for tc90522 demodulator chips.
> The chip contains 4 demod modules that run in parallel and are independently
> controllable via separate I2C addresses.
> Two of the modules are for ISDB-T and the rest for ISDB-S.
> It is used in earthsoft pt3 cards.
> 
> Note that this driver does not init the chip,
> because the initilization sequence / register setting is not disclosed.
> Thus, the driver assumes that the chips are initilized externally
> by its parent board driver before fe->ops->init() are called.
> Earthsoft PT3 PCIe card, for example, contains the init sequence
> in its private memory and provides a command to trigger the sequence.
> 
> Signed-off-by: Akihiro Tsukada <tskd08@gmail.com>

> +
> +struct dvb_frontend *
> +tc90522_attach(const struct tc90522_config *cfg, struct i2c_adapter *i2c)
> +{
> +	struct tc90522_state *state;
> +	const struct dvb_frontend_ops *ops;
> +	struct i2c_adapter *adap;
> +	int ret;
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return NULL;
> +
> +	memcpy(&state->cfg, cfg, sizeof(*cfg));
> +	state->i2c = i2c;
> +	ops = TC90522_IS_ISDBS(cfg->addr) ? &tc90522_ops_sat : &tc90522_ops_ter;
> +	memcpy(&state->dvb_fe.ops, ops, sizeof(*ops));
> +	state->dvb_fe.demodulator_priv = state;
> +
> +	adap = &state->tuner_i2c;
> +	adap->owner = i2c->owner;
> +	adap->algo = &tc90522_tuner_i2c_algo;
> +	adap->dev.parent = i2c->dev.parent;
> +	strlcpy(adap->name, "tc90522 tuner", sizeof(adap->name));
> +	i2c_set_adapdata(adap, state);
> +	ret = i2c_add_adapter(adap);
> +	if (ret < 0) {
> +		kfree(state);
> +		return NULL;
> +	}
> +	dev_info(&i2c->dev, "Toshiba TC90522 attached.\n");
> +	return &state->dvb_fe;
> +}
> +EXPORT_SYMBOL(tc90522_attach);
> +
> +
> +struct i2c_adapter *tc90522_get_tuner_i2c(struct dvb_frontend *fe)
> +{
> +	struct tc90522_state *state;
> +
> +	state = fe->demodulator_priv;
> +	return &state->tuner_i2c;
> +}
> +EXPORT_SYMBOL(tc90522_get_tuner_i2c);
> +
> +
> +MODULE_DESCRIPTION("Toshiba TC90522 frontend");
> +MODULE_AUTHOR("Akihiro TSUKADA");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/dvb-frontends/tc90522.h b/drivers/media/dvb-frontends/tc90522.h
> new file mode 100644
> index 0000000..d55a6be
> --- /dev/null
> +++ b/drivers/media/dvb-frontends/tc90522.h
> @@ -0,0 +1,63 @@
> +/*
> + * Toshiba TC90522 Demodulator
> + *
> + * Copyright (C) 2014 Akihiro Tsukada <tskd08@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/*
> + * The demod has 4 input (2xISDB-T and 2xISDB-S),
> + * and provides independent sub modules for each input.
> + * As the sub modules work in parallel and have the separate i2c addr's,
> + * this driver treats each sub module as one demod device.
> + */
> +
> +#ifndef TC90522_H
> +#define TC90522_H
> +
> +#include <linux/i2c.h>
> +#include <linux/kconfig.h>
> +#include "dvb_frontend.h"
> +
> +#define TC90522_IS_ISDBS(addr) (addr & 1)
> +
> +#define TC90522T_CMD_SET_LNA 1
> +#define TC90522S_CMD_SET_LNB 1
> +
> +struct tc90522_config {
> +	u8 addr;
> +};
> +
> +#if IS_ENABLED(CONFIG_DVB_TC90522)
> +
> +extern struct dvb_frontend *tc90522_attach(const struct tc90522_config *cfg,
> +		struct i2c_adapter *i2c);
> +
> +extern struct i2c_adapter *tc90522_get_tuner_i2c(struct dvb_frontend *fe);
> +

it sounds wrong to export a second function besides tc90522_attach.
This way there is a hard dependency of the bridge driver to the demod
driver.
In this case it is the only possible demod, but in general it violates
the design of demod drivers and their connection to bridge drivers.

si2168_probe at least has a solution for this:
Write the pointer to the new i2c adapter into location stored in "struct
i2c_adapter **" in the config structure.

Regards
Matthias

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Akihiro TSUKADA Aug. 31, 2014, 1:32 p.m. UTC | #2
Hi Matthias,
thanks for the comment.

> it sounds wrong to export a second function besides tc90522_attach.
> This way there is a hard dependency of the bridge driver to the demod
> driver.
> In this case it is the only possible demod, but in general it violates
> the design of demod drivers and their connection to bridge drivers.

I agree. I missed that point.

> 
> si2168_probe at least has a solution for this:
> Write the pointer to the new i2c adapter into location stored in "struct
> i2c_adapter **" in the config structure.

I'll look into the si2168 code and update tc90522 in v3.

regards,
akihiro

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Aug. 31, 2014, 7:48 p.m. UTC | #3
On 08/31/2014 04:32 PM, Akihiro TSUKADA wrote:
> Hi Matthias,
> thanks for the comment.
>
>> it sounds wrong to export a second function besides tc90522_attach.
>> This way there is a hard dependency of the bridge driver to the demod
>> driver.
>> In this case it is the only possible demod, but in general it violates
>> the design of demod drivers and their connection to bridge drivers.
>
> I agree. I missed that point.
>
>>
>> si2168_probe at least has a solution for this:
>> Write the pointer to the new i2c adapter into location stored in "struct
>> i2c_adapter **" in the config structure.
>
> I'll look into the si2168 code and update tc90522 in v3.

Also, I would like to see all new drivers (demod and tuner) implemented 
as a standard kernel I2C drivers (or any other bus). I have converted 
already quite many drivers, si2168, si2157, m88ds3103, m88ts2022, 
it913x, tda18212, ...
When drivers are using proper kernel driver models, it allows using 
kernel services. For example dev_ / pr_ logging (it does not work 
properly without), RegMap API, I2C client, I2C multiplex, and so...

Here is few recent examples:
https://patchwork.linuxtv.org/patch/25495/
https://patchwork.linuxtv.org/patch/25152/
https://patchwork.linuxtv.org/patch/25146/

regards
Antti
  
Akihiro TSUKADA Sept. 1, 2014, 9:54 a.m. UTC | #4
Hi,

> Also, I would like to see all new drivers (demod and tuner) implemented
> as a standard kernel I2C drivers (or any other bus). I have converted
> already quite many drivers, si2168, si2157, m88ds3103, m88ts2022,
> it913x, tda18212, ...

I wrote the code in the old style using dvb_attach()
because (I felt) it is simpler than using i2c_new_device() by
introducing new i2c-related data structures,
registering to both dvb and i2c, without any new practical
features that i2c client provides.

But if the use of dvb_attach() is (almost) deprecated and
i2c client driver is the standard/prefered way,
I'll convert my code.

regards,
akihiro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Sept. 6, 2014, 2:02 a.m. UTC | #5
On 09/01/2014 12:54 PM, Akihiro TSUKADA wrote:
> Hi,
>
>> Also, I would like to see all new drivers (demod and tuner) implemented
>> as a standard kernel I2C drivers (or any other bus). I have converted
>> already quite many drivers, si2168, si2157, m88ds3103, m88ts2022,
>> it913x, tda18212, ...
>
> I wrote the code in the old style using dvb_attach()
> because (I felt) it is simpler than using i2c_new_device() by
> introducing new i2c-related data structures,
> registering to both dvb and i2c, without any new practical
> features that i2c client provides.

Of course it is simpler to do old style as you could copy & paste older 
drivers and so. However, for a long term we must get rid of all DVB 
specific hacks and use common kernel solutions. The gap between common 
kernel solutions and DVB proprietary is already too big, without any 
good reason - just a laziness of developers to find out proper solutions 
as adding hacks is easier.

I mentioned quite many reasons earlier and If you look that driver you 
will see you use dev_foo() logging, that does not even work properly 
unless you convert driver to some kernel binding model (I2C on that 
case) (as I explained earlier).

There is also review issues. For more people do own tricks and hacks the 
harder code is review and also maintain as you don't never know what 
breaks when you do small change, which due to some trick used causes 
some other error.

Here is one example I fixed recently:
https://patchwork.linuxtv.org/patch/25776/

Lets mention that I am not even now fully happy to solution, even it 
somehow now works. Proper solution is implement clock source and clock 
client. Then register client to that source. And when client needs a 
clock (or power) it makes call to enable clock.

> But if the use of dvb_attach() is (almost) deprecated and
> i2c client driver is the standard/prefered way,
> I'll convert my code.

regards
Antti
  
Mauro Carvalho Chehab Sept. 6, 2014, 2:27 a.m. UTC | #6
Em Sat, 06 Sep 2014 05:02:15 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> On 09/01/2014 12:54 PM, Akihiro TSUKADA wrote:
> > Hi,
> >
> >> Also, I would like to see all new drivers (demod and tuner) implemented
> >> as a standard kernel I2C drivers (or any other bus). I have converted
> >> already quite many drivers, si2168, si2157, m88ds3103, m88ts2022,
> >> it913x, tda18212, ...
> >
> > I wrote the code in the old style using dvb_attach()
> > because (I felt) it is simpler than using i2c_new_device() by
> > introducing new i2c-related data structures,
> > registering to both dvb and i2c, without any new practical
> > features that i2c client provides.
> 
> Of course it is simpler to do old style as you could copy & paste older 
> drivers and so. However, for a long term we must get rid of all DVB 
> specific hacks and use common kernel solutions. The gap between common 
> kernel solutions and DVB proprietary is already too big, without any 
> good reason - just a laziness of developers to find out proper solutions 
> as adding hacks is easier.
> 
> I mentioned quite many reasons earlier and If you look that driver you 
> will see you use dev_foo() logging, that does not even work properly 
> unless you convert driver to some kernel binding model (I2C on that 
> case) (as I explained earlier).
> 
> There is also review issues. For more people do own tricks and hacks the 
> harder code is review and also maintain as you don't never know what 
> breaks when you do small change, which due to some trick used causes 
> some other error.
> 
> Here is one example I fixed recently:
> https://patchwork.linuxtv.org/patch/25776/

Yes, using the I2C binding way provides a better decoupling than using the
legacy way. The current dvb_attach() macros are hacks that were created
by the time where the I2C standard bind didn't work with DVB.

We need to move on.

> 
> Lets mention that I am not even now fully happy to solution, even it 
> somehow now works. Proper solution is implement clock source and clock 
> client. Then register client to that source. And when client needs a 
> clock (or power) it makes call to enable clock.

Well, we need to discuss more about that, because you need to convince
me first about that ;)

We had already some discussions about that related to V4L2 I2C devices.
The consensus we've reached is that it makes sense to use the clock
framework only for the cases where the bridge driver doesn't know anything
about the clock to be used by a given device, e. g. in the case where this
data comes from the Device Tree (embedded systems).

In the case where the bridge is the ownership of the information that will
be used by a given device model (clock, serial/parallel mode, etc), then
a series of data information should be passed by a call from the bridge driver
to the device at setup time, and doing it in an atomic way is the best
way to go.

Anyway, such discussions don't belong to this thread. For the PT3 and
tc90522 to be merged, the only pending stuff to be done is to use the
I2C binding.

Akihiro-san,

Please change the driver to use the I2C model as pointed by Antti.

Thank you!
Mauro

> > But if the use of dvb_attach() is (almost) deprecated and
> > i2c client driver is the standard/prefered way,
> > I'll convert my code.
> 
> regards
> Antti
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Sept. 6, 2014, 3 a.m. UTC | #7
On 09/06/2014 05:27 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 06 Sep 2014 05:02:15 +0300
> Antti Palosaari <crope@iki.fi> escreveu:

>> Lets mention that I am not even now fully happy to solution, even it
>> somehow now works. Proper solution is implement clock source and clock
>> client. Then register client to that source. And when client needs a
>> clock (or power) it makes call to enable clock.
>
> Well, we need to discuss more about that, because you need to convince
> me first about that ;)
>
> We had already some discussions about that related to V4L2 I2C devices.
> The consensus we've reached is that it makes sense to use the clock
> framework only for the cases where the bridge driver doesn't know anything
> about the clock to be used by a given device, e. g. in the case where this
> data comes from the Device Tree (embedded systems).
>
> In the case where the bridge is the ownership of the information that will
> be used by a given device model (clock, serial/parallel mode, etc), then
> a series of data information should be passed by a call from the bridge driver
> to the device at setup time, and doing it in an atomic way is the best
> way to go.

For AF9033/IT9133 demod + tuner I resolved it like that:
https://patchwork.linuxtv.org/patch/25772/
https://patchwork.linuxtv.org/patch/25774/

It is demod which provides clock for tuner. It is very common situation 
nowadays, one or more clocks are shared. And clock sharing is routed via 
chips so that there is clock gates you have enable/disable for power 
management reasons.

Currently we just enable clocks always. Clock output is put on when 
driver is attached and it is never disabled after that, leaving power 
management partly broken.

Lets take a example, dual tuner case:
tuner#0 gets clock from Xtal
tuner#1 gets clock from #tuner0

All possible use cases are:
1) #tuner0 off & #tuner1 off
2) #tuner0 on & #tuner1 off
3) #tuner1 off & #tuner1 on
4) #tuner1 on & #tuner1 on

you will need, as per aforementioned use case:
1) #tuner0 clock out off & #tuner1 clock out off
2) #tuner0 clock out off & #tuner1 clock out off
3) #tuner0 clock out on & #tuner1 clock out off
4) #tuner0 clock out on & #tuner1 clock out off

Implementing that currently is simply impossible. But if you use clock 
framework (or what ever its name is) I think it is possible to implement 
that properly. When tuner#1 driver needs a clock, it calls "get clock" 
and that call is routed to #tuner0 which enables clock.

And that was not even the most complicated case, as many times clock is 
routed to demod and USB bridge too.

Quite same situation is for power on/off gpios (which should likely 
implemented as a regulator). Also there is many times reset gpio (for PM 
chip is powered off by switching power totally off *or* chip is put to 
reset using GPIO)

regards
Antti
  
Mauro Carvalho Chehab Sept. 6, 2014, 3:11 a.m. UTC | #8
Em Sat, 06 Sep 2014 06:00:28 +0300
Antti Palosaari <crope@iki.fi> escreveu:

> On 09/06/2014 05:27 AM, Mauro Carvalho Chehab wrote:
> > Em Sat, 06 Sep 2014 05:02:15 +0300
> > Antti Palosaari <crope@iki.fi> escreveu:
> 
> >> Lets mention that I am not even now fully happy to solution, even it
> >> somehow now works. Proper solution is implement clock source and clock
> >> client. Then register client to that source. And when client needs a
> >> clock (or power) it makes call to enable clock.
> >
> > Well, we need to discuss more about that, because you need to convince
> > me first about that ;)
> >
> > We had already some discussions about that related to V4L2 I2C devices.
> > The consensus we've reached is that it makes sense to use the clock
> > framework only for the cases where the bridge driver doesn't know anything
> > about the clock to be used by a given device, e. g. in the case where this
> > data comes from the Device Tree (embedded systems).
> >
> > In the case where the bridge is the ownership of the information that will
> > be used by a given device model (clock, serial/parallel mode, etc), then
> > a series of data information should be passed by a call from the bridge driver
> > to the device at setup time, and doing it in an atomic way is the best
> > way to go.
> 
> For AF9033/IT9133 demod + tuner I resolved it like that:
> https://patchwork.linuxtv.org/patch/25772/
> https://patchwork.linuxtv.org/patch/25774/
> 
> It is demod which provides clock for tuner. It is very common situation 
> nowadays, one or more clocks are shared. And clock sharing is routed via 
> chips so that there is clock gates you have enable/disable for power 
> management reasons.
> 
> Currently we just enable clocks always. Clock output is put on when 
> driver is attached and it is never disabled after that, leaving power 
> management partly broken.
> 
> Lets take a example, dual tuner case:
> tuner#0 gets clock from Xtal
> tuner#1 gets clock from #tuner0
> 
> All possible use cases are:
> 1) #tuner0 off & #tuner1 off
> 2) #tuner0 on & #tuner1 off
> 3) #tuner1 off & #tuner1 on
> 4) #tuner1 on & #tuner1 on
> 
> you will need, as per aforementioned use case:
> 1) #tuner0 clock out off & #tuner1 clock out off
> 2) #tuner0 clock out off & #tuner1 clock out off
> 3) #tuner0 clock out on & #tuner1 clock out off
> 4) #tuner0 clock out on & #tuner1 clock out off
> 
> Implementing that currently is simply impossible. But if you use clock 
> framework (or what ever its name is) I think it is possible to implement 
> that properly. When tuner#1 driver needs a clock, it calls "get clock" 
> and that call is routed to #tuner0 which enables clock.
> 
> And that was not even the most complicated case, as many times clock is 
> routed to demod and USB bridge too.
> 
> Quite same situation is for power on/off gpios (which should likely 
> implemented as a regulator). Also there is many times reset gpio (for PM 
> chip is powered off by switching power totally off *or* chip is put to 
> reset using GPIO)

Ok, in the above scenario, I agree that using the clock framework
makes sense, but, on devices where the clock is independent
(e. g. each chip has its on XTAL), I'm yet to see a scenario where
using the clock framework will simplify the code or bring some extra
benefit.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Sept. 6, 2014, 3:20 a.m. UTC | #9
On 09/06/2014 06:11 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 06 Sep 2014 06:00:28 +0300
> Antti Palosaari <crope@iki.fi> escreveu:
>
>> On 09/06/2014 05:27 AM, Mauro Carvalho Chehab wrote:
>>> Em Sat, 06 Sep 2014 05:02:15 +0300
>>> Antti Palosaari <crope@iki.fi> escreveu:
>>
>>>> Lets mention that I am not even now fully happy to solution, even it
>>>> somehow now works. Proper solution is implement clock source and clock
>>>> client. Then register client to that source. And when client needs a
>>>> clock (or power) it makes call to enable clock.
>>>
>>> Well, we need to discuss more about that, because you need to convince
>>> me first about that ;)
>>>
>>> We had already some discussions about that related to V4L2 I2C devices.
>>> The consensus we've reached is that it makes sense to use the clock
>>> framework only for the cases where the bridge driver doesn't know anything
>>> about the clock to be used by a given device, e. g. in the case where this
>>> data comes from the Device Tree (embedded systems).
>>>
>>> In the case where the bridge is the ownership of the information that will
>>> be used by a given device model (clock, serial/parallel mode, etc), then
>>> a series of data information should be passed by a call from the bridge driver
>>> to the device at setup time, and doing it in an atomic way is the best
>>> way to go.
>>
>> For AF9033/IT9133 demod + tuner I resolved it like that:
>> https://patchwork.linuxtv.org/patch/25772/
>> https://patchwork.linuxtv.org/patch/25774/
>>
>> It is demod which provides clock for tuner. It is very common situation
>> nowadays, one or more clocks are shared. And clock sharing is routed via
>> chips so that there is clock gates you have enable/disable for power
>> management reasons.
>>
>> Currently we just enable clocks always. Clock output is put on when
>> driver is attached and it is never disabled after that, leaving power
>> management partly broken.
>>
>> Lets take a example, dual tuner case:
>> tuner#0 gets clock from Xtal
>> tuner#1 gets clock from #tuner0
>>
>> All possible use cases are:
>> 1) #tuner0 off & #tuner1 off
>> 2) #tuner0 on & #tuner1 off
>> 3) #tuner1 off & #tuner1 on
>> 4) #tuner1 on & #tuner1 on
>>
>> you will need, as per aforementioned use case:
>> 1) #tuner0 clock out off & #tuner1 clock out off
>> 2) #tuner0 clock out off & #tuner1 clock out off
>> 3) #tuner0 clock out on & #tuner1 clock out off
>> 4) #tuner0 clock out on & #tuner1 clock out off
>>
>> Implementing that currently is simply impossible. But if you use clock
>> framework (or what ever its name is) I think it is possible to implement
>> that properly. When tuner#1 driver needs a clock, it calls "get clock"
>> and that call is routed to #tuner0 which enables clock.
>>
>> And that was not even the most complicated case, as many times clock is
>> routed to demod and USB bridge too.
>>
>> Quite same situation is for power on/off gpios (which should likely
>> implemented as a regulator). Also there is many times reset gpio (for PM
>> chip is powered off by switching power totally off *or* chip is put to
>> reset using GPIO)
>
> Ok, in the above scenario, I agree that using the clock framework
> makes sense, but, on devices where the clock is independent
> (e. g. each chip has its on XTAL), I'm yet to see a scenario where
> using the clock framework will simplify the code or bring some extra
> benefit.

And I resolved it like that for IT9135:
https://patchwork.linuxtv.org/patch/25763/

1) defined tuner role config parameter (master feeds a clock, slave does 
not)
2) master is then never put 100% deep sleep

Comment on code explains that:
/*
  * Writing '0x00' to master tuner register '0x80ec08' causes slave tuner
  * communication lost. Due to that, we cannot put master full sleep.
  */

but it will be much more elegant solution to use clock framework which 
allows implementing correct power management.

regards
Antti
  
Akihiro TSUKADA Sept. 6, 2014, 6:09 a.m. UTC | #10
Moi!

> Yes, using the I2C binding way provides a better decoupling than using the
> legacy way. The current dvb_attach() macros are hacks that were created
> by the time where the I2C standard bind didn't work with DVB.

I understand. I converted my code to use i2c binding model,
but I'm uncertain about a few things.

1. How to load the modules of i2c driver?
Currently I use request_module()/module_put()
like an example (ddbrige-core.c) from Antti does,
but I'd prefer implicit module loading/ref-counting
like in dvb_attach() if it exists.


2. Is there a standard way to pass around dvb_frontend*, i2c_client*,
regmap* between bridge(dvb_adapter) and demod/tuner drivers?
Currently I use config structure for the purpose, which is set to
dev.platform_data (via i2c_board_info/i2c_new_device()) or
dev.driver_data (via i2c_{get,set}_clientdata()),
but using config as both IN/OUT looks a bit hacky.

3. Should I also use RegMap API for register access?
I tried using it but gave up,
because it does not fit well to one of my use-case,
where (only) reads must be done via 0xfb register, like
   READ(reg, buf, len) -> [addr/w, 0xfb, reg], [addr/r, buf[0]...],
   WRITE(reg, buf, len) -> [addr/w, reg, buf[0]...],
and regmap looked to me overkill for 8bit-reg, 8bit-val cases
and did not simplify the code.
so I'd like to go without RegMap if possible,
since I'm already puzzled enough by I2C binding, regmap, clock source,
as well as dvb-core, PCI ;)

regards,
Akihiro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Sept. 6, 2014, 6:51 a.m. UTC | #11
moikka!

On 09/06/2014 09:09 AM, Akihiro TSUKADA wrote:
> Moi!
>
>> Yes, using the I2C binding way provides a better decoupling than using the
>> legacy way. The current dvb_attach() macros are hacks that were created
>> by the time where the I2C standard bind didn't work with DVB.
>
> I understand. I converted my code to use i2c binding model,
> but I'm uncertain about a few things.
>
> 1. How to load the modules of i2c driver?
> Currently I use request_module()/module_put()
> like an example (ddbrige-core.c) from Antti does,
> but I'd prefer implicit module loading/ref-counting
> like in dvb_attach() if it exists.

Maybe I haven't found the best way yet, but that was implementation I 
made for AF9035 driver:
https://patchwork.linuxtv.org/patch/25764/

Basically it is 2 functions, af9035_add_i2c_dev() and af9035_del_i2c_dev()

> 2. Is there a standard way to pass around dvb_frontend*, i2c_client*,
> regmap* between bridge(dvb_adapter) and demod/tuner drivers?
> Currently I use config structure for the purpose, which is set to
> dev.platform_data (via i2c_board_info/i2c_new_device()) or
> dev.driver_data (via i2c_{get,set}_clientdata()),
> but using config as both IN/OUT looks a bit hacky.

In my understanding, platform_data is place to pass environment specific 
config to driver. get/set client_data() is aimed to carry pointer for 
device specific instance "state" inside a driver. Is it possible to set 
I2C client data before calling i2c_new_device() and pass pointer to driver?

There is also IOCTL style command for I2C, but it is legacy and should 
not be used.

Documentation/i2c/busses/i2c-ocores

Yet, using config to OUT seems to be bit hacky for my eyes too. I though 
replacing all OUT with ops when converted af9033 driver. Currently 
caller fills struct af9033_ops and then af9033 I2C probe populates ops. 
See that patch:
https://patchwork.linuxtv.org/patch/25746/

Does this kind of ops sounds any better?

EXPORT_SYMBOL() is easiest way to offer outputs, like 
EXPORT_SYMBOL(get_frontend), EXPORT_SYMBOL(get_i2c_adapter). But we want 
avoid those exported symbols.

> 3. Should I also use RegMap API for register access?
> I tried using it but gave up,
> because it does not fit well to one of my use-case,
> where (only) reads must be done via 0xfb register, like
>     READ(reg, buf, len) -> [addr/w, 0xfb, reg], [addr/r, buf[0]...],
>     WRITE(reg, buf, len) -> [addr/w, reg, buf[0]...],
> and regmap looked to me overkill for 8bit-reg, 8bit-val cases
> and did not simplify the code.
> so I'd like to go without RegMap if possible,
> since I'm already puzzled enough by I2C binding, regmap, clock source,
> as well as dvb-core, PCI ;)

I prefer RegMap API, but I am only one who has started using it yet. And 
I haven't converted any demod driver having I2C bus/gate/repeater for 
tuner to that API. It is because of I2C locking with I2C mux adapter, 
you need use unlocked version of i2c_transfer() for I2C mux as I2C bus 
lock is already taken. RegMap API does not support that, but I think it 
should be possible if you implement own xfer callback for regmap. For RF 
tuners RegMap API should be trivial and it will reduce ~100 LOC from driver.

But if you decide avoid RegMap API, I ask you add least implementing 
those I2C read / write function parameters similarly than RegMap API 
does. Defining all kind of weird register write / read functions makes 
life harder. I converted recently IT913x driver to RegMap API and 
biggest pain was there very different register read / write routines. So 
I need to do a lot of work in order convert functions first some common 
style and then it was trivial to change RegMap API.

https://patchwork.linuxtv.org/patch/25766/
https://patchwork.linuxtv.org/patch/25757/

I quickly overlooked that demod driver and one which looked crazy was 
LNA stuff. You implement set_lna callback in demod, but it is then 
passed back to PCI driver using frontend callback. Is there some reason 
you hooked it via demod? You could implement set_lna in PCI driver too.

regards
Antti
  
Antti Palosaari Sept. 6, 2014, 7:13 a.m. UTC | #12
On 09/06/2014 09:09 AM, Akihiro TSUKADA wrote:
> 3. Should I also use RegMap API for register access?
> I tried using it but gave up,
> because it does not fit well to one of my use-case,
> where (only) reads must be done via 0xfb register, like
>     READ(reg, buf, len) -> [addr/w, 0xfb, reg], [addr/r, buf[0]...],
>     WRITE(reg, buf, len) -> [addr/w, reg, buf[0]...],
> and regmap looked to me overkill for 8bit-reg, 8bit-val cases
> and did not simplify the code.
> so I'd like to go without RegMap if possible,
> since I'm already puzzled enough by I2C binding, regmap, clock source,
> as well as dvb-core, PCI ;)

That is MaxLinear MxL301RF tuner I2C. Problem is there that it uses 
write + STOP + write, so you should not even do that as a one I2C 
i2c_transfer. All I2C messages send using i2c_transfer are send so 
called REPEATED START condition.

I ran that same problem ears ago in a case of, surprise, MxL5007 tuner.
https://patchwork.linuxtv.org/patch/17847/

I think you could just write wanted register to command register 0xfb. 
And after that all the reads are coming from that active register until 
you change it.

RegMap API cannot handle I2C command format like that, it relies 
repeated start for reads.

Si2157 / Si2168 are using I2C access with STOP condition - but it is 
otherwise bad example as there is firmware API, not register API. Look 
still as a example.

regards
Antti
  
Akihiro TSUKADA Sept. 6, 2014, 7:35 p.m. UTC | #13
moikka!,

> Basically it is 2 functions, af9035_add_i2c_dev() and af9035_del_i2c_dev()

I used request_module()/try_module_get()/module_put()
just like the above example (and bridge-core.c).
It works, but when I unload bridge driver(earth_pt3),
its demod and tuner modules stay loaded, with the refcount of 0.
Is it ok that the auto loaded modules remain with 0 ref count?

> Yet, using config to OUT seems to be bit hacky for my eyes too. I though
> replacing all OUT with ops when converted af9033 driver. Currently
> caller fills struct af9033_ops and then af9033 I2C probe populates ops.
> See that patch:
> https://patchwork.linuxtv.org/patch/25746/
> 
> Does this kind of ops sounds any better?

Do you mean using ops in struct config?
if so, I don't find much difference with the current situation
where demod/tuner probe() sets dvb_frontend* to config->fe. 

> I quickly overlooked that demod driver and one which looked crazy was
> LNA stuff. You implement set_lna callback in demod, but it is then
> passed back to PCI driver using frontend callback. Is there some reason
> you hooked it via demod? You could implement set_lna in PCI driver too.

Stupidly I forgot that FE's ops can be set from the PCI driver.
I will remove those callbacks and set the corresponding ops instead.

regards,
akihiro

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antti Palosaari Sept. 7, 2014, 1:05 a.m. UTC | #14
On 09/06/2014 10:35 PM, Akihiro TSUKADA wrote:
> moikka!,
>
>> Basically it is 2 functions, af9035_add_i2c_dev() and af9035_del_i2c_dev()
>
> I used request_module()/try_module_get()/module_put()
> just like the above example (and bridge-core.c).
> It works, but when I unload bridge driver(earth_pt3),
> its demod and tuner modules stay loaded, with the refcount of 0.
> Is it ok that the auto loaded modules remain with 0 ref count?

So there is no other problem than those modules were left loaded? If you 
could unload those using rmmod it is OK then. Ref counting is here to 
prevent unloading demod and tuner driver while those are used by some 
other module. So when bridge is loaded, you should not be able to unload 
demod or tuner. But when bridge is unloaded, you should be able to 
unload demod and tuner.

And your question, I think there is no way to unload modules 
automatically or at least no need.

>> Yet, using config to OUT seems to be bit hacky for my eyes too. I though
>> replacing all OUT with ops when converted af9033 driver. Currently
>> caller fills struct af9033_ops and then af9033 I2C probe populates ops.
>> See that patch:
>> https://patchwork.linuxtv.org/patch/25746/
>>
>> Does this kind of ops sounds any better?
>
> Do you mean using ops in struct config?
> if so, I don't find much difference with the current situation
> where demod/tuner probe() sets dvb_frontend* to config->fe.

Alloc driver specific ops in bridge driver, then put pointer to that ops 
to config struct. Driver fills ops during probe. Maybe that patch clears 
the idea:
af9033: Don't export functions for the hardware filter
https://patchwork.linuxtv.org/patch/23087/

Functionality is not much different than passing pointer to frontend 
pointer from bridge to I2C demod driver via platform_data...

Somehow you will need to transfer data during driver loading and there 
is not many alternatives:
* platform data pointer
* driver data pointer
* export function
* i2c_clients_command (legacy)
* device ID string (not very suitable)
* + the rest from i2c client, not related at all

regards
Antti
  

Patch

diff --git a/drivers/media/dvb-frontends/Kconfig b/drivers/media/dvb-frontends/Kconfig
index aa5ae22..123adb2 100644
--- a/drivers/media/dvb-frontends/Kconfig
+++ b/drivers/media/dvb-frontends/Kconfig
@@ -648,6 +648,14 @@  config DVB_MB86A20S
 	  A driver for Fujitsu mb86a20s ISDB-T/ISDB-Tsb demodulator.
 	  Say Y when you want to support this frontend.
 
+config DVB_TC90522
+	tristate "Toshiba TC90522"
+	depends on DVB_CORE && I2C
+	default m if !MEDIA_SUBDRV_AUTOSELECT
+	help
+	  A Toshiba TC90522 2xISDB-T + 2xISDB-S demodulator.
+	  Say Y when you want to support this frontend.
+
 comment "Digital terrestrial only tuners/PLL"
 	depends on DVB_CORE
 
diff --git a/drivers/media/dvb-frontends/Makefile b/drivers/media/dvb-frontends/Makefile
index fc4e689..00cc299 100644
--- a/drivers/media/dvb-frontends/Makefile
+++ b/drivers/media/dvb-frontends/Makefile
@@ -114,3 +114,4 @@  obj-$(CONFIG_DVB_RTL2832_SDR) += rtl2832_sdr.o
 obj-$(CONFIG_DVB_M88RS2000) += m88rs2000.o
 obj-$(CONFIG_DVB_AF9033) += af9033.o
 obj-$(CONFIG_DVB_AS102_FE) += as102_fe.o
+obj-$(CONFIG_DVB_TC90522) += tc90522.o
diff --git a/drivers/media/dvb-frontends/tc90522.c b/drivers/media/dvb-frontends/tc90522.c
new file mode 100644
index 0000000..1f0f6f7
--- /dev/null
+++ b/drivers/media/dvb-frontends/tc90522.c
@@ -0,0 +1,857 @@ 
+/*
+ * Toshiba TC90522 Demodulator
+ *
+ * Copyright (C) 2014 Akihiro Tsukada <tskd08@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/dvb/frontend.h>
+#include "dvb_math.h"
+#include "tc90522.h"
+
+#define TC90522_I2C_THRU_REG 0xfe
+
+#define TC90522_MODULE_IDX(addr) (((u8)(addr) & 0x02U) >> 1)
+
+enum tc90522_tuning_status {
+	STATUS_IDLE,
+	STATUS_SET_FREQ,
+	STATUS_CHECK_TUNER,
+	STATUS_CHECK_DEMOD,
+	STATUS_TRACK,
+};
+
+struct tc90522_state {
+	struct dvb_frontend dvb_fe;
+
+	struct tc90522_config cfg;
+	struct i2c_adapter *i2c;
+	struct i2c_adapter tuner_i2c;
+	enum tc90522_tuning_status tuning_status;
+	int retry_count;
+
+	bool lna;
+};
+
+struct reg_val {
+	u8 reg;
+	u8 val;
+};
+
+
+static int
+reg_write(struct tc90522_state *state, const struct reg_val *regs, int num)
+{
+	int i, ret;
+	struct i2c_msg msg;
+
+	ret = 0;
+	msg.addr = state->cfg.addr;
+	msg.flags = 0;
+	msg.len = 2;
+	for (i = 0; i < num; i++) {
+		msg.buf = (u8 *)&regs[i];
+		ret = i2c_transfer(state->i2c, &msg, 1);
+		if (ret < 0)
+			break;
+	}
+	return ret;
+}
+
+static int reg_read(struct tc90522_state *state, u8 reg, u8 *val, u8 len)
+{
+	struct i2c_msg msgs[2] = {
+		{
+			.addr = state->cfg.addr,
+			.flags = 0,
+			.buf = &reg,
+			.len = 1,
+		},
+		{
+			.addr = state->cfg.addr,
+			.flags = I2C_M_RD,
+			.buf = val,
+			.len = len,
+		},
+	};
+
+	return i2c_transfer(state->i2c, msgs, ARRAY_SIZE(msgs));
+}
+
+static int enable_lna(struct dvb_frontend *fe, bool on)
+{
+	struct tc90522_state *state;
+
+	state = fe->demodulator_priv;
+	/* delegate to the parent board */
+	if (fe->callback)
+		fe->callback(fe, DVB_FRONTEND_COMPONENT_DEMOD,
+				TC90522T_CMD_SET_LNA, on);
+	state->lna = on;
+	return 0;
+}
+
+static int tc90522s_set_tsid(struct dvb_frontend *fe)
+{
+	struct reg_val set_tsid[] = {
+		{ 0x8f, 00 },
+		{ 0x90, 00 }
+	};
+
+	set_tsid[0].val = (fe->dtv_property_cache.stream_id & 0xff00) >> 8;
+	set_tsid[1].val = fe->dtv_property_cache.stream_id & 0xff;
+	return reg_write(fe->demodulator_priv, set_tsid, ARRAY_SIZE(set_tsid));
+}
+
+static int tc90522t_set_layers(struct dvb_frontend *fe)
+{
+	struct reg_val rv;
+	u8 laysel;
+
+	laysel = ~fe->dtv_property_cache.isdbt_layer_enabled & 0x07;
+	laysel = (laysel & 0x01) << 2 | (laysel & 0x02) | (laysel & 0x04) >> 2;
+	rv.reg = 0x71;
+	rv.val = laysel;
+	return reg_write(fe->demodulator_priv, &rv, 1);
+}
+
+/* frontend ops */
+
+static int tc90522s_read_status(struct dvb_frontend *fe, fe_status_t *status)
+{
+	int ret;
+	u8 reg;
+
+	*status = 0;
+	ret = reg_read(fe->demodulator_priv, 0xc3, &reg, 1);
+	if (ret < 0)
+		return ret;
+
+	if (reg & 0x80) /* input level under min ? */
+		return 0;
+	*status |= FE_HAS_SIGNAL;
+
+	if (reg & 0x60) /* carrier? */
+		return 0;
+	*status |= FE_HAS_CARRIER | FE_HAS_VITERBI | FE_HAS_SYNC;
+
+	if (reg & 0x10)
+		return 0;
+	if (reg_read(fe->demodulator_priv, 0xc5, &reg, 1) < 0 || !(reg & 0x03))
+		return 0;
+	*status |= FE_HAS_LOCK;
+	return 0;
+}
+
+static int tc90522t_read_status(struct dvb_frontend *fe, fe_status_t *status)
+{
+	int ret;
+	u8 reg;
+
+	*status = 0;
+	ret = reg_read(fe->demodulator_priv, 0x96, &reg, 1);
+	if (ret < 0)
+		return ret;
+
+	if (reg & 0xe0) {
+		*status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI
+				| FE_HAS_SYNC | FE_HAS_LOCK;
+		return 0;
+	}
+
+	ret = reg_read(fe->demodulator_priv, 0x80, &reg, 1);
+	if (ret < 0)
+		return ret;
+
+	if (reg & 0xf0)
+		return 0;
+	*status |= FE_HAS_SIGNAL | FE_HAS_CARRIER;
+
+	if (reg & 0x0c)
+		return 0;
+	*status |= FE_HAS_SYNC | FE_HAS_VITERBI;
+
+	if (reg & 0x02)
+		return 0;
+	*status |= FE_HAS_LOCK;
+	return 0;
+}
+
+static const fe_code_rate_t fec_conv_sat[] = {
+	FEC_NONE, /* unused */
+	FEC_1_2, /* for BPSK */
+	FEC_1_2, FEC_2_3, FEC_3_4, FEC_5_6, FEC_7_8, /* for QPSK */
+	FEC_2_3, /* for 8PSK. (trellis code) */
+};
+
+static int tc90522s_get_frontend(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	struct dtv_frontend_properties *c;
+	struct dtv_fe_stats *stats;
+	int ret, i;
+	int layers;
+	u8 val[10], v;
+	u32 cndat;
+
+	state = fe->demodulator_priv;
+	c = &fe->dtv_property_cache;
+	c->delivery_system = SYS_ISDBS;
+
+	layers = 0;
+	ret = reg_read(state, 0xe8, val, 3);
+	if (ret == 0) {
+		int slots;
+
+		/* high/single layer */
+		v = (val[0] & 0x70) >> 4;
+		c->modulation = (v == 7) ? PSK_8 : QPSK;
+		c->fec_inner = fec_conv_sat[v];
+		c->layer[0].fec = c->fec_inner;
+		c->layer[0].modulation = c->modulation;
+		c->layer[0].segment_count = val[1] & 0x3f; /* slots */
+
+		/* low layer */
+		v = (val[0] & 0x07);
+		c->layer[1].fec = fec_conv_sat[v];
+		if (v == 0)  /* no low layer */
+			c->layer[1].segment_count = 0;
+		else
+			c->layer[1].segment_count = val[2] & 0x3f; /* slots */
+		/* actually, BPSK if v==1, but not defined in fe_modulation_t */
+		c->layer[1].modulation = QPSK;
+		layers = (v > 0) ? 2 : 1;
+
+		slots =  c->layer[0].segment_count +  c->layer[1].segment_count;
+		c->symbol_rate = 28860000 * slots / 48;
+	}
+
+	/* statistics */
+
+	stats = &c->strength;
+	stats->len = 0;
+	if (fe->ops.tuner_ops.get_rf_strength_dbm) {
+		s64 str;
+
+		stats->len = 1;
+		ret = fe->ops.tuner_ops.get_rf_strength_dbm(fe, &str);
+		if (ret == 0) {
+			stats->stat[0].scale = FE_SCALE_DECIBEL;
+			stats->stat[0].svalue = str;
+		} else
+			stats->stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	}
+
+	stats = &c->cnr;
+	stats->len = 1;
+	stats->stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	cndat = 0;
+	ret = reg_read(state, 0xbc, val, 2);
+	if (ret == 0)
+		cndat = val[0] << 8 | val[1];
+	if (cndat >= 3000) {
+		u32 p, p4;
+		s64 cn;
+
+		cndat -= 3000;  /* cndat: 4.12 fixed point float */
+		/*
+		 * cnr[mdB] = -1634.6 * P^5 + 14341 * P^4 - 50259 * P^3
+		 *                 + 88977 * P^2 - 89565 * P + 58857
+		 *  (P = sqrt(cndat) / 64)
+		 */
+		/* p := sqrt(cndat) << 8 = P << 14, 2.14 fixed  point float */
+		/* cn = cnr << 3 */
+		p = int_sqrt(cndat << 16);
+		p4 = cndat * cndat;
+		cn = (-16346LL * p4 * p / 10) >> 35;
+		cn += (14341LL * p4) >> 21;
+		cn -= (50259LL * cndat * p) >> 23;
+		cn += (88977LL * cndat) >> 9;
+		cn -= (89565LL * p) >> 11;
+		cn += 58857  << 3;
+		stats->stat[0].svalue = cn >> 3;
+		stats->stat[0].scale = FE_SCALE_DECIBEL;
+	}
+
+	/* per-layer post viterbi BER (or PER? config dependent?) */
+	stats = &c->post_bit_error;
+	memset(stats, 0, sizeof(*stats));
+	stats->len = layers;
+	ret = reg_read(state, 0xeb, val, 10);
+	if (ret < 0)
+		for (i = 0; i < layers; i++)
+			stats->stat[i].scale = FE_SCALE_NOT_AVAILABLE;
+	else {
+		for (i = 0; i < layers; i++) {
+			stats->stat[i].scale = FE_SCALE_COUNTER;
+			stats->stat[i].uvalue = val[i * 5] << 16
+				| val[i * 5 + 1] << 8 | val[i * 5 + 2];
+		}
+	}
+	stats = &c->post_bit_count;
+	memset(stats, 0, sizeof(*stats));
+	stats->len = layers;
+	if (ret < 0)
+		for (i = 0; i < layers; i++)
+			stats->stat[i].scale = FE_SCALE_NOT_AVAILABLE;
+	else {
+		for (i = 0; i < layers; i++) {
+			stats->stat[i].scale = FE_SCALE_COUNTER;
+			stats->stat[i].uvalue =
+				val[i * 5 + 3] << 8 | val[i * 5 + 4];
+			stats->stat[i].uvalue *= 204 * 8;
+		}
+	}
+
+	return 0;
+}
+
+
+static const fe_transmit_mode_t tm_conv[] = {
+	TRANSMISSION_MODE_2K,
+	TRANSMISSION_MODE_4K,
+	TRANSMISSION_MODE_8K,
+	0
+};
+
+static const fe_code_rate_t fec_conv_ter[] = {
+	FEC_1_2, FEC_2_3, FEC_3_4, FEC_5_6, FEC_7_8, 0, 0, 0
+};
+
+static const fe_modulation_t mod_conv[] = {
+	DQPSK, QPSK, QAM_16, QAM_64, 0, 0, 0, 0
+};
+
+static int tc90522t_get_frontend(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	struct dtv_frontend_properties *c;
+	struct dtv_fe_stats *stats;
+	int ret, i;
+	int layers;
+	u8 val[15], mode;
+	u32 cndat;
+
+	state = fe->demodulator_priv;
+	c = &fe->dtv_property_cache;
+	c->delivery_system = SYS_ISDBT;
+	c->bandwidth_hz = 6000000;
+	mode = 1;
+	ret = reg_read(state, 0xb0, val, 1);
+	if (ret == 0) {
+		mode = (val[0] & 0xc0) >> 2;
+		c->transmission_mode = tm_conv[mode];
+		c->guard_interval = (val[0] & 0x30) >> 4;
+	}
+
+	ret = reg_read(state, 0xb2, val, 6);
+	layers = 0;
+	if (ret == 0) {
+		u8 v;
+
+		c->isdbt_partial_reception = val[0] & 0x01;
+		c->isdbt_sb_mode = (val[0] & 0xc0) == 0x01;
+
+		/* layer A */
+		v = (val[2] & 0x78) >> 3;
+		if (v == 0x0f)
+			c->layer[0].segment_count = 0;
+		else {
+			layers++;
+			c->layer[0].segment_count = v;
+			c->layer[0].fec = fec_conv_ter[(val[1] & 0x1c) >> 2];
+			c->layer[0].modulation = mod_conv[(val[1] & 0xe0) >> 5];
+			v = (val[1] & 0x03) << 1 | (val[2] & 0x80) >> 7;
+			c->layer[0].interleaving = v;
+		}
+
+		/* layer B */
+		v = (val[3] & 0x03) << 1 | (val[4] & 0xc0) >> 6;
+		if (v == 0x0f)
+			c->layer[1].segment_count = 0;
+		else {
+			layers++;
+			c->layer[1].segment_count = v;
+			c->layer[1].fec = fec_conv_ter[(val[3] & 0xe0) >> 5];
+			c->layer[1].modulation = mod_conv[(val[2] & 0x07)];
+			c->layer[1].interleaving = (val[3] & 0x1c) >> 2;
+		}
+
+		/* layer C */
+		v = (val[5] & 0x1e) >> 1;
+		if (v == 0x0f)
+			c->layer[2].segment_count = 0;
+		else {
+			layers++;
+			c->layer[2].segment_count = v;
+			c->layer[2].fec = fec_conv_ter[(val[4] & 0x07)];
+			c->layer[2].modulation = mod_conv[(val[4] & 0x38) >> 3];
+			c->layer[2].interleaving = (val[5] & 0xe0) >> 5;
+		}
+	}
+
+	/* statistics */
+
+	stats = &c->strength;
+	stats->len = 0;
+	if (fe->ops.tuner_ops.get_rf_strength_dbm) {
+		s64 str;
+
+		stats->len = 1;
+		ret = fe->ops.tuner_ops.get_rf_strength_dbm(fe, &str);
+		if (ret == 0) {
+			stats->stat[0].scale = FE_SCALE_DECIBEL;
+			stats->stat[0].svalue = str;
+		} else
+			stats->stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	}
+
+	stats = &c->cnr;
+	stats->len = 1;
+	stats->stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+	cndat = 0;
+	ret = reg_read(state, 0x8b, val, 3);
+	if (ret == 0)
+		cndat = val[0] << 16 | val[1] << 8 | val[2];
+	if (cndat != 0) {
+		u32 p, tmp;
+		s64 cn;
+
+		/*
+		 * cnr[mdB] = 0.024 P^4 - 1.6 P^3 + 39.8 P^2 + 549.1 P + 3096.5
+		 * (P = 10log10(5505024/cndat))
+		 */
+		/* cn = cnr << 3 (61.3 fixed point float */
+		/* p = 10log10(5505024/cndat) << 24  (8.24 fixed point float)*/
+		p = intlog10(5505024) - intlog10(cndat);
+		p *= 10;
+
+		cn = 24772;
+		cn += ((43827LL * p) / 10) >> 24;
+		tmp = p >> 8;
+		cn += ((3184LL * tmp * tmp) / 10) >> 32;
+		tmp = p >> 13;
+		cn -= ((128LL * tmp * tmp * tmp) / 10) >> 33;
+		tmp = p >> 18;
+		cn += ((192LL * tmp * tmp * tmp * tmp) / 1000) >> 24;
+
+		stats->stat[0].svalue = cn >> 3;
+		stats->stat[0].scale = FE_SCALE_DECIBEL;
+	}
+
+	/* per-layer post viterbi BER (or PER? config dependent?) */
+	stats = &c->post_bit_error;
+	memset(stats, 0, sizeof(*stats));
+	stats->len = layers;
+	ret = reg_read(state, 0x9d, val, 15);
+	if (ret < 0)
+		for (i = 0; i < layers; i++)
+			stats->stat[i].scale = FE_SCALE_NOT_AVAILABLE;
+	else {
+		for (i = 0; i < layers; i++) {
+			stats->stat[i].scale = FE_SCALE_COUNTER;
+			stats->stat[i].uvalue = val[i * 3] << 16
+				| val[i * 3 + 1] << 8 | val[i * 3 + 2];
+		}
+	}
+	stats = &c->post_bit_count;
+	memset(stats, 0, sizeof(*stats));
+	stats->len = layers;
+	if (ret < 0)
+		for (i = 0; i < layers; i++)
+			stats->stat[i].scale = FE_SCALE_NOT_AVAILABLE;
+	else {
+		for (i = 0; i < layers; i++) {
+			stats->stat[i].scale = FE_SCALE_COUNTER;
+			stats->stat[i].uvalue =
+				val[9 + i * 2] << 8 | val[9 + i * 2 + 1];
+			stats->stat[i].uvalue *= 204 * 8;
+		}
+	}
+
+	return 0;
+}
+
+static int tc90522s_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t volt)
+{
+	int ret;
+
+	ret = 0;
+	if (fe->callback)
+		ret = fe->callback(fe, DVB_FRONTEND_COMPONENT_DEMOD,
+				TC90522S_CMD_SET_LNB, volt);
+	return ret;
+}
+
+static int tc90522t_set_lna(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	int ret;
+
+	ret = 0;
+	state = fe->demodulator_priv;
+	if (fe->dtv_property_cache.lna != LNA_AUTO &&
+	    fe->dtv_property_cache.lna != state->lna)
+		ret = enable_lna(fe, fe->dtv_property_cache.lna);
+	return ret;
+}
+
+static const struct reg_val reset_sat = { 0x03, 0x01 };
+static const struct reg_val reset_ter = { 0x01, 0x40 };
+
+static int tc90522_set_frontend(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	int ret, i;
+	u32 status;
+
+	state = fe->demodulator_priv;
+
+	if (fe->ops.tuner_ops.set_params)
+		ret = fe->ops.tuner_ops.set_params(fe);
+	else
+		ret = -ENODEV;
+	if (ret < 0)
+		goto failed;
+
+	if (fe->ops.tuner_ops.get_status) {
+		for (i = 50; i > 0; i--) {
+			ret = fe->ops.tuner_ops.get_status(fe, &status);
+			if (ret < 0)
+				goto failed;
+			if (status & TUNER_STATUS_LOCKED)
+				break;
+			usleep_range(2000, 3000);
+		}
+		if (i <= 0) {
+			ret = -ETIMEDOUT;
+			goto failed;
+		}
+	}
+
+	if (fe->ops.delsys[0] == SYS_ISDBS) {
+		ret = tc90522s_set_tsid(fe);
+		if (ret < 0)
+			goto failed;
+		ret = reg_write(state, &reset_sat, 1);
+	} else {
+		ret = tc90522t_set_layers(fe);
+		if (ret < 0)
+			goto failed;
+		ret = reg_write(state, &reset_ter, 1);
+	}
+	if (ret < 0)
+		goto failed;
+
+	return 0;
+
+failed:
+	dev_warn(&state->tuner_i2c.dev, "(%s) failed. [adap%d-fe%d]\n",
+			__func__, fe->dvb->num, fe->id);
+	return ret;
+}
+
+static int tc90522_get_tune_settings(struct dvb_frontend *fe,
+	struct dvb_frontend_tune_settings *settings)
+{
+	if (fe->ops.delsys[0] == SYS_ISDBS) {
+		settings->min_delay_ms = 250;
+		settings->step_size = 1000;
+		settings->max_drift = settings->step_size * 2;
+	} else {
+		settings->min_delay_ms = 400;
+		settings->step_size = 142857;
+		settings->max_drift = settings->step_size;
+	}
+	return 0;
+}
+
+static int tc90522_set_if_agc(struct dvb_frontend *fe, bool on)
+{
+	struct reg_val sat_agc[] = {
+		{ 0x0a, 0x00 },
+		{ 0x10, 0x30 },
+		{ 0x11, 0x00 },
+		{ 0x03, 0x01 },
+	};
+	struct reg_val ter_agc[] = {
+		{ 0x25, 0x00 },
+		{ 0x23, 0x4c },
+		{ 0x01, 0x40 },
+	};
+	struct tc90522_state *state;
+	struct reg_val *rv;
+	int num;
+
+	state = fe->demodulator_priv;
+	if (fe->ops.delsys[0] == SYS_ISDBS) {
+		sat_agc[0].val = on ? 0xff : 0x00;
+		sat_agc[1].val |= 0x80;
+		sat_agc[1].val |= on ? 0x01 : 0x00;
+		sat_agc[2].val |= on ? 0x40 : 0x00;
+		rv = sat_agc;
+		num = ARRAY_SIZE(sat_agc);
+	} else {
+		ter_agc[0].val = on ? 0x40 : 0x00;
+		ter_agc[1].val |= on ? 0x00 : 0x01;
+		rv = ter_agc;
+		num = ARRAY_SIZE(ter_agc);
+	}
+	return reg_write(state, rv, num);
+}
+
+static const struct reg_val sleep_sat = { 0x17, 0x01 };
+static const struct reg_val sleep_ter = { 0x03, 0x90 };
+
+static int tc90522_sleep(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	int ret;
+
+	state = fe->demodulator_priv;
+	if (fe->ops.delsys[0] == SYS_ISDBS)
+		ret = reg_write(state, &sleep_sat, 1);
+	else {
+		ret = reg_write(state, &sleep_ter, 1);
+		if (ret == 0 && fe->dtv_property_cache.lna == LNA_AUTO)
+			ret = enable_lna(fe, false);
+	}
+	if (ret < 0)
+		dev_warn(&state->tuner_i2c.dev,
+			"(%s) failed. [adap%d-fe%d]\n",
+			__func__, fe->dvb->num, fe->id);
+	return ret;
+}
+
+static void tc90522_release(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+
+	state = fe->demodulator_priv;
+	i2c_del_adapter(&state->tuner_i2c);
+	fe->demodulator_priv = NULL;
+	kfree(state);
+}
+
+
+static const struct reg_val wakeup_sat = { 0x17, 0x00 };
+static const struct reg_val wakeup_ter = { 0x03, 0x80 };
+
+static int tc90522_init(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+	int ret;
+
+	/*
+	 * Because the init sequence is not public,
+	 * the parent device/driver should have init'ed the device before.
+	 * just wake up the device here.
+	 */
+
+	state = fe->demodulator_priv;
+	if (fe->ops.delsys[0] == SYS_ISDBS)
+		ret = reg_write(state, &wakeup_sat, 1);
+	else {
+		ret = reg_write(state, &wakeup_ter, 1);
+		if (ret == 0 && fe->dtv_property_cache.lna == LNA_AUTO)
+			ret = enable_lna(fe, true);
+	}
+	if (ret < 0) {
+		dev_warn(&state->tuner_i2c.dev,
+			"(%s) failed. [adap%d-fe%d]\n",
+			__func__, fe->dvb->num, fe->id);
+		return ret;
+	}
+
+	/* prefer 'all-layers' to 'none' as a default */
+	if (fe->dtv_property_cache.isdbt_layer_enabled == 0)
+		fe->dtv_property_cache.isdbt_layer_enabled = 7;
+	return tc90522_set_if_agc(fe, true);
+}
+
+
+/*
+ * tuner I2C
+ */
+
+static int
+tc90522_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct tc90522_state *state;
+	struct i2c_msg *new_msgs;
+	int i, j;
+	int ret, rd_num;
+	u8 wbuf[256];
+	u8 *p, *bufend;
+
+	rd_num = 0;
+	for (i = 0; i < num; i++)
+		if (msgs[i].flags & I2C_M_RD)
+			rd_num++;
+	new_msgs = kmalloc(sizeof(*new_msgs) * (num + rd_num), GFP_KERNEL);
+	if (!new_msgs)
+		return -ENOMEM;
+
+	state = i2c_get_adapdata(adap);
+	p = wbuf;
+	bufend = wbuf + sizeof(wbuf);
+	for (i = 0, j = 0; i < num; i++, j++) {
+		new_msgs[j].addr = state->cfg.addr;
+		new_msgs[j].flags = msgs[i].flags;
+
+		if (msgs[i].flags & I2C_M_RD) {
+			new_msgs[j].flags &= ~I2C_M_RD;
+			if (p + 2 > bufend)
+				break;
+			p[0] = TC90522_I2C_THRU_REG;
+			p[1] = msgs[i].addr << 1 | 0x01;
+			new_msgs[j].buf = p;
+			new_msgs[j].len = 2;
+			p += 2;
+			j++;
+			new_msgs[j].addr = state->cfg.addr;
+			new_msgs[j].flags = msgs[i].flags;
+			new_msgs[j].buf = msgs[i].buf;
+			new_msgs[j].len = msgs[i].len;
+			continue;
+		}
+
+		if (p + msgs[i].len + 2 > bufend)
+			break;
+		p[0] = TC90522_I2C_THRU_REG;
+		p[1] = msgs[i].addr << 1;
+		memcpy(p + 2, msgs[i].buf, msgs[i].len);
+		new_msgs[j].buf = p;
+		new_msgs[j].len = msgs[i].len + 2;
+		p += new_msgs[j].len;
+	}
+	if (i < num)
+		ret = -ENOMEM;
+	else
+		ret = i2c_transfer(state->i2c, new_msgs, j);
+	kfree(new_msgs);
+	return ret;
+}
+
+u32 tc90522_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C;
+}
+
+static const struct i2c_algorithm tc90522_tuner_i2c_algo = {
+	.master_xfer   = &tc90522_master_xfer,
+	.functionality = &tc90522_functionality,
+};
+
+
+/*
+ * exported functions
+ */
+
+static const struct dvb_frontend_ops tc90522_ops_sat = {
+	.delsys = { SYS_ISDBS },
+	.info = {
+		.name = "Toshiba TC90522 ISDB-S module",
+		.frequency_min =  950000,
+		.frequency_max = 2150000,
+		.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,
+	},
+
+	.release = tc90522_release,
+	.init = tc90522_init,
+	.sleep = tc90522_sleep,
+	.set_frontend = tc90522_set_frontend,
+	.get_tune_settings = tc90522_get_tune_settings,
+
+	.get_frontend = tc90522s_get_frontend,
+	.read_status = tc90522s_read_status,
+	.set_voltage = tc90522s_set_voltage,
+};
+
+static const struct dvb_frontend_ops tc90522_ops_ter = {
+	.delsys = { SYS_ISDBT },
+	.info = {
+		.name = "Toshiba TC90522 ISDB-T module",
+		.frequency_min = 470000000,
+		.frequency_max = 770000000,
+		.frequency_stepsize = 142857,
+		.caps = FE_CAN_INVERSION_AUTO |
+			FE_CAN_FEC_1_2  | FE_CAN_FEC_2_3 | FE_CAN_FEC_3_4 |
+			FE_CAN_FEC_5_6  | FE_CAN_FEC_7_8 | FE_CAN_FEC_AUTO |
+			FE_CAN_QPSK     | FE_CAN_QAM_16 | FE_CAN_QAM_64 |
+			FE_CAN_QAM_AUTO | FE_CAN_TRANSMISSION_MODE_AUTO |
+			FE_CAN_GUARD_INTERVAL_AUTO | FE_CAN_RECOVER |
+			FE_CAN_HIERARCHY_AUTO,
+	},
+
+	.release = tc90522_release,
+	.init = tc90522_init,
+	.sleep = tc90522_sleep,
+	.set_frontend = tc90522_set_frontend,
+	.get_tune_settings = tc90522_get_tune_settings,
+
+	.get_frontend = tc90522t_get_frontend,
+	.read_status = tc90522t_read_status,
+	.set_lna = tc90522t_set_lna,
+};
+
+struct dvb_frontend *
+tc90522_attach(const struct tc90522_config *cfg, struct i2c_adapter *i2c)
+{
+	struct tc90522_state *state;
+	const struct dvb_frontend_ops *ops;
+	struct i2c_adapter *adap;
+	int ret;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	memcpy(&state->cfg, cfg, sizeof(*cfg));
+	state->i2c = i2c;
+	ops = TC90522_IS_ISDBS(cfg->addr) ? &tc90522_ops_sat : &tc90522_ops_ter;
+	memcpy(&state->dvb_fe.ops, ops, sizeof(*ops));
+	state->dvb_fe.demodulator_priv = state;
+
+	adap = &state->tuner_i2c;
+	adap->owner = i2c->owner;
+	adap->algo = &tc90522_tuner_i2c_algo;
+	adap->dev.parent = i2c->dev.parent;
+	strlcpy(adap->name, "tc90522 tuner", sizeof(adap->name));
+	i2c_set_adapdata(adap, state);
+	ret = i2c_add_adapter(adap);
+	if (ret < 0) {
+		kfree(state);
+		return NULL;
+	}
+	dev_info(&i2c->dev, "Toshiba TC90522 attached.\n");
+	return &state->dvb_fe;
+}
+EXPORT_SYMBOL(tc90522_attach);
+
+
+struct i2c_adapter *tc90522_get_tuner_i2c(struct dvb_frontend *fe)
+{
+	struct tc90522_state *state;
+
+	state = fe->demodulator_priv;
+	return &state->tuner_i2c;
+}
+EXPORT_SYMBOL(tc90522_get_tuner_i2c);
+
+
+MODULE_DESCRIPTION("Toshiba TC90522 frontend");
+MODULE_AUTHOR("Akihiro TSUKADA");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/dvb-frontends/tc90522.h b/drivers/media/dvb-frontends/tc90522.h
new file mode 100644
index 0000000..d55a6be
--- /dev/null
+++ b/drivers/media/dvb-frontends/tc90522.h
@@ -0,0 +1,63 @@ 
+/*
+ * Toshiba TC90522 Demodulator
+ *
+ * Copyright (C) 2014 Akihiro Tsukada <tskd08@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/*
+ * The demod has 4 input (2xISDB-T and 2xISDB-S),
+ * and provides independent sub modules for each input.
+ * As the sub modules work in parallel and have the separate i2c addr's,
+ * this driver treats each sub module as one demod device.
+ */
+
+#ifndef TC90522_H
+#define TC90522_H
+
+#include <linux/i2c.h>
+#include <linux/kconfig.h>
+#include "dvb_frontend.h"
+
+#define TC90522_IS_ISDBS(addr) (addr & 1)
+
+#define TC90522T_CMD_SET_LNA 1
+#define TC90522S_CMD_SET_LNB 1
+
+struct tc90522_config {
+	u8 addr;
+};
+
+#if IS_ENABLED(CONFIG_DVB_TC90522)
+
+extern struct dvb_frontend *tc90522_attach(const struct tc90522_config *cfg,
+		struct i2c_adapter *i2c);
+
+extern struct i2c_adapter *tc90522_get_tuner_i2c(struct dvb_frontend *fe);
+
+#else
+static inline struct dvb_frontend *tc90522_attach(
+	const struct tc90522_config *cfg, struct i2c_adapter *i2c)
+{
+	printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
+	return NULL;
+}
+
+static inline struct i2c_adapter *tc90522_get_tuner_i2c(struct dvb_frontend *fe)
+{
+	printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
+	return NULL;
+}
+
+#endif
+
+#endif /* TC90522_H */