Michal Kazior <michal.kazior@xxxxxxxxx> writes: > On 25 August 2014 11:18, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote: >> Michal Kazior <michal.kazior@xxxxxxxxx> writes: >> >>> This makes it a lot easier to log and debug >>> messages if there's more than 1 ath10k device on a >>> system. >>> >>> Signed-off-by: Michal Kazior <michal.kazior@xxxxxxxxx> >> >> Did only a quick review and test but looks good to me except one problem >> in pci.c, see below. But there are some conflicts now, can you rebase >> please? I hope it's not too much work to fix them. > > I've rebased this in my internal tree already. Nice :) >> I'll also summarise the "meat" of the changes so that it's easier for >> others to review: >> > [...] >>> @@ -2566,12 +2576,12 @@ static int ath10k_pci_probe(struct pci_dev *pdev, >>> struct ath10k_pci *ar_pci; >>> u32 chip_id; >>> >>> - ath10k_dbg(ATH10K_DBG_PCI, "pci probe\n"); >>> + dev_printk(KERN_DEBUG, &pdev->dev, "pci probe\n"); >> >> But this doesn't look right. A leftover from testing, perhaps? > > At this point there's no "ar" yet. The "ar" is allocated a few lines > below. If the allocation fails then there's no "ar" so dev_err() is > used explicitly in the failpath. Ah, of course. > I think these are just 2 exceptions we can't do differently, can we? Using dev_err() is okay, but this debug print has the problem that it's now always printed. I see few options: 1) we create __ath10k_dbg(dev, mask, fmt, ...) 2) we move the debug print after ar is created 3) we remove the debug print entirely I vote for 2) -- Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
