Messages in this thread | | | Date | Tue, 27 Jul 2004 15:50:11 -0700 | From | "Randy.Dunlap" <> | Subject | Re: [announce][draft3] HVCS for inclusion in 2.6 tree |
| |
On Tue, 27 Jul 2004 15:08:58 -0500 Ryan Arnold wrote:
| Ok Andrew, here is draft3 of my patch. This patch contains fixes for | the following items: | | | Thanks for the kthread suggestions. The kthread API is awesome. My | stress tests seem to be going very well. So, if you don't have any more | comments....
I do. (this is for the first 1000 lines of the patch... more to come)
+struct hvcs_partner_info { + /* list management */ + struct list_head node; + /* partner unit address */ + unsigned int unit_address; + /*partner partition ID */ + unsigned int partition_ID; + /* CLC (79 chars) + 1 Null-term char */ + char location_code[HVCS_CLC_LENGTH + 1]; +};
Ugly comments style. Which comment goes with which data? Commenting data can be very helpful, but most of these are close to useless since they are so obvious. And put a space after "/*".
+/* Convert arch specific return codes into relevant errnos. The hvcs + * functions aren't performance sensitive, so this conversion isn't an + * issue. */
Long-comment style is /* * line1 * line2 * lineN */ (in multiple places).
+int hvcs_convert(long to_convert) +{ + switch (to_convert) { + case H_Success: + return 0; + case H_Parameter: + return -EINVAL; + case H_Hardware: + return -EIO; + case H_Busy:
Can these H_values be converted from that coding style?
+/* Helper function for hvcs_get_partner_info */ +int hvcs_next_partner(unsigned int unit_address, unsigned long last_p_partition_ID, unsigned long last_p_unit_address, unsigned long *pi_buff)
Split the function line. (multiple places)
+ memset(pi_buff,0x00,PAGE_SIZE);
Use spaces after commas.
+ /* This is a very small struct and will be freed soon */ + next_partner_info = kmalloc(sizeof(struct hvcs_partner_info), + GFP_ATOMIC);
Where is it freed?
+ will depend on arch specific apis exported from hvcserver.ko
"APIs"
+ To compile this driver as a module, choose M here: the + module will be called hvcs.ko. Additionally, this module + will depend on arch specific apis exported from hvcserver.ko + which will also be compiled when this driver is built as a + module. + config PC9800_OLDLP
This patch segment won't apply since PC9800 has been removed.
+#define __ALIGNED__ __attribute__((__aligned__(8)))
Why aligned? why 8? (just curious) Could use a comment if it's important.
+ * we commited to delivering it. But don't try to wake + * a non-existant tty. */
non-existent
+ /* remove the read masks*/ masks */
+ for (i=0;got && i<got;i++)
add spaces for readability: for (i = 0; got && i < got; i++)
+ if (!got){ if (!got) {
-- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |