2019-01-30 10:32:58

by Gonsolo

Subject: DVB-T2 Stick

Hi!

The following patch adds support for the Logilink VG0022A DVB-T2 stick.
After patching and building the kernel it shows up with lsusb and I
used w_scan to scan for channels and vlc for watching.
The original patches were from Andreas Kemnade.
The only thing that doesn't work is wake up after standby.

Do you think you can apply this patch?

Thanks
--
g

Attachments:

2019-02-19 21:30:52

by Sean Young

Subject: Re: DVB-T2 Stick

Hi,

On Wed, Jan 30, 2019 at 11:32:12AM +0100, Gonsolo wrote:
> Hi!
>
> The following patch adds support for the Logilink VG0022A DVB-T2 stick.
> After patching and building the kernel it shows up with lsusb and I
> used w_scan to scan for channels and vlc for watching.
> The original patches were from Andreas Kemnade.
> The only thing that doesn't work is wake up after standby.
>
> Do you think you can apply this patch?

I'm afraid there are many problems with this patch. First of all it looks
like support was added for a si2168 tuner but it looks like it will break
for any other si2157-type device.

Secondly there are lots of coding style issues, see:

https://www.kernel.org/doc/html/v4.10/process/coding-style.html

Thanks,

Sean
>
> Thanks
> --
> g

> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index d389f1fc237a..1f923418ff10 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -84,7 +84,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -126,7 +126,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (ret)
> goto err;
> }
> -
> +#if 0
> /* query chip revision */
> memcpy(cmd.args, "\x02", 1);
> cmd.wlen = 1;
> @@ -146,6 +146,8 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> switch (chip_id) {
> +#endif
> + switch (dev->chip_id) {
> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -166,9 +168,9 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> - cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> -
> +// dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> +// cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +//
> if (fw_name == NULL)
> goto skip_fw_download;
>
> @@ -461,6 +463,38 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + /* power up */
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else {
> + memcpy(cmd.args,
> + "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01",
> + 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result
> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> + cmd.args[4] << 0;
> + dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> + cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +
> +
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (cfg->mdev) {
> dev->mdev = cfg->mdev;
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index 50f86300d965..3592146cf49a 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -37,6 +37,7 @@ struct si2157_dev {
> u8 chiptype;
> u8 if_port;
> u32 if_frequency;
> + u32 chip_id;
> struct delayed_work stat_work;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> @@ -51,6 +52,13 @@ struct si2157_dev {
> #define SI2157_CHIPTYPE_SI2146 1
> #define SI2157_CHIPTYPE_SI2141 2
>
> +#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)
> +#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> +#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> +
> /* firmware command struct */
> #define SI2157_ARGLEN 30
> struct si2157_cmd {
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 80d3bd3a0f24..28b073bfe0d4 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1216,8 +1216,51 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
> struct si2168_config si2168_config;
> struct i2c_adapter *adapter;
>
> - dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
> -
> + //dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
> + dev_dbg(&intf->dev, "%s adap->id=%d\n", __func__, adap->id);
> +
> + /* I2C master bus 2 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 1,3 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* set gpio11 low */
> + ret = af9035_wr_reg_mask(d, 0xd8d4, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d5, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d3, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* Tuner enable using gpiot2_en, gpiot2_on and gpiot2_o (reset) */
> + ret = af9035_wr_reg_mask(d, 0xd8b8, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b9, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + msleep(200);
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> memset(&si2168_config, 0, sizeof(si2168_config));
> si2168_config.i2c_adapter = &adapter;
> si2168_config.fe = &adap->fe[0];
> @@ -2128,6 +2171,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { }
> };
> MODULE_DEVICE_TABLE(usb, af9035_id_table);

2019-10-02 05:58:28

by Gonsolo

Subject: Re: DVB-T2 Stick

Hi!

> Secondly there are lots of coding style issues, see:
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html

I addressed most of these except one "#if 0" warning from checkpatch.

> I'm afraid there are many problems with this patch. First of all it looks
> like support was added for a si2168 tuner but it looks like it will break
> for any other si2157-type device.


Can you give me a hint how to proceed here? I don't know much about
DVB tuners or kernel development.

I attached the cleaned-up patch for 5.4.0-rc1

Thanks,
g

Attachments:

2019-10-02 06:18:08

by Mauro Carvalho Chehab

Subject: Re: DVB-T2 Stick

Em Wed, 2 Oct 2019 00:19:05 +0200
Gonsolo <[email protected]> escreveu:

> Hi!
>
> > Secondly there are lots of coding style issues, see:
> > https://www.kernel.org/doc/html/v4.10/process/coding-style.html
>
> I addressed most of these except one "#if 0" warning from checkpatch.
>
> > I'm afraid there are many problems with this patch. First of all it looks
> > like support was added for a si2168 tuner but it looks like it will break
> > for any other si2157-type device.
>
>
> Can you give me a hint how to proceed here? I don't know much about
> DVB tuners or kernel development.
>
> I attached the cleaned-up patch for 5.4.0-rc1

First of all, don't attach a patch. Instead, just send it with a decent
emailer (with won't mangle whitespaces) or use git send-email...

Also, please read the Developer's section of our wiki page:

https://linuxtv.org/wiki/index.php/Developer_section

In special, the "Submitting your work" part of it.

>
> Thanks,
> g
> From 6cada6442207a67202e73721692aced665b8fdf0 Mon Sep 17 00:00:00 2001
> From: Gon Solo <[email protected]>
> Date: Tue, 1 Oct 2019 21:59:44 +0200
> Subject: [PATCH] DVB-T2 with coding style updates.
>
> ---
> drivers/media/tuners/si2157.c | 44 ++++++++++++++++++++++---
> drivers/media/tuners/si2157_priv.h | 8 +++++
> drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++++++++++++++++++++++++++-
> 3 files changed, 93 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..af50e721281b 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -75,7 +75,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -117,7 +117,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (ret)
> goto err;
> }
> -
> +#if 0
> /* query chip revision */
> memcpy(cmd.args, "\x02", 1);
> cmd.wlen = 1;
> @@ -138,6 +138,8 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> switch (chip_id) {
> +#endif
> + switch (dev->chip_id) {

You shouldn't just blindly comment out some code, as this will very likely
break support for all other devices supported by the driver...

> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -161,9 +163,9 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> - cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> -
> +// dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> +// cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +//
> if (fw_name == NULL)
> goto skip_fw_download;
>
> @@ -456,6 +458,38 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + /* power up */
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else {
> + memcpy(cmd.args,
> + "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01",
> + 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result
> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> + cmd.args[4] << 0;
> + dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> + cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +
> +

... yet, looking on what you've done, it seems that you're actually
adding support for a different tuner at the si2157 driver.

If this is the case, this should be on a separate patch, and in a way
that it will become clear that it won't break support for any existing
device.

Also, please remove the dead code, instead of commenting it out.

> #ifdef CONFIG_MEDIA_CONTROLLER
> if (cfg->mdev) {
> dev->mdev = cfg->mdev;
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index 2bda903358da..0f4090e184e9 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -28,6 +28,7 @@ struct si2157_dev {
> u8 chiptype;
> u8 if_port;
> u32 if_frequency;
> + u32 chip_id;
> struct delayed_work stat_work;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> @@ -43,6 +44,13 @@ struct si2157_dev {
> #define SI2157_CHIPTYPE_SI2141 2
> #define SI2157_CHIPTYPE_SI2177 3
>
> +#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)
> +#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> +#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> +
> /* firmware command struct */
> #define SI2157_ARGLEN 30
> struct si2157_cmd {
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..5b7a00cdcbd8 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1206,7 +1206,50 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
> struct si2168_config si2168_config;
> struct i2c_adapter *adapter;
>
> - dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
> + //dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
> + dev_dbg(&intf->dev, "%s adap->id=%d\n", __func__, adap->id);

Why did you do such change? dev_dbg can already print the function, and
much more. See:

https://lwn.net/Articles/434833/

> +
> + /* I2C master bus 2 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 1,3 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* set gpio11 low */
> + ret = af9035_wr_reg_mask(d, 0xd8d4, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d5, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d3, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* Tuner enable using gpiot2_en, gpiot2_on and gpiot2_o (reset) */
> + ret = af9035_wr_reg_mask(d, 0xd8b8, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b9, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + msleep(200);
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> + if (ret < 0)
> + goto err;

The above seems specific for your device. You need to check if the device
is USB_VID_DEXATEK, running the code only on such case.

>
> /* I2C master bus 2 clock speed 300k */
> ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> @@ -2118,6 +2161,8 @@ static const struct usb_device_id af9035_id_table[] = {
>
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> &it930x_props, "ITE 9303 Generic", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },


Thanks,
Mauro

2019-10-02 14:15:40

by Gonsolo

Subject: [PATCH] si2157: Add support for Logilink VG0022A.

Dear Mauro!

Thanks for your reply, I tried to address most of your concerns,
so please bear with me (especially this is my first email with
git send-email). :)

> First of all, don't attach a patch. Instead, just send it with a decent
> emailer (with won't mangle whitespaces) or use git send-email...

Done.

> You shouldn't just blindly comment out some code, as this will very likely
> break support for all other devices supported by the driver...

Done. I extracted power_up into its own function, chip querying is now
done in probe. I don't have enough knowledge about hardware to do the
right thing on my own. If there is anybody willing to guide me I will
spend some time on it.

The original patch where the problem is discussed is
https://lkml.kernel.org/lkml/[email protected]

> ... yet, looking on what you've done, it seems that you're actually
> adding support for a different tuner at the si2157 driver.
> If this is the case, this should be on a separate patch, and in a way
> that it will become clear that it won't break support for any existing
> device.

I'm not entirely sure how to split this up. Can you give some advice?

> Why did you do such change? dev_dbg can already print the function, and
> much more. See:

Thanks for the link. I removed these lines.

> The above seems specific for your device. You need to check if the device
> is USB_VID_DEXATEK, running the code only on such case.

Done. Though I'm not sure whether I did it the right way.

Thanks for all the advice, I hope this will be included eventually.
g

2019-10-02 14:17:33

by Gonsolo

Subject: [PATCH] si2157: Add support for Logilink VG0022A.

---
drivers/media/tuners/si2157.c | 68 +++++++++++++++++----------
drivers/media/tuners/si2157_priv.h | 1 +
drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++++++++++++++++++
3 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..8f9df2485d51 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
return ret;
}

+static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
+{
+ struct si2157_cmd cmd;
+
+ if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
+ memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
+ cmd.wlen = 9;
+ } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
+ memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
+ cmd.wlen = 10;
+ } else {
+ memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
+ cmd.wlen = 15;
+ }
+ cmd.rlen = 1;
+ return si2157_cmd_execute(client, &cmd);
+}
+
static int si2157_init(struct dvb_frontend *fe)
{
struct i2c_client *client = fe->tuner_priv;
@@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
struct si2157_cmd cmd;
const struct firmware *fw;
const char *fw_name;
- unsigned int uitmp, chip_id;
+ unsigned int uitmp;

dev_dbg(&client->dev, "\n");

@@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
if (uitmp == dev->if_frequency / 1000)
goto warm;

- /* power up */
- if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
- memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
- cmd.wlen = 9;
- } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
- memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
- cmd.wlen = 10;
- } else {
- memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
- cmd.wlen = 15;
- }
- cmd.rlen = 1;
- ret = si2157_cmd_execute(client, &cmd);
+ ret = si2157_power_up(dev, client);
if (ret)
goto err;

@@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}

- /* query chip revision */
- memcpy(cmd.args, "\x02", 1);
- cmd.wlen = 1;
- cmd.rlen = 13;
- ret = si2157_cmd_execute(client, &cmd);
- if (ret)
- goto err;
-
- chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
- cmd.args[4] << 0;
-
#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
@@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)

- switch (chip_id) {
+ switch (dev->chip_id) {
case SI2158_A20:
case SI2148_A20:
fw_name = SI2158_A20_FIRMWARE;
@@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
fe->tuner_priv = client;

+ ret = si2157_power_up(dev, client);
+ if (ret)
+ goto err;
+ /* query chip revision */
+ /* hack: do it here because after the si2168 gets 0101, commands will
+ * still be executed here but no result
+ */
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err_kfree;
+ dev->chip_id = cmd.args[1] << 24 |
+ cmd.args[2] << 16 |
+ cmd.args[3] << 8 |
+ cmd.args[4] << 0;
+ dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
+ cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
+
+
#ifdef CONFIG_MEDIA_CONTROLLER
if (cfg->mdev) {
dev->mdev = cfg->mdev;
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 2bda903358da..9ab7c88b01b4 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -28,6 +28,7 @@ struct si2157_dev {
u8 chiptype;
u8 if_port;
u32 if_frequency;
+ u32 chip_id;
struct delayed_work stat_work;

#if defined(CONFIG_MEDIA_CONTROLLER)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..83e243df59b9 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1246,6 +1246,51 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

msleep(200);

+ if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK) {
+
+ ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ /* I2C master bus 2 clock speed 300k */
+ ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ if (ret < 0)
+ goto err;
+
+ /* I2C master bus 1,3 clock speed 300k */
+ ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ if (ret < 0)
+ goto err;
+
+ /* set gpio11 low */
+ ret = af9035_wr_reg_mask(d, 0xd8d4, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ ret = af9035_wr_reg_mask(d, 0xd8d5, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ ret = af9035_wr_reg_mask(d, 0xd8d3, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ /* Tuner enable using gpiot2_en, gpiot2_on and gpiot2_o (reset) */
+ ret = af9035_wr_reg_mask(d, 0xd8b8, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ ret = af9035_wr_reg_mask(d, 0xd8b9, 0x01, 0x01);
+ if (ret < 0)
+ goto err;
+
+ ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
+ if (ret < 0)
+ goto err;
+
+ msleep(200);
+ }
+
ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
if (ret < 0)
goto err;
@@ -2119,6 +2164,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }
--
2.20.1

2019-10-02 14:52:23

by Sean Young

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

On Wed, Oct 02, 2019 at 04:13:59PM +0200, Gon Solo wrote:

You need a message and a Signed-off-by: here.

> ---
> drivers/media/tuners/si2157.c | 68 +++++++++++++++++----------
> drivers/media/tuners/si2157_priv.h | 1 +
> drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++++++++++++++++++
> 3 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..8f9df2485d51 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
> return ret;
> }
>
> +static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
> +{
> + struct si2157_cmd cmd;
> +
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> + memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
> + cmd.wlen = 10;
> + } else {
> + memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + return si2157_cmd_execute(client, &cmd);
> +}
> +
> static int si2157_init(struct dvb_frontend *fe)
> {
> struct i2c_client *client = fe->tuner_priv;
> @@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (uitmp == dev->if_frequency / 1000)
> goto warm;
>
> - /* power up */
> - if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> - memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> - cmd.wlen = 9;
> - } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> - memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
> - cmd.wlen = 10;
> - } else {
> - memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> - cmd.wlen = 15;
> - }
> - cmd.rlen = 1;
> - ret = si2157_cmd_execute(client, &cmd);
> + ret = si2157_power_up(dev, client);
> if (ret)
> goto err;
>
> @@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - /* query chip revision */
> - memcpy(cmd.args, "\x02", 1);
> - cmd.wlen = 1;
> - cmd.rlen = 13;
> - ret = si2157_cmd_execute(client, &cmd);
> - if (ret)
> - goto err;
> -
> - chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> - cmd.args[4] << 0;
> -
> #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> @@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> - switch (chip_id) {
> + switch (dev->chip_id) {
> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + ret = si2157_power_up(dev, client);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result

I don't understand. What problem are you seeing here? Why can't you do a
query chip revision first?

This needs resolving of course.

> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> + cmd.args[4] << 0;
> + dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> + cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +
> +
> #ifdef CONFIG_MEDIA_CONTROLLER
> if (cfg->mdev) {
> dev->mdev = cfg->mdev;
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index 2bda903358da..9ab7c88b01b4 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -28,6 +28,7 @@ struct si2157_dev {
> u8 chiptype;
> u8 if_port;
> u32 if_frequency;
> + u32 chip_id;
> struct delayed_work stat_work;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..83e243df59b9 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1246,6 +1246,51 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
>
> msleep(200);
>
> + if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK) {
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 2 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 1,3 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* set gpio11 low */
> + ret = af9035_wr_reg_mask(d, 0xd8d4, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d5, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d3, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* Tuner enable using gpiot2_en, gpiot2_on and gpiot2_o (reset) */
> + ret = af9035_wr_reg_mask(d, 0xd8b8, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b9, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + msleep(200);
> + }
> +
> ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> if (ret < 0)
> goto err;
> @@ -2119,6 +2164,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }
> --
> 2.20.1

2019-10-02 15:03:20

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

> You need a message and a Signed-off-by: here.

Ok, I'll try to get that right the next time.

> > + ret = si2157_power_up(dev, client);
> > + if (ret)
> > + goto err;
> > + /* query chip revision */
> > + /* hack: do it here because after the si2168 gets 0101, commands will
> > + * still be executed here but no result
>
> I don't understand. What problem are you seeing here? Why can't you do a
> query chip revision first?

This was explained here: https://lkml.org/lkml/2017/3/15/778. To quote:

If the si2157 is behind a e.g. si2168, the si2157 will at least in
some situations not be readable after the si268 got the command 0101.
It still accepts commands but the answer is just ffffff. So read the
chip id before that so the information is not lost.

The following line in kernel output is a symptome of that problem:
si2157 7-0063: unknown chip version Si21255-\xffffffff\xffffffff\xffffffff

> This needs resolving of course.

I hope so. :)

g

2019-10-02 15:55:16

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Wed, 2 Oct 2019 16:13:59 +0200
Gon Solo <[email protected]> escreveu:


All patches should have a description at the beginning of the e-mail
body, even the trivial ones.


You also forgot to add your Signed-off-by.

> ---
> drivers/media/tuners/si2157.c | 68 +++++++++++++++++----------
> drivers/media/tuners/si2157_priv.h | 1 +
> drivers/media/usb/dvb-usb-v2/af9035.c | 47 ++++++++++++++++++
> 3 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..8f9df2485d51 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -66,6 +66,24 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
> return ret;
> }
>
> +static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
> +{
> + struct si2157_cmd cmd;
> +
> + if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> + memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> + cmd.wlen = 9;
> + } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> + memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
> + cmd.wlen = 10;
> + } else {
> + memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> + cmd.wlen = 15;
> + }
> + cmd.rlen = 1;
> + return si2157_cmd_execute(client, &cmd);
> +}
> +
> static int si2157_init(struct dvb_frontend *fe)
> {
> struct i2c_client *client = fe->tuner_priv;
> @@ -75,7 +93,7 @@ static int si2157_init(struct dvb_frontend *fe)
> struct si2157_cmd cmd;
> const struct firmware *fw;
> const char *fw_name;
> - unsigned int uitmp, chip_id;
> + unsigned int uitmp;
>
> dev_dbg(&client->dev, "\n");
>
> @@ -93,19 +111,7 @@ static int si2157_init(struct dvb_frontend *fe)
> if (uitmp == dev->if_frequency / 1000)
> goto warm;
>
> - /* power up */
> - if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
> - memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
> - cmd.wlen = 9;
> - } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
> - memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
> - cmd.wlen = 10;
> - } else {
> - memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
> - cmd.wlen = 15;
> - }
> - cmd.rlen = 1;
> - ret = si2157_cmd_execute(client, &cmd);
> + ret = si2157_power_up(dev, client);
> if (ret)
> goto err;

Ok, you're moving the power-op code to a function. That's OK, but
the rule is one patch per functional change.

So, the first patch in this series should be the one moving the
power up code to a separate function, e. g. the e-mail would be
something like:

Subject: [PATCH 1/3] media: si2157: move power up code to a function

On some devices, we need to power up the device on other places,
so move the code to a separate function.

Signed-off-by: your name <your@email>

...

>
> @@ -118,17 +124,6 @@ static int si2157_init(struct dvb_frontend *fe)
> goto err;
> }
>
> - /* query chip revision */
> - memcpy(cmd.args, "\x02", 1);
> - cmd.wlen = 1;
> - cmd.rlen = 13;
> - ret = si2157_cmd_execute(client, &cmd);
> - if (ret)
> - goto err;
> -
> - chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> - cmd.args[4] << 0;
> -
> #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
> #define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
> #define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)
> @@ -137,7 +132,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
>
> - switch (chip_id) {
> + switch (dev->chip_id) {
> case SI2158_A20:
> case SI2148_A20:
> fw_name = SI2158_A20_FIRMWARE;
> @@ -456,6 +451,27 @@ static int si2157_probe(struct i2c_client *client,
> memcpy(&fe->ops.tuner_ops, &si2157_ops, sizeof(struct dvb_tuner_ops));
> fe->tuner_priv = client;
>
> + ret = si2157_power_up(dev, client);
> + if (ret)
> + goto err;
> + /* query chip revision */
> + /* hack: do it here because after the si2168 gets 0101, commands will
> + * still be executed here but no result
> + */
> + memcpy(cmd.args, "\x02", 1);
> + cmd.wlen = 1;
> + cmd.rlen = 13;
> + ret = si2157_cmd_execute(client, &cmd);
> + if (ret)
> + goto err_kfree;
> + dev->chip_id = cmd.args[1] << 24 |
> + cmd.args[2] << 16 |
> + cmd.args[3] << 8 |
> + cmd.args[4] << 0;
> + dev_info(&client->dev, "found a 'Silicon Labs Si21%d-%c%c%c'\n",
> + cmd.args[2], cmd.args[1], cmd.args[3], cmd.args[4]);
> +
> +

Why the extra blank line?

The above code seems to be a separate issue and should be handled
in separate.

I suspect, however, that the issue is actually at the bridge driver.

You should probably open the I2C gate before being able to talk
with it.

> #ifdef CONFIG_MEDIA_CONTROLLER
> if (cfg->mdev) {
> dev->mdev = cfg->mdev;
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index 2bda903358da..9ab7c88b01b4 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -28,6 +28,7 @@ struct si2157_dev {
> u8 chiptype;
> u8 if_port;
> u32 if_frequency;
> + u32 chip_id;
> struct delayed_work stat_work;
>
> #if defined(CONFIG_MEDIA_CONTROLLER)
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..83e243df59b9 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1246,6 +1246,51 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
>
> msleep(200);
>
> + if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK) {
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 2 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* I2C master bus 1,3 clock speed 300k */
> + ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + if (ret < 0)
> + goto err;
> +
> + /* set gpio11 low */
> + ret = af9035_wr_reg_mask(d, 0xd8d4, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d5, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8d3, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + /* Tuner enable using gpiot2_en, gpiot2_on and gpiot2_o (reset) */
> + ret = af9035_wr_reg_mask(d, 0xd8b8, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b9, 0x01, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
> + if (ret < 0)
> + goto err;
> +
> + msleep(200);

Hmm.... you are setting bit 1 of 0xd8b7 to 0 here...

> + }
> +
> ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
> if (ret < 0)
> goto err;

Then setting it to 1 here.

Is this a reset pin? If so, perhaps you need to do something like:

ret = af9035_wr_reg_mask(d, 0xd8b7, 0x00, 0x01);
if (ret < 0)
goto err;

msleep(200);

ret = af9035_wr_reg_mask(d, 0xd8b7, 0x01, 0x01);
if (ret < 0)
goto err;

msleep(200);

In order to wait for the reset to finish and be able to talk with the
tuner.


> @@ -2119,6 +2164,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }



Thanks,
Mauro

2019-10-02 16:01:08

by Sean Young

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

On Wed, Oct 02, 2019 at 04:44:24PM +0200, Gonsolo wrote:
> Hi!
>
> > You need a message and a Signed-off-by: here.
>
> Ok, I'll try to get that right the next time.
>
> > > + ret = si2157_power_up(dev, client);
> > > + if (ret)
> > > + goto err;
> > > + /* query chip revision */
> > > + /* hack: do it here because after the si2168 gets 0101, commands will
> > > + * still be executed here but no result
> >
> > I don't understand. What problem are you seeing here? Why can't you do a
> > query chip revision first?
>
> This was explained here: https://lkml.org/lkml/2017/3/15/778. To quote:

Antti has some great suggestions in that thread:
https://lkml.org/lkml/2017/5/24/245

Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a
logic analyser.


Sean

2019-10-02 16:12:13

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

> Antti has some great suggestions in that thread:
> https://lkml.org/lkml/2017/5/24/245
>
> Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a
> logic analyser.

I read that thread. Unfortunately I'm not a hardware engineer nor do I
have access to a logic analyser, just the device and a remote hope not
to keep my custom 4.13 kernel forever or to have to buy a new DVB T2
device. :(
In the above thread it is mentioned that even the Windows driver
receives the ffff's so maybe it is a hardware bug?

I'd love to see this driver upstream but I have no idea how to
proceed. Any suggestions?

--
g

2019-10-02 17:58:45

by JP

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi all.

On 10/2/19 5:21 PM, Gonsolo wrote:
> Hi!
>
>> Antti has some great suggestions in that thread:
>> https://lkml.org/lkml/2017/5/24/245
>>
>> Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a
>> logic analyser.
> I read that thread. Unfortunately I'm not a hardware engineer nor do I
> have access to a logic analyser, just the device and a remote hope not
> to keep my custom 4.13 kernel forever or to have to buy a new DVB T2
> device. :(
> In the above thread it is mentioned that even the Windows driver
> receives the ffff's so maybe it is a hardware bug?
Looks like it is:
http://lkml.iu.edu/hypermail/linux/kernel/1710.3/03205.html
>
> I'd love to see this driver upstream but I have no idea how to
> proceed. Any suggestions?
>

2019-10-02 18:51:15

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Wed, 2 Oct 2019 19:23:02 +0200
JP <[email protected]> escreveu:

> Hi all.
>
> On 10/2/19 5:21 PM, Gonsolo wrote:
> > Hi!
> >
> >> Antti has some great suggestions in that thread:
> >> https://lkml.org/lkml/2017/5/24/245
> >>
> >> Also note https://lkml.org/lkml/2017/5/26/357 if you have access to a
> >> logic analyser.
> > I read that thread. Unfortunately I'm not a hardware engineer nor do I
> > have access to a logic analyser, just the device and a remote hope not
> > to keep my custom 4.13 kernel forever or to have to buy a new DVB T2
> > device. :(
> > In the above thread it is mentioned that even the Windows driver
> > receives the ffff's so maybe it is a hardware bug?
> Looks like it is:
> http://lkml.iu.edu/hypermail/linux/kernel/1710.3/03205.html

Hmm... changing the pull-up register will very likely change the
timings.

There's a logic at the driver that changes the I2C bus speed to
300k (with is non-standard):


/* I2C master bus 2 clock speed 300k */
ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
if (ret < 0)
goto err;

/* I2C master bus 1,3 clock speed 300k */
ret = af9035_wr_reg(d, 0x00f103, 0x07);
if (ret < 0)
goto err;

Perhaps if we reduce the bus speed to 100k, the device will work
without the hacks.

I don't have af9035 datasheets. Perhaps Antti could shed some
light about how to change the speed to 100k, but on a quick search
at the Internet, I found this:

https://lore.kernel.org/linux-media/1312539895.2763.33.camel@Jason-Linux/

With has a comment that explains how the I2C speed is calculated on those
ITE devices:

#define p_reg_one_cycle_counter_tuner 0xF103

/* Define I2C master speed, the default value 0x07 means 366KHz (1000000000 / (24.4 * 16 * User_I2C_SPEED)). */
#define User_I2C_SPEED 0x07

error =
it9135_write_reg(data, 0, PROCESSOR_LINK,
p_reg_one_cycle_counter_tuner, User_I2C_SPEED);

So, in summary, the value for the I2C speed register is given by:

I2C_speed_register = (1000000000 / (24.4 * 16 * I2C_speed))

So, for 100 kbps, the I2C speed register should be set with a value
close to ~25,6.

Doing the reverse math, we have:

I2C_speed_register = 25 -> I2C_speed = 102,46 kbps
I2C_speed_register = 26 -> I2C_speed = 98,52 kbps

So, if we do:

/* I2C master bus 2 clock speed ~100k */
ret = af9035_wr_reg(d, 0x00f6a7, 26);
if (ret < 0)
goto err;

/* I2C master bus 1,3 clock speed ~100k */
ret = af9035_wr_reg(d, 0x00f103, 26);
if (ret < 0)
goto err;

The bus speed will reduce to 98,52 kbps, with is a typical nominal
value for I2C tuners and other devices. With that, the device should
probably work fine without needing to replace the pull up resistor.

Ok, tuner firmware load would be ~3,7 times slower, but this is
something that we do just once need a firmware).

All other I2C messages are short enough to not cause any real difference.


Could you please test the enclosing patch and see if, with that, you
can remove the hacks you added for the tuner probe to work?

Regards,
Mauro

[PATCH] media: af9035: slow down I2C bus speed on it930x devices

We have a few reports about tuner probing instability with
some it930x devices.

As it is better safe than sorry, let's slow down the I2C,
using the formula found at an old patch:

https://lore.kernel.org/linux-media/1312539895.2763.33.camel@Jason-Linux/

Signed-off-by: Mauro Carvalho Chehab <[email protected]>

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..df2c75b84be8 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,9 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/* I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed)) */
+#define I2C_SPEED_REGISTER 26
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1211,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

2019-10-03 08:07:17

by Gonsolo

Subject: [PATCH 0/1] Testing timing patch for Logilink VG0022A

Hi!

I tested the following patch on a Ubuntu Disco Dingo kernel.
Unfortunately it fails with the following error message:

[ 3.715233] dvb_usb_af9035 2-1:1.0: prechip_version=83 chip_version=01 chip_type=9306
[ 3.718385] usb 2-1: dvb_usb_v2: found a 'Logilink VG0022A' in warm state
[ 3.718479] usb 2-1: dvb_usb_v2: will pass the complete MPEG2 transport stream to the software demuxer
[ 3.718544] dvbdev: DVB: registering new adapter (Logilink VG0022A)
[ 3.720006] usbcore: registered new interface driver usbserial_generic
[ 3.720018] usbserial: USB Serial support registered for generic
[ 3.729819] si2168 1-0067: probe failed = -5
[ 3.729826] si2168: probe of 1-0067 failed with error -5
[ 3.731138] usbcore: registered new interface driver dvb_usb_af9035
[ 3.732728] usbcore: registered new interface driver btusb
[ 3.742576] media: Linux media interface: v0.10

Did I forget something or is it just not working?

Anyway, thanks for your effort. Can we try something else?

g


Gon Solo (1):
Test Mauros timing patch.

drivers/media/tuners/si2157.c | 32 ++++++++++++++++-----------
drivers/media/usb/dvb-usb-v2/af9035.c | 14 ++++++++----
2 files changed, 29 insertions(+), 17 deletions(-)

--
2.20.1

2019-10-03 08:08:57

by Gonsolo

Subject: [PATCH 1/1] Test Mauros timing patch.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/tuners/si2157.c | 32 ++++++++++++++++-----------
drivers/media/usb/dvb-usb-v2/af9035.c | 14 ++++++++----
2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index d389f1fc237a..9e20727b7e84 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -75,6 +75,24 @@ static int si2157_cmd_execute(struct i2c_client *client, struct si2157_cmd *cmd)
return ret;
}

+static int si2157_power_up(struct si2157_dev *dev, struct i2c_client *client)
+{
+ struct si2157_cmd cmd;
+
+ if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
+ memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
+ cmd.wlen = 9;
+ } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
+ memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
+ cmd.wlen = 10;
+ } else {
+ memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
+ cmd.wlen = 15;
+ }
+ cmd.rlen = 1;
+ return si2157_cmd_execute(client, &cmd);
+}
+
static int si2157_init(struct dvb_frontend *fe)
{
struct i2c_client *client = fe->tuner_priv;
@@ -102,19 +120,7 @@ static int si2157_init(struct dvb_frontend *fe)
if (uitmp == dev->if_frequency / 1000)
goto warm;

- /* power up */
- if (dev->chiptype == SI2157_CHIPTYPE_SI2146) {
- memcpy(cmd.args, "\xc0\x05\x01\x00\x00\x0b\x00\x00\x01", 9);
- cmd.wlen = 9;
- } else if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
- memcpy(cmd.args, "\xc0\x00\x0d\x0e\x00\x01\x01\x01\x01\x03", 10);
- cmd.wlen = 10;
- } else {
- memcpy(cmd.args, "\xc0\x00\x0c\x00\x00\x01\x01\x01\x01\x01\x01\x02\x00\x00\x01", 15);
- cmd.wlen = 15;
- }
- cmd.rlen = 1;
- ret = si2157_cmd_execute(client, &cmd);
+ ret = si2157_power_up(dev, client);
if (ret)
goto err;

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 80d3bd3a0f24..c4d4994e0079 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1207,6 +1207,9 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/* I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed)) */
+#define I2C_SPEED_REGISTER 26
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1575,13 +1578,13 @@ static int it930x_tuner_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed ~100k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

@@ -2128,6 +2131,9 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ }
};
MODULE_DEVICE_TABLE(usb, af9035_id_table);
--
2.20.1

2019-10-03 10:16:54

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

> Could you please test the enclosing patch and see if, with that, you
> can remove the hacks you added for the tuner probe to work?

I tested again on a vanilla media_tree with Mauro's patch attached.
Doesn't work. Dmesg output:

[ 0.788387] kernel: usb 1-1: new high-speed USB device number 2
using ehci-pci
[ 0.792384] kernel: usb 2-1: new high-speed USB device number 2
using xhci_hcd
[ 0.944937] kernel: usb 2-1: New USB device found, idVendor=1d19,
idProduct=0100, bcdDevice= 1.00
[ 0.944939] kernel: usb 2-1: New USB device strings: Mfr=1,
Product=2, SerialNumber=3
[ 0.944940] kernel: usb 2-1: Product: TS Aggregator
[ 0.944941] kernel: usb 2-1: Manufacturer: ITE Tech., Inc.
[ 0.944942] kernel: usb 2-1: SerialNumber: AF0102020700001

I then also used the following (additional) patch:

@@ -2119,6 +2122,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }

Which gives the following output:

[ 5.380989] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[ 5.380991] si2168 1-0067: firmware version: B 4.0.2
[ 5.381013] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon
Labs Si2168)...
[ 5.381018] dvbdev: dvb_create_media_entity: media entity 'Silicon
Labs Si2168' registered.
[ 5.390058] checking generic (e0000000 410000) vs hw (e0000000 10000000)
[ 5.390062] fb0: switching to inteldrmfb from EFI VGA
[ 5.390268] Console: switching to colour dummy device 80x25
[ 5.390281] i915 0000:00:02.0: vgaarb: deactivate vga console
[ 5.393438] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158
successfully attached

But when I try to use VLC I get the following:

[ 457.677363] si2168 1-0067: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 458.631034] si2168 1-0067: firmware version: B 4.0.11
[ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff

Now I'm trying other timings.

Thanks,
g

2019-10-03 10:58:59

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

Boot time:

> [ 5.380991] si2168 1-0067: firmware version: B 4.0.2

When starting VLC:

> [ 457.677363] si2168 1-0067: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [ 458.631034] si2168 1-0067: firmware version: B 4.0.11
> [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff

There are two different firmware versions, 4.0.2 and 4.0.11. Is that expected?

--
g

2019-10-03 11:07:37

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 12:13:33 +0200
Gonsolo <[email protected]> escreveu:

> Hi!
>
> > Could you please test the enclosing patch and see if, with that, you
> > can remove the hacks you added for the tuner probe to work?
>
> I tested again on a vanilla media_tree with Mauro's patch attached.
> Doesn't work. Dmesg output:
>
> [ 0.788387] kernel: usb 1-1: new high-speed USB device number 2
> using ehci-pci
> [ 0.792384] kernel: usb 2-1: new high-speed USB device number 2
> using xhci_hcd
> [ 0.944937] kernel: usb 2-1: New USB device found, idVendor=1d19,
> idProduct=0100, bcdDevice= 1.00
> [ 0.944939] kernel: usb 2-1: New USB device strings: Mfr=1,
> Product=2, SerialNumber=3
> [ 0.944940] kernel: usb 2-1: Product: TS Aggregator
> [ 0.944941] kernel: usb 2-1: Manufacturer: ITE Tech., Inc.
> [ 0.944942] kernel: usb 2-1: SerialNumber: AF0102020700001
>
> I then also used the following (additional) patch:
>
> @@ -2119,6 +2122,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }
>
> Which gives the following output:
>
> [ 5.380989] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
> [ 5.380991] si2168 1-0067: firmware version: B 4.0.2
> [ 5.381013] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon
> Labs Si2168)...
> [ 5.381018] dvbdev: dvb_create_media_entity: media entity 'Silicon
> Labs Si2168' registered.
> [ 5.390058] checking generic (e0000000 410000) vs hw (e0000000 10000000)
> [ 5.390062] fb0: switching to inteldrmfb from EFI VGA
> [ 5.390268] Console: switching to colour dummy device 80x25
> [ 5.390281] i915 0000:00:02.0: vgaarb: deactivate vga console
> [ 5.393438] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158
> successfully attached

Ok. It sounds that the issues you're facing are indeed related to timing
conditions.

>
> But when I try to use VLC I get the following:
>
> [ 457.677363] si2168 1-0067: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [ 458.631034] si2168 1-0067: firmware version: B 4.0.11
> [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff
>
> Now I'm trying other timings.

Returning 0xff, 0xff, 0xff, ... usually means that the tuner chip didn't
respond in time.

This could indicate:

1) The device requires more time to go to sane state after firmware
load and/or device reset/power up;

2) Tuner may be using I2C clock stretching, but the bridge doesn't
support it.

3) The clock used at the I2C bus is still too high;

4) The tuner is hidden by an I2C gate.


I think that using the standard I2C bus clock of 100kbps should be
enough.

I2C clock stretching seems to be unlikely for a command that it is
just getting the device's version.

What seems more likely is that this device may need some time after
firmware load to start working.

So, I would add a msleep() somewhere after the firmware update.

Thanks,
Mauro

2019-10-03 11:21:26

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 12:57:50 +0200
Gonsolo <[email protected]> escreveu:

> Hi!
>
> Boot time:
>
> > [ 5.380991] si2168 1-0067: firmware version: B 4.0.2
>
> When starting VLC:
>
> > [ 457.677363] si2168 1-0067: downloading firmware from file
> > 'dvb-demod-si2168-b40-01.fw'
> > [ 458.631034] si2168 1-0067: firmware version: B 4.0.11
> > [ 458.650309] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff
>
> There are two different firmware versions, 4.0.2 and 4.0.11. Is that expected?

It means that there's a firmware stored at the device's eeprom
(version 4.0.2). When the driver starts, it downloads a newer firmware
from the file dvb-demod-si2168-b40-01.fw.

Btw, could you please try the enclosed hack and post the results?

Thanks,
Mauro

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..3ccfd602934b 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -76,6 +76,7 @@ static int si2157_init(struct dvb_frontend *fe)
const struct firmware *fw;
const char *fw_name;
unsigned int uitmp, chip_id;
+ int i;

dev_dbg(&client->dev, "\n");

@@ -118,16 +119,32 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}

- /* query chip revision */
- memcpy(cmd.args, "\x02", 1);
- cmd.wlen = 1;
- cmd.rlen = 13;
- ret = si2157_cmd_execute(client, &cmd);
- if (ret)
- goto err;
+ for (i = 0; i < 10; i++) {
+ /* query chip revision */
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
+ cmd.args[4] << 0;

- chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
- cmd.args[4] << 0;
+ if (chip_id != 0xffffffff)
+ break;
+
+ msleep(10);
+ }
+
+ if (i)
+ dev_info(&client->dev, "Needed to wait %i ms to get chip version", i * 10);
+
+ if (chip_id == 0xffffffff) {
+ dev_err(&client->dev, "Unable to retrieve chip version\n");
+ ret = -EINVAL;
+ goto err;
+ }

#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)

2019-10-03 11:43:04

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

> It means that there's a firmware stored at the device's eeprom
> (version 4.0.2). When the driver starts, it downloads a newer firmware
> from the file dvb-demod-si2168-b40-01.fw.

Thanks for the explanation.

> Btw, could you please try the enclosed hack and post the results?

Will do in a second.

FWIW, this hack worked:

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..28a3a4f1640e 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
#define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
#define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
#define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)

switch (chip_id) {
case SI2158_A20:
@@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
case SI2177_A30:
fw_name = SI2157_A30_FIRMWARE;
break;
+ case GONZO:
+ dev_info(&client->dev, "trying null\n");
+ fw_name = NULL;
+ break;
case SI2157_A30:
case SI2147_A30:
case SI2146_A10:

2019-10-03 12:03:16

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!


> Btw, could you please try the enclosed hack and post the results?

[ 210.178948] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
[ 212.404011] si2168 1-0067: firmware version: B 4.0.25
[ 212.656467] si2157 2-0063: Needed to wait 100 ms to get chip version
[ 212.656470] si2157 2-0063: Unable to retrieve chip version

:(

g

2019-10-03 12:14:16

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 14:01:43 +0200
Gon Solo <[email protected]> escreveu:

> Hi!
>
>
> > Btw, could you please try the enclosed hack and post the results?
>
> [ 210.178948] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
> [ 212.404011] si2168 1-0067: firmware version: B 4.0.25
> [ 212.656467] si2157 2-0063: Needed to wait 100 ms to get chip version
> [ 212.656470] si2157 2-0063: Unable to retrieve chip version

well, you could try to increase the timeout - although 100 ms seems a lot
of time to me.

Thanks,
Mauro

2019-10-03 12:21:52

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> well, you could try to increase the timeout - although 100 ms seems a lot
> of time to me.

I tried 5s, still no change.

Would it be possible to include my patch, possibly with a warning like
"bogus chip version, trying with no firmware"?

g

2019-10-03 14:12:08

by Gonsolo

Subject: [PATCH 0/3] Add support for Logilink VG0022A

These three patches make the Logilink VG0022A work.
Being cheap hardware a bogus chip version is returned but this is catched in
the driver.

Gon Solo (3):
af9035: Better explain how i2c bus speed is computed.
s2157: Handle bogus chip version.
af9035: Add Logilink VG0022A id.

drivers/media/tuners/si2157.c | 19 ++++++++++++-------
drivers/media/usb/dvb-usb-v2/af9035.c | 13 +++++++++----
2 files changed, 21 insertions(+), 11 deletions(-)

--
2.20.1

2019-10-03 14:12:08

by Gonsolo

Subject: [PATCH 2/3] [PATCH] s2157: Handle bogus chip version.

On some hardware bogus chip version numbers are returned.
Try to continue without firmware.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/tuners/si2157.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..615bc8a75c64 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -129,13 +129,14 @@ static int si2157_init(struct dvb_frontend *fe)
chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
cmd.args[4] << 0;

- #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
- #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
- #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
+ #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
+ #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI_BOGUS (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)

switch (chip_id) {
case SI2158_A20:
@@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
case SI2177_A30:
fw_name = SI2157_A30_FIRMWARE;
break;
+ case SI_BOGUS:
+ dev_info(&client->dev, "Bogus chip version, trying with no firmware\n");
+ fw_name = NULL;
+ break;
case SI2157_A30:
case SI2147_A30:
case SI2146_A10:
--
2.20.1

2019-10-03 14:12:56

by Gonsolo

Subject: [PATCH 3/3] [PATCH] af9035: Add Logilink VG0022A id.

Original patch from Andreas Kemnade.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 7828f8567da0..a02cafadfb49 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -2122,6 +2122,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }
--
2.20.1

2019-10-03 14:14:34

by Gonsolo

Subject: [PATCH 1/3] [PATCH] af9035: Better explain how i2c bus speed is computed.

Original patch from Mauro Carvalho Chehab.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..7828f8567da0 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,9 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/* I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed)) */
+#define I2C_SPEED_REGISTER 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1211,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed ~300k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed ~300k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

--
2.20.1

2019-10-03 14:20:33

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 13:41:23 +0200
Gonsolo <[email protected]> escreveu:

> Hi!
>
> > It means that there's a firmware stored at the device's eeprom
> > (version 4.0.2). When the driver starts, it downloads a newer firmware
> > from the file dvb-demod-si2168-b40-01.fw.
>
> Thanks for the explanation.
>
> > Btw, could you please try the enclosed hack and post the results?
>
> Will do in a second.
>
> FWIW, this hack worked:
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..28a3a4f1640e 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
> #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
>
> switch (chip_id) {
> case SI2158_A20:
> @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> case SI2177_A30:
> fw_name = SI2157_A30_FIRMWARE;
> break;
> + case GONZO:
> + dev_info(&client->dev, "trying null\n");
> + fw_name = NULL;
> + break;
> case SI2157_A30:
> case SI2147_A30:
> case SI2146_A10:

What does it print with this hack?

Also, could you get the SI version after the reset code at
skip_fw_download, just after retrieving si2157 firmware version?

Thanks,
Mauro

2019-10-03 14:25:13

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 09:49:04 -0300
Mauro Carvalho Chehab <[email protected]> escreveu:

> Em Thu, 3 Oct 2019 13:41:23 +0200
> Gonsolo <[email protected]> escreveu:
>
> > Hi!
> >
> > > It means that there's a firmware stored at the device's eeprom
> > > (version 4.0.2). When the driver starts, it downloads a newer firmware
> > > from the file dvb-demod-si2168-b40-01.fw.
> >
> > Thanks for the explanation.
> >
> > > Btw, could you please try the enclosed hack and post the results?
> >
> > Will do in a second.
> >
> > FWIW, this hack worked:
> >
> > diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> > index e87040d6eca7..28a3a4f1640e 100644
> > --- a/drivers/media/tuners/si2157.c
> > +++ b/drivers/media/tuners/si2157.c
> > @@ -136,6 +136,7 @@ static int si2157_init(struct dvb_frontend *fe)
> > #define SI2147_A30 ('A' << 24 | 47 << 16 | '3' << 8 | '0' << 0)
> > #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> > #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> > + #define GONZO (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
> >
> > switch (chip_id) {
> > case SI2158_A20:
> > @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> > case SI2177_A30:
> > fw_name = SI2157_A30_FIRMWARE;
> > break;
> > + case GONZO:
> > + dev_info(&client->dev, "trying null\n");
> > + fw_name = NULL;
> > + break;
> > case SI2157_A30:
> > case SI2147_A30:
> > case SI2146_A10:
>
> What does it print with this hack?
>
> Also, could you get the SI version after the reset code at
> skip_fw_download, just after retrieving si2157 firmware version?

Maybe something like this would make it work?

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..86d945fd50b9 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -129,6 +129,28 @@ static int si2157_init(struct dvb_frontend *fe)
chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
cmd.args[4] << 0;

+ if (chip_id == 0xffffffff) {
+ /* reboot the tuner */
+ memcpy(cmd.args, "\x01\x01", 2);
+ cmd.wlen = 2;
+ cmd.rlen = 1;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ /* query chip revision */
+ memcpy(cmd.args, "\x02", 1);
+ cmd.wlen = 1;
+ cmd.rlen = 13;
+ ret = si2157_cmd_execute(client, &cmd);
+ if (ret)
+ goto err;
+
+ chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
+ cmd.args[4] << 0;
+
+ }
+
#define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 0)
#define SI2158_A20 ('A' << 24 | 58 << 16 | '2' << 8 | '0' << 0)
#define SI2148_A20 ('A' << 24 | 48 << 16 | '2' << 8 | '0' << 0)


Thanks,
Mauro

2019-10-03 14:29:54

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> Maybe something like this would make it work?

Nope. :(

[ 47.371022] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
[ 48.632824] si2168 1-0067: firmware version: B 4.0.25
[ 48.671268] si2157 2-0063: unknown chip version Si21255-\xff\xff\xff


g

2019-10-03 14:47:09

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Hi!

I tried downloading a new firmware via

case SI_BOGUS:
- dev_info(&client->dev, "Bogus chip version, trying
with no firmware\n");
- fw_name = NULL;
+ dev_info(&client->dev, "Bogus chip version, trying
with new firmware\n");
+ fw_name = SI2157_A30_FIRMWARE;
break;

which I downloaded from

+ //
https://github.com/CoreELEC/dvb-firmware/blob/master/firmware/dvb-tuner-si2157-a30-01.fw

resulting in

[ 209.312086] si2168 1-0067: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 211.535097] si2168 1-0067: firmware version: B 4.0.25
[ 211.554938] si2157 2-0063: Bogus chip version, trying with new firmware
[ 211.554944] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
[ 211.557978] si2157 2-0063: downloading firmware from file
'dvb-tuner-si2157-a30-01.fw'
[ 215.739092] si2157 2-0063: rebooting tuner...
[ 215.755271] si2157 2-0063: querying firmware version...
[ 215.760756] si2157 2-0063: firmware version: \xff.\xff.255

. So even with a new firmware the queried numbers are bogus.

g

2019-10-03 14:49:10

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 15:53:30 +0200
Gonsolo <[email protected]> escreveu:

> Hi!
>
> I tried downloading a new firmware via
>
> case SI_BOGUS:
> - dev_info(&client->dev, "Bogus chip version, trying
> with no firmware\n");
> - fw_name = NULL;
> + dev_info(&client->dev, "Bogus chip version, trying
> with new firmware\n");
> + fw_name = SI2157_A30_FIRMWARE;
> break;
>
> which I downloaded from
>
> + //
> https://github.com/CoreELEC/dvb-firmware/blob/master/firmware/dvb-tuner-si2157-a30-01.fw
>
> resulting in
>
> [ 209.312086] si2168 1-0067: downloading firmware from file
> 'dvb-demod-si2168-b40-01.fw'
> [ 211.535097] si2168 1-0067: firmware version: B 4.0.25
> [ 211.554938] si2157 2-0063: Bogus chip version, trying with new firmware
> [ 211.554944] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
> [ 211.557978] si2157 2-0063: downloading firmware from file
> 'dvb-tuner-si2157-a30-01.fw'
> [ 215.739092] si2157 2-0063: rebooting tuner...
> [ 215.755271] si2157 2-0063: querying firmware version...
> [ 215.760756] si2157 2-0063: firmware version: \xff.\xff.255
>
> . So even with a new firmware the queried numbers are bogus.

Try to reduce the bus speed to 10kbps
>
> g



Thanks,
Mauro

2019-10-03 14:56:41

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> Try to reduce the bus speed to 10kbps

Nope:

#define I2C_SPEED_REGISTER 260 // ~10k

[ 117.860961] si2168 1-0067: downloading firmware from file
'dvb-demod-si2168-b40-01.fw'
[ 118.958355] si2168 1-0067: firmware version: B 4.0.25
[ 118.968882] si2157 2-0063: Bogus chip version, trying with new firmware
[ 118.968888] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
[ 118.972005] si2157 2-0063: downloading firmware from file
'dvb-tuner-si2157-a30-01.fw'
[ 121.154130] si2157 2-0063: rebooting tuner...
[ 121.169626] si2157 2-0063: querying firmware version...
[ 121.172799] si2157 2-0063: firmware version: \xff.\xff.255
[ 121.172803] si2157 2-0063: querying chip revision...
[ 121.175911] si2157 2-0063: chip revision: 255.255.255.255

g



--
g

2019-10-03 15:05:37

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> So, I would add a msleep() somewhere after the firmware update.

I tried that to no avail:

release_firmware(fw);
+ msleep(1000);

[ 107.903918] si2157 2-0063: firmware version: \xff.\xff.255
[ 107.903920] si2157 2-0063: querying chip revision...
[ 107.906970] si2157 2-0063: chip revision: 255.255.255.255

--
g

2019-10-03 15:09:24

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 17:00:08 +0200
Gonsolo <[email protected]> escreveu:

> > So, I would add a msleep() somewhere after the firmware update.
>
> I tried that to no avail:
>
> release_firmware(fw);
> + msleep(1000);
>
> [ 107.903918] si2157 2-0063: firmware version: \xff.\xff.255
> [ 107.903920] si2157 2-0063: querying chip revision...
> [ 107.906970] si2157 2-0063: chip revision: 255.255.255.255
>

With the original patch you proposed, what are the logs?


Thanks,
Mauro

2019-10-03 15:19:26

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> With the original patch you proposed, what are the logs?

Which one do you mean? That one:

+ case SI_BOGUS:
+ dev_info(&client->dev, "Bogus chip version, trying
with no firmware\n");
+ fw_name = NULL;
+ break;

With this patch VLC is running but the chip revision number and
firmware version are still bogus.

Which means if you receive 0xffffffff you can force no firmware and it runs.

--
g

2019-10-03 16:06:15

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> With the original patch you proposed, what are the logs?

With the following patch applied to media_tree master:

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..4c1ab0b6876a 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -129,13 +129,14 @@ static int si2157_init(struct dvb_frontend *fe)
chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
cmd.args[4] << 0;

- #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
- #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
- #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
+ #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
+ #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
+ #define SI_BOGUS (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)

switch (chip_id) {
case SI2158_A20:
@@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
case SI2177_A30:
fw_name = SI2157_A30_FIRMWARE;
break;
+ case SI_BOGUS:
+ dev_info(&client->dev, "Bogus chip version, trying with no firmware\n");
+ fw_name = NULL;
+ break;
case SI2157_A30:
case SI2147_A30:
case SI2146_A10:
@@ -225,6 +230,7 @@ static int si2157_init(struct dvb_frontend *fe)

dev_info(&client->dev, "firmware version: %c.%c.%d\n",
cmd.args[6], cmd.args[7], cmd.args[8]);
warm:
/* init statistics in order signal app which are supported */
c->strength.len = 1;
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..a8d59cf06b1e 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,11 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/* I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed))
+ * 7 equals ~400k, 26 ~100k and 260 ~10k.
+ * */
+#define I2C_SPEED_REGISTER 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1213,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
if (ret < 0)
goto err;

@@ -2119,6 +2124,8 @@ static const struct usb_device_id af9035_id_table[] = {
/* IT930x devices */
{ DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
&it930x_props, "ITE 9303 Generic", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
{ }

the Messages at boot time are

[ 4.262882] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[ 4.262884] si2168 1-0067: firmware version: B 4.0.2
[ 4.262902] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)...
[ 4.262908] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered.
[ 4.289776] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached

and the messages when running vlc (successfully) are

[ 486.537128] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
[ 487.795436] si2168 1-0067: firmware version: B 4.0.25
[ 487.807614] si2157 2-0063: Bogus chip version, trying with no firmware
[ 487.807618] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
[ 487.833876] si2157 2-0063: firmware version: \xff.\xff.255

g

2019-10-03 16:13:31

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 18:03:36 +0200
Gon Solo <[email protected]> escreveu:

> > With the original patch you proposed, what are the logs?
>
> With the following patch applied to media_tree master:
>
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index e87040d6eca7..4c1ab0b6876a 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -129,13 +129,14 @@ static int si2157_init(struct dvb_frontend *fe)
> chip_id = cmd.args[1] << 24 | cmd.args[2] << 16 | cmd.args[3] << 8 |
> cmd.args[4] << 0;
>
> - #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
> - #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> - #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define SI2177_A30 ('A' << 24 | 77 << 16 | '3' << 8 | '0' << 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)
> + #define SI2146_A10 ('A' << 24 | 46 << 16 | '1' << 8 | '0' << 0)
> + #define SI2141_A10 ('A' << 24 | 41 << 16 | '1' << 8 | '0' << 0)
> + #define SI_BOGUS (255 << 24 | 255 << 16 | 255 << 8 | 255 << 0)
>
> switch (chip_id) {
> case SI2158_A20:
> @@ -148,6 +149,10 @@ static int si2157_init(struct dvb_frontend *fe)
> case SI2177_A30:
> fw_name = SI2157_A30_FIRMWARE;
> break;
> + case SI_BOGUS:
> + dev_info(&client->dev, "Bogus chip version, trying with no firmware\n");
> + fw_name = NULL;
> + break;
> case SI2157_A30:
> case SI2147_A30:
> case SI2146_A10:
> @@ -225,6 +230,7 @@ static int si2157_init(struct dvb_frontend *fe)
>
> dev_info(&client->dev, "firmware version: %c.%c.%d\n",
> cmd.args[6], cmd.args[7], cmd.args[8]);
> warm:
> /* init statistics in order signal app which are supported */
> c->strength.len = 1;
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..a8d59cf06b1e 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1197,6 +1197,11 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
> return ret;
> }
>
> +/* I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed))
> + * 7 equals ~400k, 26 ~100k and 260 ~10k.
> + * */
> +#define I2C_SPEED_REGISTER 7
> +
> static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
> {
> struct state *state = adap_to_priv(adap);
> @@ -1208,13 +1213,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
>
> dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);
>
> - /* I2C master bus 2 clock speed 300k */
> - ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
> + /* I2C master bus 2 clock speed */
> + ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_REGISTER);
> if (ret < 0)
> goto err;
>
> - /* I2C master bus 1,3 clock speed 300k */
> - ret = af9035_wr_reg(d, 0x00f103, 0x07);
> + /* I2C master bus 1,3 clock speed */
> + ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_REGISTER);
> if (ret < 0)
> goto err;
>
> @@ -2119,6 +2124,8 @@ static const struct usb_device_id af9035_id_table[] = {
> /* IT930x devices */
> { DVB_USB_DEVICE(USB_VID_ITETECH, USB_PID_ITETECH_IT9303,
> &it930x_props, "ITE 9303 Generic", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> { }
>
> the Messages at boot time are
>
> [ 4.262882] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
> [ 4.262884] si2168 1-0067: firmware version: B 4.0.2
> [ 4.262902] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)...
> [ 4.262908] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered.
> [ 4.289776] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached
>
> and the messages when running vlc (successfully) are
>
> [ 486.537128] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
> [ 487.795436] si2168 1-0067: firmware version: B 4.0.25
> [ 487.807614] si2157 2-0063: Bogus chip version, trying with no firmware
> [ 487.807618] si2157 2-0063: found a 'Silicon Labs Si21255-\xff\xff\xff'
> [ 487.833876] si2157 2-0063: firmware version: \xff.\xff.255

No, I mean with the first patch you sent to the ML, with the powerup
hack.


Thanks,
Mauro

2019-10-03 17:18:48

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> No, I mean with the first patch you sent to the ML, with the powerup
> hack.

Boot time:

[ 4.653257] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[ 4.653259] si2168 1-0067: firmware version: B 4.0.2
[ 4.653279] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)...
[ 4.653284] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered.
...
[ 4.694785] si2157 2-0063: found a 'Silicon Labs Si2147-A30'
[ 4.694787] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached
[ 4.717814] usb 2-1: dvb_usb_v2: 'Logilink VG0022A' successfully initialized and connected
[ 4.717880] usbcore: registered new interface driver dvb_usb_af9035

VLC time:

[ 175.490609] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
[ 176.746585] si2168 1-0067: firmware version: B 4.0.25
[ 176.781570] si2157 2-0063: firmware version: \xff.\xff.255

g

2019-10-03 18:53:46

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 18:23:26 +0200
Gon Solo <[email protected]> escreveu:

> > No, I mean with the first patch you sent to the ML, with the powerup
> > hack.
>
> Boot time:
>
> [ 4.653257] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
> [ 4.653259] si2168 1-0067: firmware version: B 4.0.2
> [ 4.653279] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)...
> [ 4.653284] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered.
> ...
> [ 4.694785] si2157 2-0063: found a 'Silicon Labs Si2147-A30'
> [ 4.694787] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached
> [ 4.717814] usb 2-1: dvb_usb_v2: 'Logilink VG0022A' successfully initialized and connected
> [ 4.717880] usbcore: registered new interface driver dvb_usb_af9035
>
> VLC time:
>
> [ 175.490609] si2168 1-0067: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
> [ 176.746585] si2168 1-0067: firmware version: B 4.0.25
> [ 176.781570] si2157 2-0063: firmware version: \xff.\xff.255

Weird... it sounds that, after si2168 has its firmware updated, it
starts interfering at si2157. Perhaps there's a bug at si2168 I2C
gate mux logic. Are you using a new Kernel like 5.2?

I guess the best is to enable the debug logs in order to see what's
happening on both cases.

You can enable all debug messages (after loading the modules) with:

# for i in $(cat /sys/kernel/debug/dynamic_debug/control |grep -E '(si21|af9035)' |cut -d' ' -f 1); do echo "file $i +p" >>/sys/kernel/debug/dynamic_debug/control; done

You could also try to disable the firmware upload at si2168 and see
if the si2157 reads will succeed.


Thanks,
Mauro

2019-10-03 18:57:38

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> Weird... it sounds that, after si2168 has its firmware updated, it
> starts interfering at si2157. Perhaps there's a bug at si2168 I2C
> gate mux logic. Are you using a new Kernel like 5.2?

Everything is based on git://linuxtv.org/media_tree.git which is at
5.4-rc1 right now.

> I guess the best is to enable the debug logs in order to see what's
> happening on both cases.
>
> You can enable all debug messages (after loading the modules) with:
>
> # for i in $(cat /sys/kernel/debug/dynamic_debug/control |grep -E '(si21|af9035)' |cut -d' ' -f 1); do echo "file $i +p" >>/sys/kernel/debug/dynamic_debug/control; done

I'll give that a try.

> You could also try to disable the firmware upload at si2168 and see
> if the si2157 reads will succeed.

That too. Thanks for the advice.


--
g

2019-10-03 19:06:21

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> You could also try to disable the firmware upload at si2168 and see
> if the si2157 reads will succeed.

I disabled the upload and while vlc wasn't working anymore I got the
following messages:

[ 168.196656] si2157 2-0063: found a 'Silicon Labs Si2147-A30'
[ 168.223009] si2157 2-0063: firmware version: 3.0.5


It really seems that the firmware upload is the culprit.

g

2019-10-03 19:09:29

by JP

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

On 10/3/19 8:32 PM, Gon Solo wrote:
>> You could also try to disable the firmware upload at si2168 and see
>> if the si2157 reads will succeed.
> I disabled the upload and while vlc wasn't working anymore I got the
> following messages:
>
> [ 168.196656] si2157 2-0063: found a 'Silicon Labs Si2147-A30'
> [ 168.223009] si2157 2-0063: firmware version: 3.0.5
>
>
> It really seems that the firmware upload is the culprit.

try other firmware?
http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/
>
> g
>
>

2019-10-03 19:12:51

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> try other firmware?
> http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/

I already downloaded version 4.0.25 from there.

-rw-r--r-- 1 root root 6,8K Apr 5 2018
/lib/firmware/dvb-demod-si2168-b40-01.fw.gonzo
-rw-rw-r-- 1 gonsolo gonsolo 16K Okt 3 13:08
/lib/firmware/dvb-demod-si2168-b40-01.fw

No difference.

--
g

2019-10-03 19:13:55

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

I also tried to add a msleep(1000) after the si2168 firmware upload;
no difference.

g

2019-10-03 19:20:04

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> try other firmware?
> http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/

I tried all of them. No difference.

--
g

2019-10-03 19:40:00

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 21:19:16 +0200
Gonsolo <[email protected]> escreveu:

> > try other firmware?
> > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/
>
> I tried all of them. No difference.

Maybe the vendor of this device wrote a different firmware. That happens.

Thanks,
Mauro

2019-10-03 19:44:34

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

From si2168.c:808:
/* Sometimes firmware returns bogus value */
if (utmp1 == 0xffff)
utmp1 = 0;

Maybe we can include my "bogus" hack to get at least Logilink running.
Maybe with an info message to tell users what is going on.

g

2019-10-03 19:47:53

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 16:39:14 -0300
Mauro Carvalho Chehab <[email protected]> escreveu:

> Em Thu, 3 Oct 2019 21:19:16 +0200
> Gonsolo <[email protected]> escreveu:
>
> > > try other firmware?
> > > http://palosaari.fi/linux/v4l-dvb/firmware/Si2168/
> >
> > I tried all of them. No difference.
>
> Maybe the vendor of this device wrote a different firmware. That happens.

Two additional comments:

1) The firmware file is likely at the Windows driver for this device
(probably using a different format). It should be possible to get
it from there.

2) Another possibility would be to add a way to tell the si2168 driver
to not try to load a firmware, using the original one. That would
require adding a field at si2168_config to allow signalizing to it
that it should not try to load a firmware file, and add a quirk at
the af9035 that would set such flag for Logilink VG0022A.

Option (1) is the best one.

Thanks,
Mauro

2019-10-03 19:53:22

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> 1) The firmware file is likely at the Windows driver for this device
> (probably using a different format). It should be possible to get
> it from there.

If you tell me how I'm willing to do this. :)

> 2) Another possibility would be to add a way to tell the si2168 driver
> to not try to load a firmware, using the original one. That would
> require adding a field at si2168_config to allow signalizing to it
> that it should not try to load a firmware file, and add a quirk at
> the af9035 that would set such flag for Logilink VG0022A.

I don't get this. Which firmware, si2168 or si2157?

I'm still for option 3: If there is a bogus chip revision number it's
likely the VG0022A and we can safely set fw to NULL, in which case
everything works.
All already working devices will continue to work as before.
With a low probability there are other devices that will return 0xffff
but a) they didn't work until now and b) they receive a clear message
that they return bogus numbers and this works just for the VG0022A, in
which case this hardware can be tested.
At last, *my* VG0022A will work without a custom kernel which I'm a
big fan of. :))

Are there any counterarguments except that it is not the cleanest
solution in the universe? ;)

--
g

2019-10-03 19:55:11

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 21:40:28 +0200
Gonsolo <[email protected]> escreveu:

> From si2168.c:808:
> /* Sometimes firmware returns bogus value */
> if (utmp1 == 0xffff)
> utmp1 = 0;

Huh? are you using the upstream Kernel? the above code is at line 215!

Please always use the upstream code when sending patches.

>
> Maybe we can include my "bogus" hack to get at least Logilink running.
> Maybe with an info message to tell users what is going on.
>
> g



Thanks,
Mauro

2019-10-03 19:59:41

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> Huh? are you using the upstream Kernel? the above code is at line 215!
> Please always use the upstream code when sending patches.

Sorry, I was confused by my vi line:
"drivers/media/dvb-frontends/si2168.c" 808 lines --26%--
212,1-8 25%"

Twelve hours behind this screen. I think I have to have a walk in the
forest right now. :)

--
g

2019-10-03 20:06:30

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Thu, 3 Oct 2019 21:51:35 +0200
Gonsolo <[email protected]> escreveu:

> > 1) The firmware file is likely at the Windows driver for this device
> > (probably using a different format). It should be possible to get
> > it from there.
>
> If you tell me how I'm willing to do this. :)

I don't know. I was not the one that extracted the firmware. I guess
Antti did it.

I suspect that there are some comments about that in the past at the
ML. seek at lore.kernel.org.

>
> > 2) Another possibility would be to add a way to tell the si2168 driver
> > to not try to load a firmware, using the original one. That would
> > require adding a field at si2168_config to allow signalizing to it
> > that it should not try to load a firmware file, and add a quirk at
> > the af9035 that would set such flag for Logilink VG0022A.
>
> I don't get this. Which firmware, si2168 or si2157?

The one that it is causing the problem. If I understood well, the
culprit was the si2168 firmware.

> I'm still for option 3: If there is a bogus chip revision number it's
> likely the VG0022A and we can safely set fw to NULL, in which case
> everything works.
> All already working devices will continue to work as before.
> With a low probability there are other devices that will return 0xffff
> but a) they didn't work until now and b) they receive a clear message
> that they return bogus numbers and this works just for the VG0022A, in
> which case this hardware can be tested.
> At last, *my* VG0022A will work without a custom kernel which I'm a
> big fan of. :))
>
> Are there any counterarguments except that it is not the cleanest
> solution in the universe? ;)

That's a really bad solution. Returning 0xff is what happens when
things go wrong during I2C transfers. Several problems can cause it,
including device misfunction. Every time someone comes with a patch
trying to ignore it, things go sideways for other devices (existing
or future ones).

Ignoring errors is always a bad idea.

Also, it is a very bad idea to load a firmware that it is not
compatible with a device. There are even cases of devices that
were burned due to that[1].

[1] XCeive has two versions of 3028/2028 chipsets. One with a
"normal power" and a "low power" version. If the firmware for
the "normal power" (version 2.7) is used at the "low power" chip
(with requires version 3.6), it makes the chipset hotter and
reduces a lot the lifetime of the tuner.

Thanks,
Mauro

2019-10-03 20:33:21

by Gonsolo

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

> I don't know. I was not the one that extracted the firmware. I guess
> Antti did it.

Ok.

> That's a really bad solution. Returning 0xff is what happens when
> things go wrong during I2C transfers. Several problems can cause it,
> including device misfunction. Every time someone comes with a patch
> trying to ignore it, things go sideways for other devices (existing
> or future ones).
>
> Ignoring errors is always a bad idea.

I understand.

Anyway, I have to give up for now. Maybe I will have some time in the
future to come back to this or somebody else can use the information
in this thread. :(

Thanks for your time, I appreciate that very much. It was worth a try. :)

--
g

2019-10-04 12:07:01

by JP

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 3 Oct 2019 21:51:35 +0200
> Gonsolo <[email protected]> escreveu:
>
>>> 1) The firmware file is likely at the Windows driver for this device
>>> (probably using a different format). It should be possible to get
>>> it from there.
>> If you tell me how I'm willing to do this. :)
> I don't know. I was not the one that extracted the firmware. I guess
> Antti did it.
>
> I suspect that there are some comments about that in the past at the
> ML. seek at lore.kernel.org.
>
>>> 2) Another possibility would be to add a way to tell the si2168 driver
>>> to not try to load a firmware, using the original one. That would
>>> require adding a field at si2168_config to allow signalizing to it
>>> that it should not try to load a firmware file, and add a quirk at
>>> the af9035 that would set such flag for Logilink VG0022A.
>> I don't get this. Which firmware, si2168 or si2157?
> The one that it is causing the problem. If I understood well, the
> culprit was the si2168 firmware.
>
>> I'm still for option 3: If there is a bogus chip revision number it's
>> likely the VG0022A and we can safely set fw to NULL, in which case
>> everything works.
>> All already working devices will continue to work as before.
>> With a low probability there are other devices that will return 0xffff
>> but a) they didn't work until now and b) they receive a clear message
>> that they return bogus numbers and this works just for the VG0022A, in
>> which case this hardware can be tested.
>> At last, *my* VG0022A will work without a custom kernel which I'm a
>> big fan of. :))
>>
>> Are there any counterarguments except that it is not the cleanest
>> solution in the universe? ;)
> That's a really bad solution. Returning 0xff is what happens when
> things go wrong during I2C transfers. Several problems can cause it,
> including device misfunction. Every time someone comes with a patch
> trying to ignore it, things go sideways for other devices (existing
> or future ones).
>
> Ignoring errors is always a bad idea.
add module param say 'gonso_hack_vg0022a'
if true, act on error by setting a flag
if this flag is set don't load firmware

Jan Pieter.

2019-10-04 12:15:28

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Fri, 4 Oct 2019 13:50:43 +0200
JP <[email protected]> escreveu:

> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 3 Oct 2019 21:51:35 +0200
> > Gonsolo <[email protected]> escreveu:
> >
> >>> 1) The firmware file is likely at the Windows driver for this device
> >>> (probably using a different format). It should be possible to get
> >>> it from there.
> >> If you tell me how I'm willing to do this. :)
> > I don't know. I was not the one that extracted the firmware. I guess
> > Antti did it.
> >
> > I suspect that there are some comments about that in the past at the
> > ML. seek at lore.kernel.org.
> >
> >>> 2) Another possibility would be to add a way to tell the si2168 driver
> >>> to not try to load a firmware, using the original one. That would
> >>> require adding a field at si2168_config to allow signalizing to it
> >>> that it should not try to load a firmware file, and add a quirk at
> >>> the af9035 that would set such flag for Logilink VG0022A.
> >> I don't get this. Which firmware, si2168 or si2157?
> > The one that it is causing the problem. If I understood well, the
> > culprit was the si2168 firmware.
> >
> >> I'm still for option 3: If there is a bogus chip revision number it's
> >> likely the VG0022A and we can safely set fw to NULL, in which case
> >> everything works.
> >> All already working devices will continue to work as before.
> >> With a low probability there are other devices that will return 0xffff
> >> but a) they didn't work until now and b) they receive a clear message
> >> that they return bogus numbers and this works just for the VG0022A, in
> >> which case this hardware can be tested.
> >> At last, *my* VG0022A will work without a custom kernel which I'm a
> >> big fan of. :))
> >>
> >> Are there any counterarguments except that it is not the cleanest
> >> solution in the universe? ;)
> > That's a really bad solution. Returning 0xff is what happens when
> > things go wrong during I2C transfers. Several problems can cause it,
> > including device misfunction. Every time someone comes with a patch
> > trying to ignore it, things go sideways for other devices (existing
> > or future ones).
> >
> > Ignoring errors is always a bad idea.
> add module param say 'gonso_hack_vg0022a'
> if true, act on error by setting a flag
> if this flag is set don't load firmware

