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 <coiby...@gmail.com> --- 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
