Making the code cleaner: Refactoring the PCI driver for a NAND Denali controller

Using the example of a PCI driver for a NAND Denali controller, I will show how simplified the code is when using macros and helper functions available in relatively recent versions of the Linux kernel.



This old driver is hardly used, but is a good example of code that can be refactored. The driver itself is located in drivers / mtd / nand / denali_pci.c .

Kconfig preparation

First of all let's touch Kconfig. Since the driver is broken according to the already classical scheme, namely: the main part + driver on the bus, following the logic (including Torvalds himself ), we need to hide the choice of the main part from the user. Along the way, we replace spaces on tabs in those lines that we change.

')

Do it once!

Application macros

The next step is to use the module_pci_driver() macro:

 --- a/drivers/mtd/nand/denali_pci.c +++ b/drivers/mtd/nand/denali_pci.c @@ -129,14 +129,4 @@ static struct pci_driver denali_pci_driver = { .remove = denali_pci_remove, }; -static int denali_init_pci(void) -{ - return pci_register_driver(&denali_pci_driver); -} -module_init(denali_init_pci); - -static void denali_exit_pci(void) -{ - pci_unregister_driver(&denali_pci_driver); -} -module_exit(denali_exit_pci); +module_pci_driver(denali_pci_driver); 

Do two!

Transition to Managed Resource API

Now we come to the most interesting, replacing calls with managed resources ( here I described them).



Let's start with a simple, devm_kzalloc() :







Since the device is connected to the PCI bus, we use the devres API for PCI devices:





Unfortunately, it is not possible to get rid of ioremap_nocache() , there is no appropriate device at hand (and I’m not at all sure that such a thing exists in the world today) to check what information is stored in the corresponding PCI BARs.



Combine the resulting in this chapter, and do three!

Additional refactoring

To restore complete beauty, replace pr_err() with dev_err() :

 --- a/drivers/mtd/nand/denali_pci.c +++ b/drivers/mtd/nand/denali_pci.c @@ -41,7 +41,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) ret = pcim_enable_device(dev); if (ret) { - pr_err("Spectra: pci_enable_device failed.\n"); + dev_err(&dev->dev, "Spectra: pci_enable_device failed.\n"); return ret; } @@ -69,19 +69,19 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) ret = pci_request_regions(dev, DENALI_NAND_NAME); if (ret) { - pr_err("Spectra: Unable to request memory regions\n"); + dev_err(&dev->dev, "Spectra: Unable to request memory regions\n"); return ret; } denali->flash_reg = ioremap_nocache(csr_base, csr_len); if (!denali->flash_reg) { - pr_err("Spectra: Unable to remap memory region\n"); + dev_err(&dev->dev, "Spectra: Unable to remap memory region\n"); return -ENOMEM; } denali->flash_mem = ioremap_nocache(mem_base, mem_len); if (!denali->flash_mem) { - pr_err("Spectra: ioremap_nocache failed!"); + dev_err(&dev->dev, "Spectra: ioremap_nocache failed!"); ret = -ENOMEM; goto failed_remap_reg; } 

Make four!



Let's sum up:

  drivers/mtd/nand/Kconfig | 13 +++++-------- drivers/mtd/nand/denali_pci.c | 43 +++++++++++-------------------------------- 2 files changed, 16 insertions(+), 40 deletions(-) 

The obvious good. Do not forget to use the available API in the new code!



UPDATE.

All 4 are already in the maintainer tree . Commit IDs: af83a67cad14, add243d5bc37, 2445d33d8523, 04868a67ed58.