Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-12 Thread Manu Abraham
On Mon, Dec 12, 2011 at 7:11 PM, Antti Palosaari  wrote:
> On 12/12/2011 02:55 PM, Manu Abraham wrote:
>>
>> On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter
>>  wrote:
>>>
>>> On 12.12.2011 05:28, Manu Abraham wrote:

 On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari  wrote:
>
> On 12/10/2011 06:44 AM, Manu Abraham wrote:
>>
>>
>>  static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>
>
> [...]
>>
>>
>> +       switch (c->delivery_system) {
>> +       case SYS_DVBT:
>> +               ret = cxd2820r_init_t(fe);
>
>
>
>> +               ret = cxd2820r_set_frontend_t(fe, p);
>
>
>
>
> Anyhow, I don't now like idea you have put .init() calls to
> .set_frontend().
> Could you move .init() happen in .init() callback as it was earlier?


 This was there in the earlier patch as well. Maybe you have a
 new issue now ? ;-)
>
>
> You mean I didn't mentioned it when you send first version? Sorry, I didn't
> looked it very carefully since I first meet stuff that was not related whole
> thing, I mean there was that code changing from .set_params() to
> .set_state(). And I stopped reading rest of the patch.
>
>
>

 ok.

 The argument what you make doesn't hold well, Why ?

 int cxd2820r_init_t(struct dvb_frontend *fe)
 {
       ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
 }


 int cxd2820r_init_c(struct dvb_frontend *fe)
 {
       ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
 }


 Now, you might like to point that, the Base I2C address location
 is different comparing DVB-T/DVBT2 to DVB-C

 So, If you have the init as in earlier with a common init, then you
 will likely init the wrong device at .init(), as init is called open().
 So, this might result in an additional register write, which could
 be avoided altogether.  One register access is not definitely
 something to brag about, but is definitely a small incremental
 difference. Other than that this register write doesn't do anything
 more than an ADC_START. So starting the ADC at init doesn't
 make sense. But does so when you want to select the right ADC.
 So definitely, this change is an improvement. Also, you can
 compare the time taken for the device to tune now. It is quite
 a lot faster compared to without this patch. So you or any other
 user should be happy. :-)


 I don't think that in any way, the init should be used at init as
 you say, which sounds pretty much incorrect.
>>>
>>>
>>> Maybe the function names should be modified to avoid confusion with the
>>> init driver callback.
>>
>>
>>
>> On another tangential thought, Is it really worth to wrap that single
>> register write with another function name ?
>>
>> instead of the current usage; ie,
>>
>> ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */
>>
>> within set_frontend()
>>
>> in set_frontend(), another thing that's wrapped up similarly is
>> the set_frontend() within the search() callback, which causes
>> another set of confusions within the driver.
>
>
> Actually there was was a lot more code first but because I ran problems
> selsys needed for T/T2 init was not known at the time .init() was called I
> moved those set_frontend. I left that in a hope I can later fix properly
> adding more stuff back to init.
>
> That is not functionality issue, it is issue about naming callbacks and what
> is functionality of each callback.
> As for these days it have been in my understanding initialization stuff are
> done in .init() and leave as less as possible code to .set_frontend().
> Leaving set_frontend() handle only tuning requests and reconfigure IF
> control etc. And if you look most demod drivers there is rather similar
> logic used.
>
> So I would like to ask what is meaning of:
> .attach()
> * create FE
> * no HW init
> * as less as possible HW I/O, mainly reading chip ID and nothing more

Generally it should be simply create a DVB frontend data structure,
if it finds a valid device.

In some clunky cases, the demodulator clock would be from the tuner,
in which case the tuner has to be attached prior to the demod; and
then later on read device details, once clocks are setup.


>
> .init()
> * do nothing here?
> * download firmware?
>

init should simply initialize the device in the lowest power
mode, where it is ready to do a tune. (in some cases tuner also
might need an init. In such cases, the tuner_ops.init can be just
called).

> .set_frontend()
> * program tuner
> * init demod?
> * tune demod
> * download firmware?

Ideally set_frontend should just setup the frontend as a whole
(tuner, demod, or maybe more devices) and return status.

