$ devlink health dump show DEVICE reporter coredump -p -j
{
"Core Registers": {
"segment": 1,
"values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
},
"Test Logic Regs": {
"segment": 2,
"values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
},
"RMII Registers": {
"segment": 3,
"values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
},
...
"Sem Registers": {
"segment": 50,
"values": [ 0,0,0,0 ]
}
}
Signed-off-by: Coiby Xu <[email protected]>
---
drivers/staging/qlge/qlge_devlink.c | 131 ++++++++++++++++++++++++++--
1 file changed, 125 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/qlge/qlge_devlink.c b/drivers/staging/qlge/qlge_devlink.c
index aa45e7e368c0..91b6600b94a9 100644
--- a/drivers/staging/qlge/qlge_devlink.c
+++ b/drivers/staging/qlge/qlge_devlink.c
@@ -1,16 +1,135 @@
#include "qlge.h"
#include "qlge_devlink.h"
-static int
-qlge_reporter_coredump(struct devlink_health_reporter *reporter,
- struct devlink_fmsg *fmsg, void *priv_ctx,
- struct netlink_ext_ack *extack)
+static int fill_seg_(struct devlink_fmsg *fmsg,
+ struct mpi_coredump_segment_header *seg_header,
+ u32 *reg_data)
{
- return 0;
+ int i;
+ int header_size = sizeof(struct mpi_coredump_segment_header);
+ int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
+ int err;
+
+ err = devlink_fmsg_pair_nest_start(fmsg, seg_header->description);
+ if (err)
+ return err;
+ err = devlink_fmsg_obj_nest_start(fmsg);
+ if (err)
+ return err;
+ err = devlink_fmsg_u32_pair_put(fmsg, "segment", seg_header->seg_num);
+ if (err)
+ return err;
+ err = devlink_fmsg_arr_pair_nest_start(fmsg, "values");
+ if (err)
+ return err;
+ for (i = 0; i < regs_num; i++) {
+ err = devlink_fmsg_u32_put(fmsg, *reg_data);
+ if (err)
+ return err;
+ reg_data++;
+ }
+ err = devlink_fmsg_obj_nest_end(fmsg);
+ if (err)
+ return err;
+ err = devlink_fmsg_arr_pair_nest_end(fmsg);
+ if (err)
+ return err;
+ err = devlink_fmsg_pair_nest_end(fmsg);
+ return err;
+}
+
+#define fill_seg(seg_hdr, seg_regs) \
+ do { \
+ err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
+ if (err) { \
+ kvfree(dump); \
+ return err; \
+ } \
+ } while (0)
+
+static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *priv_ctx,
+ struct netlink_ext_ack *extack)
+{
+ int err = 0;
+
+ struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
+ struct ql_adapter *qdev = dev->qdev;
+ struct ql_mpi_coredump *dump;
+
+ if (!netif_running(qdev->ndev))
+ return 0;
+
+ dump = kvmalloc(sizeof(*dump), GFP_KERNEL);
+ if (!dump)
+ return -ENOMEM;
+
+ err = ql_core_dump(qdev, dump);
+ if (err) {
+ kvfree(dump);
+ return err;
+ }
+
+ fill_seg(core_regs_seg_hdr, mpi_core_regs);
+ fill_seg(test_logic_regs_seg_hdr, test_logic_regs);
+ fill_seg(rmii_regs_seg_hdr, rmii_regs);
+ fill_seg(fcmac1_regs_seg_hdr, fcmac1_regs);
+ fill_seg(fcmac2_regs_seg_hdr, fcmac2_regs);
+ fill_seg(fc1_mbx_regs_seg_hdr, fc1_mbx_regs);
+ fill_seg(ide_regs_seg_hdr, ide_regs);
+ fill_seg(nic1_mbx_regs_seg_hdr, nic1_mbx_regs);
+ fill_seg(smbus_regs_seg_hdr, smbus_regs);
+ fill_seg(fc2_mbx_regs_seg_hdr, fc2_mbx_regs);
+ fill_seg(nic2_mbx_regs_seg_hdr, nic2_mbx_regs);
+ fill_seg(i2c_regs_seg_hdr, i2c_regs);
+ fill_seg(memc_regs_seg_hdr, memc_regs);
+ fill_seg(pbus_regs_seg_hdr, pbus_regs);
+ fill_seg(mde_regs_seg_hdr, mde_regs);
+ fill_seg(nic_regs_seg_hdr, nic_regs);
+ fill_seg(nic2_regs_seg_hdr, nic2_regs);
+ fill_seg(xgmac1_seg_hdr, xgmac1);
+ fill_seg(xgmac2_seg_hdr, xgmac2);
+ fill_seg(code_ram_seg_hdr, code_ram);
+ fill_seg(memc_ram_seg_hdr, memc_ram);
+ fill_seg(xaui_an_hdr, serdes_xaui_an);
+ fill_seg(xaui_hss_pcs_hdr, serdes_xaui_hss_pcs);
+ fill_seg(xfi_an_hdr, serdes_xfi_an);
+ fill_seg(xfi_train_hdr, serdes_xfi_train);
+ fill_seg(xfi_hss_pcs_hdr, serdes_xfi_hss_pcs);
+ fill_seg(xfi_hss_tx_hdr, serdes_xfi_hss_tx);
+ fill_seg(xfi_hss_rx_hdr, serdes_xfi_hss_rx);
+ fill_seg(xfi_hss_pll_hdr, serdes_xfi_hss_pll);
+
+ err = fill_seg_(fmsg, &dump->misc_nic_seg_hdr,
+ (u32 *)&dump->misc_nic_info);
+ if (err) {
+ kvfree(dump);
+ return err;
+ }
+
+ fill_seg(intr_states_seg_hdr, intr_states);
+ fill_seg(cam_entries_seg_hdr, cam_entries);
+ fill_seg(nic_routing_words_seg_hdr, nic_routing_words);
+ fill_seg(ets_seg_hdr, ets);
+ fill_seg(probe_dump_seg_hdr, probe_dump);
+ fill_seg(routing_reg_seg_hdr, routing_regs);
+ fill_seg(mac_prot_reg_seg_hdr, mac_prot_regs);
+ fill_seg(xaui2_an_hdr, serdes2_xaui_an);
+ fill_seg(xaui2_hss_pcs_hdr, serdes2_xaui_hss_pcs);
+ fill_seg(xfi2_an_hdr, serdes2_xfi_an);
+ fill_seg(xfi2_train_hdr, serdes2_xfi_train);
+ fill_seg(xfi2_hss_pcs_hdr, serdes2_xfi_hss_pcs);
+ fill_seg(xfi2_hss_tx_hdr, serdes2_xfi_hss_tx);
+ fill_seg(xfi2_hss_rx_hdr, serdes2_xfi_hss_rx);
+ fill_seg(xfi2_hss_pll_hdr, serdes2_xfi_hss_pll);
+ fill_seg(sem_regs_seg_hdr, sem_regs);
+
+ kvfree(dump);
+ return err;
}
static const struct devlink_health_reporter_ops qlge_reporter_ops = {
- .name = "dummy",
+ .name = "coredump",
.dump = qlge_reporter_coredump,
};
--
2.28.0
On Thu, Oct 08, 2020 at 07:58:04PM +0800, Coiby Xu wrote:
> -static int
> -qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> - struct devlink_fmsg *fmsg, void *priv_ctx,
> - struct netlink_ext_ack *extack)
> +static int fill_seg_(struct devlink_fmsg *fmsg,
> + struct mpi_coredump_segment_header *seg_header,
> + u32 *reg_data)
> {
> - return 0;
> + int i;
> + int header_size = sizeof(struct mpi_coredump_segment_header);
Please use the sizeof() directly in the code. Don't introduce
indirection if you can help it.
> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
> + int err;
> +
regards,
dan carpenter
On Thu, Oct 08, 2020 at 04:39:40PM +0300, Dan Carpenter wrote:
>On Thu, Oct 08, 2020 at 07:58:04PM +0800, Coiby Xu wrote:
>> -static int
>> -qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>> - struct devlink_fmsg *fmsg, void *priv_ctx,
>> - struct netlink_ext_ack *extack)
>> +static int fill_seg_(struct devlink_fmsg *fmsg,
>> + struct mpi_coredump_segment_header *seg_header,
>> + u32 *reg_data)
>> {
>> - return 0;
>> + int i;
>> + int header_size = sizeof(struct mpi_coredump_segment_header);
>
>Please use the sizeof() directly in the code. Don't introduce
>indirection if you can help it.
>
>> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
>> + int err;
>> +
>
Thank you for the suggestion!
>regards,
>dan carpenter
--
Best regards,
Coiby
On 2020-10-08 19:58 +0800, Coiby Xu wrote:
> $ devlink health dump show DEVICE reporter coredump -p -j
> {
> "Core Registers": {
> "segment": 1,
> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
> ,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
> },
> "Test Logic Regs": {
> "segment": 2,
> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
> },
> "RMII Registers": {
> "segment": 3,
> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
> },
> ...
> "Sem Registers": {
> "segment": 50,
> "values": [ 0,0,0,0 ]
> }
> }
>
> Signed-off-by: Coiby Xu <[email protected]>
> ---
> drivers/staging/qlge/qlge_devlink.c | 131 ++++++++++++++++++++++++++--
> 1 file changed, 125 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_devlink.c b/drivers/staging/qlge/qlge_devlink.c
> index aa45e7e368c0..91b6600b94a9 100644
> --- a/drivers/staging/qlge/qlge_devlink.c
> +++ b/drivers/staging/qlge/qlge_devlink.c
> @@ -1,16 +1,135 @@
> #include "qlge.h"
> #include "qlge_devlink.h"
>
> -static int
> -qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> - struct devlink_fmsg *fmsg, void *priv_ctx,
> - struct netlink_ext_ack *extack)
> +static int fill_seg_(struct devlink_fmsg *fmsg,
Please include the "qlge_" prefix.
> + struct mpi_coredump_segment_header *seg_header,
> + u32 *reg_data)
> {
> - return 0;
> + int i;
> + int header_size = sizeof(struct mpi_coredump_segment_header);
> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
> + int err;
> +
> + err = devlink_fmsg_pair_nest_start(fmsg, seg_header->description);
> + if (err)
> + return err;
> + err = devlink_fmsg_obj_nest_start(fmsg);
> + if (err)
> + return err;
> + err = devlink_fmsg_u32_pair_put(fmsg, "segment", seg_header->seg_num);
> + if (err)
> + return err;
> + err = devlink_fmsg_arr_pair_nest_start(fmsg, "values");
> + if (err)
> + return err;
> + for (i = 0; i < regs_num; i++) {
> + err = devlink_fmsg_u32_put(fmsg, *reg_data);
> + if (err)
> + return err;
> + reg_data++;
> + }
> + err = devlink_fmsg_obj_nest_end(fmsg);
> + if (err)
> + return err;
> + err = devlink_fmsg_arr_pair_nest_end(fmsg);
> + if (err)
> + return err;
> + err = devlink_fmsg_pair_nest_end(fmsg);
> + return err;
> +}
> +
> +#define fill_seg(seg_hdr, seg_regs) \
considering that this macro accesses local variables, it is not really
"function-like". I think an all-caps name would be better to tip-off the
reader.
> + do { \
> + err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
> + if (err) { \
> + kvfree(dump); \
> + return err; \
> + } \
> + } while (0)
> +
> +static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> + struct devlink_fmsg *fmsg, void *priv_ctx,
> + struct netlink_ext_ack *extack)
> +{
> + int err = 0;
> +
> + struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
Please name this variable ql_devlink, like in qlge_probe().
On 2020-10-10 18:02 +0800, Coiby Xu wrote:
[...]
> > > + do { \
> > > + err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
> > > + if (err) { \
> > > + kvfree(dump); \
> > > + return err; \
> > > + } \
> > > + } while (0)
> > > +
> > > +static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> > > + struct devlink_fmsg *fmsg, void *priv_ctx,
> > > + struct netlink_ext_ack *extack)
> > > +{
> > > + int err = 0;
> > > +
> > > + struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
> >
> > Please name this variable ql_devlink, like in qlge_probe().
>
> I happened to find the following text in drivers/staging/qlge/TODO
> > * in terms of namespace, the driver uses either qlge_, ql_ (used by
> > other qlogic drivers, with clashes, ex: ql_sem_spinlock) or nothing (with
> > clashes, ex: struct ob_mac_iocb_req). Rename everything to use the "qlge_"
> > prefix.
This comment applies to global identifiers, not local variables.
>
> So I will adopt qlge_ instead. Besides I prefer qlge_dl to ql_devlink.
Up to you but personally, I think ql_devlink is better. In any case,
"dev" is too general and often used for struct net_device pointers
instead.
On Sat, Oct 10, 2020 at 04:48:09PM +0900, Benjamin Poirier wrote:
>On 2020-10-08 19:58 +0800, Coiby Xu wrote:
>> $ devlink health dump show DEVICE reporter coredump -p -j
>> {
>> "Core Registers": {
>> "segment": 1,
>> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
>> ,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
>> },
>> "Test Logic Regs": {
>> "segment": 2,
>> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
>> },
>> "RMII Registers": {
>> "segment": 3,
>> "values": [ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 ]
>> },
>> ...
>> "Sem Registers": {
>> "segment": 50,
>> "values": [ 0,0,0,0 ]
>> }
>> }
>>
>> Signed-off-by: Coiby Xu <[email protected]>
>> ---
>> drivers/staging/qlge/qlge_devlink.c | 131 ++++++++++++++++++++++++++--
>> 1 file changed, 125 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/qlge/qlge_devlink.c b/drivers/staging/qlge/qlge_devlink.c
>> index aa45e7e368c0..91b6600b94a9 100644
>> --- a/drivers/staging/qlge/qlge_devlink.c
>> +++ b/drivers/staging/qlge/qlge_devlink.c
>> @@ -1,16 +1,135 @@
>> #include "qlge.h"
>> #include "qlge_devlink.h"
>>
>> -static int
>> -qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>> - struct devlink_fmsg *fmsg, void *priv_ctx,
>> - struct netlink_ext_ack *extack)
>> +static int fill_seg_(struct devlink_fmsg *fmsg,
>
>Please include the "qlge_" prefix.
>
>> + struct mpi_coredump_segment_header *seg_header,
>> + u32 *reg_data)
>> {
>> - return 0;
>> + int i;
>> + int header_size = sizeof(struct mpi_coredump_segment_header);
>> + int regs_num = (seg_header->seg_size - header_size) / sizeof(u32);
>> + int err;
>> +
>> + err = devlink_fmsg_pair_nest_start(fmsg, seg_header->description);
>> + if (err)
>> + return err;
>> + err = devlink_fmsg_obj_nest_start(fmsg);
>> + if (err)
>> + return err;
>> + err = devlink_fmsg_u32_pair_put(fmsg, "segment", seg_header->seg_num);
>> + if (err)
>> + return err;
>> + err = devlink_fmsg_arr_pair_nest_start(fmsg, "values");
>> + if (err)
>> + return err;
>> + for (i = 0; i < regs_num; i++) {
>> + err = devlink_fmsg_u32_put(fmsg, *reg_data);
>> + if (err)
>> + return err;
>> + reg_data++;
>> + }
>> + err = devlink_fmsg_obj_nest_end(fmsg);
>> + if (err)
>> + return err;
>> + err = devlink_fmsg_arr_pair_nest_end(fmsg);
>> + if (err)
>> + return err;
>> + err = devlink_fmsg_pair_nest_end(fmsg);
>> + return err;
>> +}
>> +
>> +#define fill_seg(seg_hdr, seg_regs) \
>
>considering that this macro accesses local variables, it is not really
>"function-like". I think an all-caps name would be better to tip-off the
>reader.
>
Thank you for this suggestion!
>> + do { \
>> + err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
>> + if (err) { \
>> + kvfree(dump); \
>> + return err; \
>> + } \
>> + } while (0)
>> +
>> +static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>> + struct devlink_fmsg *fmsg, void *priv_ctx,
>> + struct netlink_ext_ack *extack)
>> +{
>> + int err = 0;
>> +
>> + struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
>
>Please name this variable ql_devlink, like in qlge_probe().
I happened to find the following text in drivers/staging/qlge/TODO
> * in terms of namespace, the driver uses either qlge_, ql_ (used by
> other qlogic drivers, with clashes, ex: ql_sem_spinlock) or nothing (with
> clashes, ex: struct ob_mac_iocb_req). Rename everything to use the "qlge_"
> prefix.
So I will adopt qlge_ instead. Besides I prefer qlge_dl to ql_devlink.
--
Best regards,
Coiby
On Sat, Oct 10, 2020 at 10:22:30PM +0900, Benjamin Poirier wrote:
>On 2020-10-10 18:02 +0800, Coiby Xu wrote:
>[...]
>> > > + do { \
>> > > + err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
>> > > + if (err) { \
>> > > + kvfree(dump); \
>> > > + return err; \
>> > > + } \
>> > > + } while (0)
>> > > +
>> > > +static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
>> > > + struct devlink_fmsg *fmsg, void *priv_ctx,
>> > > + struct netlink_ext_ack *extack)
>> > > +{
>> > > + int err = 0;
>> > > +
>> > > + struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
>> >
>> > Please name this variable ql_devlink, like in qlge_probe().
>>
>> I happened to find the following text in drivers/staging/qlge/TODO
>> > * in terms of namespace, the driver uses either qlge_, ql_ (used by
>> > other qlogic drivers, with clashes, ex: ql_sem_spinlock) or nothing (with
>> > clashes, ex: struct ob_mac_iocb_req). Rename everything to use the "qlge_"
>> > prefix.
>
>This comment applies to global identifiers, not local variables.
Thank you for the explanation! Are you suggesting we should choose
different naming styles so we better tell global identifiers from local
variables?
>
>>
>> So I will adopt qlge_ instead. Besides I prefer qlge_dl to ql_devlink.
>
>Up to you but personally, I think ql_devlink is better. In any case,
>"dev" is too general and often used for struct net_device pointers
>instead.
Thank you for the suggestion. Another reason to use qlge_dl is many
other network drivers supporting devlink interface also adopt this kind
of style.
--
Best regards,
Coiby
On 2020-10-12 19:51 +0800, Coiby Xu wrote:
> On Sat, Oct 10, 2020 at 10:22:30PM +0900, Benjamin Poirier wrote:
> > On 2020-10-10 18:02 +0800, Coiby Xu wrote:
> > [...]
> > > > > + do { \
> > > > > + err = fill_seg_(fmsg, &dump->seg_hdr, dump->seg_regs); \
> > > > > + if (err) { \
> > > > > + kvfree(dump); \
> > > > > + return err; \
> > > > > + } \
> > > > > + } while (0)
> > > > > +
> > > > > +static int qlge_reporter_coredump(struct devlink_health_reporter *reporter,
> > > > > + struct devlink_fmsg *fmsg, void *priv_ctx,
> > > > > + struct netlink_ext_ack *extack)
> > > > > +{
> > > > > + int err = 0;
> > > > > +
> > > > > + struct qlge_devlink *dev = devlink_health_reporter_priv(reporter);
> > > >
> > > > Please name this variable ql_devlink, like in qlge_probe().
> > >
> > > I happened to find the following text in drivers/staging/qlge/TODO
> > > > * in terms of namespace, the driver uses either qlge_, ql_ (used by
> > > > other qlogic drivers, with clashes, ex: ql_sem_spinlock) or nothing (with
> > > > clashes, ex: struct ob_mac_iocb_req). Rename everything to use the "qlge_"
> > > > prefix.
> >
> > This comment applies to global identifiers, not local variables.
>
> Thank you for the explanation! Are you suggesting we should choose
> different naming styles so we better tell global identifiers from local
> variables?
That's not the main purpose IMO. Using a consistent prefix for global
identifiers (ex. "qlge_") is to avoid clashes (two drivers using the
same name, as in the examples above). Strictly speaking, it is not a
problem for symbols with internal linkage (ex. static functions) or type
definitions in local header files but it makes the code clearer because
it shows immediately that this identifier was defined in the driver.
For local variables, the name is more a matter of personal taste I think
but it should be consistent within the driver and with other users of
the same api, where applicable. A prefix is not needed but the name is
sometimes a simpler version of a type name which includes a prefix.
> > > So I will adopt qlge_ instead. Besides I prefer qlge_dl to ql_devlink.
> >
> > Up to you but personally, I think ql_devlink is better. In any case,
> > "dev" is too general and often used for struct net_device pointers
> > instead.
>
> Thank you for the suggestion. Another reason to use qlge_dl is many
> other network drivers supporting devlink interface also adopt this kind
> of style.
Sounds good. On second thought I regretted suggesting ql_devlink. While
local variable don't need any prefix; if they do have one, better not
mix qlge_ and ql_.