[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66fbd8d1-8b1a-50b5-c442-23847dc3d42f@gmail.com>
Date: Sun, 1 Oct 2023 23:43:38 +0530
From: Ayush Singh <ayushdevel1325@...il.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
greybus-dev@...ts.linaro.org
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
gregkh@...uxfoundation.org, vaishnav@...gleboard.org,
jkridner@...gleboard.org, nm@...com,
krzysztof.kozlowski+dt@...aro.org, johan@...nel.org,
elder@...nel.org
Subject: Re: [PATCH v5 2/3] greybus: Add BeaglePlay Linux Driver
>>>> + 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@kernelnewbies.org/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
Powered by blists - more mailing lists