Skip to contentSkip to conversation

PCI: quirk related clean up

PATCH In review Yinghai Lu 3 participants 11 messages Feb 16, 2012

Subsystems (4)
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/4 PCI: quirk related clean up

After commit:

| commit 3209874a1da2c51c7325e601d9634189ee178ad6
| Author: Arjan van de Ven <[email protected]>
| Date:   Mon Jan 30 20:52:07 2012 -0800
|
|    PCI: Annotate PCI quirks in initcall_debug style

will have lots of print out for quirks when initcall_debug is specified.

It turns out most of them are not really called for the devices because quirks itself will check class id and bail out early.

Try to class into quirk declaration. So we could avoid dip into these quirks.

[PATCH 1/4] PCI: Add class support in quirk handling
[PATCH 2/4] PCI: Specify class for quirk entry with PCI_ANY_ID [PATCH 3/4] PCI: Move out pci reassigndev resource alignment out of quirk [PATCH 4/4] PCI: quirk print dev name with duration

Thanks

Yinghai

author
#1 lore raw

PATCH 1/4 PCI: Add class support in quirk handling

Recently there is new support to make quirk calling will report duration time. That will boot log get more print out with initcall_debug is specified.

Found a lot of quirks are calling for not related devices.

Reason is quirk frame do not support class handling. So quirk code have to use PCI_ANY_ID for vendor/device and let quirk code itself to check class inside the func.

The patch add class and cls_shift into struct pci_fixup. Also update related macro to accept the class and shift.

---
 drivers/pci/quirks.c |   19 ++++++++++--------
 include/linux/pci.h  |   53 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 53 insertions(+), 19 deletions(-)
drivers/pci/quirks.c +11 -8
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
@@ -2914,14 +2914,15 @@ static void do_one_fixup_debug(void (*fn
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 
-	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+	printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+			fn, task_pid_nr(current), dev_name(&dev->dev));
 	calltime = ktime_get();
 	fn(dev);
 	rettime = ktime_get();
 	delta = ktime_sub(rettime, calltime);
 	duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-		duration);