In some clunky cases, there could be cases where each tune
needs a firmware download. Maybe this download can be
optimized in some paths. This depends on the device.

>
> .sleep()
> * pu

Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-12 Thread Antti Palosaari

On 12/12/2011 02:55 PM, Manu Abraham wrote:

On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter  wrote:

On 12.12.2011 05:28, Manu Abraham wrote:

On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari  wrote:

On 12/10/2011 06:44 AM, Manu Abraham wrote:


  static int cxd2820r_set_frontend(struct dvb_frontend *fe,


[...]


+   switch (c->delivery_system) {
+   case SYS_DVBT:
+   ret = cxd2820r_init_t(fe);




+   ret = cxd2820r_set_frontend_t(fe, p);




Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
Could you move .init() happen in .init() callback as it was earlier?


This was there in the earlier patch as well. Maybe you have a
new issue now ? ;-)


You mean I didn't mentioned it when you send first version? Sorry, I 
didn't looked it very carefully since I first meet stuff that was not 
related whole thing, I mean there was that code changing from 
.set_params() to .set_state(). And I stopped reading rest of the patch.





ok.

The argument what you make doesn't hold well, Why ?

int cxd2820r_init_t(struct dvb_frontend *fe)
{
   ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}


int cxd2820r_init_c(struct dvb_frontend *fe)
{
   ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}


Now, you might like to point that, the Base I2C address location
is different comparing DVB-T/DVBT2 to DVB-C

So, If you have the init as in earlier with a common init, then you
will likely init the wrong device at .init(), as init is called open().
So, this might result in an additional register write, which could
be avoided altogether.  One register access is not definitely
something to brag about, but is definitely a small incremental
difference. Other than that this register write doesn't do anything
more than an ADC_START. So starting the ADC at init doesn't
make sense. But does so when you want to select the right ADC.
So definitely, this change is an improvement. Also, you can
compare the time taken for the device to tune now. It is quite
a lot faster compared to without this patch. So you or any other
user should be happy. :-)


I don't think that in any way, the init should be used at init as
you say, which sounds pretty much incorrect.


Maybe the function names should be modified to avoid confusion with the
init driver callback.



On another tangential thought, Is it really worth to wrap that single
register write with another function name ?

instead of the current usage; ie,

ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */

within set_frontend()

in set_frontend(), another thing that's wrapped up similarly is
the set_frontend() within the search() callback, which causes
another set of confusions within the driver.


Actually there was was a lot more code first but because I ran problems 
selsys needed for T/T2 init was not known at the time .init() was called 
I moved those set_frontend. I left that in a hope I can later fix 
properly adding more stuff back to init.


That is not functionality issue, it is issue about naming callbacks and 
what is functionality of each callback.
As for these days it have been in my understanding initialization stuff 
are done in .init() and leave as less as possible code to 
.set_frontend(). Leaving set_frontend() handle only tuning requests and 
reconfigure IF control etc. And if you look most demod drivers there is 
rather similar logic used.


So I would like to ask what is meaning of:
.attach()
* create FE
* no HW init
* as less as possible HW I/O, mainly reading chip ID and nothing more

.init()
* do nothing here?
* download firmware?

.set_frontend()
* program tuner
* init demod?
* tune demod
* download firmware?

.sleep()
* put device sleep mode
* powersave


After all it is just fine for me apply that patch, but I would like to 
get clear idea what is meaning of every single callback we have. And if 
we really end up .init() is not needed and all should be put to 
.set_frontend() when possible it means I have to change all my demod 
drivers and maybe tuner drivers too.


regards
Antti


--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-12 Thread Manu Abraham
On Mon, Dec 12, 2011 at 6:13 PM, Andreas Oberritter  wrote:
> On 12.12.2011 05:28, Manu Abraham wrote:
>> On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari  wrote:
>>> On 12/10/2011 06:44 AM, Manu Abraham wrote:

  static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>>>