Adding a module param should be the last resort, only when there's
no way for the driver to autodetect.

Making af9035 to detect vg0022a is quite simple.

Considering this device's entry:

{ DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
&it930x_props, "Logilink VG0022A", NULL) },

the check, at af9035 would be:

if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
/* do something to disable firmware load */

So, no need to add any load time parameter.

It should be noticed that a change just at af9035 won't work, as the
firmware is updated by si2168 driver. So, the caller code needs to
pass a config parameter to si2168 driver.

Thanks,
Mauro

2019-10-04 13:17:43

by Mauro Carvalho Chehab

Subject: [PATCH 1/4] media: si2168: use bits instead of bool for flags

Using bool on struct is not recommended, as it wastes lots of
space. So, instead, let's use bits.

While here, convert the comments to kernel-doc format.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/dvb-frontends/si2168.h | 47 +++++++++++++----------
drivers/media/dvb-frontends/si2168_priv.h | 10 ++---
2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index 50dccb394efa..ecd21adf8950 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -9,38 +9,43 @@
#define SI2168_H

#include <linux/dvb/frontend.h>
-/*
- * I2C address
- * 0x64
+/**
+ * struct si2168_config - configuration parameters for si2168
+ *
+ * @fe:
+ * frontend returned by driver
+ * @i2c_adapter:
+ * tuner I2C adapter returned by driver
+ * @ts_mode:
+ * Transport Stream mode. Can be:
+ * - %SI2168_TS_PARALLEL
+ * - %SI2168_TS_SERIAL
+ * - %SI2168_TS_TRISTATE
+ * - %SI2168_TS_CLK_MANUAL
+ * @ts_clock_inv:
+ * TS clock inverted
+ * @ts_clock_gapped:
+ * TS clock gapped
+ * @spectral_inversion:
+ * Inverted spectrum
+ *
+ * Note:
+ * The I2C address of this demod is 0x64.
*/
struct si2168_config {
- /*
- * frontend
- * returned by driver
- */
struct dvb_frontend **fe;
-
- /*
- * tuner I2C adapter
- * returned by driver
- */
struct i2c_adapter **i2c_adapter;

- /* TS mode */
#define SI2168_TS_PARALLEL 0x06
#define SI2168_TS_SERIAL 0x03
#define SI2168_TS_TRISTATE 0x00
#define SI2168_TS_CLK_MANUAL 0x20
u8 ts_mode;

- /* TS clock inverted */
- bool ts_clock_inv;
-
- /* TS clock gapped */
- bool ts_clock_gapped;
-
- /* Inverted spectrum */
- bool spectral_inversion;
+ /* Flags */
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

#endif
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 804d5b30c697..18bea5222082 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -34,12 +34,12 @@ struct si2168_dev {
unsigned int chip_id;
unsigned int version;
const char *firmware_name;
- bool active;
- bool warm;
u8 ts_mode;
- bool ts_clock_inv;
- bool ts_clock_gapped;
- bool spectral_inversion;
+ unsigned int active:1;
+ unsigned int warm:1;
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

/* firmware command struct */
--
2.21.0

2019-10-04 13:19:34

by Mauro Carvalho Chehab

Subject: [PATCH 3/4] media: af9035: add support for Logilink VG0022A

This it930x-based device has an issue with si2068.

When the si2168 firmware that came with the device is replaced
by a new one, any I2C data received from the tuner will be
replaced by 0xff.

Probably, the vendor firmware has some patch specifically
designed for this device. So, we can't replace by the generic
firmware.

The right solution would be to extract the si2168 firmware from
the original driver and ask the driver to load the specifically
designed firmware, but, while we don't have that, the next best
solution is to just keep the original firmware at the device.

For more details, see the discussions at:
https://lore.kernel.org/linux-media/[email protected]/

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..e555483c3077 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1255,6 +1255,23 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
si2168_config.fe = &adap->fe[0];
si2168_config.ts_mode = SI2168_TS_SERIAL;

+ /*
+ * HACK: The Logilink VG0022A has a bug: when the si2168
+ * firmware that came with the device is replaced by a new
+ * one, the I2C transfers to the tuner will return just 0xff.
+ *
+ * Probably, the vendor firmware has some patch specifically
+ * designed for this device. So, we can't replace by the
+ * generic firmware. The right solution would be to extract
+ * the si2168 firmware from the original driver and ask the
+ * driver to load the specifically designed firmware, but,
+ * while we don't have that, the next best solution is to just
+ * keep the original firmware at the device.
+ */
+ if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
+ le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
+ si2168_config.dont_load_firmware = true;
+
state->af9033_config[adap->id].fe = &adap->fe[0];
state->af9033_config[adap->id].ops = &state->ops;
ret = af9035_add_i2c_dev(d, "si2168",
@@ -2121,6 +2138,8 @@ static const struct usb_device_id af9035_id_table[] = {
&it930x_props, "ITE 9303 Generic", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ }
};
MODULE_DEVICE_TABLE(usb, af9035_id_table);
--
2.21.0

2019-10-04 13:19:43

by Mauro Carvalho Chehab

Subject: [PATCH 4/4] media: af9035: add the formula used for the I2C speed

A very old patch sent to the media ML used to contain the
I2C speed formula:

https://lore.kernel.org/linux-media/1312539895.2763.33.camel@Jason-Linux/

When the ite9135 code was merged with af9035, the formula was
lost. As we might need to slow down the speed for some devices,
add the formula again.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index e555483c3077..7632b00713a0 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,15 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/*
+ * The I2C speed register is calculated with:
+ * I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed))
+ *
+ * The default speed register for it930x is 7, with means a
+ * speed of ~366 kbps
+ */
+#define I2C_SPEED_366K 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1217,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_366K);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_366K);
if (ret < 0)
goto err;

--
2.21.0

2019-10-04 13:54:12

by JP

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

On 10/4/19 2:08 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 4 Oct 2019 13:50:43 +0200
> JP <[email protected]> escreveu:
>
>> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
>>> Em Thu, 3 Oct 2019 21:51:35 +0200
>>> Gonsolo <[email protected]> escreveu:
>>>
>>>>> 1) The firmware file is likely at the Windows driver for this device
>>>>> (probably using a different format). It should be possible to get
>>>>> it from there.
>>>> If you tell me how I'm willing to do this. :)
>>> I don't know. I was not the one that extracted the firmware. I guess
>>> Antti did it.
>>>
>>> I suspect that there are some comments about that in the past at the
>>> ML. seek at lore.kernel.org.
>>>
>>>>> 2) Another possibility would be to add a way to tell the si2168 driver
>>>>> to not try to load a firmware, using the original one. That would
>>>>> require adding a field at si2168_config to allow signalizing to it
>>>>> that it should not try to load a firmware file, and add a quirk at
>>>>> the af9035 that would set such flag for Logilink VG0022A.
>>>> I don't get this. Which firmware, si2168 or si2157?
>>> The one that it is causing the problem. If I understood well, the
>>> culprit was the si2168 firmware.
>>>
>>>> I'm still for option 3: If there is a bogus chip revision number it's
>>>> likely the VG0022A and we can safely set fw to NULL, in which case
>>>> everything works.
>>>> All already working devices will continue to work as before.
>>>> With a low probability there are other devices that will return 0xffff
>>>> but a) they didn't work until now and b) they receive a clear message
>>>> that they return bogus numbers and this works just for the VG0022A, in
>>>> which case this hardware can be tested.
>>>> At last, *my* VG0022A will work without a custom kernel which I'm a
>>>> big fan of. :))
>>>>
>>>> Are there any counterarguments except that it is not the cleanest
>>>> solution in the universe? ;)
>>> That's a really bad solution. Returning 0xff is what happens when
>>> things go wrong during I2C transfers. Several problems can cause it,
>>> including device misfunction. Every time someone comes with a patch
>>> trying to ignore it, things go sideways for other devices (existing
>>> or future ones).
>>>
>>> Ignoring errors is always a bad idea.
>> add module param say 'gonso_hack_vg0022a'
>> if true, act on error by setting a flag
>> if this flag is set don't load firmware
> Adding a module param should be the last resort, only when there's
> no way for the driver to autodetect.
Remember the guy reported the hw fix? Could be that
only some receiver units are affected. Therefore  the
module param.

The hw fix was original 4k7 and 10k added. That looks
like 3k3 total and all 3 chips on the bus work. 10k per
chip. Now Gon reported that said bus works with 2 chips
active on a faulty device with 4k7 resistor, which is 2
times 10k. It looks same hw error to me.
> Making af9035 to detect vg0022a is quite simple.
>
> Considering this device's entry:
>
> { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> &it930x_props, "Logilink VG0022A", NULL) },
>
> the check, at af9035 would be:
>
> if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
> le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
> /* do something to disable firmware load */
>
> So, no need to add any load time parameter.
>
> It should be noticed that a change just at af9035 won't work, as the
> firmware is updated by si2168 driver. So, the caller code needs to
> pass a config parameter to si2168 driver.
If it is a failing pull-up resistor on only some individual receiver
units, this seems overkill to me. In my proposal I did not realized
this change in the demod driver was needed.

> Thanks,
> Mauro
>
Thank you.

2019-10-04 14:18:09

by Mauro Carvalho Chehab

Subject: Re: [PATCH] si2157: Add support for Logilink VG0022A.

Em Fri, 4 Oct 2019 15:50:18 +0200
JP <[email protected]> escreveu:

> On 10/4/19 2:08 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 4 Oct 2019 13:50:43 +0200
> > JP <[email protected]> escreveu:
> >
> >> On 10/3/19 10:03 PM, Mauro Carvalho Chehab wrote:
> >>> Em Thu, 3 Oct 2019 21:51:35 +0200
> >>> Gonsolo <[email protected]> escreveu:
> >>>
> >>>>> 1) The firmware file is likely at the Windows driver for this device
> >>>>> (probably using a different format). It should be possible to get
> >>>>> it from there.
> >>>> If you tell me how I'm willing to do this. :)
> >>> I don't know. I was not the one that extracted the firmware. I guess
> >>> Antti did it.
> >>>
> >>> I suspect that there are some comments about that in the past at the
> >>> ML. seek at lore.kernel.org.
> >>>
> >>>>> 2) Another possibility would be to add a way to tell the si2168 driver
> >>>>> to not try to load a firmware, using the original one. That would
> >>>>> require adding a field at si2168_config to allow signalizing to it
> >>>>> that it should not try to load a firmware file, and add a quirk at
> >>>>> the af9035 that would set such flag for Logilink VG0022A.
> >>>> I don't get this. Which firmware, si2168 or si2157?
> >>> The one that it is causing the problem. If I understood well, the
> >>> culprit was the si2168 firmware.
> >>>
> >>>> I'm still for option 3: If there is a bogus chip revision number it's
> >>>> likely the VG0022A and we can safely set fw to NULL, in which case
> >>>> everything works.
> >>>> All already working devices will continue to work as before.
> >>>> With a low probability there are other devices that will return 0xffff
> >>>> but a) they didn't work until now and b) they receive a clear message
> >>>> that they return bogus numbers and this works just for the VG0022A, in
> >>>> which case this hardware can be tested.
> >>>> At last, *my* VG0022A will work without a custom kernel which I'm a
> >>>> big fan of. :))
> >>>>
> >>>> Are there any counterarguments except that it is not the cleanest
> >>>> solution in the universe? ;)
> >>> That's a really bad solution. Returning 0xff is what happens when
> >>> things go wrong during I2C transfers. Several problems can cause it,
> >>> including device misfunction. Every time someone comes with a patch
> >>> trying to ignore it, things go sideways for other devices (existing
> >>> or future ones).
> >>>
> >>> Ignoring errors is always a bad idea.
> >> add module param say 'gonso_hack_vg0022a'
> >> if true, act on error by setting a flag
> >> if this flag is set don't load firmware
> > Adding a module param should be the last resort, only when there's
> > no way for the driver to autodetect.
> Remember the guy reported the hw fix? Could be that
> only some receiver units are affected. Therefore  the
> module param.
>
> The hw fix was original 4k7 and 10k added. That looks
> like 3k3 total and all 3 chips on the bus work. 10k per
> chip. Now Gon reported that said bus works with 2 chips
> active on a faulty device with 4k7 resistor, which is 2
> times 10k. It looks same hw error to me.