+	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+			fn, duration, dev_name(&dev->dev));
 }
 
 /*
@@ -2961,17 +2962,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
 			  struct pci_fixup *end)
 {
-	while (f < end) {
-		if ((f->vendor == dev->vendor || f->vendor == (u16) PCI_ANY_ID) &&
-		    (f->device == dev->device || f->device == (u16) PCI_ANY_ID)) {
+	for (; f < end; f++)
+		if ((f->class == (u32) (dev->class >> f->cls_shift) ||
+		     f->class == (u32) PCI_ANY_ID) &&
+		    (f->vendor == dev->vendor ||
+		     f->vendor == (u16) PCI_ANY_ID) &&
+		    (f->device == dev->device ||
+		     f->device == (u16) PCI_ANY_ID)) {
 			dev_dbg(&dev->dev, "calling %pF\n", f->hook);
 			if (initcall_debug)
 				do_one_fixup_debug(f->hook, dev);
 			else
 				f->hook(dev);
 		}
-		f++;
-	}
 }
 
 extern struct pci_fixup __start_pci_fixups_early[];
include/linux/pci.h +42 -11

79 lines not on this page · raw message

author
#2 lore raw

PATCH 2/4 PCI: Specify class for quirk entry with PCI_ANY_ID

In reply to Yinghai Lu

Use quirk framework class checking instead check class inside every quirk func.

That reduce quirk calling significent esp on Intel 8 sockets systems that have lots of device.
- quirk_e100_interrupt
- quirk_usb_early_handoff
- pci_fixup_video
- quirk_mmio_always_on
- quirk_cardbus_legacy

---
 arch/x86/kernel/pci-dma.c     |    5 ++--
 arch/x86/pci/fixup.c          |   12 ++++-------
 drivers/pci/quirks.c          |   43 ++++++++++++++++++++++--------------------
 drivers/usb/host/pci-quirks.c |    3 +-
 4 files changed, 33 insertions(+), 30 deletions(-)
arch/x86/kernel/pci-dma.c +3 -2
Index: linux-2.6/arch/x86/kernel/pci-dma.c
===================================================================
@@ -262,10 +262,11 @@ rootfs_initcall(pci_iommu_init);
 
 static __devinit void via_no_dac(struct pci_dev *dev)
 {
-	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI && forbid_dac == 0) {
+	if (forbid_dac == 0) {
 		dev_info(&dev->dev, "disabling DAC on VIA PCI bridge\n");
 		forbid_dac = 1;
 	}
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, PCI_ANY_ID, via_no_dac);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
+				PCI_CLASS_BRIDGE_PCI, 8, via_no_dac);
 #endif
arch/x86/pci/fixup.c +5 -7
Index: linux-2.6/arch/x86/pci/fixup.c
===================================================================
@@ -164,11 +164,11 @@ DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_V
  */
 static void __devinit pci_fixup_transparent_bridge(struct pci_dev *dev)
 {
-	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI &&
-	    (dev->device & 0xff00) == 0x2400)
+	if ((dev->device & 0xff00) == 0x2400)
 		dev->transparent = 1;
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_fixup_transparent_bridge);
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
+			 PCI_CLASS_BRIDGE_PCI, 8, pci_fixup_transparent_bridge);
 
 /*
  * Fixup for C1 Halt Disconnect problem on nForce2 systems.
@@ -322,9 +322,6 @@ static void __devinit pci_fixup_video(st
 	struct pci_bus *bus;
 	u16 config;
 
-	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return;
-
 	/* Is VGA routed to us? */
 	bus = pdev->bus;
 	while (bus) {
@@ -353,7 +350,8 @@ static void __devinit pci_fixup_video(st
 		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 	}
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+				PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
 
 
 static const struct dmi_system_id __devinitconst msi_k8t_dmi_table[] = {
drivers/pci/quirks.c +23 -20

81 lines not on this page · raw message

drivers/usb/host/pci-quirks.c +2 -1
Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
@@ -901,4 +901,5 @@ static void __devinit quirk_usb_early_ha
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
 		quirk_usb_handoff_xhci(pdev);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_usb_early_handoff);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+			PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
author
#3 lore raw

PATCH 3/4 PCI: Move out pci reassigndev resource alignment out of quirk

In reply to Yinghai Lu

That looks like abusing quirk framework.

calling it directly in pci_add_device. or do we have good place for it?

---
 drivers/pci/pci.c       |   62 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h       |    5 ---
 drivers/pci/probe.c     |    3 ++
 drivers/pci/quirks.c    |   63 ------------------------------------------------
 drivers/pci/setup-res.c |    4 ---
 5 files changed, 66 insertions(+), 71 deletions(-)
drivers/pci/pci.c +62 -0
Index: linux-2.6/drivers/pci/pci.c
===================================================================
@@ -3694,6 +3694,68 @@ int pci_is_reassigndev(struct pci_dev *d
 	return (pci_specified_resource_alignment(dev) != 0);
 }
 
+/*
+ * This function disables memory decoding and releases memory resources
+ * of the device specified by kernel's boot parameter 'pci=resource_alignment='.
+ * It also rounds up size to specified alignment.
+ * Later on, the kernel will assign page-aligned memory resource back
+ * to the device.
+ */
+void pci_reassigndev_resource_alignment(struct pci_dev *dev)
+{
+	int i;
+	struct resource *r;
+	resource_size_t align, size;
+	u16 command;
+
+	if (!pci_is_reassigndev(dev))
+		return;
+
+	if (dev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
+	    (dev->class >> 8) == PCI_CLASS_BRIDGE_HOST) {
+		dev_warn(&dev->dev,
+			"Can't reassign resources to host bridge.\n");
+		return;
+	}
+
+	dev_info(&dev->dev,
+		"Disabling memory decoding and releasing memory resources.\n");
+	pci_read_config_word(dev, PCI_COMMAND, &command);
+	command &= ~PCI_COMMAND_MEMORY;
+	pci_write_config_word(dev, PCI_COMMAND, command);
+
+	align = pci_specified_resource_alignment(dev);
+	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+		r = &dev->resource[i];
+		if (!(r->flags & IORESOURCE_MEM))
+			continue;
+		size = resource_size(r);
+		if (size < align) {
+			size = align;
+			dev_info(&dev->dev,
+				"Rounding up size of resource #%d to %#llx.\n",
+				i, (unsigned long long)size);
+		}
+		r->end = size - 1;
+		r->start = 0;
+	}
+	/* Need to disable bridge's resource window,
+	 * to enable the kernel to reassign new resource
+	 * window later on.
+	 */
+	if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
+	    (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+		for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
+			r = &dev->resource[i];
+			if (!(r->flags & IORESOURCE_MEM))
+				continue;
+			r->end = resource_size(r) - 1;
+			r->start = 0;
+		}
+		pci_disable_bridge_window(dev);
+	}
+}
+
 ssize_t pci_set_resource_alignment_param(const char *buf, size_t count)
 {
 	if (count > RESOURCE_ALIGNMENT_PARAM_SIZE - 1)
drivers/pci/pci.h +1 -4
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -230,11 +230,8 @@ static inline int pci_ari_enabled(struct
 	return bus->self && bus->self->ari_enabled;
 }
 
-#ifdef CONFIG_PCI_QUIRKS
-extern int pci_is_reassigndev(struct pci_dev *dev);
-resource_size_t pci_specified_resource_alignment(struct pci_dev *dev);
+void pci_reassigndev_resource_alignment(struct pci_dev *dev);
 extern void pci_disable_bridge_window(struct pci_dev *dev);
-#endif
 
 /* Single Root I/O Virtualization */
 struct pci_sriov {
drivers/pci/quirks.c +0 -63

69 lines not on this page · raw message

drivers/pci/setup-res.c +0 -4
Index: linux-2.6/drivers/pci/setup-res.c
===================================================================
@@ -114,7 +114,6 @@ int pci_claim_resource(struct pci_dev *d
 }
 EXPORT_SYMBOL(pci_claim_resource);
 
-#ifdef CONFIG_PCI_QUIRKS
 void pci_disable_bridge_window(struct pci_dev *dev)
 {
 	dev_info(&dev->dev, "disabling bridge mem windows\n");
@@ -127,9 +126,6 @@ void pci_disable_bridge_window(struct pc
 	pci_write_config_dword(dev, PCI_PREF_MEMORY_BASE, 0x0000fff0);
 	pci_write_config_dword(dev, PCI_PREF_BASE_UPPER32, 0xffffffff);
 }
-#endif	/* CONFIG_PCI_QUIRKS */
-
-
 
 static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
 		int resno, resource_size_t size, resource_size_t align)
drivers/pci/probe.c +3 -0
Index: linux-2.6/drivers/pci/probe.c
===================================================================
@@ -1426,6 +1426,9 @@ void pci_device_add(struct pci_dev *dev,
 	/* Fix up broken headers */
 	pci_fixup_device(pci_fixup_header, dev);
 
+	/* moved out from quirk header fixup code */
+	pci_reassigndev_resource_alignment(dev);
+
 	/* Clear the state_saved flag. */
 	dev->state_saved = false;
 
author
#4 lore raw

PATCH 4/4 PCI: quirk print dev name with duration

In reply to Yinghai Lu

So could find out which device is involved with the calling

Cc Arjan van de Ven <[email protected]>
---
 drivers/pci/quirks.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/pci/quirks.c +4 -3
Index: linux-2.6/drivers/pci/quirks.c
===================================================================
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 
-	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+	printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+			fn, task_pid_nr(current), dev_name(&dev->dev));
 	calltime = ktime_get();
 	fn(dev);
 	rettime = ktime_get();
 	delta = ktime_sub(rettime, calltime);
 	duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-		duration);
+	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+			fn, duration, dev_name(&dev->dev));
 }
 
 /*
#5 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 1 line · Yinghai Lu

So could find out which device is involved with the calling

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858

Quoted 8 lines

+++ linux-2.6/drivers/pci/quirks.c

drivers/pci/quirks.c +2 -1
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
 	ktime_t calltime, delta, rettime;
 	unsigned long long duration;
 
-	printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+	printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+			fn, task_pid_nr(current), dev_name(&dev->dev));

Might be better to use dev_dbg

	dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865

Quoted 4 lines
drivers/pci/quirks.c +2 -2
-	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-		duration);
+	printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+			fn, duration, dev_name(&dev->dev));
	dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n",
		fn, duration);
author
#6 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 18 lines · Joe Perches
Quoted 1 line · Yinghai Lu

So could find out which device is involved with the calling

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858

Quoted 8 lines

+++ linux-2.6/drivers/pci/quirks.c

drivers/pci/quirks.c +2 -1
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
       ktime_t calltime, delta, rettime;
       unsigned long long duration;
 
-     printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+     printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+                     fn, task_pid_nr(current), dev_name(&dev->dev));

Might be better to use dev_dbg

       dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));

[]

Quoted 7 lines

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865

Quoted 4 lines
drivers/pci/quirks.c +2 -2
-     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-             duration);
+     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+                     fn, duration, dev_name(&dev->dev));

       dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);

Not sure if that is going to break Arjan 's tool that analyze the boot time.

Thanks

Yinghai

#7 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 24 lines · Yinghai Lu
Quoted 23 lines · Joe Perches
Quoted 1 line · Yinghai Lu

So could find out which device is involved with the calling

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858

Quoted 8 lines

+++ linux-2.6/drivers/pci/quirks.c

drivers/pci/quirks.c +2 -1
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
      ktime_t calltime, delta, rettime;
      unsigned long long duration;
 
-     printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+     printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+                     fn, task_pid_nr(current), dev_name(&dev->dev));

Might be better to use dev_dbg

       dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));

[]

Quoted 4 lines
drivers/pci/quirks.c +2 -2
-     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-             duration);
+     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+                     fn, duration, dev_name(&dev->dev));
Quoted 4 lines
Quoted 2 lines
       dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n",
               fn, duration);

Not sure if that is going to break Arjan 's tool that analyze the boot time.

That depends on the tool code.
Either change might break it.
Did you test your change with Arjan's tool?
Do you have a link to the tool?

#8 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 25 lines · Joe Perches
Quoted 24 lines · Yinghai Lu
Quoted 23 lines · Joe Perches
Quoted 1 line · Yinghai Lu

So could find out which device is involved with the calling

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858

Quoted 8 lines

+++ linux-2.6/drivers/pci/quirks.c

drivers/pci/quirks.c +2 -1
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
      ktime_t calltime, delta, rettime;
      unsigned long long duration;
 
-     printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+     printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+                     fn, task_pid_nr(current), dev_name(&dev->dev));

Might be better to use dev_dbg

       dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));

[]

Quoted 4 lines
drivers/pci/quirks.c +2 -2
-     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-             duration);
+     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+                     fn, duration, dev_name(&dev->dev));
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
       dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n",
               fn, duration);

Not sure if that is going to break Arjan 's tool that analyze the boot time.

That depends on the tool code.

[]

Quoted 1 line

Do you have a link to the tool?

That tool would be scripts/bootgraph.pl and using
dev_dbg would break it unless it's adapted to the
change. Your change seems fine for now.

maintainer
#9 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 28 lines · Joe Perches
Quoted 27 lines · Joe Perches
Quoted 26 lines · Yinghai Lu
Quoted 25 lines · Joe Perches
Quoted 1 line · Yinghai Lu

So could find out which device is involved with the calling

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2858

Quoted 8 lines

+++ linux-2.6/drivers/pci/quirks.c

drivers/pci/quirks.c +2 -1
@@ -2854,14 +2854,15 @@ static void do_one_fixup_debug(void (*fn
       ktime_t calltime, delta, rettime;
       unsigned long long duration;
 
-     printk(KERN_DEBUG "calling  %pF @ %i\n", fn, task_pid_nr(current));
+     printk(KERN_DEBUG "calling  %pF @ %i for %s\n",
+                     fn, task_pid_nr(current), dev_name(&dev->dev));

Might be better to use dev_dbg

       dev_dbg(&dev->dev, "calling %pF @ %i\n", fn, task_pid_nr(current));

[]

drivers/pci/quirks.c static void do_one_fixup_debug(void (*fn · line 2865

Quoted 4 lines
drivers/pci/quirks.c +2 -2
-     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs\n", fn,
-             duration);
+     printk(KERN_DEBUG "pci fixup %pF returned after %lld usecs for %s\n",
+                     fn, duration, dev_name(&dev->dev));

       dev_dbg(&dev->dev, "pci fixup %pF returned after %llu usecs\n", fn, duration);

Quoted 8 lines
Quoted 2 lines
Quoted 1 line

Not sure if that is going to break Arjan 's tool that analyze the boot time.

That depends on the tool code.

[]

Quoted 1 line

Do you have a link to the tool?

That tool would be scripts/bootgraph.pl and using
dev_dbg would break it unless it's adapted to the
change.  Your change seems fine for now.

Since we're adding new functionality (analyzing quirk durations), I think it would be fair game to update the tool at the same time so it could parse the dev_dbg() format.

For things related specifically to a device, we have a pretty good record of trying to use dev_*() consistently, so I do have a preference for continuing that.

If you do change, remember that dev_dbg() is different from printk(KERN_DEBUG) in that printk(KERN_DEBUG) is always compiled-in and always goes to the dmesg, but dev_dbg() is only compiled in when DEBUG is defined, so dev_printk(KERN_DEBUG) would be a more direct replacement for printk(KERN_DEBUG).

Bjorn

#10 lore raw

Re: [PATCH 4/4] PCI: quirk print dev name with duration

Quoted 5 lines · Bjorn Helgaas

If you do change, remember that dev_dbg() is different from printk(KERN_DEBUG) in that printk(KERN_DEBUG) is always compiled-in and always goes to the dmesg, but dev_dbg() is only compiled in when DEBUG is defined, so dev_printk(KERN_DEBUG) would be a more direct replacement for printk(KERN_DEBUG).

Correct.

Which is why I usually add #define DEBUG or an equivalent CONFIG option before those are converted. Doing so allows control via dynamic debug.