@@ -0,0 +1,123 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+# Copyright (C) Sunplus Co., Ltd. 2021+%YAML1.2+---+$id:http://devicetree.org/schemas/net/sunplus,sp7021-l2sw.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Sunplus SP7021 Dual Ethernet MAC Device Tree Bindings++maintainers:+-Wells Lu <wells.lu@sunplus.com>++description:|+Sunplus SP7021 dual 10M/100M Ethernet MAC controller with Layer 2 switch.+The controller can operate at either dual Ethernet MAC mode or one Ethernet+MAC with layer 2 switch (daisy-chain) mode.+The device node of Sunplus SP7021 Ethernet (L2SW) MAC controller has+following properties.++properties:+compatible:+const:sunplus,sp7021-l2sw++reg:+items:+-description:Base address and length of the L2SW registers.+-description:Base address and length of the MOON5 registers.++reg-names:+items:+-const:l2sw+-const:moon5++interrupts:+description:|+Contains number and type of interrupt. Number should be 66.+Type should be high-level trigger+maxItems:1++clocks:+description:|+Clock controller selector for Ethernet MAC controller.+maxItems:1++resets:+description:|+Reset controller selector for Ethernet MAC controller.+maxItems:1++phy-handle1:+description:A handle to node of phy 1 in mdio node+maxItems:1++phy-handle2:+description:A handle to node of phy 2 in mdio node+maxItems:1++pinctrl-names:+description:|+Names corresponding to the numbered pinctrl states.+A pinctrl state named "default" must be defined.+const:default++pinctrl-0:+description:A handle to the 'default' state of pin configuration++nvmem-cells:+items:+-description:nvmem cell address of MAC address of MAC 1+-description:nvmem cell address of MAC address of MAC 2++nvmem-cell-names:+description:names corresponding to the nvmem cells of MAC address+items:+-const:mac_addr0+-const:mac_addr1++mode:+description:|+Set operating modes of Sunplus Dual Ethernet MAC controller.+Please set one of the following modes:+0:daisy-chain mode+1:dual NIC mode+2:daisy-chain mode but disable SA learning+$ref:/schemas/types.yaml#/definitions/uint32+enum:[0,1,2]++additionalProperties:false++required:+-compatible+-reg+-reg-names+-interrupts+-clocks+-resets+-phy-handle1+-phy-handle2+-pinctrl-names+-pinctrl-0+-nvmem-cells+-nvmem-cell-names++examples:+-|+#include <dt-bindings/interrupt-controller/irq.h>+l2sw:l2sw@9c108000 {+compatible = "sunplus,sp7021-l2sw";+reg = <0x9c108000 0x400>, <0x9c000280 0x80>;+reg-names = "l2sw", "moon5";+interrupt-parent = <&intc>;+interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;+clocks = <&clkc 0xa7>;+resets = <&rstc 0x97>;+phy-handle1 = <ð_phy0>;+phy-handle2 = <ð_phy1>;+pinctrl-names = "default";+pinctrl-0 = <&l2sw_demo_board_v3_pins>;+nvmem-cells = <&mac_addr0>, <&mac_addr1>;+nvmem-cell-names = "mac_addr0", "mac_addr1";+mode = < 1 >;+};+...
@@ -0,0 +1,20 @@+# SPDX-License-Identifier: GPL-2.0+#+# Sunplus Ethernet device configuration+#++config NET_VENDOR_SUNPLUS+ tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"+ depends on ETHERNET && SOC_SP7021+ select PHYLIB+ select PINCTRL_SPPCTL+ select COMMON_CLK_SP7021+ select RESET_SUNPLUS+ select NVMEM_SUNPLUS_OCOTP+ help+ If you have Sunplus dual 10M/100M Ethernet (with L2 switch)+ devices, say Y.+ The network device supports dual 10M/100M Ethernet interfaces,+ or one 10/100M Ethernet interface with two LAN ports.+ To compile this driver as a module, choose M here. The module+ will be called sp_l2sw.
Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates
many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and
etc.) into a single chip. It is designed for industrial control
applications.
I ran checkpatch.pl with -f option for all *.c and *.h files, like:
$ checkpatch.pl -f *.c
$ checkpatch.pl -f *.h
I got 0 warnings and 0 errors for all files.
Also, I ran checkpath.pl for patch files, except cover-letter patch, as
shown below:
$ checkpatch.pl 0001*.patch
$ checkpatch.pl 0002*.patch
I got 0 warnings and 0 errors for the 2 patch files. But the second patch
file shows some 'CHECK' items, like
"Unnecessary parentheses around...",
"Macro argument reuse 'N' may be better as'(N)'..."
I am sure the code has no problem.
Do I need to clean 'CHECK' items?
Best regards,
Please use NET_VENDOR_SUNPLUS in the same way that other
NET_VENDOR_wyxz kconfig symbols are used. It should just enable
or disable any specific device drivers under it.
--
~Randy
The "with L2 Switch" is causing lots of warning bells to ring for me.
I don't see any references to switchdev or DSA in this driver. How is
the switch managed? There have been a few examples in the past of
similar two port switches being first supported in Dual MAC
mode. Later trying to actually use the switch in the Linux was always
ran into problems, and basically needed a new driver. So i want to
make sure you don't have this problem.
In the Linux world, Ethernet switches default to having there
ports/interfaces separated. This effectively gives you your dual MAC
mode by default. You then create a Linux bridge, and add the
ports/interfaces to the bridge. switchdev is used to offload the
bridge, telling the hardware to enable the L2 switch between the
ports.
So you don't need the mode parameter in DT. switchdev tells you
this. Switchdev gives user space access to the address table etc.
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o
...
+struct l2sw_common {
Please change your prefix. l2sw is a common prefix, there are other
silicon vendors using l2sw. I would suggest sp_l2sw or spl2sw.
No inline functions in C code. Let the compiler decide.
+int phy_cfg(struct l2sw_mac *mac)
+{
+ // Bug workaround:
+ // Flow-control of phy should be enabled. L2SW IP flow-control will refer
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac->comm->phy1_addr, 4, mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));
+ mdio_write(mac->comm->phy2_addr, 4, mdio_read(mac->comm->phy2_addr, 4) | (1 << 10));
This should be in the PHY driver. The MAC driver should never need to
touch PHY registers.
quoted hunk
+++ b/drivers/net/ethernet/sunplus/l2sw_mdio.c
@@ -0,0 +1,118 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"l2sw_mdio.h"++staticintmii_read(structmii_bus*bus,intphy_id,intregnum)+{+returnmdio_read(phy_id,regnum);+}++staticintmii_write(structmii_bus*bus,intphy_id,intregnum,u16val)+{+returnmdio_write(phy_id,regnum,val);+}++u32mdio_init(structplatform_device*pdev,structnet_device*net_dev)+{+structl2sw_mac*mac=netdev_priv(net_dev);+structmii_bus*mii_bus;+structdevice_node*mdio_node;+u32ret;++mii_bus=mdiobus_alloc();+if(!mii_bus){+pr_err(" Failed to allocate mdio_bus memory!\n");+return-ENOMEM;+}++mii_bus->name="sunplus_mii_bus";+mii_bus->parent=&pdev->dev;+mii_bus->priv=mac;+mii_bus->read=mii_read;+mii_bus->write=mii_write;+snprintf(mii_bus->id,MII_BUS_ID_SIZE,"%s-mii",dev_name(&pdev->dev));++mdio_node=of_get_parent(mac->comm->phy1_node);+ret=of_mdiobus_register(mii_bus,mdio_node);+if(ret){+pr_err(" Failed to register mii bus (ret = %d)!\n",ret);+mdiobus_free(mii_bus);+returnret;+}++mac->comm->mii_bus=mii_bus;+returnret;+}++voidmdio_remove(structnet_device*net_dev)+{+structl2sw_mac*mac=netdev_priv(net_dev);++if(mac->comm->mii_bus){+mdiobus_unregister(mac->comm->mii_bus);+mdiobus_free(mac->comm->mii_bus);+mac->comm->mii_bus=NULL;+}+}
You MDIO code is pretty scattered around. Please bring it all together
in one file.
Please use NET_VENDOR_SUNPLUS in the same way that other
NET_VENDOR_wyxz kconfig symbols are used. It should just enable or
disable any specific device drivers under it.
--
~Randy
I looked up Kconfig file of other vendors, but not sure what I should do.
Do I need to modify Kconfig file in the form as shown below?
# SPDX-License-Identifier: GPL-2.0
#
# Sunplus device configuration
#
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
---help---
If you have a network (Ethernet) card belonging to this
class, say Y here.
Note that the answer to this question doesn't directly
affect the kernel: saying N will just cause the configurator
to skip all the questions about Sunplus cards. If you say Y,
you will be asked for your specific card in the following
questions.
if NET_VENDOR_SUNPLUS
config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
depends on ETHERNET && SOC_SP7021
select PHYLIB
select PINCTRL_SPPCTL
select COMMON_CLK_SP7021
select RESET_SUNPLUS
select NVMEM_SUNPLUS_OCOTP
help
If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
devices, say Y.
The network device supports dual 10M/100M Ethernet interfaces,
or one 10/100M Ethernet interface with two LAN ports.
To compile this driver as a module, choose M here. The module
will be called sp_l2sw.
endif # NET_VENDOR_SUNPLUS
Best regards,
Wells
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-03 19:31:09
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
Ideally, you want it to also build with COMPILE_TEST, so that the
driver gets build by 0-day and all the other build bots.
---help---
If you have a network (Ethernet) card belonging to this
class, say Y here.
Note that the answer to this question doesn't directly
affect the kernel: saying N will just cause the configurator
to skip all the questions about Sunplus cards. If you say Y,
you will be asked for your specific card in the following
questions.
if NET_VENDOR_SUNPLUS
config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
depends on ETHERNET && SOC_SP7021
Does it actually depend on SOC_SP7021 to build?
Andrew
Please use NET_VENDOR_SUNPLUS in the same way that other
NET_VENDOR_wyxz kconfig symbols are used. It should just enable or
disable any specific device drivers under it.
--
~Randy
I looked up Kconfig file of other vendors, but not sure what I should do.
Do I need to modify Kconfig file in the form as shown below?
Hi,
Yes, this is the correct general idea, but also consider
Andrew's comments.
Thanks.
# SPDX-License-Identifier: GPL-2.0
#
# Sunplus device configuration
#
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
---help---
If you have a network (Ethernet) card belonging to this
class, say Y here.
Note that the answer to this question doesn't directly
affect the kernel: saying N will just cause the configurator
to skip all the questions about Sunplus cards. If you say Y,
you will be asked for your specific card in the following
questions.
if NET_VENDOR_SUNPLUS
config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
depends on ETHERNET && SOC_SP7021
select PHYLIB
select PINCTRL_SPPCTL
select COMMON_CLK_SP7021
select RESET_SUNPLUS
select NVMEM_SUNPLUS_OCOTP
help
If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
devices, say Y.
The network device supports dual 10M/100M Ethernet interfaces,
or one 10/100M Ethernet interface with two LAN ports.
To compile this driver as a module, choose M here. The module
will be called sp_l2sw.
endif # NET_VENDOR_SUNPLUS
Best regards,
Wells
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-04 05:42:59
Hi,
Thanks a lot for review.
quoted
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
Ideally, you want it to also build with COMPILE_TEST, so that the driver gets
build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
quoted
---help---
If you have a network (Ethernet) card belonging to this
class, say Y here.
Note that the answer to this question doesn't directly
affect the kernel: saying N will just cause the configurator
to skip all the questions about Sunplus cards. If you say Y,
you will be asked for your specific card in the following
questions.
if NET_VENDOR_SUNPLUS
config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
depends on ETHERNET && SOC_SP7021
Does it actually depend on SOC_SP7021 to build?
Andrew
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-04 12:59:51
On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
Hi,
Thanks a lot for review.
quoted
quoted
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
So it does not actually depend on ARCH_SUNPLUS. There are a few cases
where drivers have needed to call into arch specific code, which stops
them building for any other arch.
quoted
Ideally, you want it to also build with COMPILE_TEST, so that the driver gets
build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
Yes.
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
If it will compile with COMPILE_TEST on x86, mips, etc, you should
allow it to compile with COMPILE_TEST. You get better compile testing
that way.
Andrew
From: Randy Dunlap <hidden> Date: 2021-11-04 14:55:32
On 11/4/21 5:59 AM, Andrew Lunn wrote:
On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
quoted
Hi,
Thanks a lot for review.
quoted
quoted
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
So it does not actually depend on ARCH_SUNPLUS. There are a few cases
where drivers have needed to call into arch specific code, which stops
them building for any other arch.
quoted
quoted
Ideally, you want it to also build with COMPILE_TEST, so that the driver gets
build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
Yes.
Yes, but use "||" instead of one "|".
quoted
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
If it will compile with COMPILE_TEST on x86, mips, etc, you should
allow it to compile with COMPILE_TEST. You get better compile testing
that way.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-04 17:46:59
On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
quoted
Hi,
Thanks a lot for review.
quoted
quoted
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
So it does not actually depend on ARCH_SUNPLUS. There are a few cases where
drivers have needed to call into arch specific code, which stops them building
for any other arch.
quoted
quoted
Ideally, you want it to also build with COMPILE_TEST, so that the
driver gets build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
Yes.
quoted
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
If it will compile with COMPILE_TEST on x86, mips, etc, you should allow it to
compile with COMPILE_TEST. You get better compile testing that way.
Andrew
No, we only develop arm-based SoC, never for x86 or mips.
We never compile the driver for x86 or mips machine.
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-04 17:51:30
On 11/4/21 5:59 AM, Andrew Lunn wrote:
quoted
On Thu, Nov 04, 2021 at 05:31:57AM +0000, Wells Lu 呂芳騰 wrote:
quoted
Hi,
Thanks a lot for review.
quoted
quoted
config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
Does it actually depend on ARCH_SUNPLUS? What do you make use of?
ARCH_SUNPLUS will be defined for Sunplus family series SoC.
Ethernet devices of Sunplus are designed and used for Sunplus SoC.
So far, only two SoC of Sunplus have the network device.
I'd like to show up the selection only for Sunplus SoC.
So it does not actually depend on ARCH_SUNPLUS. There are a few cases
where drivers have needed to call into arch specific code, which stops
them building for any other arch.
quoted
quoted
Ideally, you want it to also build with COMPILE_TEST, so that the
driver gets build by 0-day and all the other build bots.
I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?
depends on ARCH_SUNPLUS | COMPILE_TEST
Yes.
Yes, but use "||" instead of one "|".
quoted
quoted
Yes, the device is now only for Sunplus SP7021 SoC.
Devices in each SoC may have a bit difference because of adding new
function or improving something.
If it will compile with COMPILE_TEST on x86, mips, etc, you should
allow it to compile with COMPILE_TEST. You get better compile testing
that way.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-04 18:21:47
No, we only develop arm-based SoC, never for x86 or mips.
We never compile the driver for x86 or mips machine.
You don't, but the Linux community does build for those
architectures. Most people do tree wide refactoring work using
x86. Tree wide cleanups using x86, etc. Any changes like that could
touch your driver. The harder is it to build, the less build testing
it will get, and tree wide changes which break it are less likely to
get noticed. So you really do want it to compile cleanly for all
architectures. If it does not, it normally actually means you are
doing something wrong, something you need to fix anyway. So please do
build it for x86 and make sure it builds cleanly.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-04 19:04:25
quoted
No, we only develop arm-based SoC, never for x86 or mips.
We never compile the driver for x86 or mips machine.
You don't, but the Linux community does build for those architectures. Most
people do tree wide refactoring work using x86. Tree wide cleanups using x86,
etc. Any changes like that could touch your driver. The harder is it to build, the
less build testing it will get, and tree wide changes which break it are less likely
to get noticed. So you really do want it to compile cleanly for all
architectures. If it does not, it normally actually means you are doing
something wrong, something you need to fix anyway. So please do build it for
x86 and make sure it builds cleanly.
Andrew
Ok, I understand.
I'll add COMPILE_TEST and compile driver for x86.
Thanks,
The "with L2 Switch" is causing lots of warning bells to ring for me.
I don't see any references to switchdev or DSA in this driver. How is the
switch managed? There have been a few examples in the past of similar two
port switches being first supported in Dual MAC mode. Later trying to
actually use the switch in the Linux was always ran into problems, and
basically needed a new driver. So i want to make sure you don't have this
problem.
In the Linux world, Ethernet switches default to having there
ports/interfaces separated. This effectively gives you your dual MAC mode by
default. You then create a Linux bridge, and add the ports/interfaces to the
bridge. switchdev is used to offload the bridge, telling the hardware to
enable the L2 switch between the ports.
So you don't need the mode parameter in DT. switchdev tells you this.
Switchdev gives user space access to the address table etc.
The L2 switch of Ethernet of SP7021 is not used to forward packets
between two network interfaces.
Sunplus Dual Ethernet devices consists of one CPU port, two LAN
ports, and a L2 switch. L2 switch is a circuitry which receives packets
from CPU or LAN ports and then forwards them other ports. Rules of
forwarding packets are set by driver.
Ethernet driver of SP7021 of Sunplus supports 3 operation modes:
- Dual NIC mode
- An NIC with two LAN ports mode (daisy-chain mode)
- An NIC with two LAN ports mode 2
Dual NIC mode
Ethernet driver creates two net-device interfaces (eg: eth0 and eth1).
Each has its dedicated LAN port. For example, LAN port 0 is for
net-device interface eth0. LAN port 1 is for net-device interface
eth1. Packets from LAN port 0 will be always forwarded to eth0 and
vice versa by L2 switch. Similarly, packets from LAN port 1 will be
always forwarded to eth1 and vice versa by L2 switch. Packets will
never be forwarded between two LAN ports, or between eth0 and
LAN port 1, or between eth1 and LAN port 0. The two network
devices work independently.
An NIC with two LAN ports mode (daisy-chain mode)
Ethernet driver creates one net-device interface (eg: eth0), but the
net-device interface has two LAN ports. In this mode, a packet from
one LAN port will be either forwarded to net-device interface (eht0)
if its destination address matches MAC address of net-device
interface (eth0), or forwarded to other LAN port. A packet from
net-device interface (eth0) will be forwarded to a LAN port if its
destination address is learnt by L2 switch, or forwarded to both
LAN ports if its destination has not been learnt yet.
An NIC with two LAN ports mode 2
This mode is similar to “An NIC with two LAN ports mode”. The
difference is that a packet from net-device interface (eth0) will be
always forwarded to both LAN ports. Learning function of L2 switch
is turned off in this mode. This means L2 switch will never learn the
source address of a packet. So, it always forward packets to both
LAN ports. This mode works like you have 2-port Ethernet hub.
quoted
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o
...
quoted
+struct l2sw_common {
Please change your prefix. l2sw is a common prefix, there are other silicon
vendors using l2sw. I would suggest sp_l2sw or spl2sw.
Ok, I'll modify two struct names in next patch as shown below:
l2sw_common --> sp_common
l2sw_mac --> sp_mac
Should I also modify prefix of file name?
Ok, I'll modify mdio_read() to return -EOPNOTSUPP
when error (time-out) occurs.
quoted
+
+inline void tx_trigger(void)
No inline functions in C code. Let the compiler decide.
Ok, I'll remove 'inline' for all functions in next patch.
quoted
+int phy_cfg(struct l2sw_mac *mac)
+{
+ // Bug workaround:
+ // Flow-control of phy should be enabled. L2SW IP flow-control will refer
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac->comm->phy1_addr, 4,
mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));
quoted
+ mdio_write(mac->comm->phy2_addr, 4,
mdio_read(mac->comm->phy2_addr,
quoted
+4) | (1 << 10));
This should be in the PHY driver. The MAC driver should never need to touch
PHY registers.
Sunplus Ethernet MAC integrates MDIO controller.
So Ethernet driver has MDIO- and PHY-related code.
To work-around a circuitry bug, we need to enable
bit 10 of register 4 of PHY.
Where should we place the code?
quoted
+++ b/drivers/net/ethernet/sunplus/l2sw_mdio.c
@@ -0,0 +1,118 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"l2sw_mdio.h"++staticintmii_read(structmii_bus*bus,intphy_id,intregnum){+returnmdio_read(phy_id,regnum);+}++staticintmii_write(structmii_bus*bus,intphy_id,intregnum,u16+val){+returnmdio_write(phy_id,regnum,val);}++u32mdio_init(structplatform_device*pdev,structnet_device+*net_dev){+structl2sw_mac*mac=netdev_priv(net_dev);+structmii_bus*mii_bus;+structdevice_node*mdio_node;+u32ret;++mii_bus=mdiobus_alloc();+if(!mii_bus){+pr_err(" Failed to allocate mdio_bus memory!\n");+return-ENOMEM;+}++mii_bus->name="sunplus_mii_bus";+mii_bus->parent=&pdev->dev;+mii_bus->priv=mac;+mii_bus->read=mii_read;+mii_bus->write=mii_write;+snprintf(mii_bus->id,MII_BUS_ID_SIZE,"%s-mii",+dev_name(&pdev->dev));++mdio_node=of_get_parent(mac->comm->phy1_node);+ret=of_mdiobus_register(mii_bus,mdio_node);+if(ret){+pr_err(" Failed to register mii bus (ret = %d)!\n",ret);+mdiobus_free(mii_bus);+returnret;+}++mac->comm->mii_bus=mii_bus;+returnret;+}++voidmdio_remove(structnet_device*net_dev){+structl2sw_mac*mac=netdev_priv(net_dev);++if(mac->comm->mii_bus){+mdiobus_unregister(mac->comm->mii_bus);+mdiobus_free(mac->comm->mii_bus);+mac->comm->mii_bus=NULL;+}+}
You MDIO code is pretty scattered around. Please bring it all together in one
file.
Ok, I'll move mdio_read() and mdio_write() to 'l2sw_mdio.c',
but keep mdio_access() in 'l2sw_hal.c' because it is a function
which accesses hardware registers actually.
The "with L2 Switch" is causing lots of warning bells to ring for me.
I don't see any references to switchdev or DSA in this driver. How is the
switch managed? There have been a few examples in the past of similar two
port switches being first supported in Dual MAC mode. Later trying to
actually use the switch in the Linux was always ran into problems, and
basically needed a new driver. So i want to make sure you don't have this
problem.
In the Linux world, Ethernet switches default to having there
ports/interfaces separated. This effectively gives you your dual MAC mode by
default. You then create a Linux bridge, and add the ports/interfaces to the
bridge. switchdev is used to offload the bridge, telling the hardware to
enable the L2 switch between the ports.
So you don't need the mode parameter in DT. switchdev tells you this.
Switchdev gives user space access to the address table etc.
The L2 switch of Ethernet of SP7021 is not used to forward packets
between two network interfaces.
Sunplus Dual Ethernet devices consists of one CPU port, two LAN
ports, and a L2 switch. L2 switch is a circuitry which receives packets
from CPU or LAN ports and then forwards them other ports. Rules of
forwarding packets are set by driver.
Ethernet driver of SP7021 of Sunplus supports 3 operation modes:
- Dual NIC mode
- An NIC with two LAN ports mode (daisy-chain mode)
- An NIC with two LAN ports mode 2
Dual NIC mode
Ethernet driver creates two net-device interfaces (eg: eth0 and eth1).
Each has its dedicated LAN port. For example, LAN port 0 is for
net-device interface eth0. LAN port 1 is for net-device interface
eth1. Packets from LAN port 0 will be always forwarded to eth0 and
vice versa by L2 switch. Similarly, packets from LAN port 1 will be
always forwarded to eth1 and vice versa by L2 switch. Packets will
never be forwarded between two LAN ports, or between eth0 and
LAN port 1, or between eth1 and LAN port 0. The two network
devices work independently.
An NIC with two LAN ports mode (daisy-chain mode)
Ethernet driver creates one net-device interface (eg: eth0), but the
net-device interface has two LAN ports. In this mode, a packet from
one LAN port will be either forwarded to net-device interface (eht0)
if its destination address matches MAC address of net-device
interface (eth0), or forwarded to other LAN port. A packet from
net-device interface (eth0) will be forwarded to a LAN port if its
destination address is learnt by L2 switch, or forwarded to both
LAN ports if its destination has not been learnt yet.
An NIC with two LAN ports mode 2
This mode is similar to “An NIC with two LAN ports mode”. The
difference is that a packet from net-device interface (eth0) will be
always forwarded to both LAN ports. Learning function of L2 switch
is turned off in this mode. This means L2 switch will never learn the
source address of a packet. So, it always forward packets to both
LAN ports. This mode works like you have 2-port Ethernet hub.
So here you describe how the hardware can be used. Dual is two
interfaces. Daisy-chain is what you get by taking those two interfaces
and adding them to a bridge. The bridge then forwards frames between
the interfaces and the CPU as needed, based on learning. And your
third mode is the bridge always performs flooding.
A linux driver must follow the linux networking model. You cannot make
up your own model. In the linux world, you model the external
ports. The hardware always has two external ports, so you need to
always have two netdev interfaces. To bridge packets between those two
interfaces, you create a bridge and you add the interfaces to the
bridge. That is the model you need to follow. switchdev gives you the
API calls you need to implement this.
quoted
quoted
+struct l2sw_common {
Please change your prefix. l2sw is a common prefix, there are other silicon
vendors using l2sw. I would suggest sp_l2sw or spl2sw.
Ok, I'll modify two struct names in next patch as shown below:
l2sw_common --> sp_common
l2sw_mac --> sp_mac
Should I also modify prefix of file name?
You need to modify the prefix everywhere you use it. Function names,
variable names, all symbols. Search and replace throughout the whole
code.
You cannot rely on the name, systemd has probably renamed it. If you have
using phylib correctly, net_dev->phydev is what you want.
Ok, I'll use name of the second net device to do compare,
instead of using fixed string "eth1", in next patch.
No. There are always two interfaces. You always have two netdev
structures. Each netdev structure has a phydev. So use netdev->phydev.
This is another advantage of the Linux model. In your daisy chain
mode, how do i control the two PHYs? How do i see one is up and one is
down? How do i configure one to 10Half and the other 100Full?
quoted
quoted
+int phy_cfg(struct l2sw_mac *mac)
+{
+ // Bug workaround:
+ // Flow-control of phy should be enabled. L2SW IP flow-control will refer
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac->comm->phy1_addr, 4,
mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));
quoted
+ mdio_write(mac->comm->phy2_addr, 4,
mdio_read(mac->comm->phy2_addr,
quoted
+4) | (1 << 10));
This should be in the PHY driver. The MAC driver should never need to touch
PHY registers.
Sunplus Ethernet MAC integrates MDIO controller.
So Ethernet driver has MDIO- and PHY-related code.
To work-around a circuitry bug, we need to enable
bit 10 of register 4 of PHY.
Where should we place the code?
The silicon is integrated, but it is still a collection of standard
blocks. Linux models those blocks independently. There is a subsystem
for the MAC, a subsystem for the MDIO bus master and a subsystem for
the PHY. You register a driver with each of these subsystems. PHY
drivers live in drivers/net/phy. Put a PHY driver in there, which
includes this workaround.
Nothing to do? Seems very odd. Don't you need to tell the MAC it should do
10Mbps or 100Mbps? What about pause?
No, hardware does it automatically.
Sunplus MAC integrates MDIO controller.
It reads PHY status and set MAC automatically.
The PHY is external? So you have no idea what PHY that is? It could be
a Marvell PHY, a microchip PHY, an Atheros PHY. Often PHYs have
pages. In order to read the temperature sensor you change the page,
read a register, and then hopefully change the page back again. If the
PHY supports Fibre as well as copper, it can put the fibre registers
in a second page. The PHY driver knows about this, it will flip the
pages as needed. The phylib core has a mutex, so that only one
operation happens at a time. So a page flip does not happen
unexpectedly.
Your MAC hardware does not take this mutex. It has no idea what page
is selected when it reads registers. Instead of getting the basic mode
register, it could get the LED control register...
The MAC should never directly access the PHY. Please disable this
hardware, and use the mii_linkchange callback to configure the MAC.
quoted
So the MAC does not support pause? I'm then confused about phy_cfg().
Yes, MAC supports pause. MAC (hardware) takes care of pause
automatically.
Should I remove the two lines?
Yes.
And you need to configure the MAC based on the results of the
auto-neg.
Andrew
The "with L2 Switch" is causing lots of warning bells to ring for me.
I don't see any references to switchdev or DSA in this driver. How
is the switch managed? There have been a few examples in the past of
similar two port switches being first supported in Dual MAC mode.
Later trying to actually use the switch in the Linux was always ran
into problems, and basically needed a new driver. So i want to make
sure you don't have this problem.
In the Linux world, Ethernet switches default to having there
ports/interfaces separated. This effectively gives you your dual MAC
mode by default. You then create a Linux bridge, and add the
ports/interfaces to the bridge. switchdev is used to offload the
bridge, telling the hardware to enable the L2 switch between the ports.
So you don't need the mode parameter in DT. switchdev tells you this.
Switchdev gives user space access to the address table etc.
The L2 switch of Ethernet of SP7021 is not used to forward packets
between two network interfaces.
Sunplus Dual Ethernet devices consists of one CPU port, two LAN ports,
and a L2 switch. L2 switch is a circuitry which receives packets from
CPU or LAN ports and then forwards them other ports. Rules of
forwarding packets are set by driver.
Ethernet driver of SP7021 of Sunplus supports 3 operation modes:
- Dual NIC mode
- An NIC with two LAN ports mode (daisy-chain mode)
- An NIC with two LAN ports mode 2
Dual NIC mode
Ethernet driver creates two net-device interfaces (eg: eth0 and eth1).
Each has its dedicated LAN port. For example, LAN port 0 is for
net-device interface eth0. LAN port 1 is for net-device interface
eth1. Packets from LAN port 0 will be always forwarded to eth0 and
vice versa by L2 switch. Similarly, packets from LAN port 1 will be
always forwarded to eth1 and vice versa by L2 switch. Packets will
never be forwarded between two LAN ports, or between eth0 and LAN port
1, or between eth1 and LAN port 0. The two network devices work
independently.
An NIC with two LAN ports mode (daisy-chain mode) Ethernet driver
creates one net-device interface (eg: eth0), but the net-device
interface has two LAN ports. In this mode, a packet from one LAN port
will be either forwarded to net-device interface (eht0) if its
destination address matches MAC address of net-device interface
(eth0), or forwarded to other LAN port. A packet from net-device
interface (eth0) will be forwarded to a LAN port if its destination
address is learnt by L2 switch, or forwarded to both LAN ports if its
destination has not been learnt yet.
An NIC with two LAN ports mode 2
This mode is similar to “An NIC with two LAN ports mode”. The
difference is that a packet from net-device interface (eth0) will be
always forwarded to both LAN ports. Learning function of L2 switch is
turned off in this mode. This means L2 switch will never learn the
source address of a packet. So, it always forward packets to both LAN
ports. This mode works like you have 2-port Ethernet hub.
So here you describe how the hardware can be used. Dual is two interfaces.
Daisy-chain is what you get by taking those two interfaces and adding them to
a bridge. The bridge then forwards frames between the interfaces and the CPU
as needed, based on learning. And your third mode is the bridge always
performs flooding.
A linux driver must follow the linux networking model. You cannot make up
your own model. In the linux world, you model the external ports. The
hardware always has two external ports, so you need to always have two
netdev interfaces. To bridge packets between those two interfaces, you create
a bridge and you add the interfaces to the bridge. That is the model you need
to follow. switchdev gives you the API calls you need to implement this.
Thank you very much for your explanation.
I realize that we need to follow the Linux networking model.
I'll remove all descriptions about L2 switch or daisy-chain mode.
I'd like to modify Sunplus Ethernet driver to fulfill Linux networking model.
Here is my proposal:
SP7021 Ethernet supports 3 operation modes:
- Dual Ethernet mode
In this mode, driver creates two net-device interfaces. Each connects
to PHY. There are two LAN ports totally.
I am sorry that EMAC of SP7021 cannot support L2 switch functions
of Linux switch-device model because it only has partial function of
switch.
- One Ethernet mode
In this mode, driver creates one net-device interface. It connects to
to a PHY (There is only one LAN port).
The LAN port is then connected to a 3-port Ethernet hub.
The 3-port Ethernet hub is a hardware circuitry. All operations
(packet forwarding) are done by hardware. No software
intervention is needed. Actually, even just power-on, no software
running, two LAN ports of SP7021 work well as 2-port hub.
- One Ethernet mode 2
This is mode is similar to previous mode, but a bit different settings
to the hub.
Please kindly comment if my proposal is feasible or not
quoted
quoted
quoted
+struct l2sw_common {
Please change your prefix. l2sw is a common prefix, there are other
silicon vendors using l2sw. I would suggest sp_l2sw or spl2sw.
Ok, I'll modify two struct names in next patch as shown below:
l2sw_common --> sp_common
l2sw_mac --> sp_mac
Should I also modify prefix of file name?
You need to modify the prefix everywhere you use it. Function names,
variable names, all symbols. Search and replace throughout the whole code.
You cannot rely on the name, systemd has probably renamed it. If you
have using phylib correctly, net_dev->phydev is what you want.
Ok, I'll use name of the second net device to do compare, instead of
using fixed string "eth1", in next patch.
No. There are always two interfaces. You always have two netdev structures.
Each netdev structure has a phydev. So use netdev->phydev.
Yes, I'll modify driver to use 'netdev->phydev'.
This is another advantage of the Linux model. In your daisy chain mode, how
do i control the two PHYs? How do i see one is up and one is down? How do i
configure one to 10Half and the other 100Full?
No software intervention is needed.
Hardware circuitry of EMAC of Sunplus SP7021 does it well.
EMAC will communicate with PHY chips (via MDIO bus) automatically.
Actually, just giving power to SP7021, the two LAN ports act as 2-port
Ethernet hub, forwarding packets between ports.
quoted
quoted
quoted
+int phy_cfg(struct l2sw_mac *mac) {
+ // Bug workaround:
+ // Flow-control of phy should be enabled. L2SW IP flow-control will
refer
quoted
quoted
quoted
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac->comm->phy1_addr, 4,
mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));
quoted
+ mdio_write(mac->comm->phy2_addr, 4,
mdio_read(mac->comm->phy2_addr,
quoted
+4) | (1 << 10));
This should be in the PHY driver. The MAC driver should never need
to touch PHY registers.
Sunplus Ethernet MAC integrates MDIO controller.
So Ethernet driver has MDIO- and PHY-related code.
To work-around a circuitry bug, we need to enable bit 10 of register 4
of PHY.
Where should we place the code?
The silicon is integrated, but it is still a collection of standard blocks. Linux
models those blocks independently. There is a subsystem for the MAC, a
subsystem for the MDIO bus master and a subsystem for the PHY. You register
a driver with each of these subsystems. PHY drivers live in drivers/net/phy. Put
a PHY driver in there, which includes this workaround.
Nothing to do? Seems very odd. Don't you need to tell the MAC it
should do 10Mbps or 100Mbps? What about pause?
No, hardware does it automatically.
Sunplus MAC integrates MDIO controller.
It reads PHY status and set MAC automatically.
The PHY is external? So you have no idea what PHY that is? It could be a
Marvell PHY, a microchip PHY, an Atheros PHY. Often PHYs have pages. In order
to read the temperature sensor you change the page, read a register, and then
hopefully change the page back again. If the PHY supports Fibre as well as
copper, it can put the fibre registers in a second page. The PHY driver knows
about this, it will flip the pages as needed. The phylib core has a mutex, so that
only one operation happens at a time. So a page flip does not happen
unexpectedly.
Your MAC hardware does not take this mutex. It has no idea what page is
selected when it reads registers. Instead of getting the basic mode register, it
could get the LED control register...
The MAC should never directly access the PHY. Please disable this hardware,
and use the mii_linkchange callback to configure the MAC.
Yes, the PHYs are external. SP7021 are connected to two ICPlus IP101 PHY.
EMAC of SP7021 communicates with PHY via MDIO bus automatically after
It is setup and enabled. Sorry that the function cannot be disabled.
The mentioned bug is not a bug of PHY, but a hardware bug of EMAC.
SP7021 EMAC will read bit 10 (pause bit) of register 4 of PHY and then set
pause mode of EMAC itself. At initial, bit 10 of register 4 of PHY is 0. This
results in pause mode of EMAC be turned off. Due to timing issue, setting
bit 10 on PHY driver is not feasible, we need to set it on EMAC driver right
just after MAC is enabled.
quoted
quoted
So the MAC does not support pause? I'm then confused about phy_cfg().
quoted
Yes, MAC supports pause. MAC (hardware) takes care of pause
automatically.
Should I remove the two lines?
Yes.
Yes, I'll remove them in next patch.
And you need to configure the MAC based on the results of the auto-neg.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-08 13:16:05
SP7021 Ethernet supports 3 operation modes:
- Dual Ethernet mode
In this mode, driver creates two net-device interfaces. Each connects
to PHY. There are two LAN ports totally.
I am sorry that EMAC of SP7021 cannot support L2 switch functions
of Linux switch-device model because it only has partial function of
switch.
This is fine.
- One Ethernet mode
In this mode, driver creates one net-device interface. It connects to
to a PHY (There is only one LAN port).
The LAN port is then connected to a 3-port Ethernet hub.
The 3-port Ethernet hub is a hardware circuitry. All operations
(packet forwarding) are done by hardware. No software
intervention is needed. Actually, even just power-on, no software
running, two LAN ports of SP7021 work well as 2-port hub.
We need to dig into the details of this mode. I would initially say
no, until we really do know it is impossible to do it correctly. Even
if it is impossible to do it correctly, i'm still temped to reject
this mode.
How does spanning tree work? Who sends and receives the BPDU?
Is there PTP support? How do you send and receive the PTP frames?
Is IGMP snooping supported?
All of these have one thing in common, you need to be able to egress
frames out a specific port of the switch, and you need to know what
port a received frames ingressed on. If you can do that, you can
probably do proper support in Linux.
Is the datasheet available?
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-08 14:26:23
quoted
SP7021 Ethernet supports 3 operation modes:
- Dual Ethernet mode
In this mode, driver creates two net-device interfaces. Each connects
to PHY. There are two LAN ports totally.
I am sorry that EMAC of SP7021 cannot support L2 switch functions
of Linux switch-device model because it only has partial function of
switch.
This is fine.
Thanks a lot!
quoted
- One Ethernet mode
In this mode, driver creates one net-device interface. It connects to
to a PHY (There is only one LAN port).
The LAN port is then connected to a 3-port Ethernet hub.
The 3-port Ethernet hub is a hardware circuitry. All operations
(packet forwarding) are done by hardware. No software
intervention is needed. Actually, even just power-on, no software
running, two LAN ports of SP7021 work well as 2-port hub.
We need to dig into the details of this mode. I would initially say no, until we
really do know it is impossible to do it correctly. Even if it is impossible to do
it correctly, i'm still temped to reject this mode.
How does spanning tree work? Who sends and receives the BPDU?
Is there PTP support? How do you send and receive the PTP frames?
Is IGMP snooping supported?
All of these have one thing in common, you need to be able to egress frames
out a specific port of the switch, and you need to know what port a received
frames ingressed on. If you can do that, you can probably do proper support in
Linux.
The "L2 switch" is a very simple witch. It has 3 ports: CPU, LAN port 0 and LAN
port 1. A packet is always forwarded to other two ports if source-address (of MAC)
learning function is off, or forwarded to one of the two ports if source-address
learning function is on and source address is learnt (recorded by switch).
The switch will not recognize type of packets, regardless BPDU, PTP or any other
packets. If turning off source-address learning function, it works like an Ethernet
plus a 2-port hub.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-08 14:52:46
The switch will not recognize type of packets, regardless BPDU, PTP or any other
packets. If turning off source-address learning function, it works like an Ethernet
plus a 2-port hub.
So without STP, there is no way to stop an loop, and a broadcast storm
taking down your network?
Looking at the TX descriptor, there are two bits:
[18]: force forward to port 0
[19]: force forward to port 1
When the switch is enabled, can these two bits be used?
In the RX descriptor there is:
pkt_sp:
000: from port0
001: from port1
110: soc0 loopback
101: soc1 loopback
Are these bits used when the switch is enabled?
0.31 port control 1 (port cntl1) blocking state seems to have what you
need for STP.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-08 16:47:51
quoted
The switch will not recognize type of packets, regardless BPDU, PTP or
any other packets. If turning off source-address learning function, it
works like an Ethernet plus a 2-port hub.
So without STP, there is no way to stop an loop, and a broadcast storm taking
down your network?
Do you mean connecting two PHY ports to the same LAN? We never
connect two PHY ports to the same LAN (or hub). I never think of this
loop problem. I thought only WAN has the loop problem.
The switch has some kinds of flow control, refer to 0.2 "Flow control threshold"
and 0.3 "CPU port flow control threshold". It will drop extra packets.
How an Ethernet hub take care of this situation?
Is that reasonable to connect two ports of an Ethernet hub together?
Looking at the TX descriptor, there are two bits:
[18]: force forward to port 0
[19]: force forward to port 1
When the switch is enabled, can these two bits be used?
Yes, for example, when bit 19 of TX descriptor is enabled, a packet from CPU
port is forwarded to LAN port 0 forcibly.
In the RX descriptor there is:
pkt_sp:
000: from port0
001: from port1
110: soc0 loopback
101: soc1 loopback
Are these bits used when the switch is enabled?
Yes, E- MAC driver uses these bits to tell where a packet comes from.
Note that soc1 port (CPU port) has been removed in this chip.
0.31 port control 1 (port cntl1) blocking state seems to have what you need for
STP.
From document, if bit 17 or bit 16 of port_cntl1 register is set, only RMC
packets will be forwarded to other LAN port. I am not sure whether
enabling the bits helps the issue. Should I enable the bits?
Sorry, I don't know what is a RMC packet?
Could you please teach me?
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-08 17:33:05
On Mon, Nov 08, 2021 at 04:47:34PM +0000, Wells Lu 呂芳騰 wrote:
quoted
quoted
The switch will not recognize type of packets, regardless BPDU, PTP or
any other packets. If turning off source-address learning function, it
works like an Ethernet plus a 2-port hub.
So without STP, there is no way to stop an loop, and a broadcast storm taking
down your network?
Do you mean connecting two PHY ports to the same LAN? We never
connect two PHY ports to the same LAN (or hub). I never think of this
loop problem. I thought only WAN has the loop problem.
Any Ethernet network can have a loop. Often loops a deliberate because
they give redundancy. STP will detect this loop, and somewhere in the
network one of the switches will block traffic to break the loop. But
if something in the network breaks, the port can be unblocked to allow
traffic to flow, redundancy. Well behaved switches should always
implement STP.
How an Ethernet hub take care of this situation?
STP. Run tcpdump on your network. Depending on how your network is
configured, you might see BPDU from your building switches.
Is that reasonable to connect two ports of an Ethernet hub together?
It is not just together. You cannot guarantee any Ethernet network is
a tree. You could connect the two ports to two different hubs, but
those hubs are connected together, and so you get a loop.
quoted
Looking at the TX descriptor, there are two bits:
[18]: force forward to port 0
[19]: force forward to port 1
When the switch is enabled, can these two bits be used?
Yes, for example, when bit 19 of TX descriptor is enabled, a packet from CPU
port is forwarded to LAN port 0 forcibly.
quoted
In the RX descriptor there is:
pkt_sp:
000: from port0
001: from port1
110: soc0 loopback
101: soc1 loopback
Are these bits used when the switch is enabled?
Yes, E- MAC driver uses these bits to tell where a packet comes from.
Note that soc1 port (CPU port) has been removed in this chip.
Right. So you can have two netdev when in L2 switch mode.
You need to think about the Linux model some more. In linux,
networking hardware is there to accelerate what the Linux stack can do
in software. Take for example a simple SoC will have two Ethernet
interfaces. You can perform software bridging on those two interfaces:
ip link add name br0 type bridge
ip link set dev br0 up
ip link set dev eth0 master br0
ip link set dev eth1 master br0
The software bridge will decided which interface to send a packet
out. The software will perform learning etc.
You can use your dual MAC setup exactly like this. But you can also go
further. You can use the hardware to accelerate switching packets
between eth0 and eth1. But also Linux can still send packets out
specific ports using these bits. The software bridge and the hardware
bridge work together. This is the correct way to do this in Linux.
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-09 14:39:52
On Mon, Nov 08, 2021 at 04:47:34PM +0000, Wells Lu 呂芳騰 wrote:
quoted
quoted
quoted
The switch will not recognize type of packets, regardless BPDU,
PTP or any other packets. If turning off source-address learning
function, it works like an Ethernet plus a 2-port hub.
So without STP, there is no way to stop an loop, and a broadcast
storm taking down your network?
Do you mean connecting two PHY ports to the same LAN? We never connect
two PHY ports to the same LAN (or hub). I never think of this loop
problem. I thought only WAN has the loop problem.
Any Ethernet network can have a loop. Often loops a deliberate because they
give redundancy. STP will detect this loop, and somewhere in the network one
of the switches will block traffic to break the loop. But if something in the
network breaks, the port can be unblocked to allow traffic to flow, redundancy.
Well behaved switches should always implement STP.
I don't know how to implement STP in L2 switch like SP7021.
How about one NIC + 2-port simple frame-flooding hub?
Someone told me that some low-cost Ethernet hub just does frame-flooding
to other ports. Let users take care of use.
If this is acceptable, I'd like to have Ethernet of SP7021 have two operation
modes:
- Dual NIC mode
- Single NIC with 2-port frame-flooding hub mode
If this is not acceptable, can I, instead, implement the two operation modes:
- Dual NIC mode
- Single NIC mode
RMII pins of PHY ports of SP7021 are multiplexable. I'd like to switch RMII
pins of the second PHY for other use if single NIC mode is used.
In fact, some SP7021 boards have dual Ethernet and some have only one
Ethernet. We really need the two operation modes.
quoted
How an Ethernet hub take care of this situation?
STP. Run tcpdump on your network. Depending on how your network is
configured, you might see BPDU from your building switches.
Thanks a lot. I understand.
quoted
Is that reasonable to connect two ports of an Ethernet hub together?
It is not just together. You cannot guarantee any Ethernet network is a tree. You
could connect the two ports to two different hubs, but those hubs are
connected together, and so you get a loop.
Thanks for explanation. I got it.
quoted
quoted
Looking at the TX descriptor, there are two bits:
[18]: force forward to port 0
[19]: force forward to port 1
When the switch is enabled, can these two bits be used?
Yes, for example, when bit 19 of TX descriptor is enabled, a packet
from CPU port is forwarded to LAN port 0 forcibly.
quoted
In the RX descriptor there is:
pkt_sp:
000: from port0
001: from port1
110: soc0 loopback
101: soc1 loopback
Are these bits used when the switch is enabled?
Yes, E- MAC driver uses these bits to tell where a packet comes from.
Note that soc1 port (CPU port) has been removed in this chip.
Right. So you can have two netdev when in L2 switch mode.
You need to think about the Linux model some more. In linux, networking
hardware is there to accelerate what the Linux stack can do in software. Take
for example a simple SoC will have two Ethernet interfaces. You can perform
software bridging on those two interfaces:
ip link add name br0 type bridge
ip link set dev br0 up
ip link set dev eth0 master br0
ip link set dev eth1 master br0
The software bridge will decided which interface to send a packet out. The
software will perform learning etc.
You can use your dual MAC setup exactly like this. But you can also go further.
You can use the hardware to accelerate switching packets between eth0 and
eth1. But also Linux can still send packets out specific ports using these bits.
The software bridge and the hardware bridge work together. This is the correct
way to do this in Linux.
Sorry, I am not capable to do that.
I need to study more about Linux switch device, L2 switch of SP7021, procotols,...
So what I can use now is pure software bridge for dual Ethernet case.
quoted
Sorry, I don't know what is a RMC packet?
Sorry, i have no idea.
After looking up some data, I find RMC means reserved multi-cast.
RMC packets means packets with DA = 0x0180c2000000, 0x0180c2000002 ~ 0x0180c200000f,
except the PAUSE packet (DA = 0x0180c2000001)
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-09 15:32:52
I don't know how to implement STP in L2 switch like SP7021.
That is the nice thing about using Linux. It already knows how to
implement STP. The bridge will do it for you. You just need to add the
callbacks in the driver which are needed. Please take a look at other
switchdev drivers.
If this is acceptable, I'd like to have Ethernet of SP7021 have two operation
modes:
- Dual NIC mode
- Single NIC with 2-port frame-flooding hub mode
No, sorry. Do it correctly, or do not do it. Please start with a clean
driver doing Dual NIC mode. You can add L2 support later, once you
have done the research to understand switchdev, etc.
RMII pins of PHY ports of SP7021 are multiplexable. I'd like to switch RMII
pins of the second PHY for other use if single NIC mode is used.
In fact, some SP7021 boards have dual Ethernet and some have only one
Ethernet. We really need the two operation modes.
Only using a subset of ports in a switch is common. The common binding
for DSA switches is described in:
Documentation/devicetree/bindings/net/dsa/dsa.yaml and for example
Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml is
a memory mapped switch. Notice the reg numbers:
ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
label = "cpu";
ethernet = <&gmac0>;
};
port@2 {
reg = <2>;
label = "lan0";
phy-handle = <&phy1>;
};
reg = <1> is missing in this example. Port 1 of the switch is not
used. You can do the same with a 2 port switch, when you don't want to
make use of a port. Just don't list it in DT.
After looking up some data, I find RMC means reserved multi-cast.
RMC packets means packets with DA = 0x0180c2000000, 0x0180c2000002 ~ 0x0180c200000f,
except the PAUSE packet (DA = 0x0180c2000001)
Ah, good. BPDUs use 01:80:C2:00:00:00. So they will be passed when the
port is in blocking mode. PTP uses 01:80:C2:00:00:0E. So the hardware
designers appear to of designed a proper L2 switch with everything you
need for a managed switch. What is missing is software. The more i
learn about this hardware, the more i've convinced you need to write
proper Linux support for it, not your mode hacks.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-09 17:05:19
quoted
I don't know how to implement STP in L2 switch like SP7021.
That is the nice thing about using Linux. It already knows how to implement
STP. The bridge will do it for you. You just need to add the callbacks in the
driver which are needed. Please take a look at other switchdev drivers.
quoted
If this is acceptable, I'd like to have Ethernet of SP7021 have two
operation
modes:
- Dual NIC mode
- Single NIC with 2-port frame-flooding hub mode
No, sorry. Do it correctly, or do not do it. Please start with a clean driver doing
Dual NIC mode. You can add L2 support later, once you have done the research
to understand switchdev, etc.
Sorry, I will go with Dual NIC mode. I'll do a whole cleanup on driver for this.
Please kindly review again.
I need time to study more about switchdev and propose a plan to high
management of company. However, Sunplus is not a networking company,
but targets on Linux-based industrial control, autonomous mobile robot, ...
quoted
RMII pins of PHY ports of SP7021 are multiplexable. I'd like to switch
RMII pins of the second PHY for other use if single NIC mode is used.
In fact, some SP7021 boards have dual Ethernet and some have only one
Ethernet. We really need the two operation modes.
Only using a subset of ports in a switch is common. The common binding for
DSA switches is described in:
Documentation/devicetree/bindings/net/dsa/dsa.yaml and for example
Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml is a
memory mapped switch. Notice the reg numbers:
ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
label = "cpu";
ethernet = <&gmac0>;
};
port@2 {
reg = <2>;
label = "lan0";
phy-handle = <&phy1>;
};
reg = <1> is missing in this example. Port 1 of the switch is not used. You can
do the same with a 2 port switch, when you don't want to make use of a port.
Just don't list it in DT.
Thank you for routing me to the document.
Now I know there are switch device examples in folder dsa/.
We can refer to them when we want to make a switch.
quoted
After looking up some data, I find RMC means reserved multi-cast.
RMC packets means packets with DA = 0x0180c2000000, 0x0180c2000002 ~
0x0180c200000f, except the PAUSE packet (DA = 0x0180c2000001)
Ah, good. BPDUs use 01:80:C2:00:00:00. So they will be passed when the port is
in blocking mode. PTP uses 01:80:C2:00:00:0E. So the hardware designers
appear to of designed a proper L2 switch with everything you need for a
managed switch. What is missing is software. The more i learn about this
hardware, the more i've convinced you need to write proper Linux support for
it, not your mode hacks.
Andrew
Thanks for confirming that the L2 switch is good enough for switch device.
Actually, the IP was licensed from other company long ago. We don’t know
all details.
@@ -0,0 +1,152 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+# Copyright (C) Sunplus Co., Ltd. 2021+%YAML1.2+---+$id:http://devicetree.org/schemas/net/sunplus,sp7021-emac.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:Sunplus SP7021 Dual Ethernet MAC Device Tree Bindings++maintainers:+-Wells Lu <wells.lu@sunplus.com>++description:|+Sunplus SP7021 dual 10M/100M Ethernet MAC controller.+Device node of the controller has following properties.++properties:+compatible:+const:sunplus,sp7021-emac++reg:+items:+-description:Base address and length of the EMAC registers.+-description:Base address and length of the MOON5 registers.++reg-names:+items:+-const:emac+-const:moon5++interrupts:+description:|+Contains number and type of interrupt. Number should be 66.+Type should be high-level trigger+maxItems:1++clocks:+description:|+Clock controller selector for Ethernet MAC controller.+maxItems:1++resets:+description:|+Reset controller selector for Ethernet MAC controller.+maxItems:1++phy-handle1:+description:A handle to node of phy 1 in mdio node+maxItems:1++phy-handle2:+description:A handle to node of phy 2 in mdio node+maxItems:1++pinctrl-names:+description:|+Names corresponding to the numbered pinctrl states.+A pinctrl state named "default" must be defined.+const:default++pinctrl-0:+description:A handle to the 'default' state of pin configuration++nvmem-cells:+items:+-description:nvmem cell address of MAC address of MAC 1+-description:nvmem cell address of MAC address of MAC 2++nvmem-cell-names:+description:names corresponding to the nvmem cells of MAC address+items:+-const:mac_addr0+-const:mac_addr1++mdio:+type:object+description:Internal MDIO Bus++properties:+"#address-cells":+const:1++"#size-cells":+const:0++patternProperties:+"^ethernet-phy@[0-9]$":+type:object++description:+Integrated PHY node++properties:+reg:+maxItems:1++phy-mode:+maxItems:1++required:+-reg+-phy-mode++additionalProperties:false++required:+-compatible+-reg+-reg-names+-interrupts+-clocks+-resets+-phy-handle1+-phy-handle2+-pinctrl-0+-pinctrl-names+-nvmem-cells+-nvmem-cell-names+-mdio++examples:+-|+#include <dt-bindings/interrupt-controller/irq.h>+emac:emac@9c108000 {+compatible = "sunplus,sp7021-emac";+reg = <0x9c108000 0x400>, <0x9c000280 0x80>;+reg-names = "emac", "moon5";+interrupt-parent = <&intc>;+interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;+clocks = <&clkc 0xa7>;+resets = <&rstc 0x97>;+phy-handle1 = <ð_phy0>;+phy-handle2 = <ð_phy1>;+pinctrl-0 = <&emac_demo_board_v3_pins>;+pinctrl-names = "default";+nvmem-cells = <&mac_addr0>, <&mac_addr1>;+nvmem-cell-names = "mac_addr0", "mac_addr1";++mdio {+#address-cells = <1>;+#size-cells = <0>;+eth_phy0:ethernet-phy@0 {+reg = <0>;+phy-mode = "rmii";+};+eth_phy1:ethernet-phy@1 {+reg = <1>;+phy-mode = "rmii";+};+};+};+...
@@ -0,0 +1,231 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_desc.h"+#include"sp_define.h"++voidrx_descs_flush(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].addr1=rx_skbinfo[j].mapping;+rx_desc[j].cmd2=(j==comm->rx_desc_num[i]-1)?+EOR_BIT|comm->rx_desc_buff_size:+comm->rx_desc_buff_size;+wmb();// Set OWN_BIT after other fields are ready.+rx_desc[j].cmd1=OWN_BIT;+}+}+}++voidtx_descs_clean(structsp_common*comm)+{+u32i;+s32buflen;++if(!comm->tx_desc)+return;++for(i=0;i<TX_DESC_NUM;i++){+comm->tx_desc[i].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+comm->tx_desc[i].cmd2=0;+comm->tx_desc[i].addr1=0;+comm->tx_desc[i].addr2=0;++if(comm->tx_temp_skb_info[i].mapping){+buflen=(comm->tx_temp_skb_info[i].skb)?+comm->tx_temp_skb_info[i].skb->len:+MAC_TX_BUFF_SIZE;+dma_unmap_single(&comm->pdev->dev,comm->tx_temp_skb_info[i].mapping,+buflen,DMA_TO_DEVICE);+comm->tx_temp_skb_info[i].mapping=0;+}++if(comm->tx_temp_skb_info[i].skb){+dev_kfree_skb(comm->tx_temp_skb_info[i].skb);+comm->tx_temp_skb_info[i].skb=NULL;+}+}+}++voidrx_descs_clean(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+if(!comm->rx_skb_info[i])+continue;++rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+rx_desc[j].cmd2=0;+rx_desc[j].addr1=0;++if(rx_skbinfo[j].skb){+dma_unmap_single(&comm->pdev->dev,rx_skbinfo[j].mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);+dev_kfree_skb(rx_skbinfo[j].skb);+rx_skbinfo[j].skb=NULL;+rx_skbinfo[j].mapping=0;+}+}++kfree(rx_skbinfo);+comm->rx_skb_info[i]=NULL;+}+}++voiddescs_clean(structsp_common*comm)+{+rx_descs_clean(comm);+tx_descs_clean(comm);+}++voiddescs_free(structsp_common*comm)+{+u32i;++descs_clean(comm);+comm->tx_desc=NULL;+for(i=0;i<RX_DESC_QUEUE_NUM;i++)+comm->rx_desc[i]=NULL;++/* Free descriptor area */+if(comm->desc_base){+dma_free_coherent(&comm->pdev->dev,comm->desc_size,comm->desc_base,+comm->desc_dma);+comm->desc_base=NULL;+comm->desc_dma=0;+comm->desc_size=0;+}+}++voidtx_descs_init(structsp_common*comm)+{+memset(comm->tx_desc,'\0',sizeof(structmac_desc)*+(TX_DESC_NUM+MAC_GUARD_DESC_NUM));+}++intrx_descs_init(structsp_common*comm)+{+structsk_buff*skb;+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+comm->rx_skb_info[i]=kmalloc_array(comm->rx_desc_num[i],+sizeof(structskb_info),GFP_KERNEL);+if(!comm->rx_skb_info[i])+gotoMEM_ALLOC_FAIL;++rx_skbinfo=comm->rx_skb_info[i];+rx_desc=comm->rx_desc[i];+for(j=0;j<comm->rx_desc_num[i];j++){+skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(!skb)+gotoMEM_ALLOC_FAIL;++skb->dev=comm->ndev;+skb_reserve(skb,RX_OFFSET);/* +data +tail */++rx_skbinfo[j].skb=skb;+rx_skbinfo[j].mapping=dma_map_single(&comm->pdev->dev,skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);+rx_desc[j].addr1=rx_skbinfo[j].mapping;+rx_desc[j].addr2=0;+rx_desc[j].cmd2=(j==comm->rx_desc_num[i]-1)?+EOR_BIT|comm->rx_desc_buff_size:+comm->rx_desc_buff_size;+wmb();// Set OWN_BIT after other fields are effective.+rx_desc[j].cmd1=OWN_BIT;+}+}++return0;++MEM_ALLOC_FAIL:+rx_descs_clean(comm);+return-ENOMEM;+}++intdescs_alloc(structsp_common*comm)+{+u32i;+s32desc_size;++/* Alloc descriptor area */+desc_size=(TX_DESC_NUM+MAC_GUARD_DESC_NUM)*sizeof(structmac_desc);+for(i=0;i<RX_DESC_QUEUE_NUM;i++)+desc_size+=comm->rx_desc_num[i]*sizeof(structmac_desc);++comm->desc_base=dma_alloc_coherent(&comm->pdev->dev,desc_size,&comm->desc_dma,+GFP_KERNEL);+if(!comm->desc_base)+return-ENOMEM;++comm->desc_size=desc_size;++/* Setup Tx descriptor */+comm->tx_desc=(structmac_desc*)comm->desc_base;++/* Setup Rx descriptor */+comm->rx_desc[0]=&comm->tx_desc[TX_DESC_NUM+MAC_GUARD_DESC_NUM];+for(i=1;i<RX_DESC_QUEUE_NUM;i++)+comm->rx_desc[i]=comm->rx_desc[i-1]+comm->rx_desc_num[i-1];++return0;+}++intdescs_init(structsp_common*comm)+{+u32i,ret;++// Initialize rx descriptor's data+comm->rx_desc_num[0]=RX_QUEUE0_DESC_NUM;+#if RX_DESC_QUEUE_NUM > 1+comm->rx_desc_num[1]=RX_QUEUE1_DESC_NUM;+#endif++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+comm->rx_desc[i]=NULL;+comm->rx_skb_info[i]=NULL;+comm->rx_pos[i]=0;+}+comm->rx_desc_buff_size=MAC_RX_LEN_MAX;++// Initialize tx descriptor's data+comm->tx_done_pos=0;+comm->tx_desc=NULL;+comm->tx_pos=0;+comm->tx_desc_full=0;+for(i=0;i<TX_DESC_NUM;i++)+comm->tx_temp_skb_info[i].skb=NULL;++// Allocate tx & rx descriptors.+ret=descs_alloc(comm);+if(ret){+netdev_err(comm->ndev,"Failed to allocate tx & rx descriptors!\n");+returnret;+}++tx_descs_init(comm);++returnrx_descs_init(comm);+}
@@ -0,0 +1,606 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/clk.h>+#include<linux/reset.h>+#include<linux/nvmem-consumer.h>+#include<linux/of_net.h>+#include"sp_driver.h"+#include"sp_phy.h"++staticconstchardef_mac_addr[ETHERNET_MAC_ADDR_LEN]={+0xfc,0x4b,0xbc,0x00,0x00,0x00+};++/*********************************************************************+*+*net_device_ops+*+**********************************************************************/+staticintethernet_open(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netdev_dbg(ndev,"Open port = %x\n",mac->lan_port);++mac->comm->enable|=mac->lan_port;++hal_mac_start(mac);+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~(MAC_INT_TX|MAC_INT_RX));++netif_carrier_on(ndev);+if(netif_carrier_ok(ndev))+netif_start_queue(ndev);++return0;+}++staticintethernet_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netif_stop_queue(ndev);+netif_carrier_off(ndev);++mac->comm->enable&=~mac->lan_port;++hal_mac_stop(mac);++return0;+}++/* Transmit a packet (called by the kernel) */+staticintethernet_start_xmit(structsk_buff*skb,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structsp_common*comm=mac->comm;+u32tx_pos;+u32cmd1;+u32cmd2;+structmac_desc*txdesc;+structskb_info*skbinfo;+unsignedlongflags;++if(unlikely(comm->tx_desc_full==1)){+// No TX descriptors left. Wait for tx interrupt.+netdev_info(ndev,"TX descriptor queue full when xmit!\n");+returnNETDEV_TX_BUSY;+}++/* if skb size shorter than 60, fill it with '\0' */+if(unlikely(skb->len<ETH_ZLEN)){+if(skb_tailroom(skb)>=(ETH_ZLEN-skb->len)){+memset(__skb_put(skb,ETH_ZLEN-skb->len),'\0',+ETH_ZLEN-skb->len);+}else{+structsk_buff*old_skb=skb;++skb=dev_alloc_skb(ETH_ZLEN+TX_OFFSET);+if(skb){+memset(skb->data+old_skb->len,'\0',+ETH_ZLEN-old_skb->len);+memcpy(skb->data,old_skb->data,old_skb->len);+skb_put(skb,ETH_ZLEN);/* add data to an sk_buff */+dev_kfree_skb_irq(old_skb);+}else{+skb=old_skb;+}+}+}++spin_lock_irqsave(&comm->tx_lock,flags);+tx_pos=comm->tx_pos;+txdesc=&comm->tx_desc[tx_pos];+skbinfo=&comm->tx_temp_skb_info[tx_pos];+skbinfo->len=skb->len;+skbinfo->skb=skb;+skbinfo->mapping=dma_map_single(&comm->pdev->dev,skb->data,+skb->len,DMA_TO_DEVICE);+cmd1=(OWN_BIT|FS_BIT|LS_BIT|(mac->to_vlan<<12)|(skb->len&LEN_MASK));+cmd2=skb->len&LEN_MASK;++if(tx_pos==(TX_DESC_NUM-1))+cmd2|=EOR_BIT;++txdesc->addr1=skbinfo->mapping;+txdesc->cmd2=cmd2;+wmb();// Set OWN_BIT after other fields of descriptor are effective.+txdesc->cmd1=cmd1;++NEXT_TX(tx_pos);++if(unlikely(tx_pos==comm->tx_done_pos)){+netif_stop_queue(ndev);+comm->tx_desc_full=1;+}+comm->tx_pos=tx_pos;+wmb();// make sure settings are effective.++/* trigger gmac to transmit */+hal_tx_trigger(mac);++spin_unlock_irqrestore(&mac->comm->tx_lock,flags);+returnNETDEV_TX_OK;+}++staticvoidethernet_set_rx_mode(structnet_device*ndev)+{+if(ndev){+structsp_mac*mac=netdev_priv(ndev);+structsp_common*comm=mac->comm;+unsignedlongflags;++spin_lock_irqsave(&comm->ioctl_lock,flags);+hal_rx_mode_set(ndev);+spin_unlock_irqrestore(&comm->ioctl_lock,flags);+}+}++staticintethernet_set_mac_address(structnet_device*ndev,void*addr)+{+structsockaddr*hwaddr=(structsockaddr*)addr;+structsp_mac*mac=netdev_priv(ndev);++if(netif_running(ndev))+return-EBUSY;++memcpy(ndev->dev_addr,hwaddr->sa_data,ndev->addr_len);++/* Delete the old Ethernet MAC address */+netdev_dbg(ndev,"HW Addr = %pM\n",mac->mac_addr);+if(is_valid_ether_addr(mac->mac_addr))+hal_mac_addr_del(mac);++/* Set the Ethernet MAC address */+memcpy(mac->mac_addr,hwaddr->sa_data,ndev->addr_len);+hal_mac_addr_set(mac);++return0;+}++staticintethernet_do_ioctl(structnet_device*ndev,structifreq*ifr,intcmd)+{+structsp_mac*mac=netdev_priv(ndev);++switch(cmd){+caseSIOCGMIIPHY:+caseSIOCGMIIREG:+caseSIOCSMIIREG:+returnphy_mii_ioctl(mac->phy_dev,ifr,cmd);+}++return-EOPNOTSUPP;+}++staticvoidethernet_tx_timeout(structnet_device*ndev,unsignedinttxqueue)+{+}++staticstructnet_device_stats*ethernet_get_stats(structnet_device*ndev)+{+structsp_mac*mac;++mac=netdev_priv(ndev);+return&mac->dev_stats;+}++staticconststructnet_device_opsnetdev_ops={+.ndo_open=ethernet_open,+.ndo_stop=ethernet_stop,+.ndo_start_xmit=ethernet_start_xmit,+.ndo_set_rx_mode=ethernet_set_rx_mode,+.ndo_set_mac_address=ethernet_set_mac_address,+.ndo_do_ioctl=ethernet_do_ioctl,+.ndo_tx_timeout=ethernet_tx_timeout,+.ndo_get_stats=ethernet_get_stats,+};++char*sp7021_otp_read_mac(structdevice*dev,ssize_t*len,char*name)+{+char*ret=NULL;+structnvmem_cell*cell=nvmem_cell_get(dev,name);++if(IS_ERR_OR_NULL(cell)){+dev_err(dev,"OTP %s read failure: %ld",name,PTR_ERR(cell));+returnNULL;+}++ret=nvmem_cell_read(cell,len);+nvmem_cell_put(cell);+dev_dbg(dev,"%zd bytes are read from OTP %s.",*len,name);++returnret;+}++staticvoidcheck_mac_vendor_id_and_convert(char*mac_addr)+{+// Byte order of MAC address of some samples are reversed.+// Check vendor id and convert byte order if it is wrong.+if((mac_addr[5]==0xFC)&&(mac_addr[4]==0x4B)&&(mac_addr[3]==0xBC)&&+((mac_addr[0]!=0xFC)||(mac_addr[1]!=0x4B)||(mac_addr[2]!=0xBC))){+chartmp;++// Swap mac_addr[0] and mac_addr[5]+tmp=mac_addr[0];+mac_addr[0]=mac_addr[5];+mac_addr[5]=tmp;++// Swap mac_addr[1] and mac_addr[4]+tmp=mac_addr[1];+mac_addr[1]=mac_addr[4];+mac_addr[4]=tmp;++// Swap mac_addr[2] and mac_addr[3]+tmp=mac_addr[2];+mac_addr[2]=mac_addr[3];+mac_addr[3]=tmp;+}+}++/*********************************************************************+*+*platform_driver+*+**********************************************************************/+staticu32init_netdev(structplatform_device*pdev,inteth_no,structnet_device**r_ndev)+{+structsp_mac*mac;+structnet_device*ndev;+char*m_addr_name=(eth_no==0)?"mac_addr0":"mac_addr1";+ssize_totp_l=0;+char*otp_v;+intret;++// Allocate the devices, and also allocate sp_mac, we can get it by netdev_priv().+ndev=alloc_etherdev(sizeof(*mac));+if(!ndev){+*r_ndev=NULL;+return-ENOMEM;+}+SET_NETDEV_DEV(ndev,&pdev->dev);+ndev->netdev_ops=&netdev_ops;++mac=netdev_priv(ndev);+mac->ndev=ndev;+mac->next_ndev=NULL;++// Get property 'mac-addr0' or 'mac-addr1' from dts.+otp_v=sp7021_otp_read_mac(&pdev->dev,&otp_l,m_addr_name);+if((otp_l<6)||IS_ERR_OR_NULL(otp_v)){+dev_info(&pdev->dev,"OTP mac %s (len = %zd) is invalid, using default!\n",+m_addr_name,otp_l);+otp_l=0;+}else{+// Check if mac-address is valid or not. If not, copy from default.+memcpy(mac->mac_addr,otp_v,6);++// Byte order of Some samples are reversed. Convert byte order here.+check_mac_vendor_id_and_convert(mac->mac_addr);++if(!is_valid_ether_addr(mac->mac_addr)){+dev_info(&pdev->dev,"Invalid mac in OTP[%s] = %pM, use default!\n",+m_addr_name,mac->mac_addr);+otp_l=0;+}+}+if(otp_l!=6){+memcpy(mac->mac_addr,def_mac_addr,ETHERNET_MAC_ADDR_LEN);+mac->mac_addr[5]+=eth_no;+}++dev_info(&pdev->dev,"HW Addr = %pM\n",mac->mac_addr);++memcpy(ndev->dev_addr,mac->mac_addr,ETHERNET_MAC_ADDR_LEN);++ret=register_netdev(ndev);+if(ret){+dev_err(&pdev->dev,"Failed to register net device \"%s\"!\n",+ndev->name);+free_netdev(ndev);+*r_ndev=NULL;+returnret;+}+netdev_info(ndev,"Registered net device \"%s\" successfully.\n",ndev->name);++*r_ndev=ndev;+return0;+}++staticintsoc0_open(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32ret;++hal_mac_stop(mac);++ret=descs_init(comm);+if(ret){+netdev_err(mac->ndev,"Fail to initialize mac descriptors!\n");+descs_free(comm);+returnret;+}++mac_init(mac);+return0;+}++staticintsoc0_stop(structsp_mac*mac)+{+hal_mac_stop(mac);++descs_free(mac->comm);+return0;+}++staticintsp_probe(structplatform_device*pdev)+{+structsp_common*comm;+structresource*rc;+structnet_device*ndev,*ndev2;+structdevice_node*np;+structsp_mac*mac,*mac2;+intret;++if(platform_get_drvdata(pdev))+return-ENODEV;++// Allocate memory for 'sp_common' area.+comm=devm_kzalloc(&pdev->dev,sizeof(*comm),GFP_KERNEL);+if(!comm)+return-ENOMEM;+comm->pdev=pdev;++spin_lock_init(&comm->rx_lock);+spin_lock_init(&comm->tx_lock);+spin_lock_init(&comm->ioctl_lock);++// Get memory resoruce "emac" from dts.+rc=platform_get_resource_byname(pdev,IORESOURCE_MEM,"emac");+if(!rc){+dev_err(&pdev->dev,"No MEM resource \'emac\' found!\n");+return-ENXIO;+}+dev_dbg(&pdev->dev,"name = \"%s\", start = %pa\n",rc->name,&rc->start);++comm->sp_reg_base=devm_ioremap_resource(&pdev->dev,rc);+if(IS_ERR(comm->sp_reg_base)){+dev_err(&pdev->dev,"ioremap failed!\n");+return-ENOMEM;+}++// Get memory resoruce "moon5" from dts.+rc=platform_get_resource_byname(pdev,IORESOURCE_MEM,"moon5");+if(!rc){+dev_err(&pdev->dev,"No MEM resource \'moon5\' found!\n");+return-ENXIO;+}+dev_dbg(&pdev->dev,"name = \"%s\", start = %pa\n",rc->name,&rc->start);++// Note that moon5 is shared resource. Don't use devm_ioremap_resource().+comm->moon5_reg_base=devm_ioremap(&pdev->dev,rc->start,rc->end-rc->start+1);+if(IS_ERR(comm->moon5_reg_base)){+dev_err(&pdev->dev,"ioremap failed!\n");+return-ENOMEM;+}++// Get irq resource from dts.+ret=platform_get_irq(pdev,0);+if(ret<0)+returnret;+comm->irq=ret;++// Get clock controller.+comm->clk=devm_clk_get(&pdev->dev,NULL);+if(IS_ERR(comm->clk)){+dev_err_probe(&pdev->dev,PTR_ERR(comm->clk),+"Failed to retrieve clock controller!\n");+returnPTR_ERR(comm->clk);+}++// Get reset controller.+comm->rstc=devm_reset_control_get_exclusive(&pdev->dev,NULL);+if(IS_ERR(comm->rstc)){+dev_err_probe(&pdev->dev,PTR_ERR(comm->rstc),+"Failed to retrieve reset controller!\n");+returnPTR_ERR(comm->rstc);+}++// Enable clock.+clk_prepare_enable(comm->clk);+udelay(1);++reset_control_assert(comm->rstc);+udelay(1);+reset_control_deassert(comm->rstc);+udelay(1);++// Initialize the 1st net device.+ret=init_netdev(pdev,0,&ndev);+if(!ndev)+returnret;++platform_set_drvdata(pdev,ndev);++ndev->irq=comm->irq;+mac=netdev_priv(ndev);+mac->comm=comm;+comm->ndev=ndev;++// Get node of phy 1.+mac->phy_node=of_parse_phandle(pdev->dev.of_node,"phy-handle1",0);+if(!mac->phy_node){+netdev_info(ndev,"Cannot get node of phy 1!\n");+ret=-ENODEV;+gotoout_unregister_dev;+}++// Get address of phy from dts.+if(of_property_read_u32(mac->phy_node,"reg",&mac->phy_addr)){+mac->phy_addr=0;+netdev_info(ndev,"Cannot get address of phy 1! Set to 0.\n");+}++// Get mode of phy from dts.+if(of_get_phy_mode(mac->phy_node,&mac->phy_mode)){+mac->phy_mode=PHY_INTERFACE_MODE_RGMII_ID;+netdev_info(ndev,"Missing phy-mode of phy 1! Set to \'rgmii-id\'.\n");+}++// Request irq.+ret=devm_request_irq(&pdev->dev,comm->irq,ethernet_interrupt,0,+ndev->name,ndev);+if(ret){+netdev_err(ndev,"Failed to request irq #%d for \"%s\"!\n",+ndev->irq,ndev->name);+gotoout_unregister_dev;+}++mac->cpu_port=0x1;// soc0+mac->lan_port=0x1;// forward to port 0+mac->to_vlan=0x1;// vlan group: 0+mac->vlan_id=0x0;// vlan group: 0++// Set MAC address+hal_mac_addr_set(mac);+hal_rx_mode_set(ndev);+hal_mac_addr_table_del_all(mac);++ndev2=NULL;+np=of_parse_phandle(pdev->dev.of_node,"phy-handle2",0);+if(np){+init_netdev(pdev,1,&ndev2);+if(ndev2){+mac->next_ndev=ndev2;// Point to the second net device.++ndev2->irq=comm->irq;+mac2=netdev_priv(ndev2);+mac2->comm=comm;+mac2->phy_node=np;++if(of_property_read_u32(mac2->phy_node,"reg",&mac2->phy_addr)){+mac2->phy_addr=1;+netdev_info(ndev2,"Cannot get address of phy 2! Set to 1.\n");+}++if(of_get_phy_mode(mac2->phy_node,&mac2->phy_mode)){+mac2->phy_mode=PHY_INTERFACE_MODE_RGMII_ID;+netdev_info(ndev,"Missing phy-mode phy 2! Set to \'rgmii-id\'.\n");+}++mac2->cpu_port=0x1;// soc0+mac2->lan_port=0x2;// forward to port 1+mac2->to_vlan=0x2;// vlan group: 1+mac2->vlan_id=0x1;// vlan group: 1++hal_mac_addr_set(mac2);// Set MAC address for the 2nd net device.+hal_rx_mode_set(ndev2);+}+}++soc0_open(mac);+hal_set_rmii_tx_rx_pol(mac);+hal_phy_addr(mac);++ret=mdio_init(pdev,ndev);+if(ret){+netdev_err(ndev,"Failed to initialize mdio!\n");+gotoout_unregister_dev;+}++ret=sp_phy_probe(ndev);+if(ret){+netdev_err(ndev,"Failed to probe phy!\n");+gotoout_freemdio;+}++if(ndev2){+ret=sp_phy_probe(ndev2);+if(ret){+netdev_err(ndev2,"Failed to probe phy!\n");+unregister_netdev(ndev2);+mac->next_ndev=0;+}+}++netif_napi_add(ndev,&comm->rx_napi,rx_poll,RX_NAPI_WEIGHT);+napi_enable(&comm->rx_napi);+netif_napi_add(ndev,&comm->tx_napi,tx_poll,TX_NAPI_WEIGHT);+napi_enable(&comm->tx_napi);+return0;++out_freemdio:+if(comm->mii_bus)+mdio_remove(ndev);++out_unregister_dev:+unregister_netdev(ndev);+if(ndev2)+unregister_netdev(ndev2);++returnret;+}++staticintsp_remove(structplatform_device*pdev)+{+structnet_device*ndev,*ndev2;+structsp_mac*mac;++ndev=platform_get_drvdata(pdev);+if(!ndev)+return0;++mac=netdev_priv(ndev);++// Unregister and free 2nd net device.+ndev2=mac->next_ndev;+if(ndev2){+sp_phy_remove(ndev2);+unregister_netdev(ndev2);+free_netdev(ndev2);+}++mac->comm->enable=0;+soc0_stop(mac);++// Disable and delete napi.+napi_disable(&mac->comm->rx_napi);+netif_napi_del(&mac->comm->rx_napi);+napi_disable(&mac->comm->tx_napi);+netif_napi_del(&mac->comm->tx_napi);++sp_phy_remove(ndev);+mdio_remove(ndev);++// Unregister and free 1st net device.+unregister_netdev(ndev);+free_netdev(ndev);++clk_disable(mac->comm->clk);++return0;+}++staticconststructof_device_idsp_of_match[]={+{.compatible="sunplus,sp7021-emac"},+{/* sentinel */}+};++MODULE_DEVICE_TABLE(of,sp_of_match);++staticstructplatform_driversp_driver={+.probe=sp_probe,+.remove=sp_remove,+.driver={+.name="sp7021_emac",+.owner=THIS_MODULE,+.of_match_table=sp_of_match,+},+};++module_platform_driver(sp_driver);++MODULE_AUTHOR("Wells Lu <wells.lu@sunplus.com>");+MODULE_DESCRIPTION("Sunplus Dual 10M/100M Ethernet driver");+MODULE_LICENSE("GPL v2");
@@ -0,0 +1,331 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/iopoll.h>+#include"sp_hal.h"++voidhal_mac_stop(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg,disable;++if(comm->enable==0){+// Mask and clear all interrupts, except PORT_ST_CHG.+write_sw_int_mask0(mac,0xffffffff);+writel(0xffffffff&(~MAC_INT_PORT_ST_CHG),+comm->sp_reg_base+SP_SW_INT_STATUS_0);++// Disable cpu 0 and cpu 1.+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((0x3<<6)|reg,comm->sp_reg_base+SP_CPU_CNTL);+}++// Disable lan 0 and lan 1.+disable=((~comm->enable)&0x3)<<24;+reg=readl(comm->sp_reg_base+SP_PORT_CNTL0);+writel(disable|reg,comm->sp_reg_base+SP_PORT_CNTL0);+}++voidhal_mac_reset(structsp_mac*mac)+{+}++voidhal_mac_start(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Enable cpu port 0 (6) & port 0 crc padding (8)+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((reg&(~(0x1<<6)))|(0x1<<8),comm->sp_reg_base+SP_CPU_CNTL);++// Enable lan 0 & lan 1+reg=readl(comm->sp_reg_base+SP_PORT_CNTL0);+writel(reg&(~(comm->enable<<24)),comm->sp_reg_base+SP_PORT_CNTL0);+}++voidhal_mac_addr_set(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Write MAC address.+writel(mac->mac_addr[0]+(mac->mac_addr[1]<<8),+comm->sp_reg_base+SP_W_MAC_15_0);+writel(mac->mac_addr[2]+(mac->mac_addr[3]<<8)+(mac->mac_addr[4]<<16)++(mac->mac_addr[5]<<24),comm->sp_reg_base+SP_W_MAC_47_16);++// Set aging=1+writel((mac->cpu_port<<10)+(mac->vlan_id<<7)+(1<<4)+0x1,+comm->sp_reg_base+SP_WT_MAC_AD0);++// Wait for completing.+do{+reg=readl(comm->sp_reg_base+SP_WT_MAC_AD0);+ndelay(10);+netdev_dbg(mac->ndev,"wt_mac_ad0 = %08x\n",reg);+}while((reg&(0x1<<1))==0x0);++netdev_dbg(mac->ndev,"mac_ad0 = %08x, mac_ad = %08x%04x\n",+readl(comm->sp_reg_base+SP_WT_MAC_AD0),+readl(comm->sp_reg_base+SP_W_MAC_47_16),+readl(comm->sp_reg_base+SP_W_MAC_15_0)&0xffff);+}++voidhal_mac_addr_del(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Write MAC address.+writel(mac->mac_addr[0]+(mac->mac_addr[1]<<8),+comm->sp_reg_base+SP_W_MAC_15_0);+writel(mac->mac_addr[2]+(mac->mac_addr[3]<<8)+(mac->mac_addr[4]<<16)++(mac->mac_addr[5]<<24),comm->sp_reg_base+SP_W_MAC_47_16);++// Wait for completing.+writel((0x1<<12)+(mac->vlan_id<<7)+0x1,+comm->sp_reg_base+SP_WT_MAC_AD0);+do{+reg=readl(comm->sp_reg_base+SP_WT_MAC_AD0);+ndelay(10);+netdev_dbg(mac->ndev,"wt_mac_ad0 = %08x\n",reg);+}while((reg&(0x1<<1))==0x0);++netdev_dbg(mac->ndev,"mac_ad0 = %08x, mac_ad = %08x%04x\n",+readl(comm->sp_reg_base+SP_WT_MAC_AD0),+readl(comm->sp_reg_base+SP_W_MAC_47_16),+readl(comm->sp_reg_base+SP_W_MAC_15_0)&0xffff);+}++voidhal_mac_addr_table_del_all(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Wait for address table being idle.+do{+reg=readl(comm->sp_reg_base+SP_ADDR_TBL_SRCH);+ndelay(10);+}while(!(reg&MAC_ADDR_LOOKUP_IDLE));++// Search address table from start.+writel(readl(comm->sp_reg_base+SP_ADDR_TBL_SRCH)|MAC_BEGIN_SEARCH_ADDR,+comm->sp_reg_base+SP_ADDR_TBL_SRCH);+while(1){+do{+reg=readl(comm->sp_reg_base+SP_ADDR_TBL_ST);+ndelay(10);+netdev_dbg(mac->ndev,"addr_tbl_st = %08x\n",reg);+}while(!(reg&(MAC_AT_TABLE_END|MAC_AT_DATA_READY)));++if(reg&MAC_AT_TABLE_END)+break;++netdev_dbg(mac->ndev,"addr_tbl_st = %08x\n",reg);+netdev_dbg(mac->ndev,"@AT #%u: port=%01x, cpu=%01x, vid=%u, aging=%u, proxy=%u, mc_ingress=%u\n",+(reg>>22)&0x3ff,(reg>>12)&0x3,(reg>>10)&0x3,+(reg>>7)&0x7,(reg>>4)&0x7,(reg>>3)&0x1,+(reg>>2)&0x1);++// Delete all entries which are learnt from lan ports.+if((reg>>12)&0x3){+writel(readl(comm->sp_reg_base+SP_MAC_AD_SER0),+comm->sp_reg_base+SP_W_MAC_15_0);+writel(readl(comm->sp_reg_base+SP_MAC_AD_SER1),+comm->sp_reg_base+SP_W_MAC_47_16);++writel((0x1<<12)+(reg&(0x7<<7))+0x1,+comm->sp_reg_base+SP_WT_MAC_AD0);+do{+reg=readl(comm->sp_reg_base+SP_WT_MAC_AD0);+ndelay(10);+netdev_dbg(mac->ndev,"wt_mac_ad0 = %08x\n",reg);+}while((reg&(0x1<<1))==0x0);+netdev_dbg(mac->ndev,"mac_ad0 = %08x, mac_ad = %08x%04x\n",+readl(comm->sp_reg_base+SP_WT_MAC_AD0),+readl(comm->sp_reg_base+SP_W_MAC_47_16),+readl(comm->sp_reg_base+SP_W_MAC_15_0)&0xffff);+}++// Search next.+writel(readl(comm->sp_reg_base+SP_ADDR_TBL_SRCH)|MAC_SEARCH_NEXT_ADDR,+comm->sp_reg_base+SP_ADDR_TBL_SRCH);+}+}++voidhal_mac_init(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Disable cpu0 and cpu 1.+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((0x3<<6)|reg,comm->sp_reg_base+SP_CPU_CNTL);++// Descriptor base address+writel(mac->comm->desc_dma,comm->sp_reg_base+SP_TX_LBASE_ADDR_0);+writel(mac->comm->desc_dma+sizeof(structmac_desc)*TX_DESC_NUM,+comm->sp_reg_base+SP_TX_HBASE_ADDR_0);+writel(mac->comm->desc_dma+sizeof(structmac_desc)*(TX_DESC_NUM++MAC_GUARD_DESC_NUM),comm->sp_reg_base+SP_RX_HBASE_ADDR_0);+writel(mac->comm->desc_dma+sizeof(structmac_desc)*(TX_DESC_NUM++MAC_GUARD_DESC_NUM+RX_QUEUE0_DESC_NUM),+comm->sp_reg_base+SP_RX_LBASE_ADDR_0);++// Fc_rls_th=0x4a, Fc_set_th=0x3a, Drop_rls_th=0x2d, Drop_set_th=0x1d+writel(0x4a3a2d1d,comm->sp_reg_base+SP_FL_CNTL_TH);++// Cpu_rls_th=0x4a, Cpu_set_th=0x3a, Cpu_th=0x12, Port_th=0x12+writel(0x4a3a1212,comm->sp_reg_base+SP_CPU_FL_CNTL_TH);++// mtcc_lmt=0xf, Pri_th_l=6, Pri_th_h=6, weigh_8x_en=1+writel(0xf6680000,comm->sp_reg_base+SP_PRI_FL_CNTL);++// High-active LED+reg=readl(comm->sp_reg_base+SP_LED_PORT0);+writel(reg|(1<<28),comm->sp_reg_base+SP_LED_PORT0);++// Disable cpu port0 aging (12)+// Disable cpu port0 learning (14)+// Enable UC and MC packets+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((reg&(~((0x1<<14)|(0x3c<<0))))|(0x1<<12),+comm->sp_reg_base+SP_CPU_CNTL);++// Disable lan port SA learning.+reg=readl(comm->sp_reg_base+SP_PORT_CNTL1);+writel(reg|(0x3<<8),comm->sp_reg_base+SP_PORT_CNTL1);++// Port 0: VLAN group 0+// Port 1: VLAN group 1+writel((1<<4)+0,comm->sp_reg_base+SP_PVID_CONFIG0);++// VLAN group 0: cpu0+port0+// VLAN group 1: cpu0+port1+writel((0xa<<8)+0x9,comm->sp_reg_base+SP_VLAN_MEMSET_CONFIG0);++// RMC forward: to cpu+// LED: 60mS+// BC storm prev: 31 BC+reg=readl(comm->sp_reg_base+SP_SW_GLB_CNTL);+writel((reg&(~((0x3<<25)|(0x3<<23)|(0x3<<4))))|+(0x1<<25)|(0x1<<23)|(0x1<<4),+comm->sp_reg_base+SP_SW_GLB_CNTL);++write_sw_int_mask0(mac,MAC_INT_MASK_DEF);+}++voidhal_rx_mode_set(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structsp_common*comm=mac->comm;+u32mask,reg,rx_mode;++netdev_dbg(ndev,"ndev->flags = %08x\n",ndev->flags);++mask=(mac->lan_port<<2)|(mac->lan_port<<0);+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);++if(ndev->flags&IFF_PROMISC){/* Set promiscuous mode */+// Allow MC and unknown UC packets+rx_mode=(mac->lan_port<<2)|(mac->lan_port<<0);+}elseif((!netdev_mc_empty(ndev)&&(ndev->flags&IFF_MULTICAST))||+(ndev->flags&IFF_ALLMULTI)){+// Allow MC packets+rx_mode=(mac->lan_port<<2);+}else{+// Disable MC and unknown UC packets+rx_mode=0;+}++writel((reg&(~mask))|((~rx_mode)&mask),comm->sp_reg_base+SP_CPU_CNTL);+netdev_dbg(ndev,"cpu_cntl = %08x\n",readl(comm->sp_reg_base+SP_CPU_CNTL));+}++inthal_mdio_access(structsp_mac*mac,u8op_cd,u8phy_addr,u8reg_addr,u32wdata)+{+structsp_common*comm=mac->comm;+u32val,ret;++writel((wdata<<16)|(op_cd<<13)|(reg_addr<<8)|phy_addr,+comm->sp_reg_base+SP_PHY_CNTL_REG0);++ret=read_poll_timeout(readl,val,val&op_cd,10,1000,1,+comm->sp_reg_base+SP_PHY_CNTL_REG1);+if(ret==0)+returnval>>16;+else+returnret;+}++voidhal_tx_trigger(structsp_mac*mac)+{+structsp_common*comm=mac->comm;++writel((0x1<<1),comm->sp_reg_base+SP_CPU_TX_TRIG);+}++voidhal_set_rmii_tx_rx_pol(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Set polarity of RX and TX of RMII signal.+reg=readl(comm->moon5_reg_base+MOON5_MO4_L2SW_CLKSW_CTL);+writel(reg|(0xf<<16)|0xf,comm->moon5_reg_base+MOON5_MO4_L2SW_CLKSW_CTL);+}++voidhal_phy_addr(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg;++// Set address of phy.+reg=readl(comm->sp_reg_base+SP_MAC_FORCE_MODE);+reg=(reg&(~(0x1f<<16)))|((mac->phy_addr&0x1f)<<16);+if(mac->next_ndev){+structnet_device*ndev2=mac->next_ndev;+structsp_mac*mac2=netdev_priv(ndev2);++reg=(reg&(~(0x1f<<24)))|((mac2->phy_addr&0x1f)<<24);+}+writel(reg,comm->sp_reg_base+SP_MAC_FORCE_MODE);+}++u32read_sw_int_mask0(structsp_mac*mac)+{+structsp_common*comm=mac->comm;++returnreadl(comm->sp_reg_base+SP_SW_INT_MASK_0);+}++voidwrite_sw_int_mask0(structsp_mac*mac,u32value)+{+structsp_common*comm=mac->comm;++writel(value,comm->sp_reg_base+SP_SW_INT_MASK_0);+}++voidwrite_sw_int_status0(structsp_mac*mac,u32value)+{+structsp_common*comm=mac->comm;++writel(value,comm->sp_reg_base+SP_SW_INT_STATUS_0);+}++u32read_sw_int_status0(structsp_mac*mac)+{+structsp_common*comm=mac->comm;++returnreadl(comm->sp_reg_base+SP_SW_INT_STATUS_0);+}++u32read_port_ability(structsp_mac*mac)+{+structsp_common*comm=mac->comm;++returnreadl(comm->sp_reg_base+SP_PORT_ABILITY);+}
@@ -0,0 +1,286 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_define.h"+#include"sp_int.h"+#include"sp_driver.h"+#include"sp_hal.h"++staticvoidport_status_change(structsp_mac*mac)+{+u32reg;+structnet_device*ndev=mac->ndev;++reg=read_port_ability(mac);+if(!netif_carrier_ok(ndev)&&(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_on(ndev);+netif_start_queue(ndev);+}elseif(netif_carrier_ok(ndev)&&!(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_off(ndev);+netif_stop_queue(ndev);+}++if(mac->next_ndev){+structnet_device*ndev2=mac->next_ndev;++if(!netif_carrier_ok(ndev2)&&(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_on(ndev2);+netif_start_queue(ndev2);+}elseif(netif_carrier_ok(ndev2)&&!(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_off(ndev2);+netif_stop_queue(ndev2);+}+}+}++staticvoidrx_skb(structsp_mac*mac,structsk_buff*skb)+{+mac->dev_stats.rx_packets++;+mac->dev_stats.rx_bytes+=skb->len;+netif_receive_skb(skb);+}++intrx_poll(structnapi_struct*napi,intbudget)+{+structsp_common*comm=container_of(napi,structsp_common,rx_napi);+structsp_mac*mac=netdev_priv(comm->ndev);+structsk_buff*skb,*new_skb;+structskb_info*sinfo;+structmac_desc*desc;+structmac_desc*h_desc;+u32rx_pos,pkg_len;+u32cmd;+u32num,rx_count;+s32queue;+intndev2_pkt;+structnet_device_stats*dev_stats;++spin_lock(&comm->rx_lock);++// Process high-priority queue and then low-priority queue.+for(queue=0;queue<RX_DESC_QUEUE_NUM;queue++){+rx_pos=comm->rx_pos[queue];+rx_count=comm->rx_desc_num[queue];++for(num=0;num<rx_count;num++){+sinfo=comm->rx_skb_info[queue]+rx_pos;+desc=comm->rx_desc[queue]+rx_pos;+cmd=desc->cmd1;++if(cmd&OWN_BIT)+break;++if((cmd&PKTSP_MASK)==PKTSP_PORT1){+structsp_mac*mac2;++ndev2_pkt=1;+mac2=(mac->next_ndev)?netdev_priv(mac->next_ndev):NULL;+dev_stats=(mac2)?&mac2->dev_stats:&mac->dev_stats;+}else{+ndev2_pkt=0;+dev_stats=&mac->dev_stats;+}++pkg_len=cmd&LEN_MASK;+if(unlikely((cmd&ERR_CODE)||(pkg_len<64))){+dev_stats->rx_length_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++if(unlikely(cmd&RX_IP_CHKSUM_BIT)){+dev_stats->rx_crc_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++// Allocate an skbuff for receiving.+new_skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(unlikely(!new_skb)){+dev_stats->rx_dropped++;+gotoNEXT;+}+new_skb->dev=mac->ndev;++dma_unmap_single(&comm->pdev->dev,sinfo->mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);++skb=sinfo->skb;+skb->ip_summed=CHECKSUM_NONE;++/*skb_put will judge if tail exceeds end, but __skb_put won't */+__skb_put(skb,(pkg_len-4>comm->rx_desc_buff_size)?+comm->rx_desc_buff_size:pkg_len-4);++sinfo->mapping=dma_map_single(&comm->pdev->dev,new_skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);+sinfo->skb=new_skb;++if(ndev2_pkt){+structnet_device*netdev2=mac->next_ndev;++if(netdev2){+skb->protocol=eth_type_trans(skb,netdev2);+rx_skb(netdev_priv(netdev2),skb);+}+}else{+skb->protocol=eth_type_trans(skb,mac->ndev);+rx_skb(mac,skb);+}++desc->addr1=sinfo->mapping;++NEXT:+desc->cmd2=(rx_pos==comm->rx_desc_num[queue]-1)?+EOR_BIT|MAC_RX_LEN_MAX:MAC_RX_LEN_MAX;+wmb();// Set OWN_BIT after other fields of descriptor are effective.+desc->cmd1=OWN_BIT|(comm->rx_desc_buff_size&LEN_MASK);++NEXT_RX(queue,rx_pos);++// If there are packets in high-priority queue,+// stop processing low-priority queue.+if((queue==1)&&((h_desc->cmd1&OWN_BIT)==0))+break;+}++comm->rx_pos[queue]=rx_pos;++// Save pointer to last rx descriptor of high-priority queue.+if(queue==0)+h_desc=comm->rx_desc[queue]+rx_pos;+}++spin_unlock(&comm->rx_lock);++wmb();// make sure settings are effective.+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~MAC_INT_RX);++napi_complete(napi);+return0;+}++inttx_poll(structnapi_struct*napi,intbudget)+{+structsp_common*comm=container_of(napi,structsp_common,tx_napi);+structsp_mac*mac=netdev_priv(comm->ndev);+u32tx_done_pos;+u32cmd;+structskb_info*skbinfo;+structsp_mac*smac;++spin_lock(&comm->tx_lock);++tx_done_pos=comm->tx_done_pos;+while((tx_done_pos!=comm->tx_pos)||(comm->tx_desc_full==1)){+cmd=comm->tx_desc[tx_done_pos].cmd1;+if(cmd&OWN_BIT)+break;++skbinfo=&comm->tx_temp_skb_info[tx_done_pos];+if(unlikely(!skbinfo->skb))+netdev_err(mac->ndev,"skb is null!\n");++smac=mac;+if(mac->next_ndev&&((cmd&TO_VLAN_MASK)==TO_VLAN_GROUP1))+smac=netdev_priv(mac->next_ndev);++if(unlikely(cmd&(ERR_CODE))){+smac->dev_stats.tx_errors++;+}else{+smac->dev_stats.tx_packets++;+smac->dev_stats.tx_bytes+=skbinfo->len;+}++dma_unmap_single(&comm->pdev->dev,skbinfo->mapping,skbinfo->len,+DMA_TO_DEVICE);+skbinfo->mapping=0;+dev_kfree_skb_irq(skbinfo->skb);+skbinfo->skb=NULL;++NEXT_TX(tx_done_pos);+if(comm->tx_desc_full==1)+comm->tx_desc_full=0;+}++comm->tx_done_pos=tx_done_pos;+if(!comm->tx_desc_full){+if(netif_queue_stopped(mac->ndev))+netif_wake_queue(mac->ndev);++if(mac->next_ndev){+if(netif_queue_stopped(mac->next_ndev))+netif_wake_queue(mac->next_ndev);+}+}++spin_unlock(&comm->tx_lock);++wmb();// make sure settings are effective.+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~MAC_INT_TX);++napi_complete(napi);+return0;+}++irqreturn_tethernet_interrupt(intirq,void*dev_id)+{+structnet_device*ndev;+structsp_mac*mac;+structsp_common*comm;+u32status;++ndev=(structnet_device*)dev_id;+if(unlikely(!ndev)){+netdev_err(ndev,"ndev is null!\n");+gotoOUT;+}++mac=netdev_priv(ndev);+comm=mac->comm;++status=read_sw_int_status0(mac);+if(unlikely(status==0)){+netdev_err(ndev,"Interrput status is null!\n");+gotoOUT;+}+write_sw_int_status0(mac,status);++if(status&MAC_INT_RX){+// Disable RX interrupts.+write_sw_int_mask0(mac,read_sw_int_mask0(mac)|MAC_INT_RX);++if(unlikely(status&MAC_INT_RX_DES_ERR)){+netdev_err(ndev,"Illegal RX Descriptor!\n");+mac->dev_stats.rx_fifo_errors++;+}+if(napi_schedule_prep(&comm->rx_napi))+__napi_schedule(&comm->rx_napi);+}++if(status&MAC_INT_TX){+// Disable TX interrupts.+write_sw_int_mask0(mac,read_sw_int_mask0(mac)|MAC_INT_TX);++if(unlikely(status&MAC_INT_TX_DES_ERR)){+netdev_err(ndev,"Illegal TX Descriptor Error\n");+mac->dev_stats.tx_fifo_errors++;+mac_soft_reset(mac);+wmb();// make sure settings are effective.+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~MAC_INT_TX);+}else{+if(napi_schedule_prep(&comm->tx_napi))+__napi_schedule(&comm->tx_napi);+}+}++if(status&MAC_INT_PORT_ST_CHG)/* link status changed */+port_status_change(mac);++OUT:+returnIRQ_HANDLED;+}
@@ -0,0 +1,90 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_mdio.h"++u32mdio_read(structsp_mac*mac,u32phy_id,u16regnum)+{+intret;++ret=hal_mdio_access(mac,MDIO_READ_CMD,phy_id,regnum,0);+if(ret<0)+return-EOPNOTSUPP;++returnret;+}++u32mdio_write(structsp_mac*mac,u32phy_id,u32regnum,u16val)+{+intret;++ret=hal_mdio_access(mac,MDIO_WRITE_CMD,phy_id,regnum,val);+if(ret<0)+return-EOPNOTSUPP;++return0;+}++staticintmii_read(structmii_bus*bus,intphy_id,intregnum)+{+structsp_mac*mac=bus->priv;++returnmdio_read(mac,phy_id,regnum);+}++staticintmii_write(structmii_bus*bus,intphy_id,intregnum,u16val)+{+structsp_mac*mac=bus->priv;++returnmdio_write(mac,phy_id,regnum,val);+}++u32mdio_init(structplatform_device*pdev,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structmii_bus*mii_bus;+structdevice_node*mdio_node;+intret;++mii_bus=mdiobus_alloc();+if(!mii_bus){+netdev_err(ndev,"Failed to allocate mdio_bus memory!\n");+return-ENOMEM;+}++mii_bus->name="sunplus_mii_bus";+mii_bus->parent=&pdev->dev;+mii_bus->priv=mac;+mii_bus->read=mii_read;+mii_bus->write=mii_write;+snprintf(mii_bus->id,MII_BUS_ID_SIZE,"%s-mii",dev_name(&pdev->dev));++mdio_node=of_get_parent(mac->phy_node);+if(!mdio_node){+netdev_err(ndev,"Failed to get mdio_node!\n");+return-ENODATA;+}++ret=of_mdiobus_register(mii_bus,mdio_node);+if(ret){+netdev_err(ndev,"Failed to register mii bus!\n");+mdiobus_free(mii_bus);+returnret;+}++mac->comm->mii_bus=mii_bus;+returnret;+}++voidmdio_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->comm->mii_bus){+mdiobus_unregister(mac->comm->mii_bus);+mdiobus_free(mac->comm->mii_bus);+mac->comm->mii_bus=NULL;+}+}
@@ -0,0 +1,64 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_phy.h"+#include"sp_mdio.h"++staticvoidmii_linkchange(structnet_device*netdev)+{+}++intsp_phy_probe(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structphy_device*phydev;+inti;++phydev=of_phy_connect(ndev,mac->phy_node,mii_linkchange,+0,mac->phy_mode);+if(!phydev){+netdev_err(ndev,"\"%s\" failed to connect to phy!\n",ndev->name);+return-ENODEV;+}++for(i=0;i<sizeof(phydev->supported)/sizeof(long);i++)+phydev->advertising[i]=phydev->supported[i];++phydev->irq=PHY_MAC_INTERRUPT;+mac->phy_dev=phydev;++// Bug workaround:+// Flow-control of phy should be enabled. MAC flow-control will refer+// to the bit to decide to enable or disable flow-control.+mdio_write(mac,mac->phy_addr,4,mdio_read(mac,mac->phy_addr,4)|(1<<10));++return0;+}++voidsp_phy_start(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_start(mac->phy_dev);+}++voidsp_phy_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_stop(mac->phy_dev);+}++voidsp_phy_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev){+phy_disconnect(mac->phy_dev);+mac->phy_dev=NULL;+}+}
@@ -0,0 +1,231 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_desc.h"+#include"sp_define.h"++voidrx_descs_flush(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].addr1=rx_skbinfo[j].mapping;+rx_desc[j].cmd2=(j==comm->rx_desc_num[i]-1)?+EOR_BIT|comm->rx_desc_buff_size:+comm->rx_desc_buff_size;+wmb();// Set OWN_BIT after other fields are ready.+rx_desc[j].cmd1=OWN_BIT;+}+}+}++voidtx_descs_clean(structsp_common*comm)+{+u32i;+s32buflen;++if(!comm->tx_desc)+return;++for(i=0;i<TX_DESC_NUM;i++){+comm->tx_desc[i].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+comm->tx_desc[i].cmd2=0;+comm->tx_desc[i].addr1=0;+comm->tx_desc[i].addr2=0;++if(comm->tx_temp_skb_info[i].mapping){+buflen=(comm->tx_temp_skb_info[i].skb)?+comm->tx_temp_skb_info[i].skb->len:+MAC_TX_BUFF_SIZE;+dma_unmap_single(&comm->pdev->dev,comm->tx_temp_skb_info[i].mapping,+buflen,DMA_TO_DEVICE);+comm->tx_temp_skb_info[i].mapping=0;+}++if(comm->tx_temp_skb_info[i].skb){+dev_kfree_skb(comm->tx_temp_skb_info[i].skb);+comm->tx_temp_skb_info[i].skb=NULL;+}+}+}++voidrx_descs_clean(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+if(!comm->rx_skb_info[i])+continue;++rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+rx_desc[j].cmd2=0;+rx_desc[j].addr1=0;++if(rx_skbinfo[j].skb){+dma_unmap_single(&comm->pdev->dev,rx_skbinfo[j].mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);+dev_kfree_skb(rx_skbinfo[j].skb);+rx_skbinfo[j].skb=NULL;+rx_skbinfo[j].mapping=0;+}+}++kfree(rx_skbinfo);+comm->rx_skb_info[i]=NULL;+}+}++voiddescs_clean(structsp_common*comm)+{+rx_descs_clean(comm);+tx_descs_clean(comm);+}++voiddescs_free(structsp_common*comm)+{+u32i;++descs_clean(comm);+comm->tx_desc=NULL;+for(i=0;i<RX_DESC_QUEUE_NUM;i++)+comm->rx_desc[i]=NULL;++/* Free descriptor area */+if(comm->desc_base){+dma_free_coherent(&comm->pdev->dev,comm->desc_size,comm->desc_base,+comm->desc_dma);+comm->desc_base=NULL;+comm->desc_dma=0;+comm->desc_size=0;+}+}++voidtx_descs_init(structsp_common*comm)+{+memset(comm->tx_desc,'\0',sizeof(structmac_desc)*+(TX_DESC_NUM+MAC_GUARD_DESC_NUM));+}++intrx_descs_init(structsp_common*comm)+{+structsk_buff*skb;+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+comm->rx_skb_info[i]=kmalloc_array(comm->rx_desc_num[i],+sizeof(structskb_info),GFP_KERNEL);+if(!comm->rx_skb_info[i])+gotoMEM_ALLOC_FAIL;++rx_skbinfo=comm->rx_skb_info[i];+rx_desc=comm->rx_desc[i];+for(j=0;j<comm->rx_desc_num[i];j++){+skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(!skb)+gotoMEM_ALLOC_FAIL;++skb->dev=comm->ndev;+skb_reserve(skb,RX_OFFSET);/* +data +tail */++rx_skbinfo[j].skb=skb;+rx_skbinfo[j].mapping=dma_map_single(&comm->pdev->dev,skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);
@@ -0,0 +1,606 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/clk.h>+#include<linux/reset.h>+#include<linux/nvmem-consumer.h>+#include<linux/of_net.h>+#include"sp_driver.h"+#include"sp_phy.h"++staticconstchardef_mac_addr[ETHERNET_MAC_ADDR_LEN]={+0xfc,0x4b,0xbc,0x00,0x00,0x00+};++/*********************************************************************+*+*net_device_ops+*+**********************************************************************/+staticintethernet_open(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netdev_dbg(ndev,"Open port = %x\n",mac->lan_port);++mac->comm->enable|=mac->lan_port;++hal_mac_start(mac);+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~(MAC_INT_TX|MAC_INT_RX));++netif_carrier_on(ndev);+if(netif_carrier_ok(ndev))+netif_start_queue(ndev);++return0;+}++staticintethernet_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netif_stop_queue(ndev);+netif_carrier_off(ndev);++mac->comm->enable&=~mac->lan_port;++hal_mac_stop(mac);++return0;+}++/* Transmit a packet (called by the kernel) */+staticintethernet_start_xmit(structsk_buff*skb,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structsp_common*comm=mac->comm;+u32tx_pos;+u32cmd1;+u32cmd2;+structmac_desc*txdesc;+structskb_info*skbinfo;+unsignedlongflags;++if(unlikely(comm->tx_desc_full==1)){+// No TX descriptors left. Wait for tx interrupt.+netdev_info(ndev,"TX descriptor queue full when xmit!\n");+returnNETDEV_TX_BUSY;
@@ -0,0 +1,331 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/iopoll.h>+#include"sp_hal.h"++voidhal_mac_stop(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg,disable;++if(comm->enable==0){+// Mask and clear all interrupts, except PORT_ST_CHG.+write_sw_int_mask0(mac,0xffffffff);+writel(0xffffffff&(~MAC_INT_PORT_ST_CHG),+comm->sp_reg_base+SP_SW_INT_STATUS_0);++// Disable cpu 0 and cpu 1.+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((0x3<<6)|reg,comm->sp_reg_base+SP_CPU_CNTL);+}++// Disable lan 0 and lan 1.+disable=((~comm->enable)&0x3)<<24;+reg=readl(comm->sp_reg_base+SP_PORT_CNTL0);+writel(disable|reg,comm->sp_reg_base+SP_PORT_CNTL0);+}++voidhal_mac_reset(structsp_mac*mac)+{+}
@@ -0,0 +1,286 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_define.h"+#include"sp_int.h"+#include"sp_driver.h"+#include"sp_hal.h"++staticvoidport_status_change(structsp_mac*mac)+{+u32reg;+structnet_device*ndev=mac->ndev;++reg=read_port_ability(mac);+if(!netif_carrier_ok(ndev)&&(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_on(ndev);+netif_start_queue(ndev);+}elseif(netif_carrier_ok(ndev)&&!(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_off(ndev);+netif_stop_queue(ndev);+}++if(mac->next_ndev){+structnet_device*ndev2=mac->next_ndev;++if(!netif_carrier_ok(ndev2)&&(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_on(ndev2);+netif_start_queue(ndev2);+}elseif(netif_carrier_ok(ndev2)&&!(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_off(ndev2);+netif_stop_queue(ndev2);+}+}+}++staticvoidrx_skb(structsp_mac*mac,structsk_buff*skb)+{+mac->dev_stats.rx_packets++;+mac->dev_stats.rx_bytes+=skb->len;+netif_receive_skb(skb);+}++intrx_poll(structnapi_struct*napi,intbudget)+{+structsp_common*comm=container_of(napi,structsp_common,rx_napi);+structsp_mac*mac=netdev_priv(comm->ndev);+structsk_buff*skb,*new_skb;+structskb_info*sinfo;+structmac_desc*desc;+structmac_desc*h_desc;+u32rx_pos,pkg_len;+u32cmd;+u32num,rx_count;+s32queue;+intndev2_pkt;+structnet_device_stats*dev_stats;++spin_lock(&comm->rx_lock);++// Process high-priority queue and then low-priority queue.+for(queue=0;queue<RX_DESC_QUEUE_NUM;queue++){+rx_pos=comm->rx_pos[queue];+rx_count=comm->rx_desc_num[queue];++for(num=0;num<rx_count;num++){+sinfo=comm->rx_skb_info[queue]+rx_pos;+desc=comm->rx_desc[queue]+rx_pos;+cmd=desc->cmd1;++if(cmd&OWN_BIT)+break;++if((cmd&PKTSP_MASK)==PKTSP_PORT1){+structsp_mac*mac2;++ndev2_pkt=1;+mac2=(mac->next_ndev)?netdev_priv(mac->next_ndev):NULL;+dev_stats=(mac2)?&mac2->dev_stats:&mac->dev_stats;+}else{+ndev2_pkt=0;+dev_stats=&mac->dev_stats;+}++pkg_len=cmd&LEN_MASK;+if(unlikely((cmd&ERR_CODE)||(pkg_len<64))){+dev_stats->rx_length_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++if(unlikely(cmd&RX_IP_CHKSUM_BIT)){+dev_stats->rx_crc_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++// Allocate an skbuff for receiving.+new_skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(unlikely(!new_skb)){+dev_stats->rx_dropped++;+gotoNEXT;+}+new_skb->dev=mac->ndev;++dma_unmap_single(&comm->pdev->dev,sinfo->mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);++skb=sinfo->skb;+skb->ip_summed=CHECKSUM_NONE;++/*skb_put will judge if tail exceeds end, but __skb_put won't */+__skb_put(skb,(pkg_len-4>comm->rx_desc_buff_size)?+comm->rx_desc_buff_size:pkg_len-4);++sinfo->mapping=dma_map_single(&comm->pdev->dev,new_skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);
@@ -0,0 +1,90 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_mdio.h"++u32mdio_read(structsp_mac*mac,u32phy_id,u16regnum)+{+intret;++ret=hal_mdio_access(mac,MDIO_READ_CMD,phy_id,regnum,0);+if(ret<0)+return-EOPNOTSUPP;++returnret;+}++u32mdio_write(structsp_mac*mac,u32phy_id,u32regnum,u16val)+{+intret;++ret=hal_mdio_access(mac,MDIO_WRITE_CMD,phy_id,regnum,val);+if(ret<0)+return-EOPNOTSUPP;++return0;+}++staticintmii_read(structmii_bus*bus,intphy_id,intregnum)+{+structsp_mac*mac=bus->priv;++returnmdio_read(mac,phy_id,regnum);+}++staticintmii_write(structmii_bus*bus,intphy_id,intregnum,u16val)+{+structsp_mac*mac=bus->priv;++returnmdio_write(mac,phy_id,regnum,val);+}++u32mdio_init(structplatform_device*pdev,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structmii_bus*mii_bus;+structdevice_node*mdio_node;+intret;++mii_bus=mdiobus_alloc();+if(!mii_bus){+netdev_err(ndev,"Failed to allocate mdio_bus memory!\n");+return-ENOMEM;+}++mii_bus->name="sunplus_mii_bus";+mii_bus->parent=&pdev->dev;+mii_bus->priv=mac;+mii_bus->read=mii_read;+mii_bus->write=mii_write;+snprintf(mii_bus->id,MII_BUS_ID_SIZE,"%s-mii",dev_name(&pdev->dev));++mdio_node=of_get_parent(mac->phy_node);+if(!mdio_node){+netdev_err(ndev,"Failed to get mdio_node!\n");+return-ENODATA;+}++ret=of_mdiobus_register(mii_bus,mdio_node);+if(ret){+netdev_err(ndev,"Failed to register mii bus!\n");+mdiobus_free(mii_bus);+returnret;+}++mac->comm->mii_bus=mii_bus;+returnret;+}++voidmdio_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->comm->mii_bus){+mdiobus_unregister(mac->comm->mii_bus);+mdiobus_free(mac->comm->mii_bus);+mac->comm->mii_bus=NULL;+}+}
@@ -0,0 +1,64 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_phy.h"+#include"sp_mdio.h"++staticvoidmii_linkchange(structnet_device*netdev)+{+}++intsp_phy_probe(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structphy_device*phydev;+inti;++phydev=of_phy_connect(ndev,mac->phy_node,mii_linkchange,+0,mac->phy_mode);+if(!phydev){+netdev_err(ndev,"\"%s\" failed to connect to phy!\n",ndev->name);+return-ENODEV;+}++for(i=0;i<sizeof(phydev->supported)/sizeof(long);i++)+phydev->advertising[i]=phydev->supported[i];++phydev->irq=PHY_MAC_INTERRUPT;+mac->phy_dev=phydev;++// Bug workaround:+// Flow-control of phy should be enabled. MAC flow-control will refer+// to the bit to decide to enable or disable flow-control.+mdio_write(mac,mac->phy_addr,4,mdio_read(mac,mac->phy_addr,4)|(1<<10));++return0;+}++voidsp_phy_start(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_start(mac->phy_dev);+}++voidsp_phy_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_stop(mac->phy_dev);+}++voidsp_phy_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev){+phy_disconnect(mac->phy_dev);+mac->phy_dev=NULL;+}+}
From: Rob Herring <robh@kernel.org> Date: 2021-11-11 14:58:14
On Thu, 11 Nov 2021 17:04:20 +0800, Wells Lu wrote:
Add bindings documentation for Sunplus SP7021.
Signed-off-by: Wells Lu <redacted>
---
Changes in V2
- Added mdio and phy sub-nodes.
.../bindings/net/sunplus,sp7021-emac.yaml | 152 +++++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 159 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml:94:12: [warning] wrong indentation: expected 10 but found 11 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1553831
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
I would suggest you structure this differently to make it clear it is
a two port switch:
ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
phy-handle = <ð_phy0>;
phy-mode = "rmii";
}
port@1 {
reg = <1>;
phy-handle = <ð_phy1>;
phy-mode = "rmii";
}
}
Andrew
I would suggest you structure this differently to make it clear it is a two port switch:
ethernet-ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
phy-handle = <ð_phy0>;
phy-mode = "rmii";
}
port@1 {
reg = <1>;
phy-handle = <ð_phy1>;
phy-mode = "rmii";
}
}
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-12 02:58:05
Hi,
On Thu, 11 Nov 2021 17:04:20 +0800, Wells Lu wrote:
quoted
Add bindings documentation for Sunplus SP7021.
Signed-off-by: Wells Lu <redacted>
---
Changes in V2
- Added mdio and phy sub-nodes.
.../bindings/net/sunplus,sp7021-emac.yaml | 152 +++++++++++++++++++++
MAINTAINERS | 7 +
2 files changed, 159 insertions(+)
create mode 100644
Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml:94:12: [warning] wrong
indentation: expected 10 but found 11 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1553831
This check can fail if there are any dependencies. The base for a patch series is generally the most
recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure
'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Thank you very much for review and tests.
I'll fix wrong indentation next patch.
You should not be exposing these functions, if you do, that means
another part of your code performs MDIO bus read/write operations
without using the appropriate layer, so no.
+
+static int mii_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+ struct sp_mac *mac = bus->priv;
+
+ return mdio_read(mac, phy_id, regnum);
+}
+
+static int mii_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+ struct sp_mac *mac = bus->priv;
+
+ return mdio_write(mac, phy_id, regnum, val);
+}
+
+u32 mdio_init(struct platform_device *pdev, struct net_device *ndev)
Those function names need to be prefixed with sp_ to denote the driver
local scope, this applies for your entire patch set.
[snip]
Please scope your functions better, and name them sp_mdio_read, etc.
because mdio_read() is way too generic. Also, can you please follow the
same prototype as what include/linux/mdio.h has for the mdiobus->read
and ->write calls, that is phy_id is int, regnum is u32, etc.
Does your MAC fully auto-configure based on the PHY's link parameters,
if so, how does it do it? You most certainly need to act on duplex
changes, or speed changes no?
+
+int sp_phy_probe(struct net_device *ndev)
+{
+ struct sp_mac *mac = netdev_priv(ndev);
+ struct phy_device *phydev;
+ int i;
+
+ phydev = of_phy_connect(ndev, mac->phy_node, mii_linkchange,
+ 0, mac->phy_mode);
+ if (!phydev) {
+ netdev_err(ndev, "\"%s\" failed to connect to phy!\n", ndev->name);
+ return -ENODEV;
+ }
+
+ for (i = 0; i < sizeof(phydev->supported) / sizeof(long); i++)
+ phydev->advertising[i] = phydev->supported[i];
+
+ phydev->irq = PHY_MAC_INTERRUPT;
+ mac->phy_dev = phydev;
+
+ // Bug workaround:
+ // Flow-control of phy should be enabled. MAC flow-control will refer
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac, mac->phy_addr, 4, mdio_read(mac, mac->phy_addr, 4) | (1 << 10));
This is a layering violation, and you should not be doing those things
here, if you need to advertise flow control, then please set
ADVERTISE_PAUSE_CAP and/or ADVERTISE_PAUSE_ASYM accordingly, see whether
phy_set_asym_pause() can do what you need it to.
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-12 23:24:55
Hi Florian
You are basically pointing out issues i already pointed out in
previous versions, and have been ignored :-(
Wells, please look at the comments i made on your earlier
versions. Those comments are still valid and need addressing.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-12 23:59:04
+void rx_descs_flush(struct sp_common *comm)
As both Florian and I have said, you need a prefix for all your
functions, structures, etc. sp_ is not the best prefix either, it is
not very unique. spl2sw_ would be better.
I suggest you avoid any references to hal. It makes people think you
have ported a driver from some other operating system and then put a
layer of code on top of it. That is not how you do it in Linux. This
is a Linux driver, nothing else.
Is this actually adding an entry into the address translation table?
If so, make this clear in the function name. You are not setting the
MAC address, you are just adding a static forwarding entry.
@@ -0,0 +1,212 @@+/* SPDX-License-Identifier: GPL-2.0 */+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#ifndef __SP_DEFINE_H__+#define __SP_DEFINE_H__++#include<linux/module.h>+#include<linux/init.h>+#include<linux/sched.h>+#include<linux/kernel.h>+#include<linux/slab.h>+#include<linux/errno.h>+#include<linux/types.h>+#include<linux/interrupt.h>+#include<linux/kdev_t.h>+#include<linux/in.h>+#include<linux/netdevice.h>+#include<linux/etherdevice.h>+#include<linux/ip.h>+#include<linux/tcp.h>+#include<linux/skbuff.h>+#include<linux/ethtool.h>+#include<linux/platform_device.h>+#include<linux/phy.h>+#include<linux/mii.h>+#include<linux/if_vlan.h>+#include<linux/io.h>+#include<linux/dma-mapping.h>+#include<linux/of_address.h>+#include<linux/of_mdio.h>++//define MAC interrupt status bit
please embrace all comments with /* */
Do you mean to modify comment, for example,
//define MAC interrupt status bit
to
/* define MAC interrupt status bit */
for all commets in .h and .c files?
@@ -0,0 +1,231 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_desc.h"+#include"sp_define.h"++voidrx_descs_flush(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].addr1=rx_skbinfo[j].mapping;+rx_desc[j].cmd2=(j==comm->rx_desc_num[i]-1)?+EOR_BIT|comm->rx_desc_buff_size:+comm->rx_desc_buff_size;+wmb();// Set OWN_BIT after other fields are ready.+rx_desc[j].cmd1=OWN_BIT;+}+}+}++voidtx_descs_clean(structsp_common*comm)+{+u32i;+s32buflen;++if(!comm->tx_desc)+return;++for(i=0;i<TX_DESC_NUM;i++){+comm->tx_desc[i].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+comm->tx_desc[i].cmd2=0;+comm->tx_desc[i].addr1=0;+comm->tx_desc[i].addr2=0;++if(comm->tx_temp_skb_info[i].mapping){+buflen=(comm->tx_temp_skb_info[i].skb)?+comm->tx_temp_skb_info[i].skb->len:+MAC_TX_BUFF_SIZE;+dma_unmap_single(&comm->pdev->dev,comm->tx_temp_skb_info[i].mapping,+buflen,DMA_TO_DEVICE);+comm->tx_temp_skb_info[i].mapping=0;+}++if(comm->tx_temp_skb_info[i].skb){+dev_kfree_skb(comm->tx_temp_skb_info[i].skb);+comm->tx_temp_skb_info[i].skb=NULL;+}+}+}++voidrx_descs_clean(structsp_common*comm)+{+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+if(!comm->rx_skb_info[i])+continue;++rx_desc=comm->rx_desc[i];+rx_skbinfo=comm->rx_skb_info[i];+for(j=0;j<comm->rx_desc_num[i];j++){+rx_desc[j].cmd1=0;+wmb();// Clear OWN_BIT and then set other fields.+rx_desc[j].cmd2=0;+rx_desc[j].addr1=0;++if(rx_skbinfo[j].skb){+dma_unmap_single(&comm->pdev->dev,rx_skbinfo[j].mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);+dev_kfree_skb(rx_skbinfo[j].skb);+rx_skbinfo[j].skb=NULL;+rx_skbinfo[j].mapping=0;+}+}++kfree(rx_skbinfo);+comm->rx_skb_info[i]=NULL;+}+}++voiddescs_clean(structsp_common*comm)+{+rx_descs_clean(comm);+tx_descs_clean(comm);+}++voiddescs_free(structsp_common*comm)+{+u32i;++descs_clean(comm);+comm->tx_desc=NULL;+for(i=0;i<RX_DESC_QUEUE_NUM;i++)+comm->rx_desc[i]=NULL;++/* Free descriptor area */+if(comm->desc_base){+dma_free_coherent(&comm->pdev->dev,comm->desc_size,comm->desc_base,+comm->desc_dma);+comm->desc_base=NULL;+comm->desc_dma=0;+comm->desc_size=0;+}+}++voidtx_descs_init(structsp_common*comm)+{+memset(comm->tx_desc,'\0',sizeof(structmac_desc)*+(TX_DESC_NUM+MAC_GUARD_DESC_NUM));+}++intrx_descs_init(structsp_common*comm)+{+structsk_buff*skb;+u32i,j;+structmac_desc*rx_desc;+structskb_info*rx_skbinfo;++for(i=0;i<RX_DESC_QUEUE_NUM;i++){+comm->rx_skb_info[i]=kmalloc_array(comm->rx_desc_num[i],+sizeof(structskb_info),GFP_KERNEL);+if(!comm->rx_skb_info[i])+gotoMEM_ALLOC_FAIL;++rx_skbinfo=comm->rx_skb_info[i];+rx_desc=comm->rx_desc[i];+for(j=0;j<comm->rx_desc_num[i];j++){+skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(!skb)+gotoMEM_ALLOC_FAIL;++skb->dev=comm->ndev;+skb_reserve(skb,RX_OFFSET);/* +data +tail */++rx_skbinfo[j].skb=skb;+rx_skbinfo[j].mapping=dma_map_single(&comm->pdev->dev,skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);
it may fail
Yes, I'll add error check in next patch as shown below:
rx_skbinfo[j].mapping = dma_map_single(&comm->pdev->dev, skb->data,
comm->rx_desc_buff_size,
DMA_FROM_DEVICE);
if (dma_mapping_error(&comm->pdev->dev, rx_skbinfo[j].mapping))
goto mem_alloc_fail;
[...]
Return 0;
mem_alloc_fail:
rx_desc_clean(comm);
return -ENOMEM;
}
@@ -0,0 +1,606 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/clk.h>+#include<linux/reset.h>+#include<linux/nvmem-consumer.h>+#include<linux/of_net.h>+#include"sp_driver.h"+#include"sp_phy.h"++staticconstchardef_mac_addr[ETHERNET_MAC_ADDR_LEN]={+0xfc,0x4b,0xbc,0x00,0x00,0x00+};++/*********************************************************************+*+*net_device_ops+*+**********************************************************************/+staticintethernet_open(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netdev_dbg(ndev,"Open port = %x\n",mac->lan_port);++mac->comm->enable|=mac->lan_port;++hal_mac_start(mac);+write_sw_int_mask0(mac,read_sw_int_mask0(mac)&~(MAC_INT_TX|MAC_INT_RX));++netif_carrier_on(ndev);+if(netif_carrier_ok(ndev))+netif_start_queue(ndev);++return0;+}++staticintethernet_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++netif_stop_queue(ndev);+netif_carrier_off(ndev);++mac->comm->enable&=~mac->lan_port;++hal_mac_stop(mac);++return0;+}++/* Transmit a packet (called by the kernel) */+staticintethernet_start_xmit(structsk_buff*skb,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structsp_common*comm=mac->comm;+u32tx_pos;+u32cmd1;+u32cmd2;+structmac_desc*txdesc;+structskb_info*skbinfo;+unsignedlongflags;++if(unlikely(comm->tx_desc_full==1)){+// No TX descriptors left. Wait for tx interrupt.+netdev_info(ndev,"TX descriptor queue full when xmit!\n");+returnNETDEV_TX_BUSY;
Do you really have to return NETDEV_TX_BUSY?
(tx_desc_full == 1) means there is no TX descriptor left in ring buffer.
So there is no way to do new transmit. Return 'busy' directly.
I am not sure if this is a correct process or not.
Could you please teach is there any other way to take care of this case?
Drop directly?
I am not sure the meaning of this comment.
Do you mean to modify:
// Initialize the 1st net device.
ret = init_netdev(pdev, 0, &ndev);
if (!ndev)
return ret;
to:
// Initialize the 1st net device.
ret = init_netdev(pdev, 0, &ndev);
if (ret)
return ret;
?
quoted
+ }
+ SET_NETDEV_DEV(ndev, &pdev->dev);
+ ndev->netdev_ops = &netdev_ops;
+
+ mac = netdev_priv(ndev);
+ mac->ndev = ndev;
+ mac->next_ndev = NULL;
+
+ // Get property 'mac-addr0' or 'mac-addr1' from dts.
+ otp_v = sp7021_otp_read_mac(&pdev->dev, &otp_l, m_addr_name);
+ if ((otp_l < 6) || IS_ERR_OR_NULL(otp_v)) {
+ dev_info(&pdev->dev, "OTP mac %s (len = %zd) is invalid, using default!\n",
+ m_addr_name, otp_l);
+ otp_l = 0;
+ } else {
+ // Check if mac-address is valid or not. If not, copy from default.
+ memcpy(mac->mac_addr, otp_v, 6);
+
+ // Byte order of Some samples are reversed. Convert byte order here.
+ check_mac_vendor_id_and_convert(mac->mac_addr);
+
+ if (!is_valid_ether_addr(mac->mac_addr)) {
+ dev_info(&pdev->dev, "Invalid mac in OTP[%s] = %pM, use default!\n",
+ m_addr_name, mac->mac_addr);
+ otp_l = 0;
+ }
+ }
+ if (otp_l != 6) {
+ memcpy(mac->mac_addr, def_mac_addr, ETHERNET_MAC_ADDR_LEN);
+ mac->mac_addr[5] += eth_no;
+ }
+
+ dev_info(&pdev->dev, "HW Addr = %pM\n", mac->mac_addr);
+
+ memcpy(ndev->dev_addr, mac->mac_addr, ETHERNET_MAC_ADDR_LEN);
+
+ ret = register_netdev(ndev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register net device \"%s\"!\n",
+ ndev->name);
+ free_netdev(ndev);
+ *r_ndev = NULL;
+ return ret;
+ }
+ netdev_info(ndev, "Registered net device \"%s\" successfully.\n", ndev->name);
+
+ *r_ndev = ndev;
+ return 0;
+}
+
+static int soc0_open(struct sp_mac *mac)
+{
+ struct sp_common *comm = mac->comm;
+ u32 ret;
+
+ hal_mac_stop(mac);
+
+ ret = descs_init(comm);
+ if (ret) {
+ netdev_err(mac->ndev, "Fail to initialize mac descriptors!\n");
+ descs_free(comm);
+ return ret;
+ }
+
+ mac_init(mac);
+ return 0;
+}
+
+static int soc0_stop(struct sp_mac *mac)
+{
+ hal_mac_stop(mac);
+
+ descs_free(mac->comm);
+ return 0;
+}
+
+static int sp_probe(struct platform_device *pdev)
+{
+ struct sp_common *comm;
+ struct resource *rc;
+ struct net_device *ndev, *ndev2;
+ struct device_node *np;
+ struct sp_mac *mac, *mac2;
+ int ret;
+
+ if (platform_get_drvdata(pdev))
+ return -ENODEV;
+
+ // Allocate memory for 'sp_common' area.
+ comm = devm_kzalloc(&pdev->dev, sizeof(*comm), GFP_KERNEL);
+ if (!comm)
+ return -ENOMEM;
+ comm->pdev = pdev;
+
+ spin_lock_init(&comm->rx_lock);
+ spin_lock_init(&comm->tx_lock);
+ spin_lock_init(&comm->ioctl_lock);
+
+ // Get memory resoruce "emac" from dts.
+ rc = platform_get_resource_byname(pdev, IORESOURCE_MEM, "emac");
+ if (!rc) {
+ dev_err(&pdev->dev, "No MEM resource \'emac\' found!\n");
+ return -ENXIO;
+ }
+ dev_dbg(&pdev->dev, "name = \"%s\", start = %pa\n", rc->name, &rc->start);
+
+ comm->sp_reg_base = devm_ioremap_resource(&pdev->dev, rc);
+ if (IS_ERR(comm->sp_reg_base)) {
+ dev_err(&pdev->dev, "ioremap failed!\n");
+ return -ENOMEM;
+ }
+
+ // Get memory resoruce "moon5" from dts.
+ rc = platform_get_resource_byname(pdev, IORESOURCE_MEM, "moon5");
+ if (!rc) {
+ dev_err(&pdev->dev, "No MEM resource \'moon5\' found!\n");
+ return -ENXIO;
+ }
+ dev_dbg(&pdev->dev, "name = \"%s\", start = %pa\n", rc->name, &rc->start);
+
+ // Note that moon5 is shared resource. Don't use devm_ioremap_resource().
+ comm->moon5_reg_base = devm_ioremap(&pdev->dev, rc->start, rc->end - rc->start + 1);
+ if (IS_ERR(comm->moon5_reg_base)) {
+ dev_err(&pdev->dev, "ioremap failed!\n");
+ return -ENOMEM;
+ }
+
+ // Get irq resource from dts.
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0)
+ return ret;
+ comm->irq = ret;
+
+ // Get clock controller.
+ comm->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(comm->clk)) {
+ dev_err_probe(&pdev->dev, PTR_ERR(comm->clk),
+ "Failed to retrieve clock controller!\n");
+ return PTR_ERR(comm->clk);
+ }
+
+ // Get reset controller.
+ comm->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(comm->rstc)) {
+ dev_err_probe(&pdev->dev, PTR_ERR(comm->rstc),
+ "Failed to retrieve reset controller!\n");
+ return PTR_ERR(comm->rstc);
+ }
+
+ // Enable clock.
+ clk_prepare_enable(comm->clk);
+ udelay(1);
+
+ reset_control_assert(comm->rstc);
+ udelay(1);
+ reset_control_deassert(comm->rstc);
+ udelay(1);
+
+ // Initialize the 1st net device.
+ ret = init_netdev(pdev, 0, &ndev);
+ if (!ndev)
+ return ret;
+
+ platform_set_drvdata(pdev, ndev);
+
+ ndev->irq = comm->irq;
+ mac = netdev_priv(ndev);
+ mac->comm = comm;
+ comm->ndev = ndev;
+
+ // Get node of phy 1.
+ mac->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle1", 0);
+ if (!mac->phy_node) {
+ netdev_info(ndev, "Cannot get node of phy 1!\n");
+ ret = -ENODEV;
+ goto out_unregister_dev;
+ }
+
+ // Get address of phy from dts.
+ if (of_property_read_u32(mac->phy_node, "reg", &mac->phy_addr)) {
+ mac->phy_addr = 0;
+ netdev_info(ndev, "Cannot get address of phy 1! Set to 0.\n");
+ }
+
+ // Get mode of phy from dts.
+ if (of_get_phy_mode(mac->phy_node, &mac->phy_mode)) {
+ mac->phy_mode = PHY_INTERFACE_MODE_RGMII_ID;
+ netdev_info(ndev, "Missing phy-mode of phy 1! Set to \'rgmii-id\'.\n");
+ }
+
+ // Request irq.
+ ret = devm_request_irq(&pdev->dev, comm->irq, ethernet_interrupt, 0,
+ ndev->name, ndev);
+ if (ret) {
+ netdev_err(ndev, "Failed to request irq #%d for \"%s\"!\n",
+ ndev->irq, ndev->name);
+ goto out_unregister_dev;
+ }
+
+ mac->cpu_port = 0x1; // soc0
+ mac->lan_port = 0x1; // forward to port 0
+ mac->to_vlan = 0x1; // vlan group: 0
+ mac->vlan_id = 0x0; // vlan group: 0
+
+ // Set MAC address
+ hal_mac_addr_set(mac);
+ hal_rx_mode_set(ndev);
+ hal_mac_addr_table_del_all(mac);
+
+ ndev2 = NULL;
+ np = of_parse_phandle(pdev->dev.of_node, "phy-handle2", 0);
+ if (np) {
+ init_netdev(pdev, 1, &ndev2);
+ if (ndev2) {
+ mac->next_ndev = ndev2; // Point to the second net device.
+
+ ndev2->irq = comm->irq;
+ mac2 = netdev_priv(ndev2);
+ mac2->comm = comm;
+ mac2->phy_node = np;
+
+ if (of_property_read_u32(mac2->phy_node, "reg", &mac2->phy_addr)) {
+ mac2->phy_addr = 1;
+ netdev_info(ndev2, "Cannot get address of phy 2! Set to 1.\n");
+ }
+
+ if (of_get_phy_mode(mac2->phy_node, &mac2->phy_mode)) {
+ mac2->phy_mode = PHY_INTERFACE_MODE_RGMII_ID;
+ netdev_info(ndev, "Missing phy-mode phy 2! Set to \'rgmii-id\'.\n");
+ }
+
+ mac2->cpu_port = 0x1; // soc0
+ mac2->lan_port = 0x2; // forward to port 1
+ mac2->to_vlan = 0x2; // vlan group: 1
+ mac2->vlan_id = 0x1; // vlan group: 1
+
+ hal_mac_addr_set(mac2); // Set MAC address for the 2nd net device.
+ hal_rx_mode_set(ndev2);
+ }
+ }
+
+ soc0_open(mac);
+ hal_set_rmii_tx_rx_pol(mac);
+ hal_phy_addr(mac);
+
+ ret = mdio_init(pdev, ndev);
+ if (ret) {
+ netdev_err(ndev, "Failed to initialize mdio!\n");
+ goto out_unregister_dev;
+ }
+
+ ret = sp_phy_probe(ndev);
+ if (ret) {
+ netdev_err(ndev, "Failed to probe phy!\n");
+ goto out_freemdio;
+ }
+
+ if (ndev2) {
+ ret = sp_phy_probe(ndev2);
+ if (ret) {
+ netdev_err(ndev2, "Failed to probe phy!\n");
+ unregister_netdev(ndev2);
+ mac->next_ndev = 0;
+ }
+ }
+
+ netif_napi_add(ndev, &comm->rx_napi, rx_poll, RX_NAPI_WEIGHT);
+ napi_enable(&comm->rx_napi);
+ netif_napi_add(ndev, &comm->tx_napi, tx_poll, TX_NAPI_WEIGHT);
+ napi_enable(&comm->tx_napi);
+ return 0;
+
+out_freemdio:
+ if (comm->mii_bus)
+ mdio_remove(ndev);
+
+out_unregister_dev:
+ unregister_netdev(ndev);
+ if (ndev2)
+ unregister_netdev(ndev2);
+
+ return ret;
+}
+
+static int sp_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev, *ndev2;
+ struct sp_mac *mac;
+
+ ndev = platform_get_drvdata(pdev);
+ if (!ndev)
+ return 0;
+
+ mac = netdev_priv(ndev);
+
+ // Unregister and free 2nd net device.
+ ndev2 = mac->next_ndev;
+ if (ndev2) {
+ sp_phy_remove(ndev2);
+ unregister_netdev(ndev2);
+ free_netdev(ndev2);
+ }
+
+ mac->comm->enable = 0;
+ soc0_stop(mac);
+
+ // Disable and delete napi.
+ napi_disable(&mac->comm->rx_napi);
+ netif_napi_del(&mac->comm->rx_napi);
+ napi_disable(&mac->comm->tx_napi);
+ netif_napi_del(&mac->comm->tx_napi);
+
+ sp_phy_remove(ndev);
+ mdio_remove(ndev);
+
+ // Unregister and free 1st net device.
+ unregister_netdev(ndev);
+ free_netdev(ndev);
+
+ clk_disable(mac->comm->clk);
+
+ return 0;
+}
+
+static const struct of_device_id sp_of_match[] = {
+ {.compatible = "sunplus,sp7021-emac"},
+ { /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, sp_of_match);
+
+static struct platform_driver sp_driver = {
+ .probe = sp_probe,
+ .remove = sp_remove,
+ .driver = {
+ .name = "sp7021_emac",
+ .owner = THIS_MODULE,
+ .of_match_table = sp_of_match,
+ },
+};
+
+module_platform_driver(sp_driver);
+
+MODULE_AUTHOR("Wells Lu [off-list ref]");
+MODULE_DESCRIPTION("Sunplus Dual 10M/100M Ethernet driver");
+MODULE_LICENSE("GPL v2");
@@ -0,0 +1,331 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include<linux/iopoll.h>+#include"sp_hal.h"++voidhal_mac_stop(structsp_mac*mac)+{+structsp_common*comm=mac->comm;+u32reg,disable;++if(comm->enable==0){+// Mask and clear all interrupts, except PORT_ST_CHG.+write_sw_int_mask0(mac,0xffffffff);+writel(0xffffffff&(~MAC_INT_PORT_ST_CHG),+comm->sp_reg_base+SP_SW_INT_STATUS_0);++// Disable cpu 0 and cpu 1.+reg=readl(comm->sp_reg_base+SP_CPU_CNTL);+writel((0x3<<6)|reg,comm->sp_reg_base+SP_CPU_CNTL);+}++// Disable lan 0 and lan 1.+disable=((~comm->enable)&0x3)<<24;+reg=readl(comm->sp_reg_base+SP_PORT_CNTL0);+writel(disable|reg,comm->sp_reg_base+SP_PORT_CNTL0);+}++voidhal_mac_reset(structsp_mac*mac)+{+}
@@ -0,0 +1,286 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_define.h"+#include"sp_int.h"+#include"sp_driver.h"+#include"sp_hal.h"++staticvoidport_status_change(structsp_mac*mac)+{+u32reg;+structnet_device*ndev=mac->ndev;++reg=read_port_ability(mac);+if(!netif_carrier_ok(ndev)&&(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_on(ndev);+netif_start_queue(ndev);+}elseif(netif_carrier_ok(ndev)&&!(reg&PORT_ABILITY_LINK_ST_P0)){+netif_carrier_off(ndev);+netif_stop_queue(ndev);+}++if(mac->next_ndev){+structnet_device*ndev2=mac->next_ndev;++if(!netif_carrier_ok(ndev2)&&(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_on(ndev2);+netif_start_queue(ndev2);+}elseif(netif_carrier_ok(ndev2)&&!(reg&PORT_ABILITY_LINK_ST_P1)){+netif_carrier_off(ndev2);+netif_stop_queue(ndev2);+}+}+}++staticvoidrx_skb(structsp_mac*mac,structsk_buff*skb)+{+mac->dev_stats.rx_packets++;+mac->dev_stats.rx_bytes+=skb->len;+netif_receive_skb(skb);+}++intrx_poll(structnapi_struct*napi,intbudget)+{+structsp_common*comm=container_of(napi,structsp_common,rx_napi);+structsp_mac*mac=netdev_priv(comm->ndev);+structsk_buff*skb,*new_skb;+structskb_info*sinfo;+structmac_desc*desc;+structmac_desc*h_desc;+u32rx_pos,pkg_len;+u32cmd;+u32num,rx_count;+s32queue;+intndev2_pkt;+structnet_device_stats*dev_stats;++spin_lock(&comm->rx_lock);++// Process high-priority queue and then low-priority queue.+for(queue=0;queue<RX_DESC_QUEUE_NUM;queue++){+rx_pos=comm->rx_pos[queue];+rx_count=comm->rx_desc_num[queue];++for(num=0;num<rx_count;num++){+sinfo=comm->rx_skb_info[queue]+rx_pos;+desc=comm->rx_desc[queue]+rx_pos;+cmd=desc->cmd1;++if(cmd&OWN_BIT)+break;++if((cmd&PKTSP_MASK)==PKTSP_PORT1){+structsp_mac*mac2;++ndev2_pkt=1;+mac2=(mac->next_ndev)?netdev_priv(mac->next_ndev):NULL;+dev_stats=(mac2)?&mac2->dev_stats:&mac->dev_stats;+}else{+ndev2_pkt=0;+dev_stats=&mac->dev_stats;+}++pkg_len=cmd&LEN_MASK;+if(unlikely((cmd&ERR_CODE)||(pkg_len<64))){+dev_stats->rx_length_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++if(unlikely(cmd&RX_IP_CHKSUM_BIT)){+dev_stats->rx_crc_errors++;+dev_stats->rx_dropped++;+gotoNEXT;+}++// Allocate an skbuff for receiving.+new_skb=__dev_alloc_skb(comm->rx_desc_buff_size+RX_OFFSET,+GFP_ATOMIC|GFP_DMA);+if(unlikely(!new_skb)){+dev_stats->rx_dropped++;+gotoNEXT;+}+new_skb->dev=mac->ndev;++dma_unmap_single(&comm->pdev->dev,sinfo->mapping,+comm->rx_desc_buff_size,DMA_FROM_DEVICE);++skb=sinfo->skb;+skb->ip_summed=CHECKSUM_NONE;++/*skb_put will judge if tail exceeds end, but __skb_put won't */+__skb_put(skb,(pkg_len-4>comm->rx_desc_buff_size)?+comm->rx_desc_buff_size:pkg_len-4);++sinfo->mapping=dma_map_single(&comm->pdev->dev,new_skb->data,+comm->rx_desc_buff_size,+DMA_FROM_DEVICE);
may fail
Yes, I'll add error check in next patch as shown below:
sinfo->mapping = dma_map_single(&comm->pdev->dev, new_skb->data,
comm->rx_desc_buff_size,
DMA_FROM_DEVICE);
if (dma_mapping_error(&comm->pdev->dev, sinfo->mapping)) {
dev_kfree_skb(new_skb);
dev_stats->rx_errors++;
desc->cmd2 = (rx_pos == comm->rx_desc_num[queue] - 1) ?
EOR_BIT : 0;
goto rx_poll_err;
}
sinfo->skb = new_skb;
[...]
desc->addr1 = sinfo->mapping;
rx_poll_next:
desc->cmd2 = (rx_pos == comm->rx_desc_num[queue] - 1) ?
EOR_BIT | MAC_RX_LEN_MAX : MAC_RX_LEN_MAX;
rx_poll_err:
wmb(); // Set OWN_BIT after other fields of descriptor are effective.
desc->cmd1 = OWN_BIT;
NEXT_RX(queue, rx_pos);
@@ -0,0 +1,90 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_mdio.h"++u32mdio_read(structsp_mac*mac,u32phy_id,u16regnum)+{+intret;++ret=hal_mdio_access(mac,MDIO_READ_CMD,phy_id,regnum,0);+if(ret<0)+return-EOPNOTSUPP;++returnret;+}++u32mdio_write(structsp_mac*mac,u32phy_id,u32regnum,u16val)+{+intret;++ret=hal_mdio_access(mac,MDIO_WRITE_CMD,phy_id,regnum,val);+if(ret<0)+return-EOPNOTSUPP;++return0;+}++staticintmii_read(structmii_bus*bus,intphy_id,intregnum)+{+structsp_mac*mac=bus->priv;++returnmdio_read(mac,phy_id,regnum);+}++staticintmii_write(structmii_bus*bus,intphy_id,intregnum,u16val)+{+structsp_mac*mac=bus->priv;++returnmdio_write(mac,phy_id,regnum,val);+}++u32mdio_init(structplatform_device*pdev,structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structmii_bus*mii_bus;+structdevice_node*mdio_node;+intret;++mii_bus=mdiobus_alloc();+if(!mii_bus){+netdev_err(ndev,"Failed to allocate mdio_bus memory!\n");+return-ENOMEM;+}++mii_bus->name="sunplus_mii_bus";+mii_bus->parent=&pdev->dev;+mii_bus->priv=mac;+mii_bus->read=mii_read;+mii_bus->write=mii_write;+snprintf(mii_bus->id,MII_BUS_ID_SIZE,"%s-mii",dev_name(&pdev->dev));++mdio_node=of_get_parent(mac->phy_node);+if(!mdio_node){+netdev_err(ndev,"Failed to get mdio_node!\n");+return-ENODATA;+}++ret=of_mdiobus_register(mii_bus,mdio_node);+if(ret){+netdev_err(ndev,"Failed to register mii bus!\n");+mdiobus_free(mii_bus);+returnret;+}++mac->comm->mii_bus=mii_bus;+returnret;+}++voidmdio_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->comm->mii_bus){+mdiobus_unregister(mac->comm->mii_bus);+mdiobus_free(mac->comm->mii_bus);+mac->comm->mii_bus=NULL;+}+}
@@ -0,0 +1,64 @@+// SPDX-License-Identifier: GPL-2.0+/* Copyright Sunplus Technology Co., Ltd.+*Allrightsreserved.+*/++#include"sp_phy.h"+#include"sp_mdio.h"++staticvoidmii_linkchange(structnet_device*netdev)+{+}++intsp_phy_probe(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);+structphy_device*phydev;+inti;++phydev=of_phy_connect(ndev,mac->phy_node,mii_linkchange,+0,mac->phy_mode);+if(!phydev){+netdev_err(ndev,"\"%s\" failed to connect to phy!\n",ndev->name);+return-ENODEV;+}++for(i=0;i<sizeof(phydev->supported)/sizeof(long);i++)+phydev->advertising[i]=phydev->supported[i];++phydev->irq=PHY_MAC_INTERRUPT;+mac->phy_dev=phydev;++// Bug workaround:+// Flow-control of phy should be enabled. MAC flow-control will refer+// to the bit to decide to enable or disable flow-control.+mdio_write(mac,mac->phy_addr,4,mdio_read(mac,mac->phy_addr,4)|(1<<10));++return0;+}++voidsp_phy_start(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_start(mac->phy_dev);+}++voidsp_phy_stop(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev)+phy_stop(mac->phy_dev);+}++voidsp_phy_remove(structnet_device*ndev)+{+structsp_mac*mac=netdev_priv(ndev);++if(mac->phy_dev){+phy_disconnect(mac->phy_dev);+mac->phy_dev=NULL;+}+}
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-13 15:35:02
quoted
quoted
+//define MAC interrupt status bit
please embrace all comments with /* */
Do you mean to modify comment, for example,
//define MAC interrupt status bit
to
/* define MAC interrupt status bit */
Yes. The Kernel is written in C, so C style comments are preferred
over C++ comments, even if later versions of the C standard allow C++
style comments.
You should also read the netdev FAQ, which makes some specific
comments about how multi-line comments should be formatted.
Yes, I'll add error check in next patch as shown below:
rx_skbinfo[j].mapping = dma_map_single(&comm->pdev->dev, skb->data,
comm->rx_desc_buff_size,
DMA_FROM_DEVICE);
if (dma_mapping_error(&comm->pdev->dev, rx_skbinfo[j].mapping))
goto mem_alloc_fail;
If it is clear how to fix the code, just do it. No need to tell us
what you are going to do, we will see the change when reviewing the
next version.
quoted
quoted
+/* Transmit a packet (called by the kernel) */
+static int ethernet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct sp_mac *mac = netdev_priv(ndev);
+ struct sp_common *comm = mac->comm;
+ u32 tx_pos;
+ u32 cmd1;
+ u32 cmd2;
+ struct mac_desc *txdesc;
+ struct skb_info *skbinfo;
+ unsigned long flags;
+
+ if (unlikely(comm->tx_desc_full == 1)) {
+ // No TX descriptors left. Wait for tx interrupt.
+ netdev_info(ndev, "TX descriptor queue full when xmit!\n");
+ return NETDEV_TX_BUSY;
Do you really have to return NETDEV_TX_BUSY?
(tx_desc_full == 1) means there is no TX descriptor left in ring buffer.
So there is no way to do new transmit. Return 'busy' directly.
I am not sure if this is a correct process or not.
Could you please teach is there any other way to take care of this case?
Drop directly?
There are a few hundred examples to follow, other MAC drivers. What do
they do when out of TX buffers? Find the most common pattern, and
follow it.
You should also thinking about the netdev_info(). Do you really want
to spam the kernel log? Say you are connected to a 10/Half link, and
the application is trying to send UDP at 100Mbps, Won't you see a lot
of these messages? change it to _debug(), or rate limit it.
static void ethernet_tx_timeout(struct net_device *ndev, unsigned int txqueue)
{
struct sp_mac *mac = netdev_priv(ndev);
struct net_device *ndev2;
unsigned long flags;
netdev_err(ndev, "TX timed out!\n");
ndev->stats.tx_errors++;
spin_lock_irqsave(&mac->comm->tx_lock, flags);
netif_stop_queue(ndev);
ndev2 = mac->next_ndev;
if (ndev2)
netif_stop_queue(ndev2);
hal_mac_stop(mac);
hal_mac_init(mac);
hal_mac_start(mac);
// Accept TX packets again.
netif_trans_update(ndev);
netif_wake_queue(ndev);
if (ndev2) {
netif_trans_update(ndev2);
netif_wake_queue(ndev2);
}
spin_unlock_irqrestore(&mac->comm->tx_lock, flags);
}
Is that ok?
This ndev2 stuff is not nice. You probably need a cleaner abstract of
two netdev's sharing one TX and RX ring. See if there are any other
switchdev drivers with a similar structure you can copy. Maybe
cpsw_new.c? But be careful with that driver. cpsw is a bit of a mess
due to an incorrect initial design with respect to its L2 switch. A
lot of my initial comments are to stop you making the same mistakes.
Andrew
You should not be exposing these functions, if you do, that means another part of your
code performs MDIO bus read/write operations without using the appropriate layer, so no.
Yes, I'll re-declare the two functions as static functions.
quoted
+
+static int mii_read(struct mii_bus *bus, int phy_id, int regnum) {
+ struct sp_mac *mac = bus->priv;
+
+ return mdio_read(mac, phy_id, regnum); }
+
+static int mii_write(struct mii_bus *bus, int phy_id, int regnum, u16
+val) {
+ struct sp_mac *mac = bus->priv;
+
+ return mdio_write(mac, phy_id, regnum, val); }
+
+u32 mdio_init(struct platform_device *pdev, struct net_device *ndev)
Those function names need to be prefixed with sp_ to denote the driver local scope, this
applies for your entire patch set.
Yes, I'll add vendor-specified prefix to the two functions and all the other functions in
the drivers.
Please scope your functions better, and name them sp_mdio_read, etc.
because mdio_read() is way too generic. Also, can you please follow the same prototype
as what include/linux/mdio.h has for the mdiobus->read and ->write calls, that is phy_id
is int, regnum is u32, etc.
Yes, I'll re-declare the two functions, mdio_read() and mdio_write(), as static
functions and add vendor-specified prefix to them.
The wrong declaration of the two functions will be removed from the header file.
Does your MAC fully auto-configure based on the PHY's link parameters, if so, how does
it do it? You most certainly need to act on duplex changes, or speed changes no?
Yes, it does. SP7021 MAC communicates with PHY automatically.
It reads link status (half- or full-duplex, 10M or 100M) from PHY
and sets itself automatically.
It also reads port status (link up or down) and generates
interrupt to driver.
quoted
+
+int sp_phy_probe(struct net_device *ndev) {
+ struct sp_mac *mac = netdev_priv(ndev);
+ struct phy_device *phydev;
+ int i;
+
+ phydev = of_phy_connect(ndev, mac->phy_node, mii_linkchange,
+ 0, mac->phy_mode);
+ if (!phydev) {
+ netdev_err(ndev, "\"%s\" failed to connect to phy!\n", ndev->name);
+ return -ENODEV;
+ }
+
+ for (i = 0; i < sizeof(phydev->supported) / sizeof(long); i++)
+ phydev->advertising[i] = phydev->supported[i];
+
+ phydev->irq = PHY_MAC_INTERRUPT;
+ mac->phy_dev = phydev;
+
+ // Bug workaround:
+ // Flow-control of phy should be enabled. MAC flow-control will refer
+ // to the bit to decide to enable or disable flow-control.
+ mdio_write(mac, mac->phy_addr, 4, mdio_read(mac, mac->phy_addr, 4) |
+(1 << 10));
This is a layering violation, and you should not be doing those things here, if you need
to advertise flow control, then please set ADVERTISE_PAUSE_CAP and/or ADVERTISE_PAUSE_ASYM
accordingly, see whether
phy_set_asym_pause() can do what you need it to.
Yes, I'll remove the statement, instead, use phy_set_asym_pause().
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-15 14:39:55
Hi Andrew,
Thank you for reminding.
I'll do my best to address all comments.
I fully understand that all reviewers' comments should be addressed.
If I lost addressing any comments, please kindly remind me again.
Best regards,
Wells
Hi Florian
You are basically pointing out issues i already pointed out in previous versions, and have
been ignored :-(
Wells, please look at the comments i made on your earlier versions. Those comments are
still valid and need addressing.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-16 17:10:07
Hi,
quoted
+void rx_descs_flush(struct sp_common *comm)
As both Florian and I have said, you need a prefix for all your functions, structures,
etc. sp_ is not the best prefix either, it is not very unique. spl2sw_ would be better.
I'll add prefix spl2sw for all functions, structures, file-names in next patch.
I thought comment for revising prefix is only for structures, function and file name
with prefix l2sw_ because 'l2sw_' has been used by other modules.
Now I know prefix is necessary for all in this driver, except local variables and
structure members.
This does not have the locally administered bit set. Should it? Or is this and address
from your OUI?
This is default MAC address when MAC address in NVMEM is not found.
Fc:4b:bc:00:00:00 is OUI of "Sunplus Technology Co., Ltd.".
Can I keep this? or it should be removed?
quoted
+static void ethernet_set_rx_mode(struct net_device *ndev) {
+ if (ndev) {
How can ndev be NULL?
Yes, I'll remove 'if (ndev) {' statement in next patch.
It is redundant.
I suggest you avoid any references to hal. It makes people think you have ported a driver
from some other operating system and then put a layer of code on top of it. That is not
how you do it in Linux. This is a Linux driver, nothing else.
Yes, I'll change file name 'sp_hal.c' to 'spl2sw_hw.c'.
Function name in this file will also be changed, for example:
hal_mac_stop() --> spl2sw_hw_mac_stop()
Is this actually adding an entry into the address translation table?
If so, make this clear in the function name. You are not setting the MAC address, you are
just adding a static forwarding entry.
Yes, this is actually adding an entry into address table.
I'll change function name to spl2sw_mac_add_addr() in next patch.
Is the name ok?
I'll move it into 'spl2sw_mdio.c' in next patch.
I put all hardware-related functions in sp_hal.c (will be changed to spl2sw_hw.c).
All functions in other files won't touch hardware registers.
This seems not Linux driver style.
As i said before, the hardware never directly communicates with the PHY. So you can remove
this.
I'll remove this function in next patch.
But now I cannot find a way to disable hardware 'auto rmii' function.
If I remove this function right now, MAC may get wrong status of PHY
from wrong address because SP7021 MAC communicates with PHY
automatically. This may cause more problem.
I am consulting with ASIC engineer. Hopefully, someone can find
a way to disable the auto function.
What happened about my request to return -EOPNOTSUPP for C45 requests?
Sorry for overlooking the comment!
I am not sure how to check C45 request. Should I add statements like:
if (regnum & MII_ADDR_C45)
Return -EOPNOTSUPP;
for mdio_read() and mdio_write()?
Andrew
Thank you very much for your review!
Best regards,
Wells
This does not have the locally administered bit set. Should it? Or is this and address
from your OUI?
This is default MAC address when MAC address in NVMEM is not found.
Fc:4b:bc:00:00:00 is OUI of "Sunplus Technology Co., Ltd.".
Can I keep this? or it should be removed?
Please add a comment about whos OUI it is.
It is however more normal to use a random MAC address if no other MAC
address is available. That way, you avoid multiple devices on one LAN
using the same default MAC address.
Looks very odd. The two netdev should be independent.
I don't understand your comment.
ndev checks PORT_ABILITY_LINK_ST_P0
ndev2 checks PORT_ABILITY_LINK_ST_P1
They are independent already.
I would try to remove the mac->next_ndev. I think without that, you
will get a cleaner abstraction. You might want to keep an array of mac
pointers in your top level shared structure.
Andrew
+ // unregister and free net device.
+ unregister_netdev(net_dev2);
+ free_netdev(net_dev2);
+ mac->next_netdev = NULL;
+ pr_info(" Unregistered and freed net device \"eth1\"!\n");
+
+ comm->dual_nic = 0;
+ mac_switch_mode(mac);
+ rx_mode_set(net_dev);
+ mac_hw_addr_del(mac2);
+
mac2 is net_dev2 private data (*), so it will become freed after
free_netdev() call.
FWIW the latest `smatch` should warn about this type of bugs.
Yes, this is indeed a bug.
But the code paragraph has been removed thoroughly in [PATCH v2].
quoted
+ // If eth0 is up, turn on lan 0 and 1 when
+ // switching to daisy-chain mode.
+ if (comm->enable & 0x1)
+ comm->enable = 0x3;
[code snip]
quoted
+static int l2sw_remove(struct platform_device *pdev) {
+ struct net_device *net_dev;
+ struct net_device *net_dev2;
+ struct l2sw_mac *mac;
+
+ net_dev = platform_get_drvdata(pdev);
+ if (!net_dev)
+ return 0;
+ mac = netdev_priv(net_dev);
+
+ // Unregister and free 2nd net device.
+ net_dev2 = mac->next_netdev;
+ if (net_dev2) {
+ unregister_netdev(net_dev2);
+ free_netdev(net_dev2);
+ }
+
Is it save here to free mac->next_netdev before unregistering "parent"
netdev? I haven't checked the whole code, just asking :)
Yes, I think it is save.
netdev2 should be unregistered and freed before net_dev.
If net_dev is unregistered and freed in advance,
mac->next_netdev becomes danger because 'mac' has been freed.
This is indeed a bug.
But the statement, Kfree(mac->comm);, has been removed in [PATCH v2].
In [PATCH v2], structure data 'mac->comm' is allocated by
devm_kzalloc(). No more need to free it here.
quoted
+ return 0;
+}
I haven't read the whole thread, i am sorry if these questions were already discussed.
With regards,
Pavel Skripkin
Thank you very much for your review!
Best regards,
Wells Lu
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-18 08:17:51
Hi,
quoted
quoted
quoted
+//define MAC interrupt status bit
please embrace all comments with /* */
Do you mean to modify comment, for example,
//define MAC interrupt status bit
to
/* define MAC interrupt status bit */
Yes. The Kernel is written in C, so C style comments are preferred over C++ comments, even
if later versions of the C standard allow C++ style comments.
I'll modify all comments to C style in next patch.
You should also read the netdev FAQ, which makes some specific comments about how multi-line
comments should be formatted.
Thanks for routing me to the document.
I'll use the new format for multi-line comments.
---
It is requested that you make it look like this:
/* foobar blah blah blah
* another line of text
*/
quoted
Yes, I'll add error check in next patch as shown below:
rx_skbinfo[j].mapping = dma_map_single(&comm->pdev->dev, skb->data,
comm->rx_desc_buff_size,
DMA_FROM_DEVICE);
if (dma_mapping_error(&comm->pdev->dev, rx_skbinfo[j].mapping))
goto mem_alloc_fail;
If it is clear how to fix the code, just do it. No need to tell us what you are going to
do, we will see the change when reviewing the next version.
Thanks, I see.
quoted
quoted
quoted
+/* Transmit a packet (called by the kernel) */ static int
+ethernet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct sp_mac *mac = netdev_priv(ndev);
+ struct sp_common *comm = mac->comm;
+ u32 tx_pos;
+ u32 cmd1;
+ u32 cmd2;
+ struct mac_desc *txdesc;
+ struct skb_info *skbinfo;
+ unsigned long flags;
+
+ if (unlikely(comm->tx_desc_full == 1)) {
+ // No TX descriptors left. Wait for tx interrupt.
+ netdev_info(ndev, "TX descriptor queue full when xmit!\n");
+ return NETDEV_TX_BUSY;
Do you really have to return NETDEV_TX_BUSY?
(tx_desc_full == 1) means there is no TX descriptor left in ring buffer.
So there is no way to do new transmit. Return 'busy' directly.
I am not sure if this is a correct process or not.
Could you please teach is there any other way to take care of this case?
Drop directly?
There are a few hundred examples to follow, other MAC drivers. What do they do when out
of TX buffers? Find the most common pattern, and follow it.
But some drivers return NETDEV_TX_BUSY, some drivers drop packet and return NETDEV_TX_OK
Some drivers seem do not take care this issue. I am not sure.
You should also thinking about the netdev_info(). Do you really want to spam the kernel
log? Say you are connected to a 10/Half link, and the application is trying to send UDP
at 100Mbps, Won't you see a lot of these messages? change it to _debug(), or rate limit
it.
Yes, I'll modify most netdev_info() to netdev_dbg() in next patch.
quoted
static void ethernet_tx_timeout(struct net_device *ndev, unsigned int
txqueue) {
struct sp_mac *mac = netdev_priv(ndev);
struct net_device *ndev2;
unsigned long flags;
netdev_err(ndev, "TX timed out!\n");
ndev->stats.tx_errors++;
spin_lock_irqsave(&mac->comm->tx_lock, flags);
netif_stop_queue(ndev);
ndev2 = mac->next_ndev;
if (ndev2)
netif_stop_queue(ndev2);
hal_mac_stop(mac);
hal_mac_init(mac);
hal_mac_start(mac);
// Accept TX packets again.
netif_trans_update(ndev);
netif_wake_queue(ndev);
if (ndev2) {
netif_trans_update(ndev2);
netif_wake_queue(ndev2);
}
spin_unlock_irqrestore(&mac->comm->tx_lock, flags); }
Is that ok?
This ndev2 stuff is not nice. You probably need a cleaner abstract of two netdev's sharing
one TX and RX ring. See if there are any other switchdev drivers with a similar structure
you can copy. Maybe cpsw_new.c? But be careful with that driver. cpsw is a bit of a mess
due to an incorrect initial design with respect to its L2 switch. A lot of my initial comments
are to stop you making the same mistakes.
I'll define a array (pointer to struct net_dev) in driver private (shared)
structure to access to all net devices. No more mac->next_ndev;.
Andrew
Thank you very much for your review.
Best regards,
Wells
This does not have the locally administered bit set. Should it? Or
is this and address from your OUI?
This is default MAC address when MAC address in NVMEM is not found.
Fc:4b:bc:00:00:00 is OUI of "Sunplus Technology Co., Ltd.".
Can I keep this? or it should be removed?
Please add a comment about whos OUI it is.
It is however more normal to use a random MAC address if no other MAC address is available.
That way, you avoid multiple devices on one LAN using the same default MAC address.
Yes, I'll add a comment about the OUI and also use 'get_random_int() % 255'
to generate the latest 3 octets (controller specific).
Looks very odd. The two netdev should be independent.
I don't understand your comment.
ndev checks PORT_ABILITY_LINK_ST_P0
ndev2 checks PORT_ABILITY_LINK_ST_P1
They are independent already.
I would try to remove the mac->next_ndev. I think without that, you will get a cleaner
abstraction. You might want to keep an array of mac pointers in your top level shared
structure.
Yes, I'll define a array (pointer to struct net_dev or mac) in driver private (shared)
structure to access to all net devices. No more mac->next_ndev;.
Andrew
Thank you very much for your review.
Best regards,
Wells
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-25 11:40:20
Hi Andrew,
Regarding hardware 'auto rmii' function of SP7021, we find a
way to 'disable' it.
We can use 'force' mode of MAC to set link up/down, speeds,
full/half-duplex, flow-control forcibly.
Although MAC still keeps sending MDIO commands to PHY and get
its status, but the read-back status has no use because MAC
is in 'force' mode.
Due to hardware design, we still need to set PHY address,
because MDIO controller of SP7021 only sends out MDIO
commands with the same address listed in PHY address
registers. The function below needs to be kept.
As i said before, the hardware never directly communicates with the PHY. So you can remove
this.
If it is ok, I'll modify code and send a new patch.
Best regards,
Wells
quoted
quoted
+void rx_descs_flush(struct sp_common *comm)
As both Florian and I have said, you need a prefix for all your
functions, structures, etc. sp_ is not the best prefix either, it is not very unique.
spl2sw_ would be better.
I'll add prefix spl2sw for all functions, structures, file-names in next patch.
I thought comment for revising prefix is only for structures, function and file name with
prefix l2sw_ because 'l2sw_' has been used by other modules.
Now I know prefix is necessary for all in this driver, except local variables and structure
members.
This does not have the locally administered bit set. Should it? Or is this and address
from your OUI?
This is default MAC address when MAC address in NVMEM is not found.
Fc:4b:bc:00:00:00 is OUI of "Sunplus Technology Co., Ltd.".
Can I keep this? or it should be removed?
quoted
quoted
+static void ethernet_set_rx_mode(struct net_device *ndev) {
+ if (ndev) {
How can ndev be NULL?
Yes, I'll remove 'if (ndev) {' statement in next patch.
It is redundant.
I suggest you avoid any references to hal. It makes people think you have ported a driver
from some other operating system and then put a layer of code on top of it. That is not
how you do it in Linux. This is a Linux driver, nothing else.
Yes, I'll change file name 'sp_hal.c' to 'spl2sw_hw.c'.
Function name in this file will also be changed, for example:
hal_mac_stop() --> spl2sw_hw_mac_stop()
Is this actually adding an entry into the address translation table?
If so, make this clear in the function name. You are not setting the MAC address, you
are
quoted
just adding a static forwarding entry.
Yes, this is actually adding an entry into address table.
I'll change function name to spl2sw_mac_add_addr() in next patch.
Is the name ok?
I'll move it into 'spl2sw_mdio.c' in next patch.
I put all hardware-related functions in sp_hal.c (will be changed to spl2sw_hw.c).
All functions in other files won't touch hardware registers.
This seems not Linux driver style.
As i said before, the hardware never directly communicates with the PHY. So you can remove
this.
I'll remove this function in next patch.
But now I cannot find a way to disable hardware 'auto rmii' function.
If I remove this function right now, MAC may get wrong status of PHY
from wrong address because SP7021 MAC communicates with PHY
automatically. This may cause more problem.
I am consulting with ASIC engineer. Hopefully, someone can find
a way to disable the auto function.
What happened about my request to return -EOPNOTSUPP for C45 requests?
Sorry for overlooking the comment!
I am not sure how to check C45 request. Should I add statements like:
if (regnum & MII_ADDR_C45)
Return -EOPNOTSUPP;
for mdio_read() and mdio_write()?
quoted
Andrew
Thank you very much for your review!
Best regards,
Wells
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-25 15:22:50
Due to hardware design, we still need to set PHY address,
because MDIO controller of SP7021 only sends out MDIO
commands with the same address listed in PHY address
registers. The function below needs to be kept.
I suggest you actually set it to some other address. One of the
good/bad things about MDIO is you have no idea if the device is
there. A read to a device which does not exist just returns 0xffff,
not an error. So i would set the address of 0x1f. I've never seen a
PHY actually use that address.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-26 03:58:49
Hi Andrew,
I set phy-id registers to 30 and 31 and found the read-back
values of mdio read commands from CPU are all 0x0000.
I consulted with an ASIC engineer. She confirmed that if
phy-id of a mdio command from CPU does not match any
phy-id registers, the mdio command will not be sent out.
She explained if phy-id of a mdio command does not match
any phy-id registers (represent addresses of external PHYs),
why MAC needs to send a command to non-existing PHY?
Best regards,
Wells Lu
呂芳騰
智能運算專案/Smart Computing Program
家庭平台事業群/Home Entertainment Business Unit
凌陽科技/Sunplus Technology Co., Ltd.
地址:300新竹科學園區創新一路19號
19, Innovation 1st Road,
Science-based Industrial Park
Hsin-Chu, Taiwan 300
TEL:886-3-5786005 ext. 2580
-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Thursday, November 25, 2021 11:21 PM
To: Wells Lu 呂芳騰 <redacted>
Cc: Wells Lu <wellslutw@gmail.com>; davem@davemloft.net; kuba@kernel.org;
robh+dt@kernel.org; netdev@vger.kernel.org; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org; p.zabel@pengutronix.de; Vincent Shih 施錕鴻
[off-list ref]
Subject: Re: [PATCH v2 2/2] net: ethernet: Add driver for Sunplus SP7021
quoted
Due to hardware design, we still need to set PHY address, because MDIO
controller of SP7021 only sends out MDIO commands with the same
address listed in PHY address registers. The function below needs to
be kept.
I suggest you actually set it to some other address. One of the good/bad things about MDIO
is you have no idea if the device is there. A read to a device which does not exist just
returns 0xffff, not an error. So i would set the address of 0x1f. I've never seen a PHY
actually use that address.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-26 14:40:56
On Fri, Nov 26, 2021 at 03:56:28AM +0000, Wells Lu 呂芳騰 wrote:
Hi Andrew,
I set phy-id registers to 30 and 31 and found the read-back
values of mdio read commands from CPU are all 0x0000.
I consulted with an ASIC engineer. She confirmed that if
phy-id of a mdio command from CPU does not match any
phy-id registers, the mdio command will not be sent out.
She explained if phy-id of a mdio command does not match
any phy-id registers (represent addresses of external PHYs),
why MAC needs to send a command to non-existing PHY?
Reads or writes on a real PHY which Linux is driving can have side
effects. There is a link statue register which latches. Read it once,
you get the last status, read it again, you get the current status. If
the MAC hardware is reading this register as well a Linux, bad things
will happen. A read on the interrupt status register often clears the
interrupts. So Linux will not see the interrupts.
So you need to make sure you hardware is not touching a PHY which
Linux uses. Which is why i suggested using MDIO bus address 31, which
generally does not have a PHY at that address.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-26 16:15:03
Hi Andrew,
From data provided by ASIC engineer, MAC of SP7021 only
reads the 4 registers of PHY:
0: Control register
1: Status register
4: Auto-negotiation advertisement register
5: Auto-negotiation link partner ability register
It does not read any other registers of PHY.
Best regards,
Wells Lu
quoted
Hi Andrew,
I set phy-id registers to 30 and 31 and found the read-back values of
mdio read commands from CPU are all 0x0000.
I consulted with an ASIC engineer. She confirmed that if phy-id of a
mdio command from CPU does not match any phy-id registers, the mdio
command will not be sent out.
She explained if phy-id of a mdio command does not match any phy-id
registers (represent addresses of external PHYs), why MAC needs to
send a command to non-existing PHY?
Reads or writes on a real PHY which Linux is driving can have side effects. There is a
link statue register which latches. Read it once, you get the last status, read it again,
you get the current status. If the MAC hardware is reading this register as well a Linux,
bad things will happen. A read on the interrupt status register often clears the interrupts.
So Linux will not see the interrupts.
So you need to make sure you hardware is not touching a PHY which Linux uses. Which is
why i suggested using MDIO bus address 31, which generally does not have a PHY at that
address.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-26 18:09:13
On Fri, Nov 26, 2021 at 04:12:46PM +0000, Wells Lu 呂芳騰 wrote:
Hi Andrew,
From data provided by ASIC engineer, MAC of SP7021 only
reads the 4 registers of PHY:
0: Control register
1: Status register
This is the register which has latching of the link
status. genphy_update_link() expects this latching behaviour, and if
the hardware reads the register, that behaviour is not going to
happen.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-26 19:25:46
Hi Andrew,
I read specification of ICPlus IP101G (10M/100M PHY).
Bits of register 0 (control) and register 1 (status)
are R/W or RO type. They will not be cleared after
read. No matter how many times they are read, the
read-back value is the same.
For example,
Value of register 0 (control) is 0x3100
Value of register 1 (status) is 0x786d
The read-back values are always the same unless you
unplug the cable.
Besides, we use polling mode (phydev->irq = PHY_POLL)
PHY state-machine is triggered by using 1-Hz work-
queue, not interrupt.
We didn't find any problem after many tests after
using 'force' mode.
Can we go with this approach?
Best regards,
Wells
quoted
Hi Andrew,
From data provided by ASIC engineer, MAC of SP7021 only reads the 4
registers of PHY:
0: Control register
1: Status register
This is the register which has latching of the link status. genphy_update_link() expects
this latching behaviour, and if the hardware reads the register, that behaviour is not
going to happen.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2021-11-26 19:34:50
On Fri, Nov 26, 2021 at 07:13:23PM +0000, Wells Lu 呂芳騰 wrote:
Hi Andrew,
I read specification of ICPlus IP101G (10M/100M PHY).
Bits of register 0 (control) and register 1 (status)
are R/W or RO type. They will not be cleared after
read. No matter how many times they are read, the
read-back value is the same.
Please read 802.3,
Section 22.2.4.2: Status register (Register 1)
Table 22-8 Status register bit definitions
Bit 1.2 Link Status is marked as RO/LL, meaning read only Latching
low.
Can we go with this approach?
You need to not make any read on the PHY which Linux is driving.
Configure the hardware to read on an address where there is no PHY.
Andrew
From: Wells Lu 呂芳騰 <hidden> Date: 2021-11-29 11:19:28
Hi Andrew,
Thanks a lot for explanation!.
You need to not make any read on the PHY which Linux is driving.
Configure the hardware to read on an address where there is no PHY.
I will modify code to set hardware external PHY address to 31.
In mii_read() or mii_write() functions, set hardware external PHY
address to real PHY address, so that mii read or write command can
be sent out by hardware. Set hardware external PHY address back to
31 after mii read or write command done.
Best regards,
Wells Lu