Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, Jan 14, 2007 at 04:29:48PM -0800, Greg KH wrote: > On Sun, Jan 14, 2007 at 07:24:21PM +0200, Ahmed S. Darwish wrote: > > Substitue intel_rng magic PCI IDs values used in the IDs table > > with the macros defined in pci_ids.h > > Why not use the PCI_DEVICE() macro too? It should make the lines even > smaller. Just for the patch applier, It seems that my mailer got insane. I've updated the patch to include PCI_DEVICE as Mr. Greg KH said But it appears in the LKML above his mail. So if no more suggestions come, the final patch is in: http://lkml.org/lkml/2007/1/15/110 Thanks, -- Ahmed S. Darwish http://darwish-07.blogspot.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, Jan 14, 2007 at 04:29:48PM -0800, Greg KH wrote:
> Why not use the PCI_DEVICE() macro too? It should make the lines even
> smaller.
Hi all, Here's the updated patch.
Substitue magic values used in the PCI IDs table with macros defined in
pci_ids.h + using the PCI_DEVICE macro.
Signed-off-by: Ahmed S. Darwish <[EMAIL PROTECTED]>
---
I've used a script to generate that patch, then checked the results manually
to make sure that it's (hopefully) correct.
#!/bin/bash
INTEL_RNG_FILE=drivers/char/hw_random/intel-rng.c
TMP_FILE=$(mktemp)
# File to hold Magic PCI deviceIDs in the "pci_device_id pci_tbl[]"
MAGIC_DEVICE_IDS=$(mktemp)
# All pci_tbl[] contents begin with a "{ 0x8086"
grep "{ 0x8086" drivers/char/hw_random/intel-rng.c > $TMP_FILE
awk ' { print $3 } ' $TMP_FILE | cut -d, -f1 | grep "0x" > $MAGIC_DEVICE_IDS
sed -i "s/{ 0x8086/{ PCI_VENDOR_ID_INTEL/g" $INTEL_RNG_FILE
# For each magic number in MAGIC_DEVICE_IDS, find its defined macro
# in pci_ids.h then use it in the PCI IDs table.
for i in $(cat $MAGIC_DEVICE_IDS); do
macro=$(grep "PCI_DEVICE_ID" include/linux/pci_ids.h | grep "$i" | awk ' {
print $2 } ')
if [[ -n "$macro" ]] ; then
sed -i "s/{ PCI_VENDOR_ID_INTEL, ${i},.*}/{
PCI_DEVICE(PCI_VENDOR_ID_INTEL, ${macro}) }/" \
$INTEL_RNG_FILE
else
sed -i "s/{ PCI_VENDOR_ID_INTEL, ${i},.*}/{
PCI_DEVICE(PCI_VENDOR_ID_INTEL, ${i}) }/" \
$INTEL_RNG_FILE
fi
done
rm -f $TMP_FILE
rm -f $MAGIC_DEVICE_IDS
diff --git a/drivers/char/hw_random/intel-rng.c
b/drivers/char/hw_random/intel-rng.c
index f22e78e..d25d5e1 100644
--- a/drivers/char/hw_random/intel-rng.c
+++ b/drivers/char/hw_random/intel-rng.c
@@ -96,49 +96,49 @@
*/
static const struct pci_device_id pci_tbl[] = {
/* AA
- { 0x8086, 0x2418, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2410, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AA */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_8) }, */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0) },
/* AB
- { 0x8086, 0x2428, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AB */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_8) }, */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0) },
/* ??
- { 0x8086, 0x2430, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x2430) }, */
/* BAM, CAM, DBM, FBM, GxM
- { 0x8086, 0x2448, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x244c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BAM */
- { 0x8086, 0x248c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CAM */
- { 0x8086, 0x24cc, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DBM */
- { 0x8086, 0x2641, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* FBM */
- { 0x8086, 0x27b9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM */
- { 0x8086, 0x27bd, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM DH */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_6) }, */
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_10) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_12) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH6_1) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH7_1) },
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH7_31) },
/* BA, CA, DB, Ex, 6300, Fx, 631x/632x, Gx
- { 0x8086, 0x244e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2440, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BA */
- { 0x8086, 0x2480, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CA */
- { 0x8086, 0x24c0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DB */
- { 0x8086, 0x24d0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Ex */
- { 0x8086, 0x25a1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 6300 */
- { 0x8086, 0x2640, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Fx */
- { 0x8086, 0x2670, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2671, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2672, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2673, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2674, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2675, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2676, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2677, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2678, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2679, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267b, PCI_ANY_ID, PCI_ANY_I
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, Jan 14, 2007 at 04:29:48PM -0800, Greg KH wrote: > On Sun, Jan 14, 2007 at 07:24:21PM +0200, Ahmed S. Darwish wrote: > > Substitue intel_rng magic PCI IDs values used in the IDs table > > with the macros defined in pci_ids.h > > Why not use the PCI_DEVICE() macro too? It should make the lines even > smaller. Ooh yes, I forgot about this macro. Updating the patch. Thanks :). -- Ahmed S. Darwish http://darwish-07.blogspot.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, Jan 14, 2007 at 09:37:21AM -0800, Arjan van de Ven wrote: > On Sun, 2007-01-14 at 19:24 +0200, Ahmed S. Darwish wrote: > > Substitue intel_rng magic PCI IDs values used in the IDs table > > with the macros defined in pci_ids.h > > > Hi, > > > hmm this is actually the opposite direction than most of the kernel is > heading in, mostly because the pci_ids.h file is a major maintenance > pain. > > Afaik the current "rule" is: if a PCI ID is only used in one driver, use > the numeric value and not (add) a symbolic constant. I think you understood me wrong .. I haven't added new macros in pci_ids.h, I've just used the macros already defined there like the PCI_VENDOR_ID_INTEL and others. -- Ahmed S. Darwish http://darwish-07.blogspot.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Mon, Jan 15, 2007 at 01:07:18AM +0200, Ahmed S. Darwish wrote: > On Mon, Jan 15, 2007 at 06:31:01AM +1100, Dave Airlie wrote: > > On 1/15/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: > > >On Sun, 2007-01-14 at 19:24 +0200, Ahmed S. Darwish wrote: > > >> Substitue intel_rng magic PCI IDs values used in the IDs table > > >> with the macros defined in pci_ids.h > > >> > > >Hi, > > > > > >hmm this is actually the opposite direction than most of the kernel is > > >heading in, mostly because the pci_ids.h file is a major maintenance > > >pain. > > > > > >Afaik the current "rule" is: if a PCI ID is only used in one driver, use > > >the numeric value and not (add) a symbolic constant. > > > > > > > My guess is that the RNG is on the LPC so the values are used in a few > > places.. > > > > Will pci_ids.h be removed from the tree some time in the future then ? No, it should contain ids that are used in multiple places (vendor ids are one such example.) And in this case, the majority of the patch is just using the vendor id, which in my opinion, is a good idea. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, Jan 14, 2007 at 07:24:21PM +0200, Ahmed S. Darwish wrote: > Substitue intel_rng magic PCI IDs values used in the IDs table > with the macros defined in pci_ids.h Why not use the PCI_DEVICE() macro too? It should make the lines even smaller. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Mon, Jan 15, 2007 at 06:31:01AM +1100, Dave Airlie wrote: > On 1/15/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: > >On Sun, 2007-01-14 at 19:24 +0200, Ahmed S. Darwish wrote: > >> Substitue intel_rng magic PCI IDs values used in the IDs table > >> with the macros defined in pci_ids.h > >> > >Hi, > > > >hmm this is actually the opposite direction than most of the kernel is > >heading in, mostly because the pci_ids.h file is a major maintenance > >pain. > > > >Afaik the current "rule" is: if a PCI ID is only used in one driver, use > >the numeric value and not (add) a symbolic constant. > > > > My guess is that the RNG is on the LPC so the values are used in a few > places.. > Will pci_ids.h be removed from the tree some time in the future then ? -- Ahmed S. Darwish http://darwish-07.blogspot.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On 1/15/07, Arjan van de Ven <[EMAIL PROTECTED]> wrote: On Sun, 2007-01-14 at 19:24 +0200, Ahmed S. Darwish wrote: > Substitue intel_rng magic PCI IDs values used in the IDs table > with the macros defined in pci_ids.h > Hi, hmm this is actually the opposite direction than most of the kernel is heading in, mostly because the pci_ids.h file is a major maintenance pain. Afaik the current "rule" is: if a PCI ID is only used in one driver, use the numeric value and not (add) a symbolic constant. My guess is that the RNG is on the LPC so the values are used in a few places.. Dave. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
On Sun, 2007-01-14 at 19:24 +0200, Ahmed S. Darwish wrote: > Substitue intel_rng magic PCI IDs values used in the IDs table > with the macros defined in pci_ids.h > Hi, hmm this is actually the opposite direction than most of the kernel is heading in, mostly because the pci_ids.h file is a major maintenance pain. Afaik the current "rule" is: if a PCI ID is only used in one driver, use the numeric value and not (add) a symbolic constant. Greetings, Arjan van de Ven - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2.6.20-rc5] intel_rng: substitue magic PCI IDs with macros
Substitue intel_rng magic PCI IDs values used in the IDs table
with the macros defined in pci_ids.h
Signed-off-by: Ahmed S. Darwish <[EMAIL PROTECTED]>
---
I've used a small script to generate this patch then manually tried to
make sure it's (hopefully) correct.
#!/bin/bash
INTEL_RNG_FILE=drivers/char/hw_random/intel-rng.c
TMP_FILE=$(mktemp)
MAGIC_NUMS_FILE=$(mktemp)
# grep the contents of "pci_device_id pci_tbl[]"
grep "{ 0x8086" drivers/char/hw_random/intel-rng.c > $TMP_FILE
# Extract the magic numbers to be replaced
cat $TMP_FILE | awk ' { print $3 } ' | cut -d, -f1 | grep "0x" >
$MAGIC_NUMS_FILE
sed -i "s/0x8086/PCI_VENDOR_ID_INTEL/g" $INTEL_RNG_FILE
# For each magic number in MAGIC_NUMS_FILE, find its defined macro
# in pci_ids.h then replace them.
for i in $(cat $MAGIC_NUMS_FILE); do
var=$(grep "PCI_DEVICE_ID" include/linux/pci_ids.h | grep "$i" | awk ' {
print $2 } ')
if [[ -n "$var" ]] ; then
# sed: Side effect of replacing the magic number in the whole file
# Collisions with other magics belonging to another family ?
# Manual checking reveals no bad collisions happen.
sed -i "s/${i}/${var}/g" $INTEL_RNG_FILE
fi
done
rm -f $TMP_FILE
rm -f $MAGIC_NUMS_FILE
diff --git a/drivers/char/hw_random/intel-rng.c
b/drivers/char/hw_random/intel-rng.c
index f22e78e..85b0374 100644
--- a/drivers/char/hw_random/intel-rng.c
+++ b/drivers/char/hw_random/intel-rng.c
@@ -95,50 +95,96 @@
* want to register another driver on the same PCI id.
*/
static const struct pci_device_id pci_tbl[] = {
-/* AA
- { 0x8086, 0x2418, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2410, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AA */
-/* AB
- { 0x8086, 0x2428, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2420, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* AB */
-/* ??
- { 0x8086, 0x2430, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
-/* BAM, CAM, DBM, FBM, GxM
- { 0x8086, 0x2448, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x244c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BAM */
- { 0x8086, 0x248c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CAM */
- { 0x8086, 0x24cc, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DBM */
- { 0x8086, 0x2641, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* FBM */
- { 0x8086, 0x27b9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM */
- { 0x8086, 0x27bd, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* GxM DH */
-/* BA, CA, DB, Ex, 6300, Fx, 631x/632x, Gx
- { 0x8086, 0x244e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2440, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* BA */
- { 0x8086, 0x2480, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* CA */
- { 0x8086, 0x24c0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* DB */
- { 0x8086, 0x24d0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Ex */
- { 0x8086, 0x25a1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 6300 */
- { 0x8086, 0x2640, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Fx */
- { 0x8086, 0x2670, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2671, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2672, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2673, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2674, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2675, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2676, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2677, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2678, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x2679, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267a, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267b, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267c, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267d, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x267f, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* 631x/632x */
- { 0x8086, 0x27b8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* Gx */
-/* E
- { 0x8086, 0x245e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, */
- { 0x8086, 0x2450, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0, }, /* E */
+/*
+ * AA
+ * { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_8, PCI_ANY_ID,
+ * PCI_ANY_ID, 0, 0, 0, },
+ */
+ { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AA_0, PCI_ANY_ID,
+ PCI_ANY_ID, 0, 0, 0, },
+/*
+ * AB
+ * { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_8, PCI_ANY_ID,
+ * PCI_ANY_ID, 0, 0, 0, },
+ */
+ { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801AB_0, PCI_ANY_ID,
+ PCI_ANY_ID, 0, 0, 0, },
+/*
+ * ??
+ * { PCI_VENDOR_ID_INTEL, 0x2430, PCI_ANY_ID, PCI_ANY_ID, 0
