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

[Ayush Singh]

> > > > +	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