Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Neftin, Sasha <sasha.neftin@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v1 1/1] igc: Remove copper fiber switch control
Date: Mon, 2 Mar 2020 16:44:17 -0800	[thread overview]
Message-ID: <549bd226-b5fa-eb68-44a5-f77dcaf28c5a@intel.com> (raw)
In-Reply-To: <CAKgT0UdFW1KbAzsUX2o7-UDD1ay70euxs6Ts=RjYN4dfFEE6Fg@mail.gmail.com>

On 3/2/2020 13:26, Alexander Duyck wrote:
> On Mon, Mar 2, 2020 at 12:23 PM Sasha Neftin <sasha.neftin@intel.com> wrote:
>>
>> i225 device support copper mode only
>> PHY signal detect indication for copper fiber switch
>> not applicable to i225 part
>>
>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> 
> So there are a couple issues with this patch.
> 
> All the changes in igc_ethtool.c are broken at this point. Once a
> register is defined in regs_buff you cannot change it. Otherwise you
> cannot debug this in the future. You would be better off just skipping
> the register that you were storing CONNSW and let it default to zero
> instead of doing all of the shifting you are doing. You can just skip
> over the register in the dump in ethtool assuming there is even a file
> for the device that hasbeen added.
> 
This change not affected igc_ethtool.c behavior. I see the same behavior 
on my setup.
Actually ethtool --register-dump not called (as properly).get_regs 
callback from igc_ethtool.c. This is not related to this patch and I 
need investigate and fix it.
ethtool --register-dump <adapter> show me row generic data. Data is 
really from i225 registers, but not parcered as for other drivers.