I'm not so sure. From the reports from Gonsolo, in the case of
this specific issue with VG0022A, the device is not unstable. It is
just that it works fine with the vendor-provided firmware, while it
breaks with the new one.

We don't know that the same thing would happen with the original
reported bug. The only way to be sure would be to obtain the same
hardware from the original post and test it to check if the device
has issues without replacing the resistor.

Even the original reporter can't help, as his device was modified,
and the issue won't be there anymore.

Btw, if we end by noticing this bug happening on other it931x
device models, we could simply disable firmware load for all of them,
but we need more tests and reports before changing the behavior for
other models, as older firmwares may have other problems.

> > Making af9035 to detect vg0022a is quite simple.
> >
> > Considering this device's entry:
> >
> > { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> > &it930x_props, "Logilink VG0022A", NULL) },
> >
> > the check, at af9035 would be:
> >
> > if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
> > le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
> > /* do something to disable firmware load */
> >
> > So, no need to add any load time parameter.
> >
> > It should be noticed that a change just at af9035 won't work, as the
> > firmware is updated by si2168 driver. So, the caller code needs to
> > pass a config parameter to si2168 driver.

> If it is a failing pull-up resistor on only some individual receiver
> units, this seems overkill to me. In my proposal I did not realized
> this change in the demod driver was needed.

Agreed, but we have no means to know that until someone buys other
units of the VG0022A and do tests with and without the patch.

Thanks,
Mauro

2019-10-09 21:46:53

by Gonsolo

Subject: Re: [PATCH 3/4] media: af9035: add support for Logilink VG0022A

> This it930x-based device has an issue with si2068.
>
> When the si2168 firmware that came with the device is replaced
> by a new one, any I2C data received from the tuner will be
> replaced by 0xff.
>
> Probably, the vendor firmware has some patch specifically
> designed for this device. So, we can't replace by the generic
> firmware.
>
> The right solution would be to extract the si2168 firmware from
> the original driver and ask the driver to load the specifically
> designed firmware, but, while we don't have that, the next best
> solution is to just keep the original firmware at the device.

Unfortunately, after applying these four patches it doesn't work for me.
The messages when inserting the stick:

