| Message ID | 20210601003325.1631980-10-olteanv@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert xpcs to phylink_pcs_ops | expand |
On Tue, Jun 01, 2021 at 01:10:33PM +0100, Russell King (Oracle) wrote: > On Tue, Jun 01, 2021 at 03:33:25AM +0300, Vladimir Oltean wrote: > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > > .validate = stmmac_validate, > > - .mac_pcs_get_state = stmmac_mac_pcs_get_state, > > - .mac_config = stmmac_mac_config, > > mac_config is still a required function. This is correct, thanks. VK, would you mind testing again with this extra patch added to the mix? If it works, I will add it to the series in v3, ordered properly. -----------------------------[ cut here]-----------------------------
On Wed, Jun 02, 2021 at 05:02:33PM +0300, Vladimir Oltean wrote: > On Wed, Jun 02, 2021 at 02:47:49PM +0100, Russell King (Oracle) wrote: > > On Wed, Jun 02, 2021 at 04:43:21PM +0300, Vladimir Oltean wrote: > > > On Tue, Jun 01, 2021 at 01:10:33PM +0100, Russell King (Oracle) wrote: > > > > On Tue, Jun 01, 2021 at 03:33:25AM +0300, Vladimir Oltean wrote: > > > > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > > > > > .validate = stmmac_validate, > > > > > - .mac_pcs_get_state = stmmac_mac_pcs_get_state, > > > > > - .mac_config = stmmac_mac_config, > > > > > > > > mac_config is still a required function. > > > > > > This is correct, thanks. > > > > > > VK, would you mind testing again with this extra patch added to the mix? > > > If it works, I will add it to the series in v3, ordered properly. > > > > > > -----------------------------[ cut here]----------------------------- > > > From a79863027998451c73d5bbfaf1b77cf6097a110c Mon Sep 17 00:00:00 2001 > > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > Date: Wed, 2 Jun 2021 16:35:55 +0300 > > > Subject: [PATCH] net: phylink: allow the mac_config method to be missing if > > > pcs_ops are provided > > > > > > The pcs_config method from struct phylink_pcs_ops does everything that > > > the mac_config method from struct phylink_mac_ops used to do in the > > > legacy approach of driving a MAC PCS. So allow drivers to not implement > > > the mac_config method if there is nothing to do. Keep the method > > > required for setups that do not provide pcs_ops. > > > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > > --- > > > drivers/net/phy/phylink.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > > index 96d8e88b4e46..a8842c6ce3a2 100644 > > > --- a/drivers/net/phy/phylink.c > > > +++ b/drivers/net/phy/phylink.c > > > @@ -415,6 +415,9 @@ static void phylink_resolve_flow(struct phylink_link_state *state) > > > static void phylink_mac_config(struct phylink *pl, > > > const struct phylink_link_state *state) > > > { > > > + if (!pl->mac_ops->mac_config) > > > + return; > > > + > > > phylink_dbg(pl, > > > "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n", > > > __func__, phylink_an_mode_str(pl->cur_link_an_mode), > > > @@ -1192,6 +1195,12 @@ void phylink_start(struct phylink *pl) > > > > > > ASSERT_RTNL(); > > > > > > + /* The mac_ops::mac_config method may be absent only if the > > > + * pcs_ops are present. > > > + */ > > > + if (WARN_ON_ONCE(!pl->mac_ops->mac_config && !pl->pcs_ops)) > > > + return; > > > + > > > phylink_info(pl, "configuring for %s/%s link mode\n", > > > phylink_an_mode_str(pl->cur_link_an_mode), > > > phy_modes(pl->link_config.interface)); > > > > I would rather we didn't do that - I suspect your case is not the > > common scenario, so please add a dummy function to stmmac instead. > > Ok, in that case the only delta to be applied should be: > > -----------------------------[ cut here]----------------------------- > >From 998569108392befc591f790e46fe5dcd1d0b6278 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Wed, 2 Jun 2021 17:00:12 +0300 > Subject: [PATCH] fixup! net: pcs: xpcs: convert to phylink_pcs_ops > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index d5685a74f3b7..704aa91b145a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1000,6 +1000,12 @@ static void stmmac_validate(struct phylink_config *config, > xpcs_validate(priv->hw->xpcs, supported, state); > } > > +static void stmmac_mac_config(struct phylink_config *config, unsigned int mode, > + const struct phylink_link_state *state) > +{ > + /* Nothing to do, xpcs_config() handles everything */ > +} > + > static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up) > { > struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; > -----------------------------[ cut here]----------------------------- No luck.. I am still seeing the same kernel panic on my side with the additional patch applied: [ 12.513652] intel-eth-pci 0000:00:1e.4 enp0s30f4: FPE workqueue start [ 12.520079] intel-eth-pci 0000:00:1e.4 enp0s30f4: configuring for inband/sgmii link mode [ 12.528141] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 12.535072] #PF: supervisor instruction fetch in kernel mode [ 12.540719] #PF: error_code(0x0010) - not-present page [ 12.545843] PGD 0 P4D 0 [ 12.548382] Oops: 0010 [#1] PREEMPT SMP NOPTI [ 12.552733] CPU: 3 PID: 2657 Comm: connmand Tainted: G U 5.13.0-rc3-intel-lts #79 [ 12.561485] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLIFUI1.R00.3373.AF0.2009230546 09/23/2020 [ 12.574803] RIP: 0010:0x0 [ 12.577436] Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6. [ 12.584280] RSP: 0018:ffffaa05404efb78 EFLAGS: 00010246 [ 12.589489] RAX: 0000000000000000 RBX: ffffa0fb91758c00 RCX: 0000000000000000 [ 12.596600] RDX: ffffaa05404efba0 RSI: 0000000000000002 RDI: ffffa0facc63c4d8 [ 12.603711] RBP: ffffaa05404efba0 R08: 0000000000000000 R09: ffffaa05404ef888 [ 12.610825] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000001 [ 12.617935] R13: ffffa0facc6388c0 R14: 0000000000000006 R15: 0000000000000001 [ 12.625040] FS: 00007f4a6d5b87c0(0000) GS:ffffa0fc57f80000(0000) knlGS:0000000000000000 [ 12.633096] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 12.638825] CR2: ffffffffffffffd6 CR3: 000000010e3f6004 CR4: 0000000000770ee0 [ 12.645932] PKRU: 55555554 [ 12.648641] Call Trace: [ 12.651095] phylink_major_config+0x5e/0x1a0 [phylink] [ 12.656219] phylink_start+0x204/0x2c0 [phylink] [ 12.660833] stmmac_open+0x3d0/0x9f0 [stmmac] [ 12.665186] __dev_open+0xe7/0x180 [ 12.668594] __dev_change_flags+0x174/0x1d0 [ 12.672773] ? __thaw_task+0x40/0x40 [ 12.676348] ? arch_stack_walk+0x9e/0xf0 [ 12.680266] dev_change_flags+0x21/0x60 [ 12.684100] devinet_ioctl+0x5e8/0x750 [ 12.687847] inet_ioctl+0x190/0x1c0 [ 12.691335] ? dev_ioctl+0x26d/0x4c0 [ 12.694907] sock_do_ioctl+0x44/0x140 [ 12.698569] ? alloc_empty_file+0x61/0xb0 [ 12.702572] sock_ioctl+0x22c/0x320 [ 12.706061] __x64_sys_ioctl+0x80/0xb0 [ 12.709808] do_syscall_64+0x42/0x80 [ 12.713381] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 12.718425] RIP: 0033:0x7f4a6db054bb [ 12.721995] Code: 0f 1e fa 48 8b 05 c5 69 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 95 69 0c 00 f7 d8 64 89 01 48 [ 12.740656] RSP: 002b:00007ffdeab00da8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 12.748194] RAX: ffffffffffffffda RBX: 0000562741681100 RCX: 00007f4a6db054bb [ 12.755300] RDX: 00007ffdeab00db0 RSI: 0000000000008914 RDI: 0000000000000010 [ 12.762405] RBP: 0000000000000010 R08: 0000562741681100 R09: 0000000000000000 [ 12.769510] R10: 000056274162e010 R11: 0000000000000246 R12: 0000000000000000 [ 12.776613] R13: 00007ffdeab00db0 R14: 0000000000000001 R15: 00007ffdeab014f0 [ 12.783721] Modules linked in: bluetooth ecryptfs hid_sensor_incl_3d hid_sensor_magn_3d hid_sensor_gyro_3d hid_sensor_als hid_sensor_accel_3d hid_sensor_trigger hid_sensor_iio_common hid_sensor_custom hid_sensor_hub intel_ishtp_loader intel_ishtp_hid mxl_gpy x86_pkg_temp_thermal ax88179_178a dwmac_intel usbnet mii stmmac dwc3 atkbd kvm_intel pcs_xpcs mei_wdt phylink mei_hdcp udc_core kvm libps2 libphy mei_me i915 irqbypass spi_pxa2xx_platform intel_rapl_msr pcspkr dw_dmac wdat_wdt i2c_i801 dw_dmac_core i2c_smbus intel_ish_ipc mei intel_ishtp dwc3_pci thermal tpm_crb tpm_tis tpm_tis_core intel_pmc_core parport_pc i8042 parport tpm sch_fq_codel uhid fuse configfs snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_pcm snd_timer snd soundcore [ 12.851290] CR2: 0000000000000000 [ 12.854603] ---[ end trace 52b9ca69be06d1e3 ]--- My git log:- dc25bc46886e (HEAD -> master) phy: maxlinear: add Maxlinear GPY115/21x/24x driver d059f32fcef1 fixup! net: pcs: xpcs: convert to phylink_pcs_ops 1a1a9c7267c1 net: pcs: xpcs: convert to phylink_pcs_ops ec25f3c80108 net: pcs: xpcs: convert to mdio_device 912a23a5768a net: pcs: xpcs: use mdiobus_c45_addr in xpcs_{read,write} af2bd0a4e32c net: pcs: xpcs: export xpcs_probe 7e4eea51132f net: pcs: xpcs: export xpcs_config_eee 37d5e086fc83 net: pcs: xpcs: export xpcs_validate d58487edb1a8 net: pcs: xpcs: make the checks related to the PHY interface mode stateless 2f0eba94af8d net: pcs: xpcs: there is only one PHY ID 67148dd06896 net: pcs: xpcs: delete shim definition for mdio_xpcs_get_ops() 5fe8e519e44f (origin/master, origin/HEAD) Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next HTH, VK
On Wed, Jun 02, 2021 at 06:14:47PM +0300, Vladimir Oltean wrote: > On Wed, Jun 02, 2021 at 11:10:56PM +0800, Wong Vee Khee wrote: > > On Wed, Jun 02, 2021 at 05:57:30PM +0300, Vladimir Oltean wrote: > > > On Wed, Jun 02, 2021 at 10:43:36PM +0800, Wong Vee Khee wrote: > > > > On Wed, Jun 02, 2021 at 05:02:33PM +0300, Vladimir Oltean wrote: > > > > > On Wed, Jun 02, 2021 at 02:47:49PM +0100, Russell King (Oracle) wrote: > > > > > > On Wed, Jun 02, 2021 at 04:43:21PM +0300, Vladimir Oltean wrote: > > > > > > > On Tue, Jun 01, 2021 at 01:10:33PM +0100, Russell King (Oracle) wrote: > > > > > > > > On Tue, Jun 01, 2021 at 03:33:25AM +0300, Vladimir Oltean wrote: > > > > > > > > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > > > > > > > > > .validate = stmmac_validate, > > > > > > > > > - .mac_pcs_get_state = stmmac_mac_pcs_get_state, > > > > > > > > > - .mac_config = stmmac_mac_config, > > > > > > > > > > > > > > > > mac_config is still a required function. > > > > > > > > > > > > > > This is correct, thanks. > > > > > > > > > > > > > > VK, would you mind testing again with this extra patch added to the mix? > > > > > > > If it works, I will add it to the series in v3, ordered properly. > > > > > > > > > > > > > > -----------------------------[ cut here]----------------------------- > > > > > > > From a79863027998451c73d5bbfaf1b77cf6097a110c Mon Sep 17 00:00:00 2001 > > > > > > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > > > > Date: Wed, 2 Jun 2021 16:35:55 +0300 > > > > > > > Subject: [PATCH] net: phylink: allow the mac_config method to be missing if > > > > > > > pcs_ops are provided > > > > > > > > > > > > > > The pcs_config method from struct phylink_pcs_ops does everything that > > > > > > > the mac_config method from struct phylink_mac_ops used to do in the > > > > > > > legacy approach of driving a MAC PCS. So allow drivers to not implement > > > > > > > the mac_config method if there is nothing to do. Keep the method > > > > > > > required for setups that do not provide pcs_ops. > > > > > > > > > > > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > > > > --- > > > > > > > drivers/net/phy/phylink.c | 9 +++++++++ > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > > > > > > > index 96d8e88b4e46..a8842c6ce3a2 100644 > > > > > > > --- a/drivers/net/phy/phylink.c > > > > > > > +++ b/drivers/net/phy/phylink.c > > > > > > > @@ -415,6 +415,9 @@ static void phylink_resolve_flow(struct phylink_link_state *state) > > > > > > > static void phylink_mac_config(struct phylink *pl, > > > > > > > const struct phylink_link_state *state) > > > > > > > { > > > > > > > + if (!pl->mac_ops->mac_config) > > > > > > > + return; > > > > > > > + > > > > > > > phylink_dbg(pl, > > > > > > > "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n", > > > > > > > __func__, phylink_an_mode_str(pl->cur_link_an_mode), > > > > > > > @@ -1192,6 +1195,12 @@ void phylink_start(struct phylink *pl) > > > > > > > > > > > > > > ASSERT_RTNL(); > > > > > > > > > > > > > > + /* The mac_ops::mac_config method may be absent only if the > > > > > > > + * pcs_ops are present. > > > > > > > + */ > > > > > > > + if (WARN_ON_ONCE(!pl->mac_ops->mac_config && !pl->pcs_ops)) > > > > > > > + return; > > > > > > > + > > > > > > > phylink_info(pl, "configuring for %s/%s link mode\n", > > > > > > > phylink_an_mode_str(pl->cur_link_an_mode), > > > > > > > phy_modes(pl->link_config.interface)); > > > > > > > > > > > > I would rather we didn't do that - I suspect your case is not the > > > > > > common scenario, so please add a dummy function to stmmac instead. > > > > > > > > > > Ok, in that case the only delta to be applied should be: > > > > > > > > > > -----------------------------[ cut here]----------------------------- > > > > > >From 998569108392befc591f790e46fe5dcd1d0b6278 Mon Sep 17 00:00:00 2001 > > > > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > > Date: Wed, 2 Jun 2021 17:00:12 +0300 > > > > > Subject: [PATCH] fixup! net: pcs: xpcs: convert to phylink_pcs_ops > > > > > > > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > > > > --- > > > > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > > index d5685a74f3b7..704aa91b145a 100644 > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > > > @@ -1000,6 +1000,12 @@ static void stmmac_validate(struct phylink_config *config, > > > > > xpcs_validate(priv->hw->xpcs, supported, state); > > > > > } > > > > > > > > > > +static void stmmac_mac_config(struct phylink_config *config, unsigned int mode, > > > > > + const struct phylink_link_state *state) > > > > > +{ > > > > > + /* Nothing to do, xpcs_config() handles everything */ > > > > > +} > > > > > + > > > > > static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up) > > > > > { > > > > > struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg; > > > > > -----------------------------[ cut here]----------------------------- > > > > > > > > No luck.. I am still seeing the same kernel panic on my side with the > > > > additional patch applied: > > > > > > > > [ 12.513652] intel-eth-pci 0000:00:1e.4 enp0s30f4: FPE workqueue start > > > > [ 12.520079] intel-eth-pci 0000:00:1e.4 enp0s30f4: configuring for inband/sgmii link mode > > > > [ 12.528141] BUG: kernel NULL pointer dereference, address: 0000000000000000 > > > > [ 12.535072] #PF: supervisor instruction fetch in kernel mode > > > > [ 12.540719] #PF: error_code(0x0010) - not-present page > > > > [ 12.545843] PGD 0 P4D 0 > > > > [ 12.548382] Oops: 0010 [#1] PREEMPT SMP NOPTI > > > > [ 12.552733] CPU: 3 PID: 2657 Comm: connmand Tainted: G U 5.13.0-rc3-intel-lts #79 > > > > [ 12.561485] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLIFUI1.R00.3373.AF0.2009230546 09/23/2020 > > > > [ 12.574803] RIP: 0010:0x0 > > > > [ 12.577436] Code: Unable to access opcode bytes at RIP 0xffffffffffffffd6. > > > > [ 12.584280] RSP: 0018:ffffaa05404efb78 EFLAGS: 00010246 > > > > [ 12.589489] RAX: 0000000000000000 RBX: ffffa0fb91758c00 RCX: 0000000000000000 > > > > [ 12.596600] RDX: ffffaa05404efba0 RSI: 0000000000000002 RDI: ffffa0facc63c4d8 > > > > [ 12.603711] RBP: ffffaa05404efba0 R08: 0000000000000000 R09: ffffaa05404ef888 > > > > [ 12.610825] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000001 > > > > [ 12.617935] R13: ffffa0facc6388c0 R14: 0000000000000006 R15: 0000000000000001 > > > > [ 12.625040] FS: 00007f4a6d5b87c0(0000) GS:ffffa0fc57f80000(0000) knlGS:0000000000000000 > > > > [ 12.633096] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > > [ 12.638825] CR2: ffffffffffffffd6 CR3: 000000010e3f6004 CR4: 0000000000770ee0 > > > > [ 12.645932] PKRU: 55555554 > > > > [ 12.648641] Call Trace: > > > > [ 12.651095] phylink_major_config+0x5e/0x1a0 [phylink] > > > > [ 12.656219] phylink_start+0x204/0x2c0 [phylink] > > > > [ 12.660833] stmmac_open+0x3d0/0x9f0 [stmmac] > > > > [ 12.665186] __dev_open+0xe7/0x180 > > > > [ 12.668594] __dev_change_flags+0x174/0x1d0 > > > > [ 12.672773] ? __thaw_task+0x40/0x40 > > > > [ 12.676348] ? arch_stack_walk+0x9e/0xf0 > > > > [ 12.680266] dev_change_flags+0x21/0x60 > > > > [ 12.684100] devinet_ioctl+0x5e8/0x750 > > > > [ 12.687847] inet_ioctl+0x190/0x1c0 > > > > [ 12.691335] ? dev_ioctl+0x26d/0x4c0 > > > > [ 12.694907] sock_do_ioctl+0x44/0x140 > > > > [ 12.698569] ? alloc_empty_file+0x61/0xb0 > > > > [ 12.702572] sock_ioctl+0x22c/0x320 > > > > [ 12.706061] __x64_sys_ioctl+0x80/0xb0 > > > > [ 12.709808] do_syscall_64+0x42/0x80 > > > > [ 12.713381] entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > > > > > My git log:- > > > > dc25bc46886e (HEAD -> master) phy: maxlinear: add Maxlinear GPY115/21x/24x driver > > > > d059f32fcef1 fixup! net: pcs: xpcs: convert to phylink_pcs_ops > > > > 1a1a9c7267c1 net: pcs: xpcs: convert to phylink_pcs_ops > > > > ec25f3c80108 net: pcs: xpcs: convert to mdio_device > > > > 912a23a5768a net: pcs: xpcs: use mdiobus_c45_addr in xpcs_{read,write} > > > > af2bd0a4e32c net: pcs: xpcs: export xpcs_probe > > > > 7e4eea51132f net: pcs: xpcs: export xpcs_config_eee > > > > 37d5e086fc83 net: pcs: xpcs: export xpcs_validate > > > > d58487edb1a8 net: pcs: xpcs: make the checks related to the PHY interface mode stateless > > > > 2f0eba94af8d net: pcs: xpcs: there is only one PHY ID > > > > 67148dd06896 net: pcs: xpcs: delete shim definition for mdio_xpcs_get_ops() > > > > 5fe8e519e44f (origin/master, origin/HEAD) Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next > > > > > > Yikes, stupid me, I forgot to add this piece to the delta, the new > > > callback wasn't doing anything, but it also didn't warn me that the > > > static function is unused in my build: > > > > > > -----------------------------[ cut here]----------------------------- > > > >From 7a2b56091fd480ae1018ac964756368904a41a0c Mon Sep 17 00:00:00 2001 > > > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > > Date: Wed, 2 Jun 2021 17:54:58 +0300 > > > Subject: [PATCH] fixup! fixup! net: pcs: xpcs: convert to phylink_pcs_ops > > > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > > --- > > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > index 704aa91b145a..5b5edfdccab3 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > @@ -1137,6 +1137,7 @@ static void stmmac_mac_link_up(struct phylink_config *config, > > > > > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > > > .validate = stmmac_validate, > > > + .mac_config = stmmac_mac_config, > > > .mac_link_down = stmmac_mac_link_down, > > > .mac_link_up = stmmac_mac_link_up, > > > }; > > > -----------------------------[ cut here]----------------------------- > > > > It works on my setup now after applying the last fixup patch! :) > > Thanks again for testing. > Just for clarity, "it works" includes traffic at 10/100/1000, right? > If I'm not mistaken, you tested with sgmii and MLO_INBAND_AN, correct? > On the sja1105 I am testing with sgmii and no MLO_INBAND_AN, so not > exactly the same setup (that, plus my hardware needs a few more changes > still). Yes, I tested by performing ping on different link speeds (10/100/1000M), with SGMII and Auto-negotiation enabled. Link speed is changed manually using ethtool. VK
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 4bcd1d340766..8a83f9e1e95b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -503,8 +503,7 @@ struct mac_device_info { const struct stmmac_hwtimestamp *ptp; const struct stmmac_tc_ops *tc; const struct stmmac_mmc_ops *mmc; - const struct mdio_xpcs_ops *xpcs; - struct mdio_xpcs_args *xpcs_args; + struct mdio_xpcs_args *xpcs; struct mii_regs mii; /* MII register Addresses */ struct mac_link link; void __iomem *pcsr; /* vpointer to device CSRs */ diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h index 5014b260844b..91f7592a0189 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h @@ -612,14 +612,6 @@ struct stmmac_mmc_ops { #define stmmac_mmc_read(__priv, __args...) \ stmmac_do_void_callback(__priv, mmc, read, __args) -/* XPCS callbacks */ -#define stmmac_xpcs_config(__priv, __args...) \ - stmmac_do_callback(__priv, xpcs, config, __args) -#define stmmac_xpcs_get_state(__priv, __args...) \ - stmmac_do_callback(__priv, xpcs, get_state, __args) -#define stmmac_xpcs_link_up(__priv, __args...) \ - stmmac_do_callback(__priv, xpcs, link_up, __args) - struct stmmac_regs_off { u32 ptp_off; u32 mmc_off; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 050576ee704d..d0ce608b81c3 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -721,7 +721,7 @@ static int stmmac_ethtool_op_set_eee(struct net_device *dev, "Setting EEE tx-lpi is not supported\n"); if (priv->hw->xpcs) { - ret = xpcs_config_eee(priv->hw->xpcs_args, + ret = xpcs_config_eee(priv->hw->xpcs, priv->plat->mult_fact_100ns, edata->eee_enabled); if (ret) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 426c8f891f5a..d5685a74f3b7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -997,29 +997,7 @@ static void stmmac_validate(struct phylink_config *config, /* If PCS is supported, check which modes it supports. */ if (priv->hw->xpcs) - xpcs_validate(priv->hw->xpcs_args, supported, state); -} - -static void stmmac_mac_pcs_get_state(struct phylink_config *config, - struct phylink_link_state *state) -{ - struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); - - state->link = 0; - stmmac_xpcs_get_state(priv, priv->hw->xpcs_args, state); -} - -static void stmmac_mac_config(struct phylink_config *config, unsigned int mode, - const struct phylink_link_state *state) -{ - struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); - - stmmac_xpcs_config(priv, priv->hw->xpcs_args, state); -} - -static void stmmac_mac_an_restart(struct phylink_config *config) -{ - /* Not Supported */ + xpcs_validate(priv->hw->xpcs, supported, state); } static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up) @@ -1061,8 +1039,6 @@ static void stmmac_mac_link_up(struct phylink_config *config, struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); u32 ctrl; - stmmac_xpcs_link_up(priv, priv->hw->xpcs_args, speed, interface); - ctrl = readl(priv->ioaddr + MAC_CTRL_REG); ctrl &= ~priv->hw->link.speed_mask; @@ -1155,9 +1131,6 @@ static void stmmac_mac_link_up(struct phylink_config *config, static const struct phylink_mac_ops stmmac_phylink_mac_ops = { .validate = stmmac_validate, - .mac_pcs_get_state = stmmac_mac_pcs_get_state, - .mac_config = stmmac_mac_config, - .mac_an_restart = stmmac_mac_an_restart, .mac_link_down = stmmac_mac_link_down, .mac_link_up = stmmac_mac_link_up, }; @@ -1234,6 +1207,7 @@ static int stmmac_init_phy(struct net_device *dev) static int stmmac_phy_setup(struct stmmac_priv *priv) { + struct stmmac_mdio_bus_data *mdio_bus_data = priv->plat->mdio_bus_data; struct fwnode_handle *fwnode = of_fwnode_handle(priv->plat->phylink_node); int mode = priv->plat->phy_interface; struct phylink *phylink; @@ -1241,8 +1215,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv) priv->phylink_config.dev = &priv->dev->dev; priv->phylink_config.type = PHYLINK_NETDEV; priv->phylink_config.pcs_poll = true; - priv->phylink_config.ovr_an_inband = - priv->plat->mdio_bus_data->xpcs_an_inband; + priv->phylink_config.ovr_an_inband = mdio_bus_data->xpcs_an_inband; if (!fwnode) fwnode = dev_fwnode(priv->device); @@ -1252,6 +1225,12 @@ static int stmmac_phy_setup(struct stmmac_priv *priv) if (IS_ERR(phylink)) return PTR_ERR(phylink); + if (mdio_bus_data->has_xpcs) { + struct mdio_xpcs_args *xpcs = priv->hw->xpcs; + + phylink_set_pcs(phylink, &xpcs->pcs); + } + priv->phylink = phylink; return 0; } @@ -3652,7 +3631,7 @@ int stmmac_open(struct net_device *dev) if (priv->hw->pcs != STMMAC_PCS_TBI && priv->hw->pcs != STMMAC_PCS_RTBI && (!priv->hw->xpcs || - xpcs_get_an_mode(priv->hw->xpcs_args, mode) != DW_AN_C73)) { + xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) { ret = stmmac_init_phy(dev); if (ret) { netdev_err(priv->dev, diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c index 9b4bf78d2eaa..6312a152c8ad 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c @@ -444,14 +444,6 @@ int stmmac_mdio_register(struct net_device *ndev) max_addr = PHY_MAX_ADDR; } - if (mdio_bus_data->has_xpcs) { - priv->hw->xpcs = mdio_xpcs_get_ops(); - if (!priv->hw->xpcs) { - err = -ENODEV; - goto bus_register_fail; - } - } - if (mdio_bus_data->needs_reset) new_bus->reset = &stmmac_mdio_reset; @@ -526,11 +518,11 @@ int stmmac_mdio_register(struct net_device *ndev) continue; } - priv->hw->xpcs_args = xpcs; + priv->hw->xpcs = xpcs; break; } - if (!priv->hw->xpcs_args) { + if (!priv->hw->xpcs) { dev_warn(dev, "No XPCS found\n"); err = -ENODEV; goto no_xpcs_found; @@ -563,8 +555,8 @@ int stmmac_mdio_unregister(struct net_device *ndev) return 0; if (priv->hw->xpcs) { - mdio_device_free(priv->hw->xpcs_args->mdiodev); - xpcs_destroy(priv->hw->xpcs_args); + mdio_device_free(priv->hw->xpcs->mdiodev); + xpcs_destroy(priv->hw->xpcs); } mdiobus_unregister(priv->mii); diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c index 3e09850b8318..f1092771ab70 100644 --- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c @@ -100,6 +100,9 @@ /* VR MII EEE Control 1 defines */ #define DW_VR_MII_EEE_TRN_LPI BIT(0) /* Transparent Mode Enable */ +#define phylink_pcs_to_xpcs(pl_pcs) \ + container_of((pl_pcs), struct mdio_xpcs_args, pcs) + static const int xpcs_usxgmii_features[] = { ETHTOOL_LINK_MODE_Pause_BIT, ETHTOOL_LINK_MODE_Asym_Pause_BIT, @@ -448,7 +451,7 @@ static int xpcs_get_max_usxgmii_speed(const unsigned long *supported) return max; } -static int xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed) +static void xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed) { int ret, speed_sel; @@ -473,33 +476,40 @@ static int xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed) break; default: /* Nothing to do here */ - return -EINVAL; + return; } ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1); if (ret < 0) - return ret; + goto out; ret = xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_EN); if (ret < 0) - return ret; + goto out; ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1); if (ret < 0) - return ret; + goto out; ret &= ~DW_USXGMII_SS_MASK; ret |= speed_sel | DW_USXGMII_FULL; ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret); if (ret < 0) - return ret; + goto out; ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1); if (ret < 0) - return ret; + goto out; + + ret = xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_RST); + if (ret < 0) + goto out; + + return; - return xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_RST); +out: + pr_err("%s: XPCS access returned %pe\n", __func__, ERR_PTR(ret)); } static int _xpcs_config_aneg_c73(struct mdio_xpcs_args *xpcs, @@ -829,19 +839,19 @@ static int xpcs_config_aneg_c37_sgmii(struct mdio_xpcs_args *xpcs) return xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret); } -static int xpcs_config(struct mdio_xpcs_args *xpcs, - const struct phylink_link_state *state) +static int xpcs_do_config(struct mdio_xpcs_args *xpcs, + phy_interface_t interface, unsigned int mode) { const struct xpcs_compat *compat; int ret; - compat = xpcs_find_compat(xpcs->id, state->interface); + compat = xpcs_find_compat(xpcs->id, interface); if (!compat) return -ENODEV; switch (compat->an_mode) { case DW_AN_C73: - if (state->an_enabled) { + if (phylink_autoneg_inband(mode)) { ret = xpcs_config_aneg_c73(xpcs, compat); if (ret) return ret; @@ -859,6 +869,16 @@ static int xpcs_config(struct mdio_xpcs_args *xpcs, return 0; } +static int xpcs_config(struct phylink_pcs *pcs, unsigned int mode, + phy_interface_t interface, + const unsigned long *advertising, + bool permit_pause_to_mac) +{ + struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs); + + return xpcs_do_config(xpcs, interface, mode); +} + static int xpcs_get_state_c73(struct mdio_xpcs_args *xpcs, struct phylink_link_state *state, const struct xpcs_compat *compat) @@ -877,7 +897,7 @@ static int xpcs_get_state_c73(struct mdio_xpcs_args *xpcs, state->link = 0; - return xpcs_config(xpcs, state); + return xpcs_do_config(xpcs, state->interface, MLO_AN_INBAND); } if (state->an_enabled && xpcs_aneg_done_c73(xpcs, state, compat)) { @@ -934,41 +954,45 @@ static int xpcs_get_state_c37_sgmii(struct mdio_xpcs_args *xpcs, return 0; } -static int xpcs_get_state(struct mdio_xpcs_args *xpcs, - struct phylink_link_state *state) +static void xpcs_get_state(struct phylink_pcs *pcs, + struct phylink_link_state *state) { + struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs); const struct xpcs_compat *compat; int ret; compat = xpcs_find_compat(xpcs->id, state->interface); if (!compat) - return -ENODEV; + return; switch (compat->an_mode) { case DW_AN_C73: ret = xpcs_get_state_c73(xpcs, state, compat); - if (ret) - return ret; + if (ret) { + pr_err("xpcs_get_state_c73 returned %pe\n", + ERR_PTR(ret)); + return; + } break; case DW_AN_C37_SGMII: ret = xpcs_get_state_c37_sgmii(xpcs, state); - if (ret) - return ret; + if (ret) { + pr_err("xpcs_get_state_c37_sgmii returned %pe\n", + ERR_PTR(ret)); + } break; default: - return -1; + return; } - - return 0; } -static int xpcs_link_up(struct mdio_xpcs_args *xpcs, int speed, - phy_interface_t interface) +static void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode, + phy_interface_t interface, int speed, int duplex) { + struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs); + if (interface == PHY_INTERFACE_MODE_USXGMII) return xpcs_config_usxgmii(xpcs, speed); - - return 0; } static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs) @@ -1009,6 +1033,12 @@ static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs) return 0xffffffff; } +static const struct phylink_pcs_ops xpcs_phylink_ops = { + .pcs_config = xpcs_config, + .pcs_get_state = xpcs_get_state, + .pcs_link_up = xpcs_link_up, +}; + struct mdio_xpcs_args *xpcs_create(struct mdio_device *mdiodev, phy_interface_t interface) { @@ -1039,6 +1069,9 @@ struct mdio_xpcs_args *xpcs_create(struct mdio_device *mdiodev, goto out; } + xpcs->pcs.ops = &xpcs_phylink_ops; + xpcs->pcs.poll = true; + ret = xpcs_soft_reset(xpcs, compat); if (ret) goto out; @@ -1061,16 +1094,4 @@ void xpcs_destroy(struct mdio_xpcs_args *xpcs) } EXPORT_SYMBOL_GPL(xpcs_destroy); -static struct mdio_xpcs_ops xpcs_ops = { - .config = xpcs_config, - .get_state = xpcs_get_state, - .link_up = xpcs_link_up, -}; - -struct mdio_xpcs_ops *mdio_xpcs_get_ops(void) -{ - return &xpcs_ops; -} -EXPORT_SYMBOL_GPL(mdio_xpcs_get_ops); - MODULE_LICENSE("GPL v2"); diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h index 57a199393d63..0860a5b59f10 100644 --- a/include/linux/pcs/pcs-xpcs.h +++ b/include/linux/pcs/pcs-xpcs.h @@ -19,19 +19,10 @@ struct xpcs_id; struct mdio_xpcs_args { struct mdio_device *mdiodev; const struct xpcs_id *id; -}; - -struct mdio_xpcs_ops { - int (*config)(struct mdio_xpcs_args *xpcs, - const struct phylink_link_state *state); - int (*get_state)(struct mdio_xpcs_args *xpcs, - struct phylink_link_state *state); - int (*link_up)(struct mdio_xpcs_args *xpcs, int speed, - phy_interface_t interface); + struct phylink_pcs pcs; }; int xpcs_get_an_mode(struct mdio_xpcs_args *xpcs, phy_interface_t interface); -struct mdio_xpcs_ops *mdio_xpcs_get_ops(void); void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported, struct phylink_link_state *state); int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,