diff mbox series

[v2,05/33] common/cpt: add common code for init routine

Message ID 1536033560-21541-6-git-send-email-ajoseph@caviumnetworks.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series Adding Cavium's OcteonTX crypto PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anoob Joseph Sept. 4, 2018, 3:58 a.m. UTC 
  From: Anoob Joseph <anoob.joseph@caviumnetworks.com>

Adding code identified common for OcteonTX family crypto devices. This
patch is adding the code required by the structures and code path of
init routine.

Signed-off-by: Ankur Dwivedi <ankur.dwivedi@caviumnetworks.com>
Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
Signed-off-by: Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
Signed-off-by: Nithin Dabilpuram <nithin.dabilpuram@caviumnetworks.com>
Signed-off-by: Ragothaman Jayaraman <rjayaraman@caviumnetworks.com>
Signed-off-by: Srisivasubramanian S <ssrinivasan@caviumnetworks.com>
Signed-off-by: Tejasree Kondoj <kondoj.tejasree@caviumnetworks.com>
---
 drivers/common/cpt/cpt_common.h | 54 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 drivers/common/cpt/cpt_common.h

Comments

Akhil Goyal Sept. 17, 2018, 10:45 a.m. UTC | #1 
On 9/4/2018 9:28 AM, Anoob Joseph wrote:
> From: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>
> Adding code identified common for OcteonTX family crypto devices. This
> patch is adding the code required by the structures and code path of
> init routine.
>
> Signed-off-by: Ankur Dwivedi <ankur.dwivedi@caviumnetworks.com>
> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> Signed-off-by: Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
> Signed-off-by: Nithin Dabilpuram <nithin.dabilpuram@caviumnetworks.com>
> Signed-off-by: Ragothaman Jayaraman <rjayaraman@caviumnetworks.com>
> Signed-off-by: Srisivasubramanian S <ssrinivasan@caviumnetworks.com>
> Signed-off-by: Tejasree Kondoj <kondoj.tejasree@caviumnetworks.com>
> ---
>   drivers/common/cpt/cpt_common.h | 54 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
>   create mode 100644 drivers/common/cpt/cpt_common.h
>
> diff --git a/drivers/common/cpt/cpt_common.h b/drivers/common/cpt/cpt_common.h
> new file mode 100644
> index 0000000..feca5fe
> --- /dev/null
> +++ b/drivers/common/cpt/cpt_common.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Cavium, Inc
> + */
> +
> +#ifndef _CPT_COMMON_H_
> +#define _CPT_COMMON_H_
> +
> +/*
> + * This file defines common macros and structs
> + */
> +
> +/*
> + * Macros to determine CPT model. Driver makefile will define CPT_MODEL
> + * accordingly
> + */
> +#define CRYPTO_OCTEONTX		0x1
> +
> +#define AE_TYPE 1
> +#define SE_TYPE 2
> +
> +/* cpt instance */
> +struct cpt_instance {
> +	uint32_t queue_id;
> +	uintptr_t rsvd;
> +};
> +
> +struct cptvf_meta_info {
> +	void *cptvf_meta_pool;
> +	int cptvf_op_mlen;
> +	int cptvf_op_sb_mlen;
> +};
> +
> +struct rid {
> +	uintptr_t rid;
> +		/**< Request id of a crypto operation */
no need for extra tab for comments.
> +};
> +
> +/*
> + * Pending queue structure
> + *
> + */
> +struct pending_queue {
> +	uint16_t enq_tail;
> +	uint16_t deq_head;
> +	uint16_t soft_qlen;
> +		/**< Software expected queue length */
> +	uint16_t p_doorbell;
> +	struct rid *rid_queue;
> +		/**< Array of pending requests */
> +	uint64_t pending_count;
> +		/**< Pending requests count */
> +};
better to add comment for each element of structure.
Also remove extra tab for comments(here and any other place if any.)
> +
> +#endif /* _CPT_COMMON_H_ */
Thomas Monjalon Sept. 17, 2018, 11:46 a.m. UTC | #2 
17/09/2018 12:45, Akhil Goyal:
> On 9/4/2018 9:28 AM, Anoob Joseph wrote:
> > +struct pending_queue {
> > +	uint16_t enq_tail;
> > +	uint16_t deq_head;
> > +	uint16_t soft_qlen;
> > +		/**< Software expected queue length */
> > +	uint16_t p_doorbell;
> > +	struct rid *rid_queue;
> > +		/**< Array of pending requests */
> > +	uint64_t pending_count;
> > +		/**< Pending requests count */
> > +};
> better to add comment for each element of structure.
> Also remove extra tab for comments(here and any other place if any.)

I don't understand this trend in the community about doing comments
_after_ the item _and_ not on the same line.
The default style should be commenting _before_.
And if you feel it is better to have the comment on the same line,
then you can comment _after_, but on the same line.
Anoob Joseph Sept. 17, 2018, 12:29 p.m. UTC | #3 
Hi Thomas,