>>> [...]

 +       switch (c->delivery_system) {
 +       case SYS_DVBT:
 +               ret = cxd2820r_init_t(fe);
>>>
>>>
 +               ret = cxd2820r_set_frontend_t(fe, p);
>>>
>>>
>>>
>>> Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
>>> Could you move .init() happen in .init() callback as it was earlier?
>>
>> This was there in the earlier patch as well. Maybe you have a
>> new issue now ? ;-)
>>
>> ok.
>>
>> The argument what you make doesn't hold well, Why ?
>>
>> int cxd2820r_init_t(struct dvb_frontend *fe)
>> {
>>       ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
>> }
>>
>>
>> int cxd2820r_init_c(struct dvb_frontend *fe)
>> {
>>       ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
>> }
>>
>>
>> Now, you might like to point that, the Base I2C address location
>> is different comparing DVB-T/DVBT2 to DVB-C
>>
>> So, If you have the init as in earlier with a common init, then you
>> will likely init the wrong device at .init(), as init is called open().
>> So, this might result in an additional register write, which could
>> be avoided altogether.  One register access is not definitely
>> something to brag about, but is definitely a small incremental
>> difference. Other than that this register write doesn't do anything
>> more than an ADC_START. So starting the ADC at init doesn't
>> make sense. But does so when you want to select the right ADC.
>> So definitely, this change is an improvement. Also, you can
>> compare the time taken for the device to tune now. It is quite
>> a lot faster compared to without this patch. So you or any other
>> user should be happy. :-)
>>
>>
>> I don't think that in any way, the init should be used at init as
>> you say, which sounds pretty much incorrect.
>
> Maybe the function names should be modified to avoid confusion with the
> init driver callback.


On another tangential thought, Is it really worth to wrap that single
register write with another function name ?

instead of the current usage; ie,

ret = cxd2820r_wr_reg(priv, 0x00085, 0x07); /* Start ADC */

within set_frontend()

in set_frontend(), another thing that's wrapped up similarly is
the set_frontend() within the search() callback, which causes
another set of confusions within the driver.


Regards,
Manu
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-12 Thread Andreas Oberritter
On 12.12.2011 05:28, Manu Abraham wrote:
> On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari  wrote:
>> On 12/10/2011 06:44 AM, Manu Abraham wrote:
>>>
>>>  static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>>
>> [...]
>>>
>>> +   switch (c->delivery_system) {
>>> +   case SYS_DVBT:
>>> +   ret = cxd2820r_init_t(fe);
>>
>>
>>> +   ret = cxd2820r_set_frontend_t(fe, p);
>>
>>
>>
>> Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
>> Could you move .init() happen in .init() callback as it was earlier?
> 
> This was there in the earlier patch as well. Maybe you have a
> new issue now ? ;-)
> 
> ok.
> 
> The argument what you make doesn't hold well, Why ?
> 
> int cxd2820r_init_t(struct dvb_frontend *fe)
> {
>   ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
> }
> 
> 
> int cxd2820r_init_c(struct dvb_frontend *fe)
> {
>   ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
> }
> 
> 
> Now, you might like to point that, the Base I2C address location
> is different comparing DVB-T/DVBT2 to DVB-C
> 
> So, If you have the init as in earlier with a common init, then you
> will likely init the wrong device at .init(), as init is called open().
> So, this might result in an additional register write, which could
> be avoided altogether.  One register access is not definitely
> something to brag about, but is definitely a small incremental
> difference. Other than that this register write doesn't do anything
> more than an ADC_START. So starting the ADC at init doesn't
> make sense. But does so when you want to select the right ADC.
> So definitely, this change is an improvement. Also, you can
> compare the time taken for the device to tune now. It is quite
> a lot faster compared to without this patch. So you or any other
> user should be happy. :-)
> 
> 
> I don't think that in any way, the init should be used at init as
> you say, which sounds pretty much incorrect.

Maybe the function names should be modified to avoid confusion with the
init driver callback.

Regards,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-11 Thread Manu Abraham
On Sat, Dec 10, 2011 at 5:17 PM, Antti Palosaari  wrote:
> Hello Manu,
> That patch looks now much acceptable than the older for my eyes, since you
> removed that .set_state() (change from .set_params() to .set_state()) I
> criticized. Thanks!
>


