Зробимо код чистіше: рефакторинг драйвер PCI для контролера NAND Denali

На прикладі драйвер PCI для контролера NAND Denali я покажу як спрощується код за використання макросів і функцій-помічників, доступних у відносно свіжих версіях ядра Linux.

Цей старий драйвер навряд використовується, але є хорошим прикладом коду, який можна отрефакторить. Сам драйвер знаходиться вdrivers/mtd/nand/denali_pci.c.

Підготовка Kconfig
Насамперед торкнемося Kconfig. Оскільки драйвер розбитий вже класичною схемою, а саме: основна частина + драйвер на шині, слідуючи логіці (в тому числі і самого Торвальдса) нам необхідно приховати вибір основної частини від користувача. Попутно замінюємо прогалини на табуляції у тих рядках, які змінюємо.

Роби раз!

Застосування макросів
Наступним кроком буде застосування макросу
module_pci_driver()
:

--- 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);


Роби два!

Перехід на API керованих ресурсів
Тепер переходимо до найцікавішого, заміні викликів на керовані ресурси (ось тут я описував їх).

Почнемо з простого,
devm_kzalloc()
:

Дивимося що вийшло
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -35,14 +35,14 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
unsigned long csr_len, mem_len;
struct denali_nand_info *denali;

- denali = kzalloc(sizeof(*denali), GFP_KERNEL);
+ denali = devm_kzalloc(&dev->dev, sizeof(*denali), GFP_KERNEL);
if (!denali)
return-ENOMEM;

ret = pci_enable_device(dev);
if (ret) {
pr_err("Spectra: pci_enable_device failed.\n");
- goto failed_alloc_memery;
+ return ret;
}

if (id->driver_data == INTEL_CE4100) {
@@ -103,9 +103,6 @@ failed_req_regions:
pci_release_regions(dev);
failed_enable_dev:
pci_disable_device(dev);
-failed_alloc_memery:
- kfree(denali);
-
return ret;
}

@@ -119,7 +116,6 @@ static void denali_pci_remove(struct pci_dev *dev)
iounmap(denali->flash_mem);
pci_release_regions(dev);
pci_disable_device(dev);
- kfree(denali);
}




Оскільки пристрій підключений до шини PCI, скористаємося devres API для пристроїв PCI:

Дивимося що вийшло
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -30,7 +30,7 @@ MODULE_DEVICE_TABLE(pci, denali_pci_ids);

static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
{
- int ret = -ENODEV;
+ int ret;
resource_size_t csr_base, mem_base;
unsigned long csr_len, mem_len;
struct denali_nand_info *denali;
@@ -39,7 +39,7 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
if (!denali)
return-ENOMEM;

- ret = pci_enable_device(dev);
+ ret = pcim_enable_device(dev);
if (ret) {
pr_err("Spectra: pci_enable_device failed.\n");
return ret;
@@ -70,14 +70,13 @@ 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");
- goto failed_enable_dev;
+ return ret;
}

denali->flash_reg = ioremap_nocache(csr_base, csr_len);
if (!denali->flash_reg) {
pr_err("Spectra: Unable to memory remap region\n");
- ret = -ENOMEM;
- goto failed_req_regions;
+ return-ENOMEM;
}

denali->flash_mem = ioremap_nocache(mem_base, mem_len);
@@ -99,10 +98,6 @@ failed_remap_mem:
iounmap(denali->flash_mem);
failed_remap_reg:
iounmap(denali->flash_reg);
-failed_req_regions:
- pci_release_regions(dev);
-failed_enable_dev:
- pci_disable_device(dev);
return ret;
}

@@ -114,8 +109,6 @@ static void denali_pci_remove(struct pci_dev *dev)
denali_remove(denali);
iounmap(denali->flash_reg);
iounmap(denali->flash_mem);
- pci_release_regions(dev);
- pci_disable_device(dev);
}



На жаль позбутися
ioremap_nocache()
не представляється можливим, немає відповідного пристрою під рукою (та й я взагалі не впевнений, що таке існує в світі сьогодні) щоб перевірити, яка інформація зберігається у відповідних PCI BAR'ах.

Об'єднуємо отримане в цій главі, і роби три!

Додатковий рефакторинг
Для наведення повної краси замінюємо
pr_err()
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 memory remap region\n");
+ dev_err(&dev->dev, "Spectra: Unable to memory remap 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;
}


Роби чотири!

Підіб'ємо підсумки:

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


Очевидна користь. Не забуваємо використовувати досьупные API в новому коді!

Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.