On 17-09-2018 17:16, Thomas Monjalon wrote:
> External Email
>
> 17/09/2018 12:45, Akhil Goyal:
>> On 9/4/2018 9:28 AM, Anoob Joseph wrote:
>>> +struct pending_queue {
>>> +   uint16_t enq_tail;
>>> +   uint16_t deq_head;
>>> +   uint16_t soft_qlen;
>>> +           /**< Software expected queue length */
>>> +   uint16_t p_doorbell;
>>> +   struct rid *rid_queue;
>>> +           /**< Array of pending requests */
>>> +   uint64_t pending_count;
>>> +           /**< Pending requests count */
>>> +};
>> better to add comment for each element of structure.
>> Also remove extra tab for comments(here and any other place if any.)
> I don't understand this trend in the community about doing comments
> _after_ the item _and_ not on the same line.
> The default style should be commenting _before_.
> And if you feel it is better to have the comment on the same line,
> then you can comment _after_, but on the same line.
Will fix it. Saw comments after the item being used frequently and 
thought that was the convention.

Thanks
Anoob
Akhil Goyal Sept. 17, 2018, 12:32 p.m. UTC | #4 
Hi Thomas,

On 9/17/2018 5:16 PM, Thomas Monjalon wrote:

> 17/09/2018 12:45, Akhil Goyal:
>> On 9/4/2018 9:28 AM, Anoob Joseph wrote:
>>> +struct pending_queue {
>>> +	uint16_t enq_tail;
>>> +	uint16_t deq_head;
>>> +	uint16_t soft_qlen;
>>> +		/**< Software expected queue length */
>>> +	uint16_t p_doorbell;
>>> +	struct rid *rid_queue;
>>> +		/**< Array of pending requests */
>>> +	uint64_t pending_count;
>>> +		/**< Pending requests count */
>>> +};
>> better to add comment for each element of structure.
>> Also remove extra tab for comments(here and any other place if any.)
> I don't understand this trend in the community about doing comments
> _after_ the item _and_ not on the same line.
> The default style should be commenting _before_.
> And if you feel it is better to have the comment on the same line,
> then you can comment _after_, but on the same line.
>
I think this should not matter, whether the comment should be before or after,

it should be consistent across the code. I can see that both are being used equally.

Shall we change the complete code beyond this driver as well?

I think whatever we choose, it should be atleast consistent within the file.

-Akhil

>
Thomas Monjalon Sept. 17, 2018, 12:51 p.m. UTC | #5 
17/09/2018 14:32, Akhil Goyal:
> 
> Hi Thomas,
> 
> On 9/17/2018 5:16 PM, Thomas Monjalon wrote:
> 
> > 17/09/2018 12:45, Akhil Goyal:
> >> On 9/4/2018 9:28 AM, Anoob Joseph wrote:
> >>> +struct pending_queue {
> >>> +	uint16_t enq_tail;
> >>> +	uint16_t deq_head;
> >>> +	uint16_t soft_qlen;
> >>> +		/**< Software expected queue length */
> >>> +	uint16_t p_doorbell;
> >>> +	struct rid *rid_queue;
> >>> +		/**< Array of pending requests */
> >>> +	uint64_t pending_count;
> >>> +		/**< Pending requests count */
> >>> +};
> >> better to add comment for each element of structure.
> >> Also remove extra tab for comments(here and any other place if any.)
> > I don't understand this trend in the community about doing comments
> > _after_ the item _and_ not on the same line.
> > The default style should be commenting _before_.
> > And if you feel it is better to have the comment on the same line,
> > then you can comment _after_, but on the same line.
> >
> I think this should not matter, whether the comment should be before or after,
> 
> it should be consistent across the code. I can see that both are being used equally.
> 
> Shall we change the complete code beyond this driver as well?
> 
> I think whatever we choose, it should be atleast consistent within the file.

Let me rephrase.

There are 3 styles:
	1/ comment on the previous line (before the item)
	2/ comment on the same line (just after the item)
	3/ comment on the next line (after the item)

I am fine with #1 and #2 but I really don't see the benefit of #3.
diff mbox series

Patch

diff --git a/drivers/common/cpt/cpt_common.h b/drivers/common/cpt/cpt_common.h
new file mode 100644
index 0000000..feca5fe
--- /dev/null
+++ b/drivers/common/cpt/cpt_common.h
@@ -0,0 +1,54 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Cavium, Inc
+ */
+
+#ifndef _CPT_COMMON_H_
+#define _CPT_COMMON_H_
+
+/*
+ * This file defines common macros and structs
+ */
+
+/*
+ * Macros to determine CPT model. Driver makefile will define CPT_MODEL
+ * accordingly
+ */
+#define CRYPTO_OCTEONTX		0x1
+
+#define AE_TYPE 1
+#define SE_TYPE 2
+
+/* cpt instance */
+struct cpt_instance {
+	uint32_t queue_id;
+	uintptr_t rsvd;
+};
+
+struct cptvf_meta_info {
+	void *cptvf_meta_pool;
+	int cptvf_op_mlen;
+	int cptvf_op_sb_mlen;
+};
+
+struct rid {
+	uintptr_t rid;
+		/**< Request id of a crypto operation */
+};
+
+/*
+ * Pending queue structure
+ *
+ */
+struct pending_queue {
+	uint16_t enq_tail;
+	uint16_t deq_head;
+	uint16_t soft_qlen;
+		/**< Software expected queue length */
+	uint16_t p_doorbell;
+	struct rid *rid_queue;
+		/**< Array of pending requests */
+	uint64_t pending_count;
+		/**< Pending requests count */
+};
+
+#endif /* _CPT_COMMON_H_ */