Hi Hannes, Thank you for the review. Please see my comment inline.. On Wed, 28 Dec 2016, 12:41am, Hannes Reinecke wrote: > On 12/23/2016 08:17 PM, Dupuis, Chad wrote: > > From: Arun Easi <arun.easi@xxxxxxxxxx> > > > > This adds the backbone required for the various HW initalizations > > which are necessary for the FCoE driver (qedf) for QLogic FastLinQ > > 4xxxx line of adapters - FW notification, resource initializations, etc. > > > > Signed-off-by: Arun Easi <arun.easi@xxxxxxxxxx> > > Signed-off-by: Yuval Mintz <yuval.mintz@xxxxxxxxxx> > > --- --8<-- snipped -- > > diff --git a/include/linux/qed/qed_fcoe_if.h b/include/linux/qed/qed_fcoe_if.h > > new file mode 100644 > > index 0000000..bd6bcb8 > > --- /dev/null > > +++ b/include/linux/qed/qed_fcoe_if.h > > @@ -0,0 +1,145 @@ > > +#ifndef _QED_FCOE_IF_H > > +#define _QED_FCOE_IF_H > > +#include <linux/types.h> > > +#include <linux/qed/qed_if.h> > > +struct qed_fcoe_stats { > > + u64 fcoe_rx_byte_cnt; > > + u64 fcoe_rx_data_pkt_cnt; > > + u64 fcoe_rx_xfer_pkt_cnt; > > + u64 fcoe_rx_other_pkt_cnt; > > + u32 fcoe_silent_drop_pkt_cmdq_full_cnt; > > + u32 fcoe_silent_drop_pkt_rq_full_cnt; > > + u32 fcoe_silent_drop_pkt_crc_error_cnt; > > + u32 fcoe_silent_drop_pkt_task_invalid_cnt; > > + u32 fcoe_silent_drop_total_pkt_cnt; > > + > > + u64 fcoe_tx_byte_cnt; > > + u64 fcoe_tx_data_pkt_cnt; > > + u64 fcoe_tx_xfer_pkt_cnt; > > + u64 fcoe_tx_other_pkt_cnt; > > +}; > > + > > +struct qed_dev_fcoe_info { > > + struct qed_dev_info common; > > + > > + void __iomem *primary_dbq_rq_addr; > > + void __iomem *secondary_bdq_rq_addr; > > +}; > > + > > +struct qed_fcoe_params_offload { > > + dma_addr_t sq_pbl_addr; > > + dma_addr_t sq_curr_page_addr; > > + dma_addr_t sq_next_page_addr; > > + > > + u8 src_mac[ETH_ALEN]; > > + u8 dst_mac[ETH_ALEN]; > > + > > + u16 tx_max_fc_pay_len; > > + u16 e_d_tov_timer_val; > > + u16 rec_tov_timer_val; > > + u16 rx_max_fc_pay_len; > > + u16 vlan_tag; > > + > > + struct fc_addr_nw s_id; > > + u8 max_conc_seqs_c3; > > + struct fc_addr_nw d_id; > > + u8 flags; > > + u8 def_q_idx; > > +}; > > + > > +#define MAX_TID_BLOCKS_FCOE (512) > > +struct qed_fcoe_tid { > > + u32 size; /* In bytes per task */ > > + u32 num_tids_per_block; > > + u8 *blocks[MAX_TID_BLOCKS_FCOE]; > > +}; > > + > > +struct qed_fcoe_cb_ops { > > + struct qed_common_cb_ops common; > > + u32 (*get_login_failures)(void *cookie); > > +}; > > + > > +void qed_fcoe_set_pf_params(struct qed_dev *cdev, > > + struct qed_fcoe_pf_params *params); > > + > > +/** > > + * struct qed_fcoe_ops - qed FCoE operations. > > + * @common: common operations pointer > > + * @fill_dev_info: fills FCoE specific information > > + * @param cdev > > + * @param info > > + * @return 0 on sucesss, otherwise error value. > > + * @register_ops: register FCoE operations > > + * @param cdev > > + * @param ops - specified using qed_iscsi_cb_ops > > + * @param cookie - driver private > > + * @ll2: light L2 operations pointer > > + * @start: fcoe in FW > > + * @param cdev > > + * @param tasks - qed will fill information about tasks > > + * return 0 on success, otherwise error value. > > + * @stop: stops fcoe in FW > > + * @param cdev > > + * return 0 on success, otherwise error value. > > + * @acquire_conn: acquire a new fcoe connection > > + * @param cdev > > + * @param handle - qed will fill handle that should be > > + * used henceforth as identifier of the > > + * connection. > > + * @param p_doorbell - qed will fill the address of the > > + * doorbell. > > + * return 0 on sucesss, otherwise error value. > > + * @release_conn: release a previously acquired fcoe connection > > + * @param cdev > > + * @param handle - the connection handle. > > + * return 0 on success, otherwise error value. > > + * @offload_conn: configures an offloaded connection > > + * @param cdev > > + * @param handle - the connection handle. > > + * @param conn_info - the configuration to use for the > > + * offload. > > + * return 0 on success, otherwise error value. > > + * @destroy_conn: stops an offloaded connection > > + * @param cdev > > + * @param handle - the connection handle. > > + * @param terminate_params > > + * return 0 on success, otherwise error value. > > + * @get_stats: gets FCoE related statistics > > + * @param cdev > > + * @param stats - pointer to struck that would be filled > > + * we stats > > + * return 0 on success, error otherwise. > > + */ > > +struct qed_fcoe_ops { > > + const struct qed_common_ops *common; > > + > > + int (*fill_dev_info)(struct qed_dev *cdev, > > + struct qed_dev_fcoe_info *info); > > + > > + void (*register_ops)(struct qed_dev *cdev, > > + struct qed_fcoe_cb_ops *ops, void *cookie); > > + > > + const struct qed_ll2_ops *ll2; > > + > > + int (*start)(struct qed_dev *cdev, struct qed_fcoe_tid *tasks); > > + > > + int (*stop)(struct qed_dev *cdev); > > + > > + int (*acquire_conn)(struct qed_dev *cdev, > > + u32 *handle, > > + u32 *fw_cid, void __iomem **p_doorbell); > > + > > + int (*release_conn)(struct qed_dev *cdev, u32 handle); > > + > > + int (*offload_conn)(struct qed_dev *cdev, > > + u32 handle, > > + struct qed_fcoe_params_offload *conn_info); > > + int (*destroy_conn)(struct qed_dev *cdev, > > + u32 handle, dma_addr_t terminate_params); > > + > > + int (*get_stats)(struct qed_dev *cdev, struct qed_fcoe_stats *stats); > > +}; > > + > I prefer to have to comments directly above the function prototypes; > that is easier to read and requires editing in one place only; this way > there's a higher likelyhood of both getting out of sync. > > But that may be personal preference only. > I too prefer the same, but when I posted the iSCSI qed part sometime ago, which had comment style like you said, I got a comment to follow kernel-doc style. So, for the FCoE series, I moved to kernel-doc style from the get go. Regards, -Arun > > +const struct qed_fcoe_ops *qed_get_fcoe_ops(void); > > +void qed_put_fcoe_ops(void); > > +#endif --8<-- snipped -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html