[PATCH 1/3] si2157: get chip id during probing

[PATCH 1/3] si2157: get chip id during probing

[Uwe Kleine-König]

From: Andreas Kemnade <andreas@kemnade.info>

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

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/media/tuners/si2157.c      | 73 +++++++++++++++++-------------
 drivers/media/tuners/si2157_priv.h |  8 ++++
 2 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 7be893def190..4d3313a611d6 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");
 
@@ -109,34 +109,7 @@ static int si2157_init(struct dvb_frontend *fe)
 	if (ret)
 		goto err;
 
-	/* Si2141 needs a second command before it answers the revision query */
-	if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
-		memcpy(cmd.args, "\xc0\x08\x01\x02\x00\x00\x01", 7);
-		cmd.wlen = 7;
-		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 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)
-
-	switch (chip_id) {
+	switch (dev->chip_id) {
 	case SI2158_A20:
 	case SI2148_A20:
 		fw_name = SI2158_A20_FIRMWARE;
@@ -157,9 +130,6 @@ 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]);
-
 	if (fw_name == NULL)
 		goto skip_fw_download;
 
@@ -451,6 +421,45 @@ 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
+	 */
+	/* Si2141 needs a second command before it answers the revision query */
+	if (dev->chiptype == SI2157_CHIPTYPE_SI2141) {
+		memcpy(cmd.args, "\xc0\x08\x01\x02\x00\x00\x01", 7);
+		cmd.wlen = 7;
+		ret = si2157_cmd_execute(client, &cmd);
+		if (ret)
+			goto err;
+	}
+
+	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) {
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 7d16934c7708..168ce7ad8ec4 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)
@@ -42,6 +43,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 {
-- 
2.20.1

Re: [PATCH 1/3] si2157: get chip id during probing

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 763 bytes --]

Hello,

On 7/7/19 10:58 PM, Uwe Kleine-König wrote:
> From: Andreas Kemnade <andreas@kemnade.info>
> 
> 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
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>

I sent this out by mistake. This patch is still needed here, but I
intend to understand and maybe improve it first. Please ignore for now.

Best regards
Uwe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] si2157: get chip id during probing

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 922 bytes --]

Hello Andreas,

On 7/7/19 11:01 PM, Uwe Kleine-König wrote:
> Hello,
> 
> On 7/7/19 10:58 PM, Uwe Kleine-König wrote:
>> From: Andreas Kemnade <andreas@kemnade.info>
>>
>> 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
>>
>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>

I wonder if you (or anyone else here reading this mail) has access to
the datasheets of the involved chips? (i.e. si2168, si2157)
I'd like to try understanding and eventually fix the addressed problem
here properly.

Best regards
Uwe


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] si2157: get chip id during probing

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]

Hello Andreas,

On Tue, Jul 09, 2019 at 08:03:29PM +0200, Andreas Kemnade wrote:
> On Mon, 8 Jul 2019 10:43:08 +0200
> Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> > On 7/7/19 11:01 PM, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > On 7/7/19 10:58 PM, Uwe Kleine-König wrote:  
> > >> From: Andreas Kemnade <andreas@kemnade.info>
> > >>
> > >> 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
> > >>
> > >> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>  
> > 
> > I wonder if you (or anyone else here reading this mail) has access to
> > the datasheets of the involved chips? (i.e. si2168, si2157)
> > I'd like to try understanding and eventually fix the addressed problem
> > here properly.

BTW, I tried to contact Silabs support asking for the datasheets. But
because this doesn't promise to generate a certain amount of selled
chips I won't get an NDA and so also won't get the datasheets. :-|

> I remember some rumors that there is a resistor on the board which needs to be
> changed, maybe some i2c pullup to properly fix it. Windows driver by chance
> does it in a order which works with the current setup, so engineers
> did not notice. I have not checked it myself.
> So the theory is that we do here some workaround for a hw problem.

Do you have some more details about these rumors?

You wrote in the commit log that the si2157 would still be writable but
not readable. Given how i2c works I wonder how this can be because both
directions use the same data line. Did you verify with a scope that the
revision query command makes it on the bus? Is the reply visible, or do
you see the 0xff bytes there, too?

Given that the transfer works before the si2168 chip is booted (this is
what the 0x01-0x01 command is about AFAIK), this doesn't sound like a
hardware issue to me.

There are still a few things I could check:

 - are there some (write only) i2c transfers later that (seem to) work?
 - is it really the 0x01 0x01 that makes reading the revision fail?

Not sure when I come around because this is my free time project and
there are more urgent things to do for me now. But if you have thoughts
about the matter, don't hesitate to tell me.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]