| Message ID | cover.1648499509.git.mchehab@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | dvb-usb: use designated initializers | expand |
(adding Benjamin Stürz and link to series) https://lore.kernel.org/all/cover.1648499509.git.mchehab@kernel.org/ On Mon, 2022-03-28 at 22:41 +0200, Mauro Carvalho Chehab wrote: > There are two DVB USB cores on media. The new one (dvb-usb-v2) > solves several problems with the previous one, but, unfortunately, > there are several drivers that weren't migrated yet. > > One of the problems with dvb-usb is that, besides the common > DVB USB table, it also uses a per-device table which require > links to the USB ID table. > > This is done, on most drivers, using a magic number, which is easy > to get outdated. > > Rewrite the drivers in order to use an enum and use designated > initializers where needed. > > This patch series was inspired on this patch: > https://patchwork.kernel.org/project/linux-media/patch/20220326165909.506926-16-benni@stuerz.xyz/ > > While it would be nice to also change the tables to be const, this > is currently not possible, as a couple drivers that depend on the > dvb-usb struct depend on it to not be const. > > Writing a patch series like that and making it properly referencing > the right entries is not fun. That's most drivers were never fixed. > So, I ended using a script to change it, adding several checks on > it, in order to avoid the risk of problems. fancy script, nice... > > Even so, I needed to manually adjust some patches. > > This is the used script: > > <script> > #!/usr/bin/perl > use strict; > use warnings; > use Text::Tabs; > use File::Find; > > my $usb_id_pre = ""; > my $usb_id_end = ""; > > my %vid; > my %pid; > > my %pid_count; > > my $state = "pre"; > > sub out_ids() > { > open OUT, "> include/media/dvb-usb-ids.h" or die; > print OUT $usb_id_pre; > foreach my $k(sort keys %vid) { > my $ln = sprintf "#define USB_VID_%-31s %s\n", $k, $vid{$k}; > $ln = unexpand($ln); > print OUT $ln; > } > > print OUT "\n/* Product IDs */\n\n"; > foreach my $k(sort keys %pid) { > my $ln = sprintf "#define USB_PID_%-39s %s\n", $k, $pid{$k}; > $ln = unexpand($ln); > print OUT $ln; > } > print OUT $usb_id_end; > close OUT; > } > > my @enum; > > sub validate_model($$) > { > my $my_vid = $1; > my $my_pid = $2; > > if (!$vid{$my_vid}) { > print "$.# ERROR! VID: $my_vid\n"; > return ""; > } > > if (!$pid{$my_pid}) { > print "$.# ERROR! PID: $my_pid\n"; > return ""; > } > > my $tmp_vid = $my_vid; > $tmp_vid =~ s/_ELECTRONIC$//; > $tmp_vid =~ s/_MICRO$//; > $tmp_vid =~ s/_1$//; > $tmp_vid =~ s/_2$//; > $tmp_vid =~ s/_UNK$//; > $tmp_vid =~ s/GTEK/GENIATECH/; > > # Change model name to be different > my $model = $tmp_vid . "_" . $my_pid; > > return $my_pid if ($my_pid =~ /$tmp_vid[_]/ && !grep(/^$my_pid$/, @enum)); > > print "NEW model: $model\n"; > > # Add the new PID to include/media/dvb-usb-ids.h > $pid{$model} = $pid{$my_pid}; > > $pid_count{$my_pid}--; > > # Drop unused PIDs > if (!$pid_count{$my_pid}) { > delete ($pid{$my_pid}); > print "Drop: $my_pid\n"; > } > > return $model if (!grep(/^$model$/, @enum)); > > print "$.# ERROR! Model $model aready exists\n"; > return ""; > } > > sub count_pid() > { > my $file = $File::Find::name; > > return if (!($file =~ /[.][ch]/)); > > open IN, $file or die "Can't open $file"; > while (<IN>) { > if (/USB_PID_([\w\_]+)/) { > $pid_count{$1}++; > next; > } > } > close IN; > } > > open IN, "include/media/dvb-usb-ids.h" or die; > while (<IN>) { > if (/#define\s+USB_VID_(\S+)\s+(\S+)/) { > $state = "vid"; > $vid{$1} = $2; > #print "VID: $1 -> $2\n"; > next; > } > if (/#define\s+USB_PID_(\S+)\s+(\S+)/) { > $state = "pid"; > $pid{$1} = $2; > #print "PID: $1 -> $2\n"; > next; > } > > if ($state eq "pre") { > $usb_id_pre .= $_; > } elsif ($state eq "pid") { > $usb_id_end .= $_; > } > } > close IN; > > find({wanted => \&count_pid, no_chdir => 1}, "drivers/media/usb/"); > > > while (scalar @ARGV) { > @enum = (); > my @dev_id; > > > my $usb_dev_id_table; > my $out = ""; > my $should_write = 1; > > my $file = $ARGV[0]; > > print "Processing $file...\n"; > > open IN, $file or die "Can't open $1"; > > my $entry = ""; > > my $pos = 0; > > while (<IN>) { > if (m/struct\s.*usb_device_id\s+([\w\_]+).*{/) { > $usb_dev_id_table = $1; > } > if (!$usb_dev_id_table) { > $out .= $_; > next; > } > > if (m,\/\*\s*(\d+)\s*\*\/,) { > if ($1 != $pos) { > printf "$.# ERROR! Count is wrong! Want %d, got %d\n", $pos, $1; > $should_write = 0; > last; > } > } > > $pos++ if (m/USB_DEVICE/); > > last if (m/^}\s*;/); > > $entry .= $_; > next if (!(m/}\s*,/)); > > if ($entry =~ m/USB_DEVICE\(USB_VID_([\w\_]+)\s*,\s*USB_PID_([\w\_]+)/) { > my $my_vid = $1; > my $my_pid = $2; > > my $model = validate_model($my_vid, $my_pid); > if ($model eq "") { > $should_write = 0; > last; > } > > push @enum, $model; > push @dev_id, "DVB_USB_DEV($my_vid, $model)"; > print "DVB_USB_DEV($my_vid, $model)\n"; > } > if ($entry =~ m/USB_DEVICE_VER\(USB_VID_([\w\_]+)\s*,\s*USB_PID_([\w\_]+)\s*,\s*(\w+)\s*,\s*(\w+)/) { > my $my_vid = $1; > my $my_pid = $2; > my $lo = $3; > my $hi = $4; > > my $model = validate_model($my_vid, $my_pid); > if ($model eq "") { > $should_write = 0; > last; > } > > push @enum, $model; > push @dev_id, "DVB_USB_DEV_VER($my_vid, $model, $lo, $hi)"; > print "DVB_USB_DEV_VER($my_vid, $model, $lo, $hi)\n"; > } > $entry = ""; > } > > if ($should_write && scalar(@enum) != $pos) { > printf "ERROR! Count is wrong! Want %d, got %d\n", $pos, scalar(@enum); > $should_write = 0; > } > > if ($usb_dev_id_table && $should_write) { > $out .= "enum {\n"; > for my $e (@enum) { > $out .= "\t$e,\n"; > } > $out .= "};\n\n"; > > $out .= "static struct usb_device_id $usb_dev_id_table\[\] = {\n"; > for my $e (@dev_id) { > $out .= "\t$e,\n"; > } > $out .= "\t{ }\n"; > $out .= "};\n\n"; > > my $start = 1; > > while (<IN>) { > next if ($start && m/^$/); > $start = 0; > while (m/$usb_dev_id_table\[(\d+)\]/g) { > my $i = $1; > > if ($1 > scalar @enum) { > print "ERROR! $usb_dev_id_table element $1 doesn't exist!\n"; > $should_write = 0; > last; > } > > my $idx = $enum[$1]; > > if (!(s,($usb_dev_id_table)\[$i\],$1\[$idx],g)) { > print "ERROR! can't replace $1!\n"; > $should_write = 0; > last; > } > } > $out .= $_; > } > close IN; > } else { > while (<IN>) { > $out .= $_; > } > print "\tunchanged\n"; > $should_write = 0; > } > > if ($should_write || $file eq "include/media/dvb-usb-ids.h") { > open OUT, ">$file" or die "Can't write on file $file"; > print OUT $out; > close OUT; > printf "\twrote\n"; > > out_ids(); > } > > shift @ARGV; > } > </script> > > > Mauro Carvalho Chehab (26): > media: dvb-usb-ids.h: sort entries > media: dvb-usb: move USB IDs to dvb-usb-ids.h > media: dvb-usb: vp702x: reference to usb ID table > media: dvb-usb: Add helper macros for using USB VID/PID > media: dvb-usb: a800: use an enum for the device number > media: af9005: use the newer dvb-usb macros for USB device > media: dvb-usb: az6027: use an enum for the device number > media: cinergyT2-core: use the newer dvb-usb macros for USB device > media: cxusb: use the newer dvb-usb macros for USB device > media: digitv: use the newer dvb-usb macros for USB device > media: dvb-usb: dtt200u: use an enum for the device number > media: dtv5100: use the newer dvb-usb macros for USB device > media: dw2102: use the newer dvb-usb macros for USB device > media: dvb-usb: gp8psk: use an enum for the device number > media: dvb-usb: m920x: use an enum for the device number > media: dvb-usb: nova-t-usb2: use an enum for the device number > media: dvb-usb: opera1: use an enum for the device number > media: dvb-usb: pctv452e: use an enum for the device number > media: technisat-usb2: use the newer dvb-usb macros for USB device > media: dvb-usb: ttusb2: use an enum for the device number > media: dvb-usb: umt-010: use an enum for the device number > media: dvb-usb: vp702x: use an enum for the device number > media: dvb-usb: vp7045: use an enum for the device number > media: dvb-usb: dibusb-mb: use an enum for the device number > media: dvb-usb: dibusb-mc: use an enum for the device number > media: dvb-usb: dib0700_devices: use an enum for the device number > > drivers/media/usb/dvb-usb/a800.c | 18 +- > drivers/media/usb/dvb-usb/af9005.c | 19 +- > drivers/media/usb/dvb-usb/az6027.c | 45 +- > drivers/media/usb/dvb-usb/cinergyT2-core.c | 10 +- > drivers/media/usb/dvb-usb/cxusb.c | 88 +-- > drivers/media/usb/dvb-usb/dib0700_devices.c | 428 +++++++------ > drivers/media/usb/dvb-usb/dibusb-mb.c | 165 ++--- > drivers/media/usb/dvb-usb/dibusb-mc.c | 88 +-- > drivers/media/usb/dvb-usb/digitv.c | 13 +- > drivers/media/usb/dvb-usb/dtt200u.c | 56 +- > drivers/media/usb/dvb-usb/dtv5100.c | 11 +- > drivers/media/usb/dvb-usb/dw2102.c | 84 ++- > drivers/media/usb/dvb-usb/gp8psk.c | 36 +- > drivers/media/usb/dvb-usb/m920x.c | 51 +- > drivers/media/usb/dvb-usb/nova-t-usb2.c | 18 +- > drivers/media/usb/dvb-usb/opera1.c | 15 +- > drivers/media/usb/dvb-usb/pctv452e.c | 22 +- > drivers/media/usb/dvb-usb/technisat-usb2.c | 11 +- > drivers/media/usb/dvb-usb/ttusb2.c | 36 +- > drivers/media/usb/dvb-usb/umt-010.c | 18 +- > drivers/media/usb/dvb-usb/vp702x.c | 23 +- > drivers/media/usb/dvb-usb/vp7045.c | 28 +- > include/media/dvb-usb-ids.h | 632 +++++++++++--------- > 23 files changed, 1071 insertions(+), 844 deletions(-) >
There are two DVB USB cores on media. The new one (dvb-usb-v2) solves several problems with the previous one, but, unfortunately, there are several drivers that weren't migrated yet. One of the problems with dvb-usb is that, besides the common DVB USB table, it also uses a per-device table which require links to the USB ID table. This is done, on most drivers, using a magic number, which is easy to get outdated. Rewrite the drivers in order to use an enum and use designated initializers where needed. This patch series was inspired on this patch: https://patchwork.kernel.org/project/linux-media/patch/20220326165909.506926-16-benni@stuerz.xyz/ While it would be nice to also change the tables to be const, this is currently not possible, as a couple drivers that depend on the dvb-usb struct depend on it to not be const. Writing a patch series like that and making it properly referencing the right entries is not fun. That's most drivers were never fixed. So, I ended using a script to change it, adding several checks on it, in order to avoid the risk of problems. Even so, I needed to manually adjust some patches. This is the used script: <script> #!/usr/bin/perl use strict; use warnings; use Text::Tabs; use File::Find; my $usb_id_pre = ""; my $usb_id_end = ""; my %vid; my %pid; my %pid_count; my $state = "pre"; sub out_ids() { open OUT, "> include/media/dvb-usb-ids.h" or die; print OUT $usb_id_pre; foreach my $k(sort keys %vid) { my $ln = sprintf "#define USB_VID_%-31s %s\n", $k, $vid{$k}; $ln = unexpand($ln); print OUT $ln; } print OUT "\n/* Product IDs */\n\n"; foreach my $k(sort keys %pid) { my $ln = sprintf "#define USB_PID_%-39s %s\n", $k, $pid{$k}; $ln = unexpand($ln); print OUT $ln; } print OUT $usb_id_end; close OUT; } my @enum; sub validate_model($$) { my $my_vid = $1; my $my_pid = $2; if (!$vid{$my_vid}) { print "$.# ERROR! VID: $my_vid\n"; return ""; } if (!$pid{$my_pid}) { print "$.# ERROR! PID: $my_pid\n"; return ""; } my $tmp_vid = $my_vid; $tmp_vid =~ s/_ELECTRONIC$//; $tmp_vid =~ s/_MICRO$//; $tmp_vid =~ s/_1$//; $tmp_vid =~ s/_2$//; $tmp_vid =~ s/_UNK$//; $tmp_vid =~ s/GTEK/GENIATECH/; # Change model name to be different my $model = $tmp_vid . "_" . $my_pid; return $my_pid if ($my_pid =~ /$tmp_vid[_]/ && !grep(/^$my_pid$/, @enum)); print "NEW model: $model\n"; # Add the new PID to include/media/dvb-usb-ids.h $pid{$model} = $pid{$my_pid}; $pid_count{$my_pid}--; # Drop unused PIDs if (!$pid_count{$my_pid}) { delete ($pid{$my_pid}); print "Drop: $my_pid\n"; } return $model if (!grep(/^$model$/, @enum)); print "$.# ERROR! Model $model aready exists\n"; return ""; } sub count_pid() { my $file = $File::Find::name; return if (!($file =~ /[.][ch]/)); open IN, $file or die "Can't open $file"; while (<IN>) { if (/USB_PID_([\w\_]+)/) { $pid_count{$1}++; next; } } close IN; } open IN, "include/media/dvb-usb-ids.h" or die; while (<IN>) { if (/#define\s+USB_VID_(\S+)\s+(\S+)/) { $state = "vid"; $vid{$1} = $2; #print "VID: $1 -> $2\n"; next; } if (/#define\s+USB_PID_(\S+)\s+(\S+)/) { $state = "pid"; $pid{$1} = $2; #print "PID: $1 -> $2\n"; next; } if ($state eq "pre") { $usb_id_pre .= $_; } elsif ($state eq "pid") { $usb_id_end .= $_; } } close IN; find({wanted => \&count_pid, no_chdir => 1}, "drivers/media/usb/"); while (scalar @ARGV) { @enum = (); my @dev_id; my $usb_dev_id_table; my $out = ""; my $should_write = 1; my $file = $ARGV[0]; print "Processing $file...\n"; open IN, $file or die "Can't open $1"; my $entry = ""; my $pos = 0; while (<IN>) { if (m/struct\s.*usb_device_id\s+([\w\_]+).*{/) { $usb_dev_id_table = $1; } if (!$usb_dev_id_table) { $out .= $_; next; } if (m,\/\*\s*(\d+)\s*\*\/,) { if ($1 != $pos) { printf "$.# ERROR! Count is wrong! Want %d, got %d\n", $pos, $1; $should_write = 0; last; } } $pos++ if (m/USB_DEVICE/); last if (m/^}\s*;/); $entry .= $_; next if (!(m/}\s*,/)); if ($entry =~ m/USB_DEVICE\(USB_VID_([\w\_]+)\s*,\s*USB_PID_([\w\_]+)/) { my $my_vid = $1; my $my_pid = $2; my $model = validate_model($my_vid, $my_pid); if ($model eq "") { $should_write = 0; last; } push @enum, $model; push @dev_id, "DVB_USB_DEV($my_vid, $model)"; print "DVB_USB_DEV($my_vid, $model)\n"; } if ($entry =~ m/USB_DEVICE_VER\(USB_VID_([\w\_]+)\s*,\s*USB_PID_([\w\_]+)\s*,\s*(\w+)\s*,\s*(\w+)/) { my $my_vid = $1; my $my_pid = $2; my $lo = $3; my $hi = $4; my $model = validate_model($my_vid, $my_pid); if ($model eq "") { $should_write = 0; last; } push @enum, $model; push @dev_id, "DVB_USB_DEV_VER($my_vid, $model, $lo, $hi)"; print "DVB_USB_DEV_VER($my_vid, $model, $lo, $hi)\n"; } $entry = ""; } if ($should_write && scalar(@enum) != $pos) { printf "ERROR! Count is wrong! Want %d, got %d\n", $pos, scalar(@enum); $should_write = 0; } if ($usb_dev_id_table && $should_write) { $out .= "enum {\n"; for my $e (@enum) { $out .= "\t$e,\n"; } $out .= "};\n\n"; $out .= "static struct usb_device_id $usb_dev_id_table\[\] = {\n"; for my $e (@dev_id) { $out .= "\t$e,\n"; } $out .= "\t{ }\n"; $out .= "};\n\n"; my $start = 1; while (<IN>) { next if ($start && m/^$/); $start = 0; while (m/$usb_dev_id_table\[(\d+)\]/g) { my $i = $1; if ($1 > scalar @enum) { print "ERROR! $usb_dev_id_table element $1 doesn't exist!\n"; $should_write = 0; last; } my $idx = $enum[$1]; if (!(s,($usb_dev_id_table)\[$i\],$1\[$idx],g)) { print "ERROR! can't replace $1!\n"; $should_write = 0; last; } } $out .= $_; } close IN; } else { while (<IN>) { $out .= $_; } print "\tunchanged\n"; $should_write = 0; } if ($should_write || $file eq "include/media/dvb-usb-ids.h") { open OUT, ">$file" or die "Can't write on file $file"; print OUT $out; close OUT; printf "\twrote\n"; out_ids(); } shift @ARGV; } </script> Mauro Carvalho Chehab (26): media: dvb-usb-ids.h: sort entries media: dvb-usb: move USB IDs to dvb-usb-ids.h media: dvb-usb: vp702x: reference to usb ID table media: dvb-usb: Add helper macros for using USB VID/PID media: dvb-usb: a800: use an enum for the device number media: af9005: use the newer dvb-usb macros for USB device media: dvb-usb: az6027: use an enum for the device number media: cinergyT2-core: use the newer dvb-usb macros for USB device media: cxusb: use the newer dvb-usb macros for USB device media: digitv: use the newer dvb-usb macros for USB device media: dvb-usb: dtt200u: use an enum for the device number media: dtv5100: use the newer dvb-usb macros for USB device media: dw2102: use the newer dvb-usb macros for USB device media: dvb-usb: gp8psk: use an enum for the device number media: dvb-usb: m920x: use an enum for the device number media: dvb-usb: nova-t-usb2: use an enum for the device number media: dvb-usb: opera1: use an enum for the device number media: dvb-usb: pctv452e: use an enum for the device number media: technisat-usb2: use the newer dvb-usb macros for USB device media: dvb-usb: ttusb2: use an enum for the device number media: dvb-usb: umt-010: use an enum for the device number media: dvb-usb: vp702x: use an enum for the device number media: dvb-usb: vp7045: use an enum for the device number media: dvb-usb: dibusb-mb: use an enum for the device number media: dvb-usb: dibusb-mc: use an enum for the device number media: dvb-usb: dib0700_devices: use an enum for the device number drivers/media/usb/dvb-usb/a800.c | 18 +- drivers/media/usb/dvb-usb/af9005.c | 19 +- drivers/media/usb/dvb-usb/az6027.c | 45 +- drivers/media/usb/dvb-usb/cinergyT2-core.c | 10 +- drivers/media/usb/dvb-usb/cxusb.c | 88 +-- drivers/media/usb/dvb-usb/dib0700_devices.c | 428 +++++++------ drivers/media/usb/dvb-usb/dibusb-mb.c | 165 ++--- drivers/media/usb/dvb-usb/dibusb-mc.c | 88 +-- drivers/media/usb/dvb-usb/digitv.c | 13 +- drivers/media/usb/dvb-usb/dtt200u.c | 56 +- drivers/media/usb/dvb-usb/dtv5100.c | 11 +- drivers/media/usb/dvb-usb/dw2102.c | 84 ++- drivers/media/usb/dvb-usb/gp8psk.c | 36 +- drivers/media/usb/dvb-usb/m920x.c | 51 +- drivers/media/usb/dvb-usb/nova-t-usb2.c | 18 +- drivers/media/usb/dvb-usb/opera1.c | 15 +- drivers/media/usb/dvb-usb/pctv452e.c | 22 +- drivers/media/usb/dvb-usb/technisat-usb2.c | 11 +- drivers/media/usb/dvb-usb/ttusb2.c | 36 +- drivers/media/usb/dvb-usb/umt-010.c | 18 +- drivers/media/usb/dvb-usb/vp702x.c | 23 +- drivers/media/usb/dvb-usb/vp7045.c | 28 +- include/media/dvb-usb-ids.h | 632 +++++++++++--------- 23 files changed, 1071 insertions(+), 844 deletions(-)