Re: [PATCH v5 2/3] greybus: Add BeaglePlay Linux Driver

From: Ayush Singh
Date: Sun Oct 01 2023 - 14:13:49 EST


+ spinlock_t tx_producer_lock;
+ /* tx_consumer_lock: HDLC consumer lock */
+ spinlock_t tx_consumer_lock;
+ struct circ_buf tx_circ_buf;
+ u16 tx_crc;
+ u8 tx_ack_seq;
+
+ u16 rx_buffer_len;
+ u8 rx_in_esc;
+ u8 rx_buffer[MAX_RX_HDLC];
+};
+
+struct hdlc_payload {
+ u16 length;
+ void *payload;
+};
+
...

+
+static int gb_serdev_init(struct gb_beagleplay *bg)
+{
+ u32 speed = 115200;
+ int ret;
+
+ serdev_device_set_drvdata(bg->serdev, bg);
+ serdev_device_set_client_ops(bg->serdev, &gb_beagleplay_ops);
+ ret = serdev_device_open(bg->serdev);
+ if (ret) {
+ return dev_err_probe(&bg->serdev->dev, ret,
+ "Unable to Open Serial Device");
+ }
Please run scripts/checkpatch.pl --strict and fix reported warnings.
Some warnings can be ignored, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.
So I do not actually get any errors here in checkpatch. I am running the
follwing:

`scripts/checkpatch.pl --codespell --strict patch/*`

I only get a warning in coverletter due to that path of DT bindings
being more than 75 character long and ` Lines should not end with a '('`.

+ if (!bg)
+ return -ENOMEM;
+
+ bg->serdev = serdev;
+ ret = gb_serdev_init(bg);
+ if (ret)
+ return ret;
+
+ ret = hdlc_init(bg);
+ if (ret)
+ goto free_serdev;
+
+ ret = gb_greybus_init(bg);
+ if (ret)
+ goto free_hdlc;
+
+ gb_beagleplay_start_svc(bg);
+
+ return 0;
+
+free_hdlc:
+ hdlc_deinit(bg);
+free_serdev:
+ gb_serdev_deinit(bg);
+ return ret;
+}
+
+static void gb_beagleplay_remove(struct serdev_device *serdev)
+{
+ struct gb_beagleplay *bg = serdev_device_get_drvdata(serdev);
+
+ gb_greybus_deinit(bg);
+ gb_beagleplay_stop_svc(bg);
+ hdlc_deinit(bg);
+ gb_serdev_deinit(bg);
+}
+
+static const struct of_device_id gb_beagleplay_of_match[] = {
+ {
+ .compatible = "beagle,play-cc1352",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gb_beagleplay_of_match);
+
+static struct serdev_device_driver gb_beagleplay_driver = {
+ .probe = gb_beagleplay_probe,
+ .remove = gb_beagleplay_remove,
+ .driver = {
+ .name = "gb_beagleplay",
+ .of_match_table = gb_beagleplay_of_match,
This is still wrongly aligned. Spaces after tab. Are you sure checkpatch
does not complain bout it?
Again, it doesn't seem to for me. Am I missing some environment
variables or options? Or maybe something wrong with my editor config
(neovim)?
You have spaces after tab, so how can this be properly aligned?

Best regards,
Krzysztof

So I just wanted to confirm, but I think spaces after tab are fine for alignment, right? I found this (https://www.mail-archive.com/kernelnewbies@xxxxxxxxxxxxxxxxx/msg13354.html) message in mailing list stating that it is fine.

It seems clang-format adds spaces for alignment less than 8 spaces. And checkpatch doesn't seem to complain as long as spaces are used for alignment (not indentation).


Sincerely,

Ayush Singh