Re: [PATCH 4/4] media: dvb/af9015, add hashes support

Previous thread: [PATCH 1/1] HID: ignore afatech 9016 by Jiri Slaby on Wednesday, January 13, 2010 - 12:59 pm. (7 messages)

Next thread: mmotm 2010-01-13-12-17 uploaded by akpm on Wednesday, January 13, 2010 - 1:17 pm. (23 messages)
From: Jiri Slaby
Date: Wednesday, January 13, 2010 - 1:00 pm

MSI digivox mini II works even with remote=2 module parameter. Check
for manufacturer and if it is Afatech, use af9015_ir_table_msi and
af9015_rc_keys_msi.

The device itself is 15a4:9016.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: linux-media@vger.kernel.org
---
 drivers/media/dvb/dvb-usb/af9015.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 8b60a60..f0d5731 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -835,9 +835,15 @@ static int af9015_read_config(struct usb_device *udev)
 					  af9015_ir_table_mygictv;
 					af9015_config.ir_table_size =
 					  ARRAY_SIZE(af9015_ir_table_mygictv);
-				} else if (!strcmp("MSI", manufacturer)) {
-					/* iManufacturer 1 MSI
-					   iProduct      2 MSI K-VOX */
+				} else if (!strcmp("MSI", manufacturer) ||
+					   !strcmp("Afatech", manufacturer)) {
+					/*
+					   iManufacturer 1 MSI
+					   iProduct      2 MSI K-VOX
+					   iManufacturer 1 Afatech
+					   iProduct      2 DVB-T 2
+					 */
+
 					af9015_properties[i].rc_key_map =
 					  af9015_rc_keys_msi;
 					af9015_properties[i].rc_key_map_size =
-- 
1.6.5.7

--

From: Antti Palosaari
Date: Thursday, January 14, 2010 - 12:09 pm

NACK

Device ID 15a4:9016 is reference design ID and it is used by vary many 
devices. Also manufacturer string "Afatech" is chipset default one. This 
leads MSI remote in question configured for many devices using default / 
reference values which I don't like good idea. Strings and other USB 
settings are stored to the device eeprom.

Empia (em28xx) driver uses eeprom hashing for identifying reference ID 
devices. This approach is better because it uses all eeprom bytes. I 
hope you could implement similar eeprom hashing to af9015.

regards
Antti
-- 
http://palosaari.fi/
--

From: Jiri Slaby
Date: Friday, January 22, 2010 - 8:10 am

So as a final patch, add support for hash and one hash entry
for MSI digi vox mini II:
iManufacturer 1 Afatech
iProduct      2 DVB-T 2
iSerial       3 010101010600001

It is now handled with proper IR and key map tables.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-media@vger.kernel.org

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/media/dvb/dvb-usb/af9015.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 796f9d5..650c913 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -788,6 +788,13 @@ static const struct af9015_setup af9015_setup_usbids[] = {
 	{ }
 };
 
+static const struct af9015_setup af9015_setup_hashes[] = {
+	{ 0xb8feb708,
+		af9015_rc_keys_msi, ARRAY_SIZE(af9015_rc_keys_msi),
+		af9015_ir_table_msi, ARRAY_SIZE(af9015_ir_table_msi) },
+	{ }
+};
+
 static void af9015_set_remote_config(struct usb_device *udev,
 		struct dvb_usb_device_properties *props)
 {
@@ -800,7 +807,10 @@ static void af9015_set_remote_config(struct usb_device *udev,
 	} else {
 		u16 vendor = le16_to_cpu(udev->descriptor.idVendor);
 
-		if (vendor == USB_VID_AFATECH) {
+		table = af9015_setup_match(af9015_config.eeprom_sum,
+				af9015_setup_hashes);
+
+		if (!table && vendor == USB_VID_AFATECH) {
 			/* Check USB manufacturer and product strings and try
 			   to determine correct remote in case of chip vendor
 			   reference IDs are used.
@@ -831,7 +841,7 @@ static void af9015_set_remote_config(struct usb_device *udev,
 					ARRAY_SIZE(af9015_ir_table_trekstor)
 				};
 			}
-		} else
+		} else if (!table)
 			table = af9015_setup_match(vendor, af9015_setup_usbids);
 	}
 
-- 
1.6.5.7

--

From: Antti Palosaari
Date: Sunday, January 24, 2010 - 4:55 pm

Acked-by: Antti Palosaari <crope@iki.fi>
-- 
http://palosaari.fi/
--

From: Jiri Slaby
Date: Friday, January 22, 2010 - 8:10 am

Add af9015_setup structure to hold (right now only remote) setup
of distinct receivers.

Add af9015_setup_match for matching ids against tables.

This is for easier matching different kind of ids against tables
to obtain setups. Currently module parameters and usb vendor ids
are switched into and matched against tables. Hashes will follow.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/dvb/dvb-usb/af9015.c |  222 ++++++++++++++---------------------
 1 files changed, 89 insertions(+), 133 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index adba90d..796f9d5 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -732,98 +732,80 @@ error:
 	return ret;
 }
 
+struct af9015_setup {
+	unsigned int id;
+	struct dvb_usb_rc_key *rc_key_map;
+	unsigned int rc_key_map_size;
+	u8 *ir_table;
+	unsigned int ir_table_size;
+};
+
+static const struct af9015_setup *af9015_setup_match(unsigned int id,
+		const struct af9015_setup *table)
+{
+	for (; table->rc_key_map; table++)
+		if (table->id == id)
+			return table;
+	return NULL;
+}
+
+static const struct af9015_setup af9015_setup_modparam[] = {
+	{ AF9015_REMOTE_A_LINK_DTU_M,
+		af9015_rc_keys_a_link, ARRAY_SIZE(af9015_rc_keys_a_link),
+		af9015_ir_table_a_link, ARRAY_SIZE(af9015_ir_table_a_link) },
+	{ AF9015_REMOTE_MSI_DIGIVOX_MINI_II_V3,
+		af9015_rc_keys_msi, ARRAY_SIZE(af9015_rc_keys_msi),
+		af9015_ir_table_msi, ARRAY_SIZE(af9015_ir_table_msi) },
+	{ AF9015_REMOTE_MYGICTV_U718,
+		af9015_rc_keys_mygictv, ARRAY_SIZE(af9015_rc_keys_mygictv),
+		af9015_ir_table_mygictv, ARRAY_SIZE(af9015_ir_table_mygictv) },
+	{ AF9015_REMOTE_DIGITTRADE_DVB_T,
+		af9015_rc_keys_digittrade, ARRAY_SIZE(af9015_rc_keys_digittrade),
+		af9015_ir_table_digittrade, ARRAY_SIZE(af9015_ir_table_digittrade) },
+	{ ...
From: Antti Palosaari
Date: Sunday, January 24, 2010 - 4:54 pm

Acked-by: Antti Palosaari <crope@iki.fi>
-- 
http://palosaari.fi/
--

From: Jiri Slaby
Date: Friday, January 22, 2010 - 8:10 am

This is just a code shuffle without functional changes. For easier
review of later changes, i.e. preparation.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/dvb/dvb-usb/af9015.c |  305 ++++++++++++++++++-----------------
 1 files changed, 157 insertions(+), 148 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index 616b3ba..adba90d 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -732,12 +732,166 @@ error:
 	return ret;
 }
 
+static void af9015_set_remote_config(struct usb_device *udev,
+		struct dvb_usb_device_properties *props)
+{
+	if (dvb_usb_af9015_remote) {
+		/* load remote defined as module param */
+		switch (dvb_usb_af9015_remote) {
+		case AF9015_REMOTE_A_LINK_DTU_M:
+			props->rc_key_map =
+			  af9015_rc_keys_a_link;
+			props->rc_key_map_size =
+			  ARRAY_SIZE(af9015_rc_keys_a_link);
+			af9015_config.ir_table = af9015_ir_table_a_link;
+			af9015_config.ir_table_size =
+			  ARRAY_SIZE(af9015_ir_table_a_link);
+			break;
+		case AF9015_REMOTE_MSI_DIGIVOX_MINI_II_V3:
+			props->rc_key_map =
+			  af9015_rc_keys_msi;
+			props->rc_key_map_size =
+			  ARRAY_SIZE(af9015_rc_keys_msi);
+			af9015_config.ir_table = af9015_ir_table_msi;
+			af9015_config.ir_table_size =
+			  ARRAY_SIZE(af9015_ir_table_msi);
+			break;
+		case AF9015_REMOTE_MYGICTV_U718:
+			props->rc_key_map =
+			  af9015_rc_keys_mygictv;
+			props->rc_key_map_size =
+			  ARRAY_SIZE(af9015_rc_keys_mygictv);
+			af9015_config.ir_table =
+			  af9015_ir_table_mygictv;
+			af9015_config.ir_table_size =
+			  ARRAY_SIZE(af9015_ir_table_mygictv);
+			break;
+		case AF9015_REMOTE_DIGITTRADE_DVB_T:
+			props->rc_key_map =
+			  af9015_rc_keys_digittrade;
+			props->rc_key_map_size =
+			  ARRAY_SIZE(af9015_rc_keys_digittrade);
+			af9015_config.ir_table =
+			  ...
From: Antti Palosaari
Date: Sunday, January 24, 2010 - 4:53 pm

Acked-by: Antti Palosaari <crope@iki.fi>

-- 
http://palosaari.fi/
--

From: Jiri Slaby
Date: Friday, January 22, 2010 - 8:11 am

What do you think about the following patches?

thanks,
-- 
js
--

From: Jiri Slaby
Date: Friday, January 22, 2010 - 8:10 am

This will be useful for matching of IR tables later.

We read the eeprom anyway for dumping. Switch the dumping to
print_hex_dump_bytes and compute hash above that by
hash = 0;
for (u32 VAL) in (eeprom):
  hash *= GOLDEN_RATIO_PRIME_32
  hash += VAL; // while preserving endinaness

The computation is moved earlier to the flow, namely from
af9015_af9013_frontend_attach to af9015_read_config, so that
we can access the sum in af9015_read_config already.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Antti Palosaari <crope@iki.fi>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-media@vger.kernel.org
---
 drivers/media/dvb/dvb-usb/af9015.c |   65 +++++++++++++++++++++++------------
 drivers/media/dvb/dvb-usb/af9015.h |    1 +
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index a365c05..616b3ba 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -21,6 +21,8 @@
  *
  */
 
+#include <linux/hash.h>
+
 #include "af9015.h"
 #include "af9013.h"
 #include "mt2060.h"
@@ -553,26 +555,45 @@ exit:
 	return ret;
 }
 
-/* dump eeprom */
-static int af9015_eeprom_dump(struct dvb_usb_device *d)
+/* hash (and dump) eeprom */
+static int af9015_eeprom_hash(struct usb_device *udev)
 {
-	u8 reg, val;
+	static const unsigned int eeprom_size = 256;
+	unsigned int reg;
+	int ret;
+	u8 val, *eeprom;
+	struct req_t req = {READ_I2C, AF9015_I2C_EEPROM, 0, 0, 1, 1, &val};
 
-	for (reg = 0; ; reg++) {
-		if (reg % 16 == 0) {
-			if (reg)
-				deb_info(KERN_CONT "\n");
-			deb_info(KERN_DEBUG "%02x:", reg);
-		}
-		if (af9015_read_reg_i2c(d, AF9015_I2C_EEPROM, reg, &val) == 0)
-			deb_info(KERN_CONT " %02x", val);
-		else
-			deb_info(KERN_CONT " --");
-		if (reg == 0xff)
-			break;
+	eeprom = kmalloc(eeprom_size, GFP_KERNEL);
+	if (eeprom == NULL)
+		return -ENOMEM;
+
+	for (reg = 0; reg < eeprom_size; reg++) {
+		req.addr = ...
From: Antti Palosaari
Date: Sunday, January 24, 2010 - 9:16 am

Hei,
Comments below.


Does this sum contain all 256 bytes from EEPROM? 256/4 is 64.

regards
Antti
-- 
http://palosaari.fi/
--

From: Jiri Slaby
Date: Sunday, January 24, 2010 - 9:35 am

Yes it does. It is computed as a hashed sum of 32-bit numbers (4 bytes)
-- speed (does not matter) and larger space of hashes. Hence the
division by 4. The cast does the trick: ((u32 *)eeprom)[reg] -- reg
index is on a 4-byte basis.

regards,
-- 
js
--

From: Antti Palosaari
Date: Sunday, January 24, 2010 - 4:52 pm

OK, true. Anyhow, I don't know if this hashing formula is good enough - 
changing it later could be really pain. I compared it to the one used 
for em28xx driver and it was different. Could someone with better 
knowledge check that?

Generally it is good and ready for submission.

Acked-by: Antti Palosaari <crope@iki.fi>

regards
Antti
-- 
http://palosaari.fi/
--

Previous thread: [PATCH 1/1] HID: ignore afatech 9016 by Jiri Slaby on Wednesday, January 13, 2010 - 12:59 pm. (7 messages)

Next thread: mmotm 2010-01-13-12-17 uploaded by akpm on Wednesday, January 13, 2010 - 1:17 pm. (23 messages)