> Also you might want to change the patch description to specify that
> the CONNSW register doesn't exist for the i225 part. Then it makes
> sense to remove the register from igc_get_regs.
> 
>> ---
>>   drivers/net/ethernet/intel/igc/igc_defines.h |   2 -
>>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 193 +++++++++++++--------------
>>   drivers/net/ethernet/intel/igc/igc_main.c    |   9 --
>>   drivers/net/ethernet/intel/igc/igc_regs.h    |   1 -
>>   4 files changed, 96 insertions(+), 109 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
>> index dd0c86ce09ed..e5116337b68d 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_defines.h
>> +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
>> @@ -91,8 +91,6 @@
>>   #define IGC_CTRL_RFCE          0x08000000  /* Receive Flow Control enable */
>>   #define IGC_CTRL_TFCE          0x10000000  /* Transmit flow control enable */
>>
>> -#define IGC_CONNSW_AUTOSENSE_EN        0x1
>> -
>>   /* As per the EAS the maximum supported size is 9.5KB (9728 bytes) */
>>   #define MAX_JUMBO_FRAME_SIZE   0x2600
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> index 69f50b8e2af3..82d0c893ed41 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> @@ -160,142 +160,141 @@ static void igc_get_regs(struct net_device *netdev,
>>          regs_buff[1] = rd32(IGC_STATUS);
>>          regs_buff[2] = rd32(IGC_CTRL_EXT);
>>          regs_buff[3] = rd32(IGC_MDIC);
>> -       regs_buff[4] = rd32(IGC_CONNSW);
>>
>>          /* NVM Register */
>> -       regs_buff[5] = rd32(IGC_EECD);
>> +       regs_buff[4] = rd32(IGC_EECD);
>>
>>          /* Interrupt */
>>          /* Reading EICS for EICR because they read the
>>           * same but EICS does not clear on read
>>           */
>> +       regs_buff[5] = rd32(IGC_EICS);
>>          regs_buff[6] = rd32(IGC_EICS);
>> -       regs_buff[7] = rd32(IGC_EICS);
>> -       regs_buff[8] = rd32(IGC_EIMS);
>> -       regs_buff[9] = rd32(IGC_EIMC);
>> -       regs_buff[10] = rd32(IGC_EIAC);
>> -       regs_buff[11] = rd32(IGC_EIAM);
>> +       regs_buff[7] = rd32(IGC_EIMS);
>> +       regs_buff[8] = rd32(IGC_EIMC);
>> +       regs_buff[9] = rd32(IGC_EIAC);
>> +       regs_buff[10] = rd32(IGC_EIAM);
>>          /* Reading ICS for ICR because they read the
>>           * same but ICS does not clear on read
>>           */
>> +       regs_buff[11] = rd32(IGC_ICS);
>>          regs_buff[12] = rd32(IGC_ICS);
>> -       regs_buff[13] = rd32(IGC_ICS);
>> -       regs_buff[14] = rd32(IGC_IMS);
>> -       regs_buff[15] = rd32(IGC_IMC);
>> -       regs_buff[16] = rd32(IGC_IAC);
>> -       regs_buff[17] = rd32(IGC_IAM);
>> +       regs_buff[13] = rd32(IGC_IMS);
>> +       regs_buff[14] = rd32(IGC_IMC);
>> +       regs_buff[15] = rd32(IGC_IAC);
>> +       regs_buff[16] = rd32(IGC_IAM);
>>
>>          /* Flow Control */
>> -       regs_buff[18] = rd32(IGC_FCAL);
>> -       regs_buff[19] = rd32(IGC_FCAH);
>> -       regs_buff[20] = rd32(IGC_FCTTV);
>> -       regs_buff[21] = rd32(IGC_FCRTL);
>> -       regs_buff[22] = rd32(IGC_FCRTH);
>> -       regs_buff[23] = rd32(IGC_FCRTV);
>> +       regs_buff[17] = rd32(IGC_FCAL);
>> +       regs_buff[18] = rd32(IGC_FCAH);
>> +       regs_buff[19] = rd32(IGC_FCTTV);
>> +       regs_buff[20] = rd32(IGC_FCRTL);
>> +       regs_buff[21] = rd32(IGC_FCRTH);
>> +       regs_buff[22] = rd32(IGC_FCRTV);
>>
>>          /* Receive */
>> -       regs_buff[24] = rd32(IGC_RCTL);
>> -       regs_buff[25] = rd32(IGC_RXCSUM);
>> -       regs_buff[26] = rd32(IGC_RLPML);
>> -       regs_buff[27] = rd32(IGC_RFCTL);
>> +       regs_buff[23] = rd32(IGC_RCTL);
>> +       regs_buff[24] = rd32(IGC_RXCSUM);
>> +       regs_buff[25] = rd32(IGC_RLPML);
>> +       regs_buff[26] = rd32(IGC_RFCTL);
>>
>>          /* Transmit */
>> -       regs_buff[28] = rd32(IGC_TCTL);
>> -       regs_buff[29] = rd32(IGC_TIPG);
>> +       regs_buff[27] = rd32(IGC_TCTL);
>> +       regs_buff[28] = rd32(IGC_TIPG);
>>
>>          /* Wake Up */
>>
>>          /* MAC */
>>
>>          /* Statistics */
>> -       regs_buff[30] = adapter->stats.crcerrs;
>> -       regs_buff[31] = adapter->stats.algnerrc;
>> -       regs_buff[32] = adapter->stats.symerrs;
>> -       regs_buff[33] = adapter->stats.rxerrc;
>> -       regs_buff[34] = adapter->stats.mpc;
>> -       regs_buff[35] = adapter->stats.scc;
>> -       regs_buff[36] = adapter->stats.ecol;
>> -       regs_buff[37] = adapter->stats.mcc;
>> -       regs_buff[38] = adapter->stats.latecol;
>> -       regs_buff[39] = adapter->stats.colc;
>> -       regs_buff[40] = adapter->stats.dc;
>> -       regs_buff[41] = adapter->stats.tncrs;
>> -       regs_buff[42] = adapter->stats.sec;
>> -       regs_buff[43] = adapter->stats.htdpmc;
>> -       regs_buff[44] = adapter->stats.rlec;
>> -       regs_buff[45] = adapter->stats.xonrxc;
>> -       regs_buff[46] = adapter->stats.xontxc;
>> -       regs_buff[47] = adapter->stats.xoffrxc;
>> -       regs_buff[48] = adapter->stats.xofftxc;
>> -       regs_buff[49] = adapter->stats.fcruc;
>> -       regs_buff[50] = adapter->stats.prc64;
>> -       regs_buff[51] = adapter->stats.prc127;
>> -       regs_buff[52] = adapter->stats.prc255;
>> -       regs_buff[53] = adapter->stats.prc511;
>> -       regs_buff[54] = adapter->stats.prc1023;
>> -       regs_buff[55] = adapter->stats.prc1522;
>> -       regs_buff[56] = adapter->stats.gprc;
>> -       regs_buff[57] = adapter->stats.bprc;
>> -       regs_buff[58] = adapter->stats.mprc;
>> -       regs_buff[59] = adapter->stats.gptc;
>> -       regs_buff[60] = adapter->stats.gorc;
>> -       regs_buff[61] = adapter->stats.gotc;
>> -       regs_buff[62] = adapter->stats.rnbc;
>> -       regs_buff[63] = adapter->stats.ruc;
>> -       regs_buff[64] = adapter->stats.rfc;
>> -       regs_buff[65] = adapter->stats.roc;
>> -       regs_buff[66] = adapter->stats.rjc;
>> -       regs_buff[67] = adapter->stats.mgprc;
>> -       regs_buff[68] = adapter->stats.mgpdc;
>> -       regs_buff[69] = adapter->stats.mgptc;
>> -       regs_buff[70] = adapter->stats.tor;
>> -       regs_buff[71] = adapter->stats.tot;
>> -       regs_buff[72] = adapter->stats.tpr;
>> -       regs_buff[73] = adapter->stats.tpt;
>> -       regs_buff[74] = adapter->stats.ptc64;
>> -       regs_buff[75] = adapter->stats.ptc127;
>> -       regs_buff[76] = adapter->stats.ptc255;
>> -       regs_buff[77] = adapter->stats.ptc511;
>> -       regs_buff[78] = adapter->stats.ptc1023;
>> -       regs_buff[79] = adapter->stats.ptc1522;
>> -       regs_buff[80] = adapter->stats.mptc;
>> -       regs_buff[81] = adapter->stats.bptc;
>> -       regs_buff[82] = adapter->stats.tsctc;
>> -       regs_buff[83] = adapter->stats.iac;
>> -       regs_buff[84] = adapter->stats.rpthc;
>> -       regs_buff[85] = adapter->stats.hgptc;
>> -       regs_buff[86] = adapter->stats.hgorc;
>> -       regs_buff[87] = adapter->stats.hgotc;
>> -       regs_buff[88] = adapter->stats.lenerrs;
>> -       regs_buff[89] = adapter->stats.scvpc;
>> -       regs_buff[90] = adapter->stats.hrmpc;
>> +       regs_buff[29] = adapter->stats.crcerrs;
>> +       regs_buff[30] = adapter->stats.algnerrc;
>> +       regs_buff[31] = adapter->stats.symerrs;
>> +       regs_buff[32] = adapter->stats.rxerrc;
>> +       regs_buff[33] = adapter->stats.mpc;
>> +       regs_buff[34] = adapter->stats.scc;
>> +       regs_buff[35] = adapter->stats.ecol;
>> +       regs_buff[36] = adapter->stats.mcc;
>> +       regs_buff[37] = adapter->stats.latecol;
>> +       regs_buff[38] = adapter->stats.colc;
>> +       regs_buff[39] = adapter->stats.dc;
>> +       regs_buff[40] = adapter->stats.tncrs;
>> +       regs_buff[41] = adapter->stats.sec;
>> +       regs_buff[42] = adapter->stats.htdpmc;
>> +       regs_buff[43] = adapter->stats.rlec;
>> +       regs_buff[44] = adapter->stats.xonrxc;
>> +       regs_buff[45] = adapter->stats.xontxc;
>> +       regs_buff[46] = adapter->stats.xoffrxc;
>> +       regs_buff[47] = adapter->stats.xofftxc;
>> +       regs_buff[48] = adapter->stats.fcruc;
>> +       regs_buff[49] = adapter->stats.prc64;
>> +       regs_buff[50] = adapter->stats.prc127;
>> +       regs_buff[51] = adapter->stats.prc255;
>> +       regs_buff[52] = adapter->stats.prc511;
>> +       regs_buff[53] = adapter->stats.prc1023;
>> +       regs_buff[54] = adapter->stats.prc1522;
>> +       regs_buff[55] = adapter->stats.gprc;
>> +       regs_buff[56] = adapter->stats.bprc;
>> +       regs_buff[57] = adapter->stats.mprc;
>> +       regs_buff[58] = adapter->stats.gptc;
>> +       regs_buff[59] = adapter->stats.gorc;
>> +       regs_buff[60] = adapter->stats.gotc;
>> +       regs_buff[61] = adapter->stats.rnbc;
>> +       regs_buff[62] = adapter->stats.ruc;
>> +       regs_buff[63] = adapter->stats.rfc;
>> +       regs_buff[64] = adapter->stats.roc;
>> +       regs_buff[65] = adapter->stats.rjc;
>> +       regs_buff[66] = adapter->stats.mgprc;
>> +       regs_buff[67] = adapter->stats.mgpdc;
>> +       regs_buff[68] = adapter->stats.mgptc;
>> +       regs_buff[69] = adapter->stats.tor;
>> +       regs_buff[70] = adapter->stats.tot;
>> +       regs_buff[71] = adapter->stats.tpr;
>> +       regs_buff[72] = adapter->stats.tpt;
>> +       regs_buff[73] = adapter->stats.ptc64;
>> +       regs_buff[74] = adapter->stats.ptc127;
>> +       regs_buff[75] = adapter->stats.ptc255;
>> +       regs_buff[76] = adapter->stats.ptc511;
>> +       regs_buff[77] = adapter->stats.ptc1023;
>> +       regs_buff[78] = adapter->stats.ptc1522;
>> +       regs_buff[79] = adapter->stats.mptc;
>> +       regs_buff[80] = adapter->stats.bptc;
>> +       regs_buff[81] = adapter->stats.tsctc;
>> +       regs_buff[82] = adapter->stats.iac;
>> +       regs_buff[83] = adapter->stats.rpthc;
>> +       regs_buff[84] = adapter->stats.hgptc;
>> +       regs_buff[85] = adapter->stats.hgorc;
>> +       regs_buff[86] = adapter->stats.hgotc;
>> +       regs_buff[87] = adapter->stats.lenerrs;
>> +       regs_buff[88] = adapter->stats.scvpc;
>> +       regs_buff[89] = adapter->stats.hrmpc;
>>
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[91 + i] = rd32(IGC_SRRCTL(i));
>> +               regs_buff[90 + i] = rd32(IGC_SRRCTL(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[95 + i] = rd32(IGC_PSRTYPE(i));
>> +               regs_buff[94 + i] = rd32(IGC_PSRTYPE(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[99 + i] = rd32(IGC_RDBAL(i));
>> +               regs_buff[98 + i] = rd32(IGC_RDBAL(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[103 + i] = rd32(IGC_RDBAH(i));
>> +               regs_buff[102 + i] = rd32(IGC_RDBAH(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[107 + i] = rd32(IGC_RDLEN(i));
>> +               regs_buff[106 + i] = rd32(IGC_RDLEN(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[111 + i] = rd32(IGC_RDH(i));
>> +               regs_buff[110 + i] = rd32(IGC_RDH(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[115 + i] = rd32(IGC_RDT(i));
>> +               regs_buff[114 + i] = rd32(IGC_RDT(i));
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[119 + i] = rd32(IGC_RXDCTL(i));
>> +               regs_buff[118 + i] = rd32(IGC_RXDCTL(i));
>>
>>          for (i = 0; i < 10; i++)
>> -               regs_buff[123 + i] = rd32(IGC_EITR(i));
>> +               regs_buff[122 + i] = rd32(IGC_EITR(i));
>>          for (i = 0; i < 16; i++)
>> -               regs_buff[139 + i] = rd32(IGC_RAL(i));
>> +               regs_buff[138 + i] = rd32(IGC_RAL(i));
>>          for (i = 0; i < 16; i++)
>> -               regs_buff[145 + i] = rd32(IGC_RAH(i));
>> +               regs_buff[144 + i] = rd32(IGC_RAH(i));
>>
>>          for (i = 0; i < 4; i++)
>> -               regs_buff[149 + i] = rd32(IGC_TDBAL(i));
>> +               regs_buff[148 + i] = rd32(IGC_TDBAL(i));
>>          for (i = 0; i < 4; i++)
>>                  regs_buff[152 + i] = rd32(IGC_TDBAH(i));
>>          for (i = 0; i < 4; i++)
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
>> index d406aaea24af..47009fe0cbde 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -4036,7 +4036,6 @@ static void igc_watchdog_task(struct work_struct *work)
>>          struct igc_hw *hw = &adapter->hw;
>>          struct igc_phy_info *phy = &hw->phy;
>>          u16 phy_data, retry_count = 20;
>> -       u32 connsw;
>>          u32 link;
>>          int i;
>>
>> @@ -4049,14 +4048,6 @@ static void igc_watchdog_task(struct work_struct *work)
>>                          link = false;
>>          }
>>
>> -       /* Force link down if we have fiber to swap to */
>> -       if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
>> -               if (hw->phy.media_type == igc_media_type_copper) {
>> -                       connsw = rd32(IGC_CONNSW);
>> -                       if (!(connsw & IGC_CONNSW_AUTOSENSE_EN))
>> -                               link = 0;
>> -               }
>> -       }
>>          if (link) {
>>                  /* Cancel scheduled suspend requests. */
>>                  pm_runtime_resume(netdev->dev.parent);
>> diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
>> index 96dee3c1a5f7..79789176fc80 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_regs.h
>> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
>> @@ -11,7 +11,6 @@
>>   #define IGC_CTRL_EXT           0x00018  /* Extended Device Control - RW */
>>   #define IGC_MDIC               0x00020  /* MDI Control - RW */
>>   #define IGC_MDICNFG            0x00E04  /* MDC/MDIO Configuration - RW */
>> -#define IGC_CONNSW             0x00034  /* Copper/Fiber switch control - RW */
>>   #define IGC_I225_PHPM          0x00E14  /* I225 PHY Power Management */
>>
>>   /* Internal Packet Buffer Size Registers */
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan at osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan


  reply	other threads:[~2020-03-03  0:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 20:23 [Intel-wired-lan] [PATCH v1 1/1] igc: Remove copper fiber switch control Sasha Neftin
2020-03-02 21:26 ` Alexander Duyck
2020-03-03  0:44   ` Neftin, Sasha [this message]
2020-03-03 16:00     ` Alexander Duyck
2020-03-03 16:53       ` Neftin, Sasha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=549bd226-b5fa-eb68-44a5-f77dcaf28c5a@intel.com \
    --to=sasha.neftin@intel.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox