On Sun, 2016-06-26 at 09:40 -0700, Vidya Sagar Ravipati wrote: > On Sun, Jun 26, 2016 at 2:33 AM, Ben Hutchings <b...@decadent.org.uk> wrote: [...] > > This looks very similar to sff8472_diags, only with the actual values > > separated from the arrays of thresholds. > > > > Can the structure and code be combined with sfpdiag.c, with the > > additional per-channel diagnostics being optional? > > Diagnostic dom information in QSFP has lot more information compared > to SFPs and as part of this checkin , basic dom information in qsfp which is > equivalent to sfp dom is getting exposed as part of this checkin. > > Here are list of fields (not complete) which are used for debugging QSFP > issues, will be added for this structure in next patch sets > a) TX/RX output amplitude conttrol > b) TX_DISABLE > b) TX_FAULT > c) TX CDR > d) RX CDR > e) RX output disable > f) Rate select option > > Please let me know if it make sense to maintain the different structure > with above explanation or whether it is required to be combined. [...]
I think there's enough information in common that it does make sense to use common reporting functions, and that in turn suggests that it would make sense to use a common structure. You could alternately have the callers in sfpdiag.c and qsfp.c extract the relevant fields and pass them into the reporting functions. The substantial duplication of reporting code from sfpid.c in your latest submission is not OK. Ben. -- Ben Hutchings compatible: Gracefully accepts erroneous data from any source
signature.asc
Description: This is a digitally signed message part