:-)

>
> On 12/10/2011 06:44 AM, Manu Abraham wrote:
>>
>>  static int cxd2820r_set_frontend(struct dvb_frontend *fe,
>
> [...]
>>
>> +       switch (c->delivery_system) {
>> +       case SYS_DVBT:
>> +               ret = cxd2820r_init_t(fe);
>
>
>> +               ret = cxd2820r_set_frontend_t(fe, p);
>
>
>
> Anyhow, I don't now like idea you have put .init() calls to .set_frontend().
> Could you move .init() happen in .init() callback as it was earlier?

This was there in the earlier patch as well. Maybe you have a
new issue now ? ;-)

ok.

The argument what you make doesn't hold well, Why ?

int cxd2820r_init_t(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}


int cxd2820r_init_c(struct dvb_frontend *fe)
{
ret = cxd2820r_wr_reg(priv, 0x00085, 0x07);
}


Now, you might like to point that, the Base I2C address location
is different comparing DVB-T/DVBT2 to DVB-C

So, If you have the init as in earlier with a common init, then you
will likely init the wrong device at .init(), as init is called open().
So, this might result in an additional register write, which could
be avoided altogether.  One register access is not definitely
something to brag about, but is definitely a small incremental
difference. Other than that this register write doesn't do anything
more than an ADC_START. So starting the ADC at init doesn't
make sense. But does so when you want to select the right ADC.
So definitely, this change is an improvement. Also, you can
compare the time taken for the device to tune now. It is quite
a lot faster compared to without this patch. So you or any other
user should be happy. :-)


I don't think that in any way, the init should be used at init as
you say, which sounds pretty much incorrect.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-10 Thread Mauro Carvalho Chehab

On 10-12-2011 11:09, Antti Palosaari wrote:

On 12/10/2011 02:47 PM, Mauro Carvalho Chehab wrote:

On 10-12-2011 09:47, Antti Palosaari wrote:

Hello Manu,
That patch looks now much acceptable than the older for my eyes, since
you removed that .set_state() (change from .set_params() to
.set_state()) I criticized. Thanks!


On 12/10/2011 06:44 AM, Manu Abraham wrote:

static int cxd2820r_set_frontend(struct dvb_frontend *fe,

[...]

+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ ret = cxd2820r_init_t(fe);



+ ret = cxd2820r_set_frontend_t(fe, p);



Anyhow, I don't now like idea you have put .init() calls to
.set_frontend(). Could you move .init() happen in .init() callback as
it was earlier?


Changing .init() to happen at set_frontend() actually makes sense for me ;)

Why didn't you like this change?


Because there is already callback named .init() just for that as I guess. 
.init() differs from .set_frontend() as init is called only once when device is 
opened whilst .set_frontend() is called every time when tuning request is done.

Anyhow, it is up to decision remove .init() from the frontends callbacks as we 
can live without.


IMO, it makes sense to not do any init before set_frontend on most devices.
An obvious exception would be if is there any device where the capabilities need
to be discovered at runtime.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-10 Thread Antti Palosaari

On 12/10/2011 02:47 PM, Mauro Carvalho Chehab wrote:

On 10-12-2011 09:47, Antti Palosaari wrote:

Hello Manu,
That patch looks now much acceptable than the older for my eyes, since
you removed that .set_state() (change from .set_params() to
.set_state()) I criticized. Thanks!


On 12/10/2011 06:44 AM, Manu Abraham wrote:

static int cxd2820r_set_frontend(struct dvb_frontend *fe,

[...]

+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ ret = cxd2820r_init_t(fe);



+ ret = cxd2820r_set_frontend_t(fe, p);



Anyhow, I don't now like idea you have put .init() calls to
.set_frontend(). Could you move .init() happen in .init() callback as
it was earlier?


Changing .init() to happen at set_frontend() actually makes sense for me ;)

Why didn't you like this change?


Because there is already callback named .init() just for that as I 
guess. .init() differs from .set_frontend() as init is called only once 
when device is opened whilst .set_frontend() is called every time when 
tuning request is done.


Anyhow, it is up to decision remove .init() from the frontends callbacks 
as we can live without.