[ 244.133448] i2c i2c-1: Added multiplexed i2c bus 2
[ 244.133455] si2168 1-0067: Silicon Labs Si2168-B40 successfully identified
[ 244.133458] si2168 1-0067: firmware version: B 4.0.2
[ 244.133500] usb 2-1: DVB: registering adapter 0 frontend 0 (Silicon Labs Si2168)...
[ 244.133514] dvbdev: dvb_create_media_entity: media entity 'Silicon Labs Si2168' registered.
[ 244.138367] si2157 2-0063: Silicon Labs Si2147/2148/2157/2158 successfully attached
[ 244.156956] usb 2-1: dvb_usb_v2: 'Logilink VG0022A' successfully initialized and connected

The messages when starting VLC:

[ 260.490253] si2157 2-0063: found a 'Silicon Labs Si2147-A30'
[ 260.541347] si2157 2-0063: firmware version: 3.0.5

But it doesn't work. :(

g

>
> For more details, see the discussions at:
> https://lore.kernel.org/linux-media/[email protected]/
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> drivers/media/usb/dvb-usb-v2/af9035.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 3afd18733614..e555483c3077 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -1255,6 +1255,23 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
> si2168_config.fe = &adap->fe[0];
> si2168_config.ts_mode = SI2168_TS_SERIAL;
>
> + /*
> + * HACK: The Logilink VG0022A has a bug: when the si2168
> + * firmware that came with the device is replaced by a new
> + * one, the I2C transfers to the tuner will return just 0xff.
> + *
> + * Probably, the vendor firmware has some patch specifically
> + * designed for this device. So, we can't replace by the
> + * generic firmware. The right solution would be to extract
> + * the si2168 firmware from the original driver and ask the
> + * driver to load the specifically designed firmware, but,
> + * while we don't have that, the next best solution is to just
> + * keep the original firmware at the device.
> + */
> + if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
> + le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
> + si2168_config.dont_load_firmware = true;
> +
> state->af9033_config[adap->id].fe = &adap->fe[0];
> state->af9033_config[adap->id].ops = &state->ops;
> ret = af9035_add_i2c_dev(d, "si2168",
> @@ -2121,6 +2138,8 @@ static const struct usb_device_id af9035_id_table[] = {
> &it930x_props, "ITE 9303 Generic", NULL) },
> { DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
> &it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
> + { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
> + &it930x_props, "Logilink VG0022A", NULL) },
> { }
> };
> MODULE_DEVICE_TABLE(usb, af9035_id_table);
> --
> 2.21.0
>

2019-10-09 22:05:04

by Gonsolo

Subject: Re: [PATCH 3/4] media: af9035: add support for Logilink VG0022A

> Unfortunately, after applying these four patches it doesn't work for me.

Your patch disables firmware loading at dvb-frontends/si2168.c:449 while
my original one disabled it at tuners/si2157.c:554:

case SI_BOGUS:
dev_info(&client->dev, "bogus chip version, trying with no firmware\n");
fw_name = NULL;

According to my dmesg the following chip is found:

[ 141.726488] si2157 10-0063: found a 'Silicon Labs Si2147-A30'
[ 141.777647] si2157 10-0063: firmware version: 3.0.5

So according to:

case SI2147_A30:
case SI2146_A10:
fw_name = NULL;

it should work. Hmmm.

g

2019-10-10 08:23:52

by Gonsolo

Subject: Re: [PATCH 3/4] media: af9035: add support for Logilink VG0022A

Hi!

I rebased Mauros patch on top of mine and this patch [3/4] is the first bad
commit. I believe these lines are the culprit:

+ if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
+ le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
+ si2168_config.dont_load_firmware = true;

> From: JP <[email protected]>
> Mauro just took the wrong firmware to skip. demod instead of tuner.
> It would not be hard to fix that.

It seems so.

g

2019-10-10 09:19:49

by Gonsolo

Subject: Re: [PATCH 3/4] media: af9035: add support for Logilink VG0022A

Hi!

This patch works for me. It disables firmware downloading for the si2157
instead of the si2168.

Convert si2157 and si2168 to kernel-doc format as suggested by Mauro.
Use bits instead of bool. Add a flag to si2157 for not loading the
firmware. Make computation of speed register clear. Add hack for
Logilink VG0022A.

Signed-off-by: Andreas Wendleder <[email protected]>

diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index 50dccb394efa..ecd21adf8950 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -9,38 +9,43 @@
#define SI2168_H

#include <linux/dvb/frontend.h>
-/*
- * I2C address
- * 0x64
+/**
+ * struct si2168_config - configuration parameters for si2168
+ *
+ * @fe:
+ * frontend returned by driver
+ * @i2c_adapter:
+ * tuner I2C adapter returned by driver
+ * @ts_mode:
+ * Transport Stream mode. Can be:
+ * - %SI2168_TS_PARALLEL
+ * - %SI2168_TS_SERIAL
+ * - %SI2168_TS_TRISTATE
+ * - %SI2168_TS_CLK_MANUAL
+ * @ts_clock_inv:
+ * TS clock inverted
+ * @ts_clock_gapped:
+ * TS clock gapped
+ * @spectral_inversion:
+ * Inverted spectrum
+ *
+ * Note:
+ * The I2C address of this demod is 0x64.
*/
struct si2168_config {
- /*
- * frontend
- * returned by driver
- */
struct dvb_frontend **fe;
-
- /*
- * tuner I2C adapter
- * returned by driver
- */
struct i2c_adapter **i2c_adapter;

- /* TS mode */
#define SI2168_TS_PARALLEL 0x06
#define SI2168_TS_SERIAL 0x03
#define SI2168_TS_TRISTATE 0x00
#define SI2168_TS_CLK_MANUAL 0x20
u8 ts_mode;

- /* TS clock inverted */
- bool ts_clock_inv;
-
- /* TS clock gapped */
- bool ts_clock_gapped;
-
- /* Inverted spectrum */
- bool spectral_inversion;
+ /* Flags */
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

#endif
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 804d5b30c697..18bea5222082 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -34,12 +34,12 @@ struct si2168_dev {
unsigned int chip_id;
unsigned int version;
const char *firmware_name;
- bool active;
- bool warm;
u8 ts_mode;
- bool ts_clock_inv;
- bool ts_clock_gapped;
- bool spectral_inversion;
+ unsigned int active:1;
+ unsigned int warm:1;
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

/* firmware command struct */
diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..898e0f9f8b70 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -118,6 +118,11 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}

+ if (dev->dont_load_firmware) {
+ dev_info(&client->dev, "device is buggy, skipping firmware download\n");
+ goto skip_fw_download;
+ }
+
/* query chip revision */
memcpy(cmd.args, "\x02", 1);
cmd.wlen = 1;
@@ -440,6 +445,7 @@ static int si2157_probe(struct i2c_client *client,
i2c_set_clientdata(client, dev);
dev->fe = cfg->fe;
dev->inversion = cfg->inversion;
+ dev->dont_load_firmware = cfg->dont_load_firmware;
dev->if_port = cfg->if_port;
dev->chiptype = (u8)id->driver_data;
dev->if_frequency = 5000000; /* default value of property 0x0706 */
diff --git a/drivers/media/tuners/si2157.h b/drivers/media/tuners/si2157.h
index c22ca784f43f..ffdece3c2eaa 100644
--- a/drivers/media/tuners/si2157.h
+++ b/drivers/media/tuners/si2157.h
@@ -11,29 +11,34 @@
#include <media/media-device.h>
#include <media/dvb_frontend.h>

-/*
- * I2C address
- * 0x60
+/**
+ * struct si2157_config - configuration parameters for si2157
+ *
+ * @fe:
+ * frontend returned by driver
+ * @mdev:
+ * media device returned by driver
+ * @inversion:
+ * spectral inversion
+ * @dont_load_firmware:
+ * Instead of uploading a new firmware, use the existing one
+ * @if_port:
+ * Port selection
+ * Select the RF interface to use (pins 9+11 or 12+13)
+ *
+ * Note:
+ * The I2C address of this demod is 0x60.
*/
struct si2157_config {
- /*
- * frontend
- */
struct dvb_frontend *fe;

#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_device *mdev;
#endif

- /*
- * Spectral Inversion
- */
- bool inversion;
+ unsigned int inversion:1;
+ unsigned int dont_load_firmware:1;

- /*
- * Port selection
- * Select the RF interface to use (pins 9+11 or 12+13)
- */
u8 if_port;
};

diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 2bda903358da..778f81b39996 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -23,8 +23,9 @@ enum si2157_pads {
struct si2157_dev {
struct mutex i2c_mutex;
struct dvb_frontend *fe;
- bool active;
- bool inversion;
+ unsigned int active:1;
+ unsigned int inversion:1;
+ unsigned int dont_load_firmware:1;
u8 chiptype;
u8 if_port;
u32 if_frequency;
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..792667ee5ebc 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,15 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/*
+ * The I2C speed register is calculated with:
+ * I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed))
+ *
+ * The default speed register for it930x is 7, with means a
+ * speed of ~366 kbps
+ */
+#define I2C_SPEED_366K 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1217,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_366K);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_366K);
if (ret < 0)
goto err;

@@ -1610,6 +1619,24 @@ static int it930x_tuner_attach(struct dvb_usb_adapter *adap)

memset(&si2157_config, 0, sizeof(si2157_config));
si2157_config.fe = adap->fe[0];
+
+ /*
+ * HACK: The Logilink VG0022A has a bug: when the si2157
+ * firmware that came with the device is replaced by a new
+ * one, the I2C transfers to the tuner will return just 0xff.
+ *
+ * Probably, the vendor firmware has some patch specifically
+ * designed for this device. So, we can't replace by the
+ * generic firmware. The right solution would be to extract
+ * the si2157 firmware from the original driver and ask the
+ * driver to load the specifically designed firmware, but,
+ * while we don't have that, the next best solution is to just
+ * keep the original firmware at the device.
+ */
+ if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
+ le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
+ si2157_config.dont_load_firmware = true;
+
si2157_config.if_port = it930x_addresses_table[state->it930x_addresses].tuner_if_port;
ret = af9035_add_i2c_dev(d, "si2157",
it930x_addresses_table[state->it930x_addresses].tuner_i2c_addr,
@@ -2121,6 +2148,8 @@ static const struct usb_device_id af9035_id_table[] = {
&it930x_props, "ITE 9303 Generic", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ }
};
MODULE_DEVICE_TABLE(usb, af9035_id_table);

2019-10-10 09:52:10

by Gonsolo

Subject: [PATCH 0/4] Add support for Logilink VG0022A.

These patches add support for the Logilink VG0022A.

Signed-off-by: Andreas Wendleder <[email protected]>

Gon Solo (4):
si2168: Use bits and convert to kernel-doc format.
si2157: Add option for not downloading firmware.
af9035: Make speed computation clear.
Add support for Logilink VG0022A.

drivers/media/dvb-frontends/si2168.h | 47 +++++++++++++----------
drivers/media/dvb-frontends/si2168_priv.h | 10 ++---
drivers/media/tuners/si2157.c | 6 +++
drivers/media/tuners/si2157.h | 33 +++++++++-------
drivers/media/tuners/si2157_priv.h | 5 ++-
drivers/media/usb/dvb-usb-v2/af9035.c | 37 ++++++++++++++++--
6 files changed, 92 insertions(+), 46 deletions(-)

--
2.20.1

2019-10-10 09:52:17

by Gonsolo

Subject: [PATCH 2/4] si2157: Add option for not downloading firmware.

While at it, convert to kernel-doc format and use bits instead of bools.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/tuners/si2157.c | 6 ++++++
drivers/media/tuners/si2157.h | 33 +++++++++++++++++-------------
drivers/media/tuners/si2157_priv.h | 5 +++--
3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index e87040d6eca7..898e0f9f8b70 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -118,6 +118,11 @@ static int si2157_init(struct dvb_frontend *fe)
goto err;
}

+ if (dev->dont_load_firmware) {
+ dev_info(&client->dev, "device is buggy, skipping firmware download\n");
+ goto skip_fw_download;
+ }
+
/* query chip revision */
memcpy(cmd.args, "\x02", 1);
cmd.wlen = 1;
@@ -440,6 +445,7 @@ static int si2157_probe(struct i2c_client *client,
i2c_set_clientdata(client, dev);
dev->fe = cfg->fe;
dev->inversion = cfg->inversion;
+ dev->dont_load_firmware = cfg->dont_load_firmware;
dev->if_port = cfg->if_port;
dev->chiptype = (u8)id->driver_data;
dev->if_frequency = 5000000; /* default value of property 0x0706 */
diff --git a/drivers/media/tuners/si2157.h b/drivers/media/tuners/si2157.h
index c22ca784f43f..ffdece3c2eaa 100644
--- a/drivers/media/tuners/si2157.h
+++ b/drivers/media/tuners/si2157.h
@@ -11,29 +11,34 @@
#include <media/media-device.h>
#include <media/dvb_frontend.h>

-/*
- * I2C address
- * 0x60
+/**
+ * struct si2157_config - configuration parameters for si2157
+ *
+ * @fe:
+ * frontend returned by driver
+ * @mdev:
+ * media device returned by driver
+ * @inversion:
+ * spectral inversion
+ * @dont_load_firmware:
+ * Instead of uploading a new firmware, use the existing one
+ * @if_port:
+ * Port selection
+ * Select the RF interface to use (pins 9+11 or 12+13)
+ *
+ * Note:
+ * The I2C address of this demod is 0x60.
*/
struct si2157_config {
- /*
- * frontend
- */
struct dvb_frontend *fe;

#if defined(CONFIG_MEDIA_CONTROLLER)
struct media_device *mdev;
#endif

- /*
- * Spectral Inversion
- */
- bool inversion;
+ unsigned int inversion:1;
+ unsigned int dont_load_firmware:1;

- /*
- * Port selection
- * Select the RF interface to use (pins 9+11 or 12+13)
- */
u8 if_port;
};

diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 2bda903358da..778f81b39996 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -23,8 +23,9 @@ enum si2157_pads {
struct si2157_dev {
struct mutex i2c_mutex;
struct dvb_frontend *fe;
- bool active;
- bool inversion;
+ unsigned int active:1;
+ unsigned int inversion:1;
+ unsigned int dont_load_firmware:1;
u8 chiptype;
u8 if_port;
u32 if_frequency;
--
2.20.1

2019-10-10 09:52:17

by Gonsolo

Subject: [PATCH 3/4] af9035: Make speed computation clear.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 3afd18733614..51e607ea3add 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1197,6 +1197,15 @@ static int af9035_frontend_attach(struct dvb_usb_adapter *adap)
return ret;
}

+/*
+ * The I2C speed register is calculated with:
+ * I2C speed register = (1000000000 / (24.4 * 16 * I2C_speed))
+ *
+ * The default speed register for it930x is 7, with means a
+ * speed of ~366 kbps
+ */
+#define I2C_SPEED_366K 7
+
static int it930x_frontend_attach(struct dvb_usb_adapter *adap)
{
struct state *state = adap_to_priv(adap);
@@ -1208,13 +1217,13 @@ static int it930x_frontend_attach(struct dvb_usb_adapter *adap)

dev_dbg(&intf->dev, "adap->id=%d\n", adap->id);

- /* I2C master bus 2 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f6a7, 0x07);
+ /* I2C master bus 2 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f6a7, I2C_SPEED_366K);
if (ret < 0)
goto err;

- /* I2C master bus 1,3 clock speed 300k */
- ret = af9035_wr_reg(d, 0x00f103, 0x07);
+ /* I2C master bus 1,3 clock speed 366k */
+ ret = af9035_wr_reg(d, 0x00f103, I2C_SPEED_366K);
if (ret < 0)
goto err;

--
2.20.1

2019-10-10 09:54:52

by Gonsolo

Subject: [PATCH 4/4] Add support for Logilink VG0022A.

This includes a hack for the device as it returns only 0xff after a new
firmware is loaded. To quote Mauro:

"When the [...] firmware that came with the device is replaced
by a new one, any I2C data received from the tuner will be
replaced by 0xff.

Probably, the vendor firmware has some patch specifically
designed for this device. So, we can't replace by the generic
firmware.

The right solution would be to extract the [...] firmware from
the original driver and ask the driver to load the specifically
designed firmware, but, while we don't have that, the next best
solution is to just keep the original firmware at the device."

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/usb/dvb-usb-v2/af9035.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 51e607ea3add..792667ee5ebc 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1619,6 +1619,24 @@ static int it930x_tuner_attach(struct dvb_usb_adapter *adap)

memset(&si2157_config, 0, sizeof(si2157_config));
si2157_config.fe = adap->fe[0];
+
+ /*
+ * HACK: The Logilink VG0022A has a bug: when the si2157
+ * firmware that came with the device is replaced by a new
+ * one, the I2C transfers to the tuner will return just 0xff.
+ *
+ * Probably, the vendor firmware has some patch specifically
+ * designed for this device. So, we can't replace by the
+ * generic firmware. The right solution would be to extract
+ * the si2157 firmware from the original driver and ask the
+ * driver to load the specifically designed firmware, but,
+ * while we don't have that, the next best solution is to just
+ * keep the original firmware at the device.
+ */
+ if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
+ le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)
+ si2157_config.dont_load_firmware = true;
+
si2157_config.if_port = it930x_addresses_table[state->it930x_addresses].tuner_if_port;
ret = af9035_add_i2c_dev(d, "si2157",
it930x_addresses_table[state->it930x_addresses].tuner_i2c_addr,
@@ -2130,6 +2148,8 @@ static const struct usb_device_id af9035_id_table[] = {
&it930x_props, "ITE 9303 Generic", NULL) },
{ DVB_USB_DEVICE(USB_VID_AVERMEDIA, USB_PID_AVERMEDIA_TD310,
&it930x_props, "AVerMedia TD310 DVB-T2", NULL) },
+ { DVB_USB_DEVICE(USB_VID_DEXATEK, 0x0100,
+ &it930x_props, "Logilink VG0022A", NULL) },
{ }
};
MODULE_DEVICE_TABLE(usb, af9035_id_table);
--
2.20.1

2019-10-10 09:55:38

by Gonsolo

Subject: [PATCH 1/4] si2168: Use bits and convert to kernel-doc format.

Signed-off-by: Gon Solo <[email protected]>
---
drivers/media/dvb-frontends/si2168.h | 47 +++++++++++++----------
drivers/media/dvb-frontends/si2168_priv.h | 10 ++---
2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
index 50dccb394efa..ecd21adf8950 100644
--- a/drivers/media/dvb-frontends/si2168.h
+++ b/drivers/media/dvb-frontends/si2168.h
@@ -9,38 +9,43 @@
#define SI2168_H

#include <linux/dvb/frontend.h>
-/*
- * I2C address
- * 0x64
+/**
+ * struct si2168_config - configuration parameters for si2168
+ *
+ * @fe:
+ * frontend returned by driver
+ * @i2c_adapter:
+ * tuner I2C adapter returned by driver
+ * @ts_mode:
+ * Transport Stream mode. Can be:
+ * - %SI2168_TS_PARALLEL
+ * - %SI2168_TS_SERIAL
+ * - %SI2168_TS_TRISTATE
+ * - %SI2168_TS_CLK_MANUAL
+ * @ts_clock_inv:
+ * TS clock inverted
+ * @ts_clock_gapped:
+ * TS clock gapped
+ * @spectral_inversion:
+ * Inverted spectrum
+ *
+ * Note:
+ * The I2C address of this demod is 0x64.
*/
struct si2168_config {
- /*
- * frontend
- * returned by driver
- */
struct dvb_frontend **fe;
-
- /*
- * tuner I2C adapter
- * returned by driver
- */
struct i2c_adapter **i2c_adapter;

- /* TS mode */
#define SI2168_TS_PARALLEL 0x06
#define SI2168_TS_SERIAL 0x03
#define SI2168_TS_TRISTATE 0x00
#define SI2168_TS_CLK_MANUAL 0x20
u8 ts_mode;

- /* TS clock inverted */
- bool ts_clock_inv;
-
- /* TS clock gapped */
- bool ts_clock_gapped;
-
- /* Inverted spectrum */
- bool spectral_inversion;
+ /* Flags */
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

#endif
diff --git a/drivers/media/dvb-frontends/si2168_priv.h b/drivers/media/dvb-frontends/si2168_priv.h
index 804d5b30c697..18bea5222082 100644
--- a/drivers/media/dvb-frontends/si2168_priv.h
+++ b/drivers/media/dvb-frontends/si2168_priv.h
@@ -34,12 +34,12 @@ struct si2168_dev {
unsigned int chip_id;
unsigned int version;
const char *firmware_name;
- bool active;
- bool warm;
u8 ts_mode;
- bool ts_clock_inv;
- bool ts_clock_gapped;
- bool spectral_inversion;
+ unsigned int active:1;
+ unsigned int warm:1;
+ unsigned int ts_clock_inv:1;
+ unsigned int ts_clock_gapped:1;
+ unsigned int spectral_inversion:1;
};

/* firmware command struct */
--
2.20.1

2019-10-10 10:11:57

by Mauro Carvalho Chehab

Subject: Re: [PATCH 0/4] Add support for Logilink VG0022A.

Patches applied.

Em Thu, 10 Oct 2019 11:50:59 +0200
Gon Solo <[email protected]> escreveu:

> These patches add support for the Logilink VG0022A.
>
> Signed-off-by: Andreas Wendleder <[email protected]>
>
> Gon Solo (4):
> si2168: Use bits and convert to kernel-doc format.
> si2157: Add option for not downloading firmware.
> af9035: Make speed computation clear.
> Add support for Logilink VG0022A.

For the si2168 and af9035 patches, I applied the version I wrote.

Regards,
Mauro

2019-10-10 10:59:10

by Gonsolo

Subject: Re: [PATCH 1/4] media: si2168: use bits instead of bool for flags

On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote:
> Using bool on struct is not recommended, as it wastes lots of
> space. So, instead, let's use bits.

Wouldn't "bool b:1;" even be better? I performed a little test:

#include <stdbool.h>
#include <stdio.h>

struct uints {
unsigned int a0;
unsigned int a1;
unsigned int a2;
unsigned int a3;
unsigned int a4;
unsigned int a5;
unsigned int a6;
unsigned int a7;
};

struct bools {
bool a0;
bool a1;
bool a2;
bool a3;
bool a4;
bool a5;
bool a6;
bool a7;
};

struct bit_uints {
unsigned int a0:1;
unsigned int a1:1;
unsigned int a2:1;
unsigned int a3:1;
unsigned int a4:1;
unsigned int a5:1;
unsigned int a6:1;
unsigned int a7:1;
};

struct bit_bools {
bool a0:1;
bool a1:1;
bool a2:1;
bool a3:1;
bool a4:1;
bool a5:1;
bool a6:1;
bool a7:1;
};

int main() {
printf("bit_uints: %ld\n", sizeof(struct bit_uints));
printf("bit_bools: %ld\n", sizeof(struct bit_bools));
printf("uints: %ld\n", sizeof(struct uints));
printf("bools: %ld\n", sizeof(struct bools));
}

Result:

bit_uints: 4
bit_bools: 1
uints: 32
bools: 8

I know with different types within the struct it looks different, but
still.

g

2019-10-10 11:35:14

by Mauro Carvalho Chehab

Subject: Re: [PATCH 1/4] media: si2168: use bits instead of bool for flags

Em Thu, 10 Oct 2019 12:55:44 +0200
Gon Solo <[email protected]> escreveu:

> On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote:
> > Using bool on struct is not recommended, as it wastes lots of
> > space. So, instead, let's use bits.
>
> Wouldn't "bool b:1;" even be better? I performed a little test:
>
> #include <stdbool.h>
> #include <stdio.h>
>
> struct uints {
> unsigned int a0;
> unsigned int a1;
> unsigned int a2;
> unsigned int a3;
> unsigned int a4;
> unsigned int a5;
> unsigned int a6;
> unsigned int a7;
> };
>
> struct bools {
> bool a0;
> bool a1;
> bool a2;
> bool a3;
> bool a4;
> bool a5;
> bool a6;
> bool a7;
> };
>
> struct bit_uints {
> unsigned int a0:1;
> unsigned int a1:1;
> unsigned int a2:1;
> unsigned int a3:1;
> unsigned int a4:1;
> unsigned int a5:1;
> unsigned int a6:1;
> unsigned int a7:1;
> };
>
> struct bit_bools {
> bool a0:1;
> bool a1:1;
> bool a2:1;
> bool a3:1;
> bool a4:1;
> bool a5:1;
> bool a6:1;
> bool a7:1;
> };
>
> int main() {
> printf("bit_uints: %ld\n", sizeof(struct bit_uints));
> printf("bit_bools: %ld\n", sizeof(struct bit_bools));
> printf("uints: %ld\n", sizeof(struct uints));
> printf("bools: %ld\n", sizeof(struct bools));
> }
>
> Result:
>
> bit_uints: 4
> bit_bools: 1
> uints: 32
> bools: 8
>
> I know with different types within the struct it looks different, but
> still.

No. In practice, the compiler will add 3 bytes of pad after bit_bools
(on 32-bit archs), due to performance reasons.

Using "unsigned int" makes it clearer.

Thanks,
Mauro

2019-10-10 11:45:31

by Mauro Carvalho Chehab

Subject: Re: [PATCH 1/4] media: si2168: use bits instead of bool for flags

Em Thu, 10 Oct 2019 08:34:23 -0300
Mauro Carvalho Chehab <[email protected]> escreveu:

> Em Thu, 10 Oct 2019 12:55:44 +0200
> Gon Solo <[email protected]> escreveu:
>
> > On Fri, Oct 04, 2019 at 10:15:22AM -0300, Mauro Carvalho Chehab wrote:
> > > Using bool on struct is not recommended, as it wastes lots of
> > > space. So, instead, let's use bits.
> >
> > Wouldn't "bool b:1;" even be better? I performed a little test:

> > Result:
> >
> > bit_uints: 4
> > bit_bools: 1

> > I know with different types within the struct it looks different, but
> > still.
>
> No. In practice, the compiler will add 3 bytes of pad after bit_bools
> (on 32-bit archs), due to performance reasons.

Btw, if you want to test, just add something after the bits, and you'll
see that it will now report the PAD bytes too:

struct bit_uints {
unsigned int a0:1;
unsigned int a1:1;
unsigned int a2:1;
unsigned int a3:1;
unsigned int a4:1;
unsigned int a5:1;
unsigned int a6:1;
unsigned int a7:1;

int i;
};

struct bit_bools {
bool a0:1;
bool a1:1;
bool a2:1;
bool a3:1;
bool a4:1;
bool a5:1;
bool a6:1;
bool a7:1;

int i;
};

bit_uints: 8
bit_bools: 8

Thanks,
Mauro

2019-10-10 11:45:45

by Gonsolo

Subject: Re: [PATCH 4/4] Add support for Logilink VG0022A.

Hi!

> "When the [...] firmware that came with the device is replaced
> by a new one, any I2C data received from the tuner will be
> replaced by 0xff.
>
> Probably, the vendor firmware has some patch specifically
> designed for this device. So, we can't replace by the generic
> firmware.
>
> The right solution would be to extract the [...] firmware from
> the original driver and ask the driver to load the specifically
> designed firmware, but, while we don't have that, the next best
> solution is to just keep the original firmware at the device."

The information in the patch is not totally correct. It is the
si2168(!) firmware download that confuses things, not the one for the
si2157. The si2157 seems to have no firmware and the problem is
that we used to bail out because we didn't recognize the bogus
chip id. The following patch corrects this.

Signed-off-by: <[email protected]>

diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
index 792667ee5ebc..5a2943e2932b 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -1621,17 +1621,20 @@ static int it930x_tuner_attach(struct dvb_usb_adapter *adap)
si2157_config.fe = adap->fe[0];

/*
- * HACK: The Logilink VG0022A has a bug: when the si2157
+ * HACK: The Logilink VG0022A has a bug: When the si2168
* firmware that came with the device is replaced by a new
* one, the I2C transfers to the tuner will return just 0xff.
*
* Probably, the vendor firmware has some patch specifically
* designed for this device. So, we can't replace by the
* generic firmware. The right solution would be to extract
- * the si2157 firmware from the original driver and ask the
+ * the si2157/68 firmware from the original driver and ask the
* driver to load the specifically designed firmware, but,
* while we don't have that, the next best solution is to just
* keep the original firmware at the device.
+ *
+ * Or, the Windows driver includes the same hack and doesn't
+ * bail out on bogus chip ids.
*/
if (le16_to_cpu(d->udev->descriptor.idVendor) == USB_VID_DEXATEK &&
le16_to_cpu(d->udev->descriptor.idProduct) == 0x0100)

2019-11-15 18:10:08

by Gonsolo

Subject: Re: [PATCH 4/4] Add support for Logilink VG0022A.

Hi!

Context: In the upcoming 5.5 kernel there will be (hopefully) support
for the Logilink VG0022A DVB-T2 usb stick. Since this device has a bug a
kernel quirk has been added. An additional quirk is needed for suspend
to work. This is documented here. Hopefully it will be picked up in
distributions or if not, owners of this device can find this
information on the internet.

Standby for this Logilink VG0022A device does not work. I had to
manually unload the module dvb_usb_af9035:

cat /lib/systemd/system-sleep/suspend-modules:

=====

case $1 in
pre)
for mod in $(</etc/suspend-modules.conf); do
rmmod $mod
done
;;
post)
for mod in $(</etc/suspend-modules.conf); do
modprobe $mod
done
;;
esac

=====

cat /etc/suspend-modules.conf:

=====

dvb_usb_af9035

=====

Cheers,
g