Re: [PATCH net-next v2 04/10] net: ethtool: Add attributes for cable test reports
From: Michal Kubecek <hidden>
Date: 2020-05-05 13:24:20
On Tue, May 05, 2020 at 03:15:48PM +0200, Andrew Lunn wrote:
quoted
quoted
+ +---------------------------------------------+--------+---------------------+ + | ``ETHTOOL_A_CABLE_TEST_HEADER`` | nested | reply header | + +---------------------------------------------+--------+---------------------+ + | ``ETHTOOL_A_CABLE_TEST_STATUS`` | u8 | completed | + +---------------------------------------------+--------+---------------------+ + | ``ETHTOOL_A_CABLE_TEST_NTF_NEST`` | nested | all the results | + +-+-------------------------------------------+--------+---------------------+ + | | ``ETHTOOL_A_CABLE_TEST_STATUS`` | u8 | completed | + +-+-------------------------------------------+--------+---------------------+You have ETHTOOL_A_CABLE_TEST_STATUS both here and on top level. AFAICS the top level attribute is the right one - but the name is ETHTOOL_A_CABLE_TEST_NTF_STATUS.Hi Michal They need better names. The first one is about the test run status. Started vs complete. A notification is sent when the test is started, and a second one at the end with the actual test results. The second status is per pair, indicating open, shorted, O.K. Maybe this second one shouldd be ETHTOOL_A_CABLE_TEST_NTF_PAIR_STATUS.
The per-pair status is ETHTOOL_A_CABLE_RESULTS_CODE (nested within ETHTOOL_A_CABLE_TEST_NTF_RESULT), isn't it? Based on the code, I would say second ETHTOOL_A_CABLE_TEST_STATUS line should be dropped and first fixed to ETHTOOL_A_CABLE_TEST_NTF_STATUS. Michal