regards
Antti

--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-10 Thread Mauro Carvalho Chehab

On 10-12-2011 09:47, Antti Palosaari wrote:

Hello Manu,
That patch looks now much acceptable than the older for my eyes, since you 
removed that .set_state() (change from .set_params() to .set_state()) I 
criticized. Thanks!


On 12/10/2011 06:44 AM, Manu Abraham wrote:

static int cxd2820r_set_frontend(struct dvb_frontend *fe,

[...]

+ switch (c->delivery_system) {
+ case SYS_DVBT:
+ ret = cxd2820r_init_t(fe);



+ ret = cxd2820r_set_frontend_t(fe, p);



Anyhow, I don't now like idea you have put .init() calls to .set_frontend(). 
Could you move .init() happen in .init() callback as it was earlier?


Changing .init() to happen at set_frontend() actually makes sense for me ;)

Why didn't you like this change?

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-10 Thread Antti Palosaari

Hello Manu,
That patch looks now much acceptable than the older for my eyes, since 
you removed that .set_state() (change from .set_params() to 
.set_state()) I criticized. Thanks!



On 12/10/2011 06:44 AM, Manu Abraham wrote:

  static int cxd2820r_set_frontend(struct dvb_frontend *fe,

[...]

+   switch (c->delivery_system) {
+   case SYS_DVBT:
+   ret = cxd2820r_init_t(fe);



+   ret = cxd2820r_set_frontend_t(fe, p);



Anyhow, I don't now like idea you have put .init() calls to 
.set_frontend(). Could you move .init() happen in .init() callback as it 
was earlier?



regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


v4 [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

2011-12-09 Thread Manu Abraham

From 4fdffdcc77228a140c944c20ce2a9f2b6c5b7658 Mon Sep 17 00:00:00 2001
From: Manu Abraham 
Date: Thu, 24 Nov 2011 20:29:53 +0530
Subject: [PATCH 09/10] CXD2820r: Query DVB frontend delivery capabilities

Override default delivery system information provided by FE_GET_INFO, so
that applications can enumerate delivery systems provided by the frontend.

Signed-off-by: Manu Abraham 
---
 drivers/media/dvb/frontends/cxd2820r_c.c|2 +-
 drivers/media/dvb/frontends/cxd2820r_core.c |  651 +--
 drivers/media/dvb/frontends/cxd2820r_priv.h |5 +-
 drivers/media/dvb/frontends/cxd2820r_t.c|2 +-
 drivers/media/dvb/frontends/cxd2820r_t2.c   |2 +-
 5 files changed, 221 insertions(+), 441 deletions(-)

diff --git a/drivers/media/dvb/frontends/cxd2820r_c.c b/drivers/media/dvb/frontends/cxd2820r_c.c
index b85f501..01baeae 100644
--- a/drivers/media/dvb/frontends/cxd2820r_c.c
+++ b/drivers/media/dvb/frontends/cxd2820r_c.c
@@ -22,7 +22,7 @@
 #include "cxd2820r_priv.h"
 
 int cxd2820r_set_frontend_c(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *params)
+			struct dvb_frontend_parameters *params)
 {
 	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
diff --git a/drivers/media/dvb/frontends/cxd2820r_core.c b/drivers/media/dvb/frontends/cxd2820r_core.c
index 036480f..5b0120a 100644
--- a/drivers/media/dvb/frontends/cxd2820r_core.c
+++ b/drivers/media/dvb/frontends/cxd2820r_core.c
@@ -240,43 +240,6 @@ error:
 	return ret;
 }
 
-/* lock FE */
-static int cxd2820r_lock(struct cxd2820r_priv *priv, int active_fe)
-{
-	int ret = 0;
-	dbg("%s: active_fe=%d", __func__, active_fe);
-
-	mutex_lock(&priv->fe_lock);
-
-	/* -1=NONE, 0=DVB-T/T2, 1=DVB-C */
-	if (priv->active_fe == active_fe)
-		;
-	else if (priv->active_fe == -1)
-		priv->active_fe = active_fe;
-	else
-		ret = -EBUSY;
-
-	mutex_unlock(&priv->fe_lock);
-
-	return ret;
-}
-
-/* unlock FE */
-static void cxd2820r_unlock(struct cxd2820r_priv *priv, int active_fe)
-{
-	dbg("%s: active_fe=%d", __func__, active_fe);
-
-	mutex_lock(&priv->fe_lock);
-
-	/* -1=NONE, 0=DVB-T/T2, 1=DVB-C */
-	if (priv->active_fe == active_fe)
-		priv->active_fe = -1;
-
-	mutex_unlock(&priv->fe_lock);
-
-	return;
-}
-
 /* 64 bit div with round closest, like DIV_ROUND_CLOSEST but 64 bit */
 u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor)
 {
@@ -284,378 +247,230 @@ u32 cxd2820r_div_u64_round_closest(u64 dividend, u32 divisor)
 }
 
 static int cxd2820r_set_frontend(struct dvb_frontend *fe,
-	struct dvb_frontend_parameters *p)
+ struct dvb_frontend_parameters *p)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 	int ret;
-	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
-
-	if (fe->ops.info.type == FE_OFDM) {
-		/* DVB-T/T2 */
-		ret = cxd2820r_lock(priv, 0);
-		if (ret)
-			return ret;
-
-		switch (priv->delivery_system) {
-		case SYS_UNDEFINED:
-			if (c->delivery_system == SYS_DVBT) {
-/* SLEEP => DVB-T */
-ret = cxd2820r_set_frontend_t(fe, p);
-			} else {
-/* SLEEP => DVB-T2 */
-ret = cxd2820r_set_frontend_t2(fe, p);
-			}
-			break;
-		case SYS_DVBT:
-			if (c->delivery_system == SYS_DVBT) {
-/* DVB-T => DVB-T */
-ret = cxd2820r_set_frontend_t(fe, p);
-			} else if (c->delivery_system == SYS_DVBT2) {
-/* DVB-T => DVB-T2 */
-ret = cxd2820r_sleep_t(fe);
-if (ret)
-	break;
-ret = cxd2820r_set_frontend_t2(fe, p);
-			}
-			break;
-		case SYS_DVBT2:
-			if (c->delivery_system == SYS_DVBT2) {
-/* DVB-T2 => DVB-T2 */
-ret = cxd2820r_set_frontend_t2(fe, p);
-			} else if (c->delivery_system == SYS_DVBT) {
-/* DVB-T2 => DVB-T */
-ret = cxd2820r_sleep_t2(fe);
-if (ret)
-	break;
-ret = cxd2820r_set_frontend_t(fe, p);
-			}
-			break;
-		default:
-			dbg("%s: error state=%d", __func__,
-priv->delivery_system);
-			ret = -EINVAL;
-		}
-	} else {
-		/* DVB-C */
-		ret = cxd2820r_lock(priv, 1);
-		if (ret)
-			return ret;
 
+	dbg("%s: delsys=%d", __func__, fe->dtv_property_cache.delivery_system);
+	switch (c->delivery_system) {
+	case SYS_DVBT:
+		ret = cxd2820r_init_t(fe);
+		if (ret < 0)
+			goto err;
+		ret = cxd2820r_set_frontend_t(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	case SYS_DVBT2:
+		ret = cxd2820r_init_t(fe);
+		if (ret < 0)
+			goto err;
+		ret = cxd2820r_set_frontend_t2(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	case SYS_DVBC_ANNEX_AC:
+		ret = cxd2820r_init_c(fe);
+		if (ret < 0)
+			goto err;
 		ret = cxd2820r_set_frontend_c(fe, p);
+		if (ret < 0)
+			goto err;
+		break;
+	default:
+		dbg("%s: error state=%d", __func__, fe->dtv_property_cache.delivery_system);
+		ret = -EINVAL;
+		break;
 	}
-
+err:
 	return ret;
 }
-
 static int cxd2820r_read_status(struct dvb_frontend *fe, fe_status_t *status)
 {
-	struct cxd2820r_priv *priv = fe->demodulator_priv;
 	int ret;
-	dbg("%s: del