Re: [PATCH v2] drivers/net/ieee802154/adf7242: Driver for ADF7242 MAC IEEE802154

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/08/2015 02:37 PM, Stefan Schmidt wrote:
Hello.

On 08/12/15 12:52, michael.hennerich@xxxxxxxxxx wrote:
From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>



+    ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
adf7242_isr,
+                    irq_type | IRQF_ONESHOT,
+                    dev_name(&spi->dev), lp);
+    if (ret)
+        goto err_hw_init;
+
+    disable_irq(spi->irq);
+
+    ret = ieee802154_register_hw(lp->hw);
+    if (ret)
+        goto err_hw_init;
+
+    dev_set_drvdata(&spi->dev, lp);
+
+    adf7242_debugfs_init(lp);

Hmm, you want to have DEBUG_FS support always enabled? I was expecting
to have it enabled behind a driver debug option as it should not be
needed during normal usage. I also expected the firmware_verify function
to be triggered from there but I don't have a string opinion on that one.

The Kconfig setup misses a "depends on DEBUG_FS".


Well - most debugfs function have empty return -ENODEV function stubs in case CONFIG_DEBUG_FS is not set.
It was done to avoid the ifdef and depends mess.
So this can stay the way it is.





+
+    dev_info(&spi->dev, "mac802154 IRQ-%d registered\n", spi->irq);
+
+    return ret;
+
+    ieee802154_unregister_hw(lp->hw);
+err_hw_init:
+    mutex_destroy(&lp->bmux);
+    ieee802154_free_hw(lp->hw);
+
+    return ret;
+}
+
+static int adf7242_remove(struct spi_device *spi)
+{
+    struct adf7242_local *lp = spi_get_drvdata(spi);
+
+    if (!IS_ERR_OR_NULL(lp->debugfs_root))
+        debugfs_remove_recursive(lp->debugfs_root);
+
+    ieee802154_unregister_hw(lp->hw);
+    mutex_destroy(&lp->bmux);
+    ieee802154_free_hw(lp->hw);
+
+    return 0;
+}
+
+static const struct of_device_id adf7242_of_match[] = {
+    { .compatible = "adi,adf7242", },
+    { },
+};
+MODULE_DEVICE_TABLE(of, adf7242_of_match);
+
+static const struct spi_device_id adf7242_device_id[] = {
+    { .name = "adf7242", },
+    { },
+};
+MODULE_DEVICE_TABLE(spi, adf7242_device_id);
+
+static struct spi_driver adf7242_driver = {
+    .id_table = adf7242_device_id,
+    .driver = {
+           .of_match_table = of_match_ptr(adf7242_of_match),
+           .name = "adf7242",
+           .owner = THIS_MODULE,
+           },
+    .probe = adf7242_probe,
+    .remove = adf7242_remove,
+};
+
+module_spi_driver(adf7242_driver);
+
+MODULE_AUTHOR("Michael Hennerich <michael.hennerich@xxxxxxxxxx>");
+MODULE_DESCRIPTION("ADF7242 IEEE802.15.4 Transceiver Driver");
+MODULE_LICENSE("GPL");


Besides the question about having DEBUG_FS enabled by default I think
this one is fine now to go in.

Reviewed-by: Stefan Schmidt <stefan@xxxxxxxxxxxxxxx>

I will do some testing on the hardware later today and scream if
something strange shows up.

regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux