[PATCH 1/3] tuners: si2157: Si2148 support.

[PATCH 1/3] tuners: si2157: Si2148 support.

[CrazyCat]

Si2148-A20 silicon tuner support.

Signed-off-by: Evgeny Plehov <EvgenyPlehov@ukr.net>
---
 drivers/media/tuners/si2157.c      | 10 ++++++----
 drivers/media/tuners/si2157.h      |  2 +-
 drivers/media/tuners/si2157_priv.h |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 25146fa..91f8290 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -1,5 +1,5 @@
 /*
- * Silicon Labs Si2147/2157/2158 silicon tuner driver
+ * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
  *
  * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
  *
@@ -112,11 +112,13 @@ static int si2157_init(struct dvb_frontend *fe)
 			cmd.args[4] << 0;
 
 	#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
+	#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
 	#define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
 	#define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
 
 	switch (chip_id) {
 	case SI2158_A20:
+	case SI2148_A20:
 		fw_file = SI2158_A20_FIRMWARE;
 		break;
 	case SI2157_A30:
@@ -309,7 +311,7 @@ static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
 
 static const struct dvb_tuner_ops si2157_ops = {
 	.info = {
-		.name           = "Silicon Labs Si2157/Si2158",
+		.name           = "Silicon Labs Si2147/2148/2157/Si2158",
 		.frequency_min  = 110000000,
 		.frequency_max  = 862000000,
 	},
@@ -373,7 +375,7 @@ static int si2157_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, s);
 
 	dev_info(&s->client->dev,
-			"Silicon Labs Si2157/Si2158 successfully attached\n");
+			"Silicon Labs Si2147/2148/2157/Si2158 successfully attached\n");
 	return 0;
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
@@ -414,7 +416,7 @@ static struct i2c_driver si2157_driver = {
 
 module_i2c_driver(si2157_driver);
 
-MODULE_DESCRIPTION("Silicon Labs Si2157/Si2158 silicon tuner driver");
+MODULE_DESCRIPTION("Silicon Labs Si2147/2148/2157/Si2158 silicon tuner driver");
 MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
 MODULE_LICENSE("GPL");
 MODULE_FIRMWARE(SI2158_A20_FIRMWARE);
diff --git a/drivers/media/tuners/si2157.h b/drivers/media/tuners/si2157.h
index d3b19ca..c439d0e 100644
--- a/drivers/media/tuners/si2157.h
+++ b/drivers/media/tuners/si2157.h
@@ -1,5 +1,5 @@
 /*
- * Silicon Labs Si2147/2157/2158 silicon tuner driver
+ * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
  *
  * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
  *
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index e71ffaf..6d2aac4 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -1,5 +1,5 @@
 /*
- * Silicon Labs Si2147/2157/2158 silicon tuner driver
+ * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
  *
  * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
  *
-- 
1.9.1

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Antti Palosaari]

Hello
I wonder if we should define own firmware for Si2148-A20 just for sure. 
Olli?

regards
Antti

On 11/14/2014 11:19 PM, CrazyCat wrote:
> Si2148-A20 silicon tuner support.
>
> Signed-off-by: Evgeny Plehov <EvgenyPlehov@ukr.net>
> ---
>   drivers/media/tuners/si2157.c      | 10 ++++++----
>   drivers/media/tuners/si2157.h      |  2 +-
>   drivers/media/tuners/si2157_priv.h |  2 +-
>   3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index 25146fa..91f8290 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -1,5 +1,5 @@
>   /*
> - * Silicon Labs Si2147/2157/2158 silicon tuner driver
> + * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
>    *
>    * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
>    *
> @@ -112,11 +112,13 @@ static int si2157_init(struct dvb_frontend *fe)
>   			cmd.args[4] << 0;
>
>   	#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> +	#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
>   	#define SI2157_A30 ('A' << 24 | 57 << 16 | '3' << 8 | '0' << 0)
>   	#define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
>
>   	switch (chip_id) {
>   	case SI2158_A20:
> +	case SI2148_A20:
>   		fw_file = SI2158_A20_FIRMWARE;
>   		break;
>   	case SI2157_A30:
> @@ -309,7 +311,7 @@ static int si2157_get_if_frequency(struct dvb_frontend *fe, u32 *frequency)
>
>   static const struct dvb_tuner_ops si2157_ops = {
>   	.info = {
> -		.name           = "Silicon Labs Si2157/Si2158",
> +		.name           = "Silicon Labs Si2147/2148/2157/Si2158",
>   		.frequency_min  = 110000000,
>   		.frequency_max  = 862000000,
>   	},
> @@ -373,7 +375,7 @@ static int si2157_probe(struct i2c_client *client,
>   	i2c_set_clientdata(client, s);
>
>   	dev_info(&s->client->dev,
> -			"Silicon Labs Si2157/Si2158 successfully attached\n");
> +			"Silicon Labs Si2147/2148/2157/Si2158 successfully attached\n");
>   	return 0;
>   err:
>   	dev_dbg(&client->dev, "failed=%d\n", ret);
> @@ -414,7 +416,7 @@ static struct i2c_driver si2157_driver = {
>
>   module_i2c_driver(si2157_driver);
>
> -MODULE_DESCRIPTION("Silicon Labs Si2157/Si2158 silicon tuner driver");
> +MODULE_DESCRIPTION("Silicon Labs Si2147/2148/2157/Si2158 silicon tuner driver");
>   MODULE_AUTHOR("Antti Palosaari <crope@iki.fi>");
>   MODULE_LICENSE("GPL");
>   MODULE_FIRMWARE(SI2158_A20_FIRMWARE);
> diff --git a/drivers/media/tuners/si2157.h b/drivers/media/tuners/si2157.h
> index d3b19ca..c439d0e 100644
> --- a/drivers/media/tuners/si2157.h
> +++ b/drivers/media/tuners/si2157.h
> @@ -1,5 +1,5 @@
>   /*
> - * Silicon Labs Si2147/2157/2158 silicon tuner driver
> + * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
>    *
>    * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
>    *
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index e71ffaf..6d2aac4 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -1,5 +1,5 @@
>   /*
> - * Silicon Labs Si2147/2157/2158 silicon tuner driver
> + * Silicon Labs Si2147/2148/2157/2158 silicon tuner driver
>    *
>    * Copyright (C) 2014 Antti Palosaari <crope@iki.fi>
>    *
>

-- 
http://palosaari.fi/

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[CrazyCat]

2148 is 2158 without analog support. Same firmware.

15.11.2014, 03:02, "Antti Palosaari" <crope@iki.fi>:
> I wonder if we should define own firmware for Si2148-A20 just for sure.
> Olli?

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Antti Palosaari]

On 11/15/2014 03:22 AM, CrazyCat wrote:
> 2148 is 2158 without analog support. Same firmware.
>
> 15.11.2014, 03:02, "Antti Palosaari" <crope@iki.fi>:
>> I wonder if we should define own firmware for Si2148-A20 just for sure.
>> Olli?

But still... I have seen one case where even same revision of si2168 
needs different firmware. It is not very clear for me how SiLabs uses 
these firmwares, but at least it seems to be:

* There is different versions done from same chips. A10 < A20 < A30 < 
B40 and so. I think that means digital logic inside of chip is changed 
when they change that revision number.

* Every chip has a ROM, containing default firmware. Firmware which 
driver downloads is just a patch to that ROM. ROM is updated regularly 
when new patch of chips are manufactured.


So currently I have feeling it is better to define as many firmware 
files as there chip revisions available, even same firmware works for 
multiple chip models/versions/revisions.

regards
Antti

-- 
http://palosaari.fi/

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Olli Salonen]

What about defining the firmware for Si2148-A20, but since the file is
the same as Si2158-A20 just point to the same file?

#define SI2148_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"

Then if Si2158-A20 would in the future get a new firmware that would
not work with Si2148, this would not break Si2148.

Another point that came to my mind is that we start to have quite a
list of chips there in the printouts (Si2147/Si2148/Si2157/Si2158) and
more is coming - I'm working with an Si2146 device currently. Should
we just say "Si214x/Si215x" there or something?

Cheers,
-olli

On 15 November 2014 03:22, CrazyCat <crazycat69@narod.ru> wrote:
> 2148 is 2158 without analog support. Same firmware.
>
> 15.11.2014, 03:02, "Antti Palosaari" <crope@iki.fi>:
>> I wonder if we should define own firmware for Si2148-A20 just for sure.
>> Olli?

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Antti Palosaari]

On 11/15/2014 12:41 PM, Olli Salonen wrote:
> What about defining the firmware for Si2148-A20, but since the file is
> the same as Si2158-A20 just point to the same file?
>
> #define SI2148_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
>
> Then if Si2158-A20 would in the future get a new firmware that would
> not work with Si2148, this would not break Si2148.

Assuming you rename possible new firmware:
dvb-tuner-si2158-a20-01.fw
dvb-tuner-si2158-a20-02.fw ?

Basically, you would not like to rename firmware when it is updated if 
it is compatible with the driver. Lets say firmware gets bug fixes, just 
introduce new firmware with same name. If driver changes are needed, 
then you have to rename it. These firmware changes are always 
problematic as you have to think possible regression - it is regression 
from the user point of view if kernel driver updates but it does not 
work as firmware incompatibility.

How about Si2146 firmware you are working?

All-in-all, with the current situation and knowledge I have, I see it is 
better to define new firmware name for that chip model and revision like 
the others. Just to make life it easier in a case Si2148-A20 and 
Si2158-A20 firmwares will be different on some case on some day. So lets 
implement it that way or explain some possible problem we could meet 
when defining own firmware file name.

> Another point that came to my mind is that we start to have quite a
> list of chips there in the printouts (Si2147/Si2148/Si2157/Si2158) and
> more is coming - I'm working with an Si2146 device currently. Should
> we just say "Si214x/Si215x" there or something?

I have no opinion.

regards
Antti

>
> Cheers,
> -olli
>
> On 15 November 2014 03:22, CrazyCat <crazycat69@narod.ru> wrote:
>> 2148 is 2158 without analog support. Same firmware.
>>
>> 15.11.2014, 03:02, "Antti Palosaari" <crope@iki.fi>:
>>> I wonder if we should define own firmware for Si2148-A20 just for sure.
>>> Olli?

-- 
http://palosaari.fi/

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Olli Salonen]

On Sat, 15 Nov 2014, Antti Palosaari wrote:

> Assuming you rename possible new firmware:
> dvb-tuner-si2158-a20-01.fw
> dvb-tuner-si2158-a20-02.fw ?
>
> Basically, you would not like to rename firmware when it is updated if it is 
> compatible with the driver. Lets say firmware gets bug fixes, just introduce 
> new firmware with same name. If driver changes are needed, then you have to 
> rename it. These firmware changes are always problematic as you have to think 
> possible regression - it is regression from the user point of view if kernel 
> driver updates but it does not work as firmware incompatibility.

Indeed, I assumed whenever there's a firmware update, we create a new 
filename for that. If that's not the case, then better to keep Si2148 and 
Si2158 totally separate. At this point the files would be identical of 
course.

> How about Si2146 firmware you are working?

No firmware loaded for that.

Cheers,
-olli

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[CrazyCat]

No need, because si214x is same si215x without analog filter path.

20.11.2014, 22:10, "Olli Salonen" <olli.salonen@iki.fi>:
> Crazycat, do you think you could change the firmware loading for Si2148
> as discussed here though and send a new patch?

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Olli Salonen]

On Mon, 17 Nov 2014, Michael Holzer wrote:

> I'd see merit to show the supported chips explicitly as otherwise users
> may be confused if a new unsupported chip  (lets assume Si2159)
> appears and the message is generic as proposed "Si215x".
> To get clarity for this case source code reading would be required.

Well, the user of a Si2159 would never see the printout as the driver 
would not be loaded for a Si2159 user. I'd say just print something like 
"Si215x/Si216x Silicon Tuner" in the printouts and list all the chips in 
the source code. But that's not something that needs to be fixed now 
anyway, we can do that later.

Crazycat, do you think you could change the firmware loading for Si2148 
as discussed here though and send a new patch?

Cheers,
-olli

Re: [PATCH 1/3] tuners: si2157: Si2148 support.

[Michael Holzer]

Hi Olli,

re ' we start to have quite a list of chips there in the printouts
(Si2147/Si2148/Si2157/Si2158) and
more is coming -...
Should we just say "Si214x/Si215x" there or something?'

I'd see merit to show the supported chips explicitly as otherwise users
may be confused if a new unsupported chip  (lets assume Si2159)
appears and the message is generic as proposed "Si215x".
To get clarity for this case source code reading would be required.
 just my 5 cent ;-)
Mike