On Mon, 2016-06-27 at 00:02 +0200, Ben Hutchings wrote: > 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.
I mean sfpdiag.c here. I didn't check for duplication from sfpid.c, but I think you've avoided that. Ben. -- Ben Hutchings Humour is the best antidote to reality.
signature.asc
Description: This is a digitally signed message part
