Re: [PATCH v3 0/6] Introduce DSA Ethernet switch class and Felix driver
On 2/8/2020 2:19 PM, Vladimir Oltean wrote: On Tue, 3 Dec 2019 at 18:18, Alex Marginean wrote: DSA stands for Distributed Switch Architecture and it is a subsystem introduced in the Linux kernel to support switches that: - have an Ethernet link up to the CPU - use some form of tagging to identify the source/destination port for Rx/Tx - may be cascaded in tree-like structures. DSA is described in depth here: https://www.kernel.org/doc/Documentation/networking/dsa/dsa.txt From the doc: Summarized, this is basically how DSA looks like from a network device perspective: |--- | CPU network device (eth0)| | | || | Switch driver | || |||| || |---| |---| |---| | sw0p0 | | sw0p1 | | sw0p2 | |---| |---| |---| This patch set introduces a DSA class in U-Boot to support drivers of such switches. DSA drivers have to implement the following ops: - enable/disable of switch ports, - insert a tag in frames being transmitted, used by the switch to select the egress port, - parse a tag in frames being received, used for Rx traffic. DSA class code deals with presentation of switch ports as Ethernet interfaces, deals with the master Ethernet device for I/O and helps with parsing of the DT assuming the structure follows the DSA kernel binding. Support for switch cascading is not included yet. This patch set also introduces a driver for the Ethernet switch integrated into NXP LS1028A, called Felix. The switch has 4 front panel ports, I/O to/fom it is done though an ENETC Ethernet interface and meta-data is carried between the switch and the driver though an additional header pre-pended to the original frame. Network commands like tftp can be used on these front panel ports. The ports are disabled unless used so they do not cause issues on network topologies that include loops. Felix as seen on LS1028A RDB: => dm tree Class Index Probed DriverName --- .. dsa 0 [ + ] felix-switch | |-- felix-switch eth 4 [ + ] dsa-port | | |-- swp0 eth 5 [ + ] dsa-port | | |-- swp1 eth 6 [ + ] dsa-port | | |-- swp2 eth 7 [ + ] dsa-port | | `-- swp3 => mdio list .. 10 - Vitesse VSC8514 <--> swp0 11 - Vitesse VSC8514 <--> swp1 12 - Vitesse VSC8514 <--> swp2 13 - Vitesse VSC8514 <--> swp3 => tftp 8000 test Using swp2 device TFTP from server 192.168.100.1; our IP address is 192.168.100.100 Filename 'test'. Load address: 0x8000 Loading: # # 6.8 MiB/s done Bytes transferred = 949880 (e7e78 hex) Changes in v3: - fix Felix platdata size - move include/dsa.h to include/net/dsa.h - updated TODO in dsa.h - other minor fixes Changes in v2: - Don't use NULL PHY in Felix driver - guard dsa.h with #ifndef __DSA__H__, somehow I missed that in v1 - added a TODO for setting master Eth in promiscuous mode - Minor fixes in patch descriptions, API comments - Added address/size-cells to LS1028A DT ports node This patch set replaces v2: https://patchwork.ozlabs.org/project/uboot/list/?series=144912 and depends on: https://patchwork.ozlabs.org/project/uboot/list/?series=144907 https://patchwork.ozlabs.org/project/uboot/list/?series=142879 Alex Marginean (6): net: introduce DSA class for Ethernet switches drivers: net: add a DSA sandbox driver test: dm: add a simple unit test for DSA class drivers: net: add Felix DSA switch driver arm: dts: ls1028a: adds Ethernet switch node and its dependencies configs: ls1028a: enable the Ethernet switch driver in defconfig arch/Kconfig | 1 + arch/arm/dts/fsl-ls1028a-rdb.dts | 36 ++ arch/arm/dts/fsl-ls1028a.dtsi| 44 +- arch/sandbox/dts/test.dts| 49 ++ configs/ls1028aqds_tfa_SECURE_BOOT_defconfig | 3 +- configs/ls1028aqds_tfa_defconfig | 3 +- configs/ls1028ardb_tfa_SECURE_BOOT_defconfig | 3 +- configs/ls1028ardb_tfa_defconfig | 3 +- drivers/net/Kconfig | 21 + drivers/net/Makefile | 1 + drivers/net/dsa_sandbox.c| 272 +++ drivers/net/fsl_enetc.h
Re: [PATCH v3 6/6] configs: ls1028a: enable the Ethernet switch driver in defconfig
On 3/13/2020 3:33 PM, Vladimir Oltean wrote: On Tue, 3 Dec 2019 at 17:23, Alex Marginean wrote: The switch driver for LS1028A Ethernet switch is now compiled in for both LS1028A boards. Signed-off-by: Alex Marginean --- configs/ls1028aqds_tfa_SECURE_BOOT_defconfig | 3 ++- configs/ls1028aqds_tfa_defconfig | 3 ++- configs/ls1028ardb_tfa_SECURE_BOOT_defconfig | 3 ++- configs/ls1028ardb_tfa_defconfig | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig index 4a01cd6715..65e467817e 100644 --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig @@ -50,8 +50,9 @@ CONFIG_PHY_ATHEROS=y CONFIG_PHY_VITESSE=y CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_E1000=y -CONFIG_FSL_ENETC=y You surely didn't want to disable FSL_ENETC here, no? I think there's a dependency set and ENETC is implied if FELIX_SWITCH is enabled. I saved these with savedefconfig, I didn't manually edit them. Alex +CONFIG_MSCC_FELIX_SWITCH=y CONFIG_PCI=y CONFIG_DM_PCI=y CONFIG_DM_PCI_COMPAT=y diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig index 1307f0d951..40d259d907 100644 --- a/configs/ls1028aqds_tfa_defconfig +++ b/configs/ls1028aqds_tfa_defconfig @@ -56,8 +56,9 @@ CONFIG_PHY_ATHEROS=y CONFIG_PHY_VITESSE=y CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_E1000=y -CONFIG_FSL_ENETC=y +CONFIG_MSCC_FELIX_SWITCH=y CONFIG_PCI=y CONFIG_DM_PCI=y CONFIG_DM_PCI_COMPAT=y diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig index d0a3310a4c..f54a6da31b 100644 --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig @@ -49,9 +49,10 @@ CONFIG_PHY_ATHEROS=y CONFIG_PHY_VITESSE=y CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_PHY_GIGE=y CONFIG_E1000=y -CONFIG_FSL_ENETC=y +CONFIG_MSCC_FELIX_SWITCH=y CONFIG_PCI=y CONFIG_DM_PCI=y CONFIG_DM_PCI_COMPAT=y diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig index 4ec7ed0920..e018e5a50e 100644 --- a/configs/ls1028ardb_tfa_defconfig +++ b/configs/ls1028ardb_tfa_defconfig @@ -56,9 +56,10 @@ CONFIG_PHY_ATHEROS=y CONFIG_PHY_VITESSE=y CONFIG_DM_ETH=y CONFIG_DM_MDIO=y +CONFIG_DM_DSA=y CONFIG_PHY_GIGE=y CONFIG_E1000=y -CONFIG_FSL_ENETC=y +CONFIG_MSCC_FELIX_SWITCH=y CONFIG_PCI=y CONFIG_DM_PCI=y CONFIG_DM_PCI_COMPAT=y -- 2.17.1 Regards, -Vladimir
Re: [PATCH] pci-host-ecam-generic: access config space independent of system-wide bus id
On 3/13/2020 12:04 PM, Vladimir Oltean wrote:
From: Vladimir Oltean
The pci-host-ecam-generic code assumes that the ECAM is the first PCI
bus in the system to be probed. Therefore, the system-wide bus number
allocated by U-Boot in sequence for it is going to be zero, which
corresponds to the memory-mapped config spaces found within it.
Reuse the logic from other PCI bus drivers, and assume that U-Boot will
allocate bus numbers in sequence for all buses within the current ECAM.
So the base number of the bus needs to be subtracted when indexing the
correct config space.
Fixes: 3675cb044e68 ("PCI: Add driver for a 'pci-host-ecam-generic' host
controller")
Signed-off-by: Vladimir Oltean
---
drivers/pci/pcie_ecam_generic.c | 36 +
1 file changed, 32 insertions(+), 4 deletions(-)
Reviewed-by: Alex Marginean
Re: [PATCH] dm: uclass: don't assign aliased seq numbers
On 12/18/2019 10:45 PM, Michael Walle wrote:
Am 2019-12-18 21:00, schrieb Alexandru Marginean:
Hi Michael,
On 12/18/2019 5:42 PM, Michael Walle wrote:
If there are aliases for an uclass, set the base for the "dynamically"
allocated numbers next to the highest alias.
Please note, that this might lead to holes in the sequences, depending
on the device tree. For example if there is only an alias "ethernet1",
the next device seq number would be 2.
In particular this fixes a problem with boards which are using ethernet
aliases but also might have network add-in cards like the E1000. If the
board is started with the add-in card and depending on the order of the
drivers, the E1000 might occupy the first ethernet device and mess up
all the hardware addresses, because the devices are now shifted by one.
As a side effect, this should also make the following commits
superfluous:
- 7f3289bf6d ("dm: device: Request next sequence number")
- 61607225d1 ("i2c: Fill req_seq in i2c_post_bind()")
Although I don't understand the root cause of the said problem.
Thomas, Michal, could you please test this and then I'd add a second
patch removing the old code.
Cc: Thomas Fitzsimmons
Cc: Michal Simek
Signed-off-by: Michael Walle
---
drivers/core/uclass.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c520ef113a..c3b325141a 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -675,13 +675,14 @@ int uclass_unbind_device(struct udevice *dev)
int uclass_resolve_seq(struct udevice *dev)
{
+ struct uclass *uc = dev->uclass;
+ struct uclass_driver *uc_drv = uc->uc_drv;
struct udevice *dup;
- int seq;
+ int seq = 0;
int ret;
assert(dev->seq == -1);
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id,
dev->req_seq,
- false, );
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false,
);
if (!ret) {
dm_warn("Device '%s': seq %d is in use by '%s'\n",
dev->name, dev->req_seq, dup->name);
@@ -693,9 +694,16 @@ int uclass_resolve_seq(struct udevice *dev)
return ret;
}
- for (seq = 0; seq < DM_MAX_SEQ; seq++) {
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, seq,
- false, );
+ if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
+ (uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
+ /* dev_read_alias_highest_id() will return -1 if there no
nits: there /is/ no and multi-line comment starts with just /* on the
1st line
yeah, I'd prefer that style too. But checkpatch.pl would complain
about it.. or so I thought.. well seems only to be the case in linux and
only in "NETWORKING_BLOCK_COMMENT_STYLE" (only for net/ and drivers/net/).
I'll definitely fix that ;)
+ * alias. Thus we can always add one.
+ */
+ seq = dev_read_alias_highest_id(uc_drv->name) + 1;
+ }
+
+ for (; seq < DM_MAX_SEQ; seq++) {
+ ret = uclass_find_device_by_seq(uc_drv->id, seq, false, );
if (ret == -ENODEV)
break;
if (ret)
Reviewed-by: Alex Marginean
Tested-by: Alex Marginean
This looks nice.
One thing I noticed is that 'dm tree' indexes now don't match dev->seq
which can be a bit confusing. The index is produced by
dev_get_uclass_index, which in effect counts devices in the uclass.
Any issue if 'dm tree' prints dev->seq instead and maybe call the
column Seq rather than Index?
Mhh, did this ever match if req_seq/aliases was used? "dm uclass" dumps
the seq and req_seq. So I don't know if that was ever intended to match.
But I'm open to suggestions.
-michael
It's certainly not something that should be part of this patch and since
I think it should be changed I can send a patch for it.
Thanks!
Alex
Re: [U-Boot] [PATCH] net/phy: Fix phy_connect() for phy addr 0
On 12/19/2019 7:30 AM, Priyanka Jain wrote: -Original Message- From: U-Boot On Behalf Of Marek Vasut Sent: Wednesday, December 18, 2019 9:47 PM To: joe.hershber...@ni.com Cc: u-boot@lists.denx.de; Tom Rini ; Joseph Hershberger Subject: Re: [U-Boot] [PATCH] net/phy: Fix phy_connect() for phy addr 0 On 12/18/19 5:15 PM, Joe Hershberger wrote: On Tue, Dec 17, 2019 at 11:55 PM Marek Vasut wrote: On 12/18/19 3:06 AM, Joe Hershberger wrote: On Tue, Dec 17, 2019 at 1:04 PM Marek Vasut wrote: On 12/17/19 7:47 PM, Joe Hershberger wrote: On Tue, Dec 17, 2019 at 11:46 AM Marek Vasut wrote: On 12/17/19 5:25 PM, Joe Hershberger wrote: Hi Marek, Hi Joe, On Tue, Dec 17, 2019 at 1:39 AM Marek Vasut wrote: On 11/7/19 9:04 PM, Joe Hershberger wrote: On Thu, Nov 7, 2019 at 1:16 PM Tom Rini wrote: On Tue, Nov 05, 2019 at 04:05:11AM +, Priyanka Jain wrote: Fix 'mask' calculation in phy_connect() for phy addr '0'. 'mask' is getting set to '0x' for phy addr '0' in phy_connect() whereas expected value is '0'. Signed-off-by: Priyanka Jain Reported-by: tetsu-aoki via github Acked-by: Joe Hershberger Sadly, this breaks systems where a PHY is at address 0. I have such an STM32MP1 system with LAN8720 PHY and since this patch, I cannot use ethernet. Please revert. It seems like a case that shouldn't have worked before. Eh? PHY at address 0 definitely did work before and must work now. Agreed that a phy at address 0 should work. Not agreed that because the value "0" used to work due to a bug that it must still. Which of these is the statement you are making? Do we already agree or disagree? I am saying that because a board worked on rc4 and does not work on rc5, this is a bug introduced by this patch in rc5 and must be fixed before the release. The address 0 is a PHY broadcast address for some PHYs, it's a fixed address for other PHYs. Thus, a PHY at address 0 must work. If this is broken now, it's a bug. The only thing this patch should change is to not access addresses other than 0. I read the data sheet for the LAN8720 and it doesn't mention anything about any broadcast behavior, so I'm not sure what you're trying to state here. Read [1] section 3.7.1 PHYAD[2:0]: PHY ADDRESS CONFIGURATION What I am saying is that there are two types of PHYs, ones which treat PHY address 0 as broadcast and ones which treat it as regular address. This one is the later and is configured as such in my case. https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww1. microchip.com%2Fdownloads%2Fen%2FDeviceDoc%2F2164B.pdfda ta=0 2%7C01%7Cpriyanka.jain%40nxp.com%7C5270d34d955647ee66ea08d783d5ab c8%7 C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637122826047859376 mp;sd ata=s22V5eU1kUe0030lbvWazQpooiM2OutlJbTxrPjbxs0%3Dreserved=0 I see. What's an example of a phy that treats 0 as broadcast? IIRC KSZ9031 does. What about this board requires the mask to be all 'f's, other than specifying the wrong phy address? It seems that in your case the phy address is not actually 0 (or the computed mask would find it), but your board dts may be setting it to 0 as an "unknown" value, but the correct unknown value should be "-1". It seems the issue is with these boards. Nope, the address is actually configured to 0 in hardware. Can you double check that? No, sorry, I know the hardware is fixed to 0. Checking it again will not change this fact. It seems there is no phy driver for this in U-Boot so the generic behavior is being used. I'm at a disadvantage of not having this board to try. Can you revert this patch and run with debug enabled for drivers/net/phy/phy.c to determine what is happening for this board? I would appreciate you helping with this. It only says "connected to Generic PHY" . So looking at the commit message, I am not really sure which board or issue does this patch fix. But if I understand the commit message right, then the aim is to set mask to 0 instead of 0x for address 0. But that's not right either, the mask should be BIT(0) = 1 for address 0, and that's what the patch actually does. I guess this then fails somewhere further down the road ... Yes, the commit message is wrong... the expected value is 1, not 0. I missed that in the review. Is the patch you sent earlier a solution for your board or something unrelated you found as a result of this discussion? It works for my board, but I wonder how many other boards got broken here. I also realized a mistake in commit message . The value of mask will be 1. This current patch was basically a fix to an issue reported at https://github.com/u-boot/u-boot/commit/afbc31948a007e03d6a1282677aafc2208f45819#commitcomment-35747179 introduced by commit afbc31948a007e03d6a1282677aafc2208f45819 (net: phy: implement fallback mechanism for negative phy addresses) Before this commit, the argument value passed to phy_find_by_mask() in phy_connect() for phy addr '0' was '1' , so
Re: [PATCH] dm: uclass: don't assign aliased seq numbers
Hi Michael,
On 12/18/2019 5:42 PM, Michael Walle wrote:
If there are aliases for an uclass, set the base for the "dynamically"
allocated numbers next to the highest alias.
Please note, that this might lead to holes in the sequences, depending
on the device tree. For example if there is only an alias "ethernet1",
the next device seq number would be 2.
In particular this fixes a problem with boards which are using ethernet
aliases but also might have network add-in cards like the E1000. If the
board is started with the add-in card and depending on the order of the
drivers, the E1000 might occupy the first ethernet device and mess up
all the hardware addresses, because the devices are now shifted by one.
As a side effect, this should also make the following commits
superfluous:
- 7f3289bf6d ("dm: device: Request next sequence number")
- 61607225d1 ("i2c: Fill req_seq in i2c_post_bind()")
Although I don't understand the root cause of the said problem.
Thomas, Michal, could you please test this and then I'd add a second
patch removing the old code.
Cc: Thomas Fitzsimmons
Cc: Michal Simek
Signed-off-by: Michael Walle
---
drivers/core/uclass.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index c520ef113a..c3b325141a 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -675,13 +675,14 @@ int uclass_unbind_device(struct udevice *dev)
int uclass_resolve_seq(struct udevice *dev)
{
+ struct uclass *uc = dev->uclass;
+ struct uclass_driver *uc_drv = uc->uc_drv;
struct udevice *dup;
- int seq;
+ int seq = 0;
int ret;
assert(dev->seq == -1);
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, dev->req_seq,
- false, );
+ ret = uclass_find_device_by_seq(uc_drv->id, dev->req_seq, false, );
if (!ret) {
dm_warn("Device '%s': seq %d is in use by '%s'\n",
dev->name, dev->req_seq, dup->name);
@@ -693,9 +694,16 @@ int uclass_resolve_seq(struct udevice *dev)
return ret;
}
- for (seq = 0; seq < DM_MAX_SEQ; seq++) {
- ret = uclass_find_device_by_seq(dev->uclass->uc_drv->id, seq,
- false, );
+ if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
+ (uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
+ /* dev_read_alias_highest_id() will return -1 if there no
nits: there /is/ no and multi-line comment starts with just /* on the
1st line
+* alias. Thus we can always add one.
+*/
+ seq = dev_read_alias_highest_id(uc_drv->name) + 1;
+ }
+
+ for (; seq < DM_MAX_SEQ; seq++) {
+ ret = uclass_find_device_by_seq(uc_drv->id, seq, false, );
if (ret == -ENODEV)
break;
if (ret)
Reviewed-by: Alex Marginean
Tested-by: Alex Marginean
This looks nice.
One thing I noticed is that 'dm tree' indexes now don't match dev->seq
which can be a bit confusing. The index is produced by
dev_get_uclass_index, which in effect counts devices in the uclass. Any
issue if 'dm tree' prints dev->seq instead and maybe call the column Seq
rather than Index?
Thanks!
Alex
Re: [PATCH] drivers: net: fsl_enetc: use write_hwaddr()
Hi Michael,
On 12/18/2019 5:47 PM, Michael Walle wrote:
Intead of setting the MAC address in enetc_start() use the proper
write_hwaddr(). U-Boot takes care of the random MAC address, too. Also,
this will correctly handle ethNmacskip etc.
Signed-off-by: Michael Walle
---
drivers/net/fsl_enetc.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index e86f3dddb5..1949530460 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -272,14 +272,19 @@ static int enetc_remove(struct udevice *dev)
return 0;
}
-/* ENETC Port MAC address registers, accepts big-endian format */
-static void enetc_set_primary_mac_addr(struct enetc_priv *priv, const u8 *addr)
+static int enetc_write_hwaddr(struct udevice *dev)
{
+ struct eth_pdata *plat = dev_get_platdata(dev);
+ struct enetc_priv *priv = dev_get_priv(dev);
+ u8 *addr = plat->enetaddr;
+
u16 lower = *(const u16 *)(addr + 4);
u32 upper = *(const u32 *)addr;
enetc_write_port(priv, ENETC_PSIPMAR0, upper);
enetc_write_port(priv, ENETC_PSIPMAR1, lower);
The SI registers hold temporary values which are cleared on FLR. These
MAC addresses will be wiped out at the next _start. The persistent
values are the one stored in IERB. Is this what you want?
+
+ return 0;
}
/* Configure port parameters (# of rings, frame size, enable port) */
@@ -410,7 +415,6 @@ static void enetc_setup_rx_bdr(struct udevice *dev)
*/
static int enetc_start(struct udevice *dev)
{
- struct eth_pdata *plat = dev_get_platdata(dev);
struct enetc_priv *priv = dev_get_priv(dev);
/* reset and enable the PCI device */
@@ -418,12 +422,6 @@ static int enetc_start(struct udevice *dev)
dm_pci_clrset_config16(dev, PCI_COMMAND, 0,
PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
- if (!is_valid_ethaddr(plat->enetaddr)) {
- enetc_dbg(dev, "invalid MAC address, generate random ...\n");
- net_random_ethaddr(plat->enetaddr);
- }
- enetc_set_primary_mac_addr(priv, plat->enetaddr);
-
This looks like the right thing to do, but with enetc_write_hwaddr
writing to IERB. Do you want to send a v2, or do you want me to pick it up?
Thanks!
Alex
enetc_enable_si_port(priv);
/* setup Tx/Rx buffer descriptors */
@@ -549,6 +547,7 @@ static const struct eth_ops enetc_ops = {
.send = enetc_send,
.recv = enetc_recv,
.stop = enetc_stop,
+ .write_hwaddr = enetc_write_hwaddr,
};
U_BOOT_DRIVER(eth_enetc) = {
Re: [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
On 12/17/2019 10:41 AM, Vladimir Oltean wrote:
On Tue, 17 Dec 2019 at 09:18, Alexandru Marginean
wrote:
On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
On Tue, 3 Dec 2019 at 17:32, Alex Marginean wrote:
+/**
+ * This function deals with additional devices around the switch as these
should
+ * have been bound to drivers by now.
+ * TODO: pick up references to other switch devices here, if we're cascaded.
+ */
+static int dm_dsa_pre_probe(struct udevice *dev)
+{
+ struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+ int i;
+
+ if (!platdata)
+ return -EINVAL;
+
+ if (ofnode_valid(platdata->master_node))
+ uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
+>master_dev);
+
+ for (i = 0; i < platdata->num_ports; i++) {
+ struct dsa_port_platdata *port = >port[i];
+
+ if (port->dev) {
+ port->dev->priv = port;
+ port->phy = dm_eth_phy_connect(port->dev);
Fixed-link interfaces don't work with DM_MDIO. That is somewhat
natural as there is no MDIO bus for a fixed-link. However the legacy
phy_connect function can be made rather easily to work with
fixed-link, since it has the necessary code for dealing with it
already. I am not, however, sure how it ever worked in the absence of
an MDIO bus.
commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
Author: Vladimir Oltean
Date: Sat Dec 14 23:25:40 2019 +0200
phy: make phy_connect_fixed work with a null mdio bus
It is utterly pointless to require an MDIO bus pointer for a fixed PHY
device. The fixed.c implementation does not require it, only
phy_device_create. Fix that.
Signed-off-by: Vladimir Oltean
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e4978..8ea5c9005291 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
dev = malloc(sizeof(*dev));
if (!dev) {
printf("Failed to allocate PHY device for %s:%d\n",
- bus->name, addr);
+ bus ? bus->name : "(null bus)", addr);
return NULL;
}
@@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
return NULL;
}
- if (addr >= 0 && addr < PHY_MAX_ADDR)
+ if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
bus->phymap[addr] = dev;
return dev;
With the patch above in place, fixed-link can also be made to work
with some logic similar to what can be seen below:
if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
phy_mode needs to be pre-parsed somewhere else as well
else
port->phy = dm_eth_phy_connect(port->dev);
How would you see fixed-link interfaces being treated? My question so
far is in the context of front-panel ports but I am interested in your
view of the CPU port situation as well.
I was thinking turning dm_eth_phy_connect into a more generic helper
that also deals with fixed links, which it does not yet. That would
How would you do that? Just put my snippet above in a higher-level
wrapper over dm_eth_phy_connect and phy_connect? Otherwise I think
it's pretty difficult and hacky to pass a null mdiodev pointer through
dm_mdio_phy_connect.
It wouldn't go through mdio_phy_connect, dm_eth_phy_connect would need
to figure out if it's a fixed link or PHY and then call the proper
function, like in your snippet.
The MDIO NULL topic is a bit more complicated I think. Some of the Eth
drivers just deal with fixed link internally and don't create a phy
device at all. If there is a PHY device I think we need to have a MDIO
bus for it, and we shouldn't piggy-back on real MDIO buses. I'm
thinking we could have a dummy MDIO bus created automatically by
ETH/MDIO uclass code for fixed link PHY devices.
Thoughts?
Alex
move the "fixed-link" if out of the driver code. Ideally the driver
should be able to call a single helper and, if the device has a DT node,
it would get back a PHY handle to either a proper PHY or to a fixed link
(from phy_connect_fixed).
Alex
+ }
+ }
+
+ return 0;
+}
Thanks,
-Vladimir
Re: [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
On 12/15/2019 3:16 PM, Vladimir Oltean wrote:
On Sun, 15 Dec 2019 at 14:53, Vladimir Oltean wrote:
Hi Alex,
On Tue, 3 Dec 2019 at 18:18, Alex Marginean wrote:
+static int felix_port_enable(struct udevice *dev, int port,
+struct phy_device *phy)
+{
+ struct felix_priv *priv = dev_get_priv(dev);
+ void *base = priv->regs_base;
+
+ out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
+FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
+
+ out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+FELIX_QSYS_SYSTEM_SW_PORT_ENA |
+FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+ if (phy)
+ phy_startup(phy);
+ return 0;
+}
+
+static void felix_port_disable(struct udevice *dev, int port,
+ struct phy_device *phy)
+{
+ struct felix_priv *priv = dev_get_priv(dev);
+ void *base = priv->regs_base;
+
+ out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
+
+ out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+ /*
+* we don't call phy_shutdown here to avoind waiting next time we use
+* the port, but the downside is that remote side will think we're
+* actively processing traffic although we are not.
+*/
+}
--
2.17.1
What is the correct general procedure here, is it to call phy_startup
so late (felix_port_enable)? I'm trying to take this driver as an
example for sja1105, which has RGMII so PCS and no autonomous in-band
AN like felix does. On this switch, it is too late to do phy_startup
now. Instead, I would need to look at phy->speed which should have
been settled by now, and reprogram my MAC with it.
My question is: don't you think phy_startup() and phy_shutdown()
belong in the DSA uclass code?
My bad, phy_startup() is synchronous and waits for PHY AN to complete.
So this is fine.
Thanks,
-Vladimir
OK, thanks :)
Alex
Re: [PATCH v3 4/6] drivers: net: add Felix DSA switch driver
On 12/15/2019 1:53 PM, Vladimir Oltean wrote:
Hi Alex,
On Tue, 3 Dec 2019 at 18:18, Alex Marginean wrote:
+static int felix_port_enable(struct udevice *dev, int port,
+struct phy_device *phy)
+{
+ struct felix_priv *priv = dev_get_priv(dev);
+ void *base = priv->regs_base;
+
+ out_le32(base + FELIX_GMII_MAC_ENA_CFG(port),
+FELIX_GMII_MAX_ENA_CFG_TX | FELIX_GMII_MAX_ENA_CFG_RX);
+
+ out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+FELIX_QSYS_SYSTEM_SW_PORT_ENA |
+FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+ if (phy)
+ phy_startup(phy);
+ return 0;
+}
+
+static void felix_port_disable(struct udevice *dev, int port,
+ struct phy_device *phy)
+{
+ struct felix_priv *priv = dev_get_priv(dev);
+ void *base = priv->regs_base;
+
+ out_le32(base + FELIX_GMII_MAC_ENA_CFG(port), 0);
+
+ out_le32(base + FELIX_QSYS_SYSTEM_SW_PORT_MODE(port),
+FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
+FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
+
+ /*
+* we don't call phy_shutdown here to avoind waiting next time we use
+* the port, but the downside is that remote side will think we're
+* actively processing traffic although we are not.
+*/
+}
--
2.17.1
What is the correct general procedure here, is it to call phy_startup
so late (felix_port_enable)? I'm trying to take this driver as an
example for sja1105, which has RGMII so PCS and no autonomous in-band
AN like felix does. On this switch, it is too late to do phy_startup
now.
Why is it too late? Is it a functional problem, or you're looking to
reduce the waiting time?
Instead, I would need to look at phy->speed which should have
been settled by now, and reprogram my MAC with it.
My question is: don't you think phy_startup() and phy_shutdown()
belong in the DSA uclass code?
Thanks,
-Vladimir
The API is similar to the one in Linux, plus I didn't want to force a
specific PHY related behavior to drivers. Sometimes it's fine to start
up the PHYs at probe and then just send/receive as needed, but that
behavior is not always acceptable. Assume there is some other host
connected to one of the front panel ports, if it sees link up it may
start doing DHCP, IPv6 discovery. I think it's generally better to keep
links down unless the port is actually in use for the benefit of the
remote end. I didn't want to force this by moving the PHY calls to
uclass code. This approach is also similar to eth uclass, it doesn't
handle phy calls for the driver either.
Alex
Re: [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
On 12/15/2019 2:08 PM, Vladimir Oltean wrote:
On Tue, 3 Dec 2019 at 17:32, Alex Marginean wrote:
+/**
+ * This function deals with additional devices around the switch as these
should
+ * have been bound to drivers by now.
+ * TODO: pick up references to other switch devices here, if we're cascaded.
+ */
+static int dm_dsa_pre_probe(struct udevice *dev)
+{
+ struct dsa_perdev_platdata *platdata = dev_get_platdata(dev);
+ int i;
+
+ if (!platdata)
+ return -EINVAL;
+
+ if (ofnode_valid(platdata->master_node))
+ uclass_find_device_by_ofnode(UCLASS_ETH, platdata->master_node,
+>master_dev);
+
+ for (i = 0; i < platdata->num_ports; i++) {
+ struct dsa_port_platdata *port = >port[i];
+
+ if (port->dev) {
+ port->dev->priv = port;
+ port->phy = dm_eth_phy_connect(port->dev);
Fixed-link interfaces don't work with DM_MDIO. That is somewhat
natural as there is no MDIO bus for a fixed-link. However the legacy
phy_connect function can be made rather easily to work with
fixed-link, since it has the necessary code for dealing with it
already. I am not, however, sure how it ever worked in the absence of
an MDIO bus.
commit 1b7e23cc7e6d0dc3fe7ae9c55390b40717ff3c3a
Author: Vladimir Oltean
Date: Sat Dec 14 23:25:40 2019 +0200
phy: make phy_connect_fixed work with a null mdio bus
It is utterly pointless to require an MDIO bus pointer for a fixed PHY
device. The fixed.c implementation does not require it, only
phy_device_create. Fix that.
Signed-off-by: Vladimir Oltean
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 80a7664e4978..8ea5c9005291 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -658,7 +658,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
dev = malloc(sizeof(*dev));
if (!dev) {
printf("Failed to allocate PHY device for %s:%d\n",
- bus->name, addr);
+ bus ? bus->name : "(null bus)", addr);
return NULL;
}
@@ -686,7 +686,7 @@ static struct phy_device *phy_device_create(struct
mii_dev *bus, int addr,
return NULL;
}
- if (addr >= 0 && addr < PHY_MAX_ADDR)
+ if (addr >= 0 && addr < PHY_MAX_ADDR && phy_id != PHY_FIXED_ID)
bus->phymap[addr] = dev;
return dev;
With the patch above in place, fixed-link can also be made to work
with some logic similar to what can be seen below:
if (ofnode_valid(ofnode_find_subnode(port->dev->node, "fixed-link")))
port->phy = phy_connect(NULL, 0, port->dev, phy_mode); //
phy_mode needs to be pre-parsed somewhere else as well
else
port->phy = dm_eth_phy_connect(port->dev);
How would you see fixed-link interfaces being treated? My question so
far is in the context of front-panel ports but I am interested in your
view of the CPU port situation as well.
I was thinking turning dm_eth_phy_connect into a more generic helper
that also deals with fixed links, which it does not yet. That would
move the "fixed-link" if out of the driver code. Ideally the driver
should be able to call a single helper and, if the device has a DT node,
it would get back a PHY handle to either a proper PHY or to a fixed link
(from phy_connect_fixed).
Alex
+ }
+ }
+
+ return 0;
+}
Thanks,
-Vladimir
Re: [PATCH v3 1/6] net: introduce DSA class for Ethernet switches
Hi Vladimir,
On 12/15/2019 1:44 PM, Vladimir Oltean wrote:
Hi Alex,
On Tue, 3 Dec 2019 at 17:32, Alex Marginean wrote:
DSA stands for Distributed Switch Architecture and it covers switches that
are connected to the CPU through an Ethernet link and generally use frame
tags to pass information about the source/destination ports to/from CPU.
Front panel ports are presented as regular ethernet devices in U-Boot and
they are expected to support the typical networking commands.
DSA switches may be cascaded, DSA class code does not currently support
this.
Signed-off-by: Alex Marginean
---
drivers/net/Kconfig| 13 ++
include/dm/uclass-id.h | 1 +
include/net/dsa.h | 141
net/Makefile | 1 +
net/dsa-uclass.c | 369 +
5 files changed, 525 insertions(+)
create mode 100644 include/net/dsa.h
create mode 100644 net/dsa-uclass.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 4182897d89..a4157cb122 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -37,6 +37,19 @@ config DM_MDIO_MUX
This is currently implemented in net/mdio-mux-uclass.c
Look in include/miiphy.h for details.
+config DM_DSA
+ bool "Enable Driver Model for DSA switches"
+ depends on DM_ETH && DM_MDIO
+ help
+ Enable Driver Model for DSA switches
+
+ Adds UCLASS_DSA class supporting switches that follow the Distributed
+ Switch Architecture (DSA). These switches rely on the presence of a
+ management switch port connected to an Ethernet controller capable of
+ receiving frames from the switch. This host Ethernet controller is
+ called "master" and "cpu" in DSA terminology.
+ This is currently implemented in net/dsa-uclass.c
+
config MDIO_SANDBOX
depends on DM_MDIO && SANDBOX
default y
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0c563d898b..8f37a91488 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -42,6 +42,7 @@ enum uclass_id {
UCLASS_DISPLAY, /* Display (e.g. DisplayPort, HDMI) */
UCLASS_DSI_HOST,/* Display Serial Interface host */
UCLASS_DMA, /* Direct Memory Access */
+ UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */
UCLASS_EFI, /* EFI managed devices */
UCLASS_ETH, /* Ethernet device */
UCLASS_FIRMWARE,/* Firmware */
diff --git a/include/net/dsa.h b/include/net/dsa.h
new file mode 100644
index 00..2387419b9d
--- /dev/null
+++ b/include/net/dsa.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#ifndef __DSA_H__
+#define __DSA_H__
+
+#include
+#include
+#include
+
+/**
+ * DSA stands for Distributed Switch Architecture and it is infrastructure
+ * intended to support drivers for Switches that rely on an intermediary
+ * Ethernet device for I/O. These switches may support cascading allowing
+ * them to be arranged as a tree.
+ * DSA is documented in detail in the Linux kernel documentation under
+ * Documentation/networking/dsa/dsa.txt
+ * The network layout of such a switch is shown below:
+ *
+ * |---
+ * | CPU network device (eth0)|
+ *
+ * | |
+ * ||
+ * | Switch driver |
+ * ||
+ * |||| ||
+ * |---| |---| |---|
+ * | sw0p0 | | sw0p1 | | sw0p2 |
+ * |---| |---| |---|
+ *
+ * In U-Boot the intent is to allow access to front panel ports (shown at the
+ * bottom of the picture) though the master Ethernet port (eth0 in the
picture).
+ * Front panel ports are presented as regular Ethernet devices in U-Boot and
+ * they are expected to support the typical networking commands.
+ * In general DSA switches require the use of tags, extra headers added both by
+ * software on Tx and by the switch on Rx. These tags carry at a minimum port
+ * information and switch information for cascaded set-ups.
+ * In U-Boot these tags are inserted and parsed by the DSA switch driver, the
+ * class code helps with headroom/tailroom for the extra headers.
+ *
+ * TODO:
+ * - handle switch cascading, for now U-Boot only supports stand-alone
switches.
+ * - propagate the master Eth MAC address to switch ports, this is used in
+ * Linux to avoid using additional MAC addresses on master Eth.
Any idea how this would be done?
The DSA master port needs to have the MAC address of the switch eth
device in its RX filter (eth_get_ops(dev)->write_hwaddr), or otherwise
be in promiscuous mode to receive packets destined to its MAC address.
Either that, or the switch eth devices need to inherit the MAC address
of the
Re: [PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
On 12/12/2019 1:12 PM, Michael Walle wrote:
Hi Alex,
Am 2019-12-11 22:01, schrieb Alexandru Marginean:
Hi Michael,
On 12/11/2019 6:03 PM, Michael Walle wrote:
Hi Alex,
Am 2019-12-11 16:37, schrieb Alexandru Marginean:
On 12/11/2019 2:16 PM, Michael Walle wrote:
Hi Vladimir,
Am 2019-12-11 13:46, schrieb Vladimir Oltean:
Hi Michael,
On Wed, 11 Dec 2019 at 00:48, Michael Walle wrote:
Am 2019-12-10 15:55, schrieb Alex Marginean:
> Passes on the primary address used by u-boot to Linux. The
code does a
> DT
> fix-up for ENETC PFs and sets the primary MAC address in IERB.
The
> address
> in IERB is restored on ENETC PCI functions at FLR.
I don't get the reason why this is done in a proprietary way.
What is
the
difference between any other network interface whose hardware
address is
set up in the generic u-boot code.
Also, shouldn't write_hwaddr callback be implemented instead of the
enetc_set_ierb_primary_mac()?
At the moment, the Linux driver ignored the device tree and only
reads
the POR values of the SIPMAR registers. The reset value of those
comes
from the IERB space, which U-Boot is configuring. So it would be good
if that behavior keeps working.
It would also be good if the Linux driver called of_get_mac_address,
so it needs the device tree binding for that. That's why both fixups
are performed, and why the generic function is not used.
yes, but u-boot already sets the mac-address/local-mac-address
property
in the device tree already in a generic way, see fdt_fixup_ethernet().
I think fdt_fixup_ethernet is not a good choice for us.
The issue is that it ties Linux DT to device indexes in U-Boot.
That's a problem if we plug an Eth PCI card in, we would need to
change Linux DT, which is definitely not desirable.
We actually do use PCI Eth cards with some of our boards and U-Boot
does pick those up and indexes shift.
are you sure? afaik it works by reading the /alias/ethernetN phandles
which gets ethNaddr assigned if you set the FDT_SEQ_MACADDR_FROM_ENV
config option. I've just tried it, here is my linux dts diff
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -17,6 +17,11 @@
#address-cells = <2>;
#size-cells = <2>;
+ aliases {
+ ethernet0 = _port0;
+ ethernet1 = _port1;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -761,10 +766,12 @@
enetc_port0: ethernet@0,0 {
compatible = "fsl,enetc";
reg = <0x00 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00
00 ];
};
enetc_port1: ethernet@0,1 {
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00
00 ];
};
enetc_mdio_pf3: mdio@0,3 {
compatible = "fsl,enetc-mdio";
That way the mapping between ethNaddr and the network device can also
be changed by the user by changing the linux device tree.
also, uboot should respect the /aliases/ethernetN, too.
I don't disagree with any of that, but that's not the issue I
mentioned. I meant actual PCI cards being plugged in, I didn't mean
having disabled ECAM functions.
In your example DT enetc port 0 is tied to /aliases/ethernet0 and to
ethaddr, enetc port 1 is tied to /aliases/ethernet1 and to eth1addr.
A LS1028 board with a PCI Eth card plugged in shows this:
Net: e1000: 68:05:ca:66:bf:bd
eth0: e1000#0 [PRIME], eth1: enetc-0, eth2: enetc-2, eth3:
swp0, eth4: swp1, eth5: swp2, eth6: swp3
In this case eth0 is the e1000 card and it uses ethaddr, enetc port 0
is eth1 and uses eth1addr. The fix-up to /aliases/ethernet0 in your
example makes enetc port 0 get the MAC address of the PCI card. If
the PCI card is removed then enetc port 0 ends up being eth0 in U-Boot
and and actually use ethaddr, not eth1addr.
Mh, I've just tried this and it seems that this is even worse. Because
if you set the ethaddr, uboot will complain about mismatching ethernet
addresses.
Between ethaddr from eeprom and e1000 ROM address? Yes, it will complain.
To make this work with a PCI card plugged in one would need to change
the aliases in Linux DT, which is not a fun thing to do.
BTW what will be the source of the network addresses, the u-boot
environment variables? (which might be set either by the user or some
kind of board specific code).
Yes, the environment variables. As far as I know these come preset
from the factory. The reference boards usually come with stickers
too, listing the preset MAC addresses.
so here is already the first
Re: [PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
Hi Michael,
On 12/11/2019 6:03 PM, Michael Walle wrote:
Hi Alex,
Am 2019-12-11 16:37, schrieb Alexandru Marginean:
On 12/11/2019 2:16 PM, Michael Walle wrote:
Hi Vladimir,
Am 2019-12-11 13:46, schrieb Vladimir Oltean:
Hi Michael,
On Wed, 11 Dec 2019 at 00:48, Michael Walle wrote:
Am 2019-12-10 15:55, schrieb Alex Marginean:
> Passes on the primary address used by u-boot to Linux. The code
does a
> DT
> fix-up for ENETC PFs and sets the primary MAC address in IERB. The
> address
> in IERB is restored on ENETC PCI functions at FLR.
I don't get the reason why this is done in a proprietary way. What is
the
difference between any other network interface whose hardware
address is
set up in the generic u-boot code.
Also, shouldn't write_hwaddr callback be implemented instead of the
enetc_set_ierb_primary_mac()?
At the moment, the Linux driver ignored the device tree and only reads
the POR values of the SIPMAR registers. The reset value of those comes
from the IERB space, which U-Boot is configuring. So it would be good
if that behavior keeps working.
It would also be good if the Linux driver called of_get_mac_address,
so it needs the device tree binding for that. That's why both fixups
are performed, and why the generic function is not used.
yes, but u-boot already sets the mac-address/local-mac-address property
in the device tree already in a generic way, see fdt_fixup_ethernet().
I think fdt_fixup_ethernet is not a good choice for us.
The issue is that it ties Linux DT to device indexes in U-Boot.
That's a problem if we plug an Eth PCI card in, we would need to
change Linux DT, which is definitely not desirable.
We actually do use PCI Eth cards with some of our boards and U-Boot
does pick those up and indexes shift.
are you sure? afaik it works by reading the /alias/ethernetN phandles
which gets ethNaddr assigned if you set the FDT_SEQ_MACADDR_FROM_ENV
config option. I've just tried it, here is my linux dts diff
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -17,6 +17,11 @@
#address-cells = <2>;
#size-cells = <2>;
+ aliases {
+ ethernet0 = _port0;
+ ethernet1 = _port1;
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -761,10 +766,12 @@
enetc_port0: ethernet@0,0 {
compatible = "fsl,enetc";
reg = <0x00 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00 00 ];
};
enetc_port1: ethernet@0,1 {
compatible = "fsl,enetc";
reg = <0x000100 0 0 0 0>;
+ local-mac-address = [ 00 00 00 00 00 00 ];
};
enetc_mdio_pf3: mdio@0,3 {
compatible = "fsl,enetc-mdio";
That way the mapping between ethNaddr and the network device can also
be changed by the user by changing the linux device tree.
also, uboot should respect the /aliases/ethernetN, too.
I don't disagree with any of that, but that's not the issue I mentioned.
I meant actual PCI cards being plugged in, I didn't mean having
disabled ECAM functions.
In your example DT enetc port 0 is tied to /aliases/ethernet0 and to
ethaddr, enetc port 1 is tied to /aliases/ethernet1 and to eth1addr.
A LS1028 board with a PCI Eth card plugged in shows this:
Net: e1000: 68:05:ca:66:bf:bd
eth0: e1000#0 [PRIME], eth1: enetc-0, eth2: enetc-2, eth3: swp0,
eth4: swp1, eth5: swp2, eth6: swp3
In this case eth0 is the e1000 card and it uses ethaddr, enetc port 0 is
eth1 and uses eth1addr. The fix-up to /aliases/ethernet0 in your
example makes enetc port 0 get the MAC address of the PCI card. If the
PCI card is removed then enetc port 0 ends up being eth0 in U-Boot and
and actually use ethaddr, not eth1addr.
To make this work with a PCI card plugged in one would need to change
the aliases in Linux DT, which is not a fun thing to do.
BTW what will be the source of the network addresses, the u-boot
environment variables? (which might be set either by the user or some
kind of board specific code).
Yes, the environment variables. As far as I know these come preset from
the factory. The reference boards usually come with stickers too,
listing the preset MAC addresses.
Also U-Boot and Linux DTs have to be in sync all the time, if we
disable one port in U-Boot but not in Linux we would mix up MAC
addresses.
I see. But that shouldn't happen with the code above. But are you sure
that this
+ uclass_get(UCLASS_ETH, );
+ uclass_foreach_dev(dev, uc) {
will work then? in my config (just enetc-0) there is only one eth device
# d
Re: [PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
On 12/11/2019 2:16 PM, Michael Walle wrote: Hi Vladimir, Am 2019-12-11 13:46, schrieb Vladimir Oltean: Hi Michael, On Wed, 11 Dec 2019 at 00:48, Michael Walle wrote: Am 2019-12-10 15:55, schrieb Alex Marginean: > Passes on the primary address used by u-boot to Linux. The code does a > DT > fix-up for ENETC PFs and sets the primary MAC address in IERB. The > address > in IERB is restored on ENETC PCI functions at FLR. I don't get the reason why this is done in a proprietary way. What is the difference between any other network interface whose hardware address is set up in the generic u-boot code. Also, shouldn't write_hwaddr callback be implemented instead of the enetc_set_ierb_primary_mac()? At the moment, the Linux driver ignored the device tree and only reads the POR values of the SIPMAR registers. The reset value of those comes from the IERB space, which U-Boot is configuring. So it would be good if that behavior keeps working. It would also be good if the Linux driver called of_get_mac_address, so it needs the device tree binding for that. That's why both fixups are performed, and why the generic function is not used. yes, but u-boot already sets the mac-address/local-mac-address property in the device tree already in a generic way, see fdt_fixup_ethernet(). I think fdt_fixup_ethernet is not a good choice for us. The issue is that it ties Linux DT to device indexes in U-Boot. That's a problem if we plug an Eth PCI card in, we would need to change Linux DT, which is definitely not desirable. We actually do use PCI Eth cards with some of our boards and U-Boot does pick those up and indexes shift. Also U-Boot and Linux DTs have to be in sync all the time, if we disable one port in U-Boot but not in Linux we would mix up MAC addresses. So as far as using generic fix-up code I'm all for it, but in this case we would need some platform specific rules to match Linux DT nodes to U-Boot eth addresses. As for the write_hwaddr callback instead of enetc_set_primary_mac_addr, that is valid but I suppose it is outside the scope of this patch. That function is related to the runtime MAC address and not to the MAC passed to Linux. according to the comment in eth-uclass.c this is not for (u-boot) runtime: /* * Devices need to write the hwaddr even if not started so that Linux * will have access to the hwaddr that u-boot stored for the device. * This is accomplished by attempting to probe each device and calling * their write_hwaddr() operation. */ and the runtime mac address for u-boot is already set enetc_start(). -michael This is fine, I'll move the IERB code to write_hwaddr. Alex
Re: [PATCH v2] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
On 12/10/2019 11:47 PM, Michael Walle wrote:
Am 2019-12-10 15:55, schrieb Alex Marginean:
Passes on the primary address used by u-boot to Linux. The code does
a DT
fix-up for ENETC PFs and sets the primary MAC address in IERB. The
address
in IERB is restored on ENETC PCI functions at FLR.
I don't get the reason why this is done in a proprietary way. What is the
difference between any other network interface whose hardware address is
set up in the generic u-boot code.
By proprietary do you mean IERB?
Network cards normally have a ROM which comes preset from the factory
with default MAC addresses. At run-time drivers can use the default
address or replace it. The MAC address in IERB is the default MAC
address for ENETC interfaces at run-time, this address is available to
the driver/stack after issuing an FLR and in the absence of a DT node
for the PCI function.
We can pass MAC addresses to Linux through DT alone, but that's more of
a hassle if Linux decides to assign the PCI function to a guest or
user-space app. Using the IERB values doesn't require any fix-up for
guest DTs.
IERB is not actually a ROM though and we need U-Boot to set factory MAC
addresses into IERB some time before Linux boots.
Also, shouldn't write_hwaddr callback be implemented instead of the
enetc_set_ierb_primary_mac()?
OK, makes sense, I will move the code there.
I'll send a v3,
Thanks!
Alex
-michael
Signed-off-by: Alex Marginean
---
The code is called fom ft_board_setup in
board/freescale/ls1028a/ls1028a.c
mostly for consistency with other LS parts. I'm open to suggestions
though.
Changes in v2:
- fixed warning for fdt_fixup_enetc_mac being implicitly declared
board/freescale/ls1028a/ls1028a.c | 5 +++
drivers/net/fsl_enetc.c | 65 ++-
drivers/net/fsl_enetc.h | 3 ++
3 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/board/freescale/ls1028a/ls1028a.c
b/board/freescale/ls1028a/ls1028a.c
index a9606b8865..1a82c95402 100644
--- a/board/freescale/ls1028a/ls1028a.c
+++ b/board/freescale/ls1028a/ls1028a.c
@@ -25,6 +25,7 @@
#include
#include
#include "../common/qixis.h"
+#include "../drivers/net/fsl_enetc.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -150,6 +151,10 @@ int ft_board_setup(void *blob, bd_t *bd)
fdt_fixup_icid(blob);
+#ifdef CONFIG_FSL_ENETC
+ fdt_fixup_enetc_mac(blob);
+#endif
+
return 0;
}
#endif
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 0ca7e838a8..3c043888db 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -14,6 +14,69 @@
#include "fsl_enetc.h"
+#define ENETC_DRIVER_NAME "enetc_eth"
+
+/*
+ * sets the MAC address in IERB registers, this setting is persistent
and
+ * carried over to Linux.
+ */
+static void enetc_set_ierb_primary_mac(struct udevice *dev, int devfn,
+ const u8 *enetaddr)
+{
+#ifdef CONFIG_ARCH_LS1028A
+/*
+ * LS1028A is the only part with IERB at this time and there are
plans to change
+ * its structure, keep this LS1028A specific for now
+ */
+#define IERB_BASE 0x1f080ULL
+#define IERB_PFMAC(pf, vf, n) (IERB_BASE + 0x8000 + (pf) * 0x100 +
(vf) * 8 \
+ + (n) * 4)
+
+static int ierb_fn_to_pf[] = {0, 1, 2, -1, -1, -1, 3};
wrong indendation
+
+ u16 lower = *(const u16 *)(enetaddr + 4);
+ u32 upper = *(const u32 *)enetaddr;
+
+ if (ierb_fn_to_pf[devfn] < 0)
+ return;
it would be easier to read if ierb_fn_to_pf[devfn] would be assigned to
a local variable.
+
+ out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 0), upper);
+ out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 1), (u32)lower);
+#endif
+}
+
+/* sets up primary MAC addresses in DT/IERB */
+void fdt_fixup_enetc_mac(void *blob)
+{
+ struct pci_child_platdata *ppdata;
+ struct eth_pdata *pdata;
+ struct udevice *dev;
+ struct uclass *uc;
+ char path[256];
+ int offset;
+ int devfn;
+
+ uclass_get(UCLASS_ETH, );
+ uclass_foreach_dev(dev, uc) {
+ if (!dev->driver || !dev->driver->name ||
+ strcmp(dev->driver->name, ENETC_DRIVER_NAME))
+ continue;
+
+ pdata = dev_get_platdata(dev);
+ ppdata = dev_get_parent_platdata(dev);
+ devfn = PCI_FUNC(ppdata->devfn);
+
+ enetc_set_ierb_primary_mac(dev, devfn, pdata->enetaddr);
+
+ snprintf(path, 256, "/soc/pcie@1f000/ethernet@%x,%x",
+ PCI_DEV(ppdata->devfn), PCI_FUNC(ppdata->devfn));
+ offset = fdt_path_offset(blob, path);
+ if (offset < 0)
+ continue;
+ fdt_setprop(blob, offset, "mac-address", pdata->enetaddr, 6);
+ }
+}
+
/*
* Bind the device:
* - set a more explicit name on the interface
@@ -583,7 +646,7 @@ static const struct eth_ops enetc_ops = {
};
U_BOOT_DRIVER(eth_enetc) = {
- .name = "enetc_eth",
+ .name = ENETC_DRIVER_NAME,
.id = UCLASS_ETH,
.bind = enetc_bind,
.probe = enetc_probe,
Re: [PATCH] drivers: net: fsl_enetc: Pass on primary MAC address to Linux
On 12/10/2019 3:54 PM, Vladimir Oltean wrote:
Hi Alex,
On Tue, 10 Dec 2019 at 16:21, Alex Marginean
wrote:
Passes on the primary address used by u-boot to Linux. The code does a DT
fix-up for ENETC PFs and sets the primary MAC address in IERB. The address
in IERB is restored on ENETC PCI functions at FLR.
Signed-off-by: Alex Marginean
---
The code is called fom ft_board_setup in board/freescale/ls1028a/ls1028a.c
mostly for consistency with other LS parts. I'm open to suggestions though.
board/freescale/ls1028a/ls1028a.c | 4 ++
drivers/net/fsl_enetc.c | 65 ++-
2 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/board/freescale/ls1028a/ls1028a.c
b/board/freescale/ls1028a/ls1028a.c
index 3977ecf896..fac03f55e9 100644
--- a/board/freescale/ls1028a/ls1028a.c
+++ b/board/freescale/ls1028a/ls1028a.c
@@ -150,6 +150,10 @@ int ft_board_setup(void *blob, bd_t *bd)
fdt_fixup_icid(blob);
+#ifdef CONFIG_FSL_ENETC
+ fdt_fixup_enetc_mac(blob);
+#endif
+
return 0;
}
#endif
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 02c1ee70d9..f8fe7d4d8d 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -14,6 +14,69 @@
#include "fsl_enetc.h"
+#define ENETC_DRIVER_NAME "enetc_eth"
+
+/*
+ * sets the MAC address in IERB registers, this setting is persistent and
+ * carried over to Linux.
+ */
+static void enetc_set_ierb_primary_mac(struct udevice *dev, int devfn,
+ const u8 *enetaddr)
+{
+#ifdef CONFIG_ARCH_LS1028A
+/*
+ * LS1028A is the only part with IERB at this time and there are plans to
change
+ * its structure, keep this LS1028A specific for now
+ */
+#define IERB_BASE 0x1f080ULL
+#define IERB_PFMAC(pf, vf, n) (IERB_BASE + 0x8000 + (pf) * 0x100 + (vf) * 8 \
++ (n) * 4)
+
+static int ierb_fn_to_pf[] = {0, 1, 2, -1, -1, -1, 3};
+
+ u16 lower = *(const u16 *)(enetaddr + 4);
+ u32 upper = *(const u32 *)enetaddr;
+
+ if (ierb_fn_to_pf[devfn] < 0)
+ return;
+
+ out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 0), upper);
+ out_le32(IERB_PFMAC(ierb_fn_to_pf[devfn], 0, 1), (u32)lower);
+#endif
+}
+
+/* sets up primary MAC addresses in DT/IERB */
+void fdt_fixup_enetc_mac(void *blob)
+{
+ struct pci_child_platdata *ppdata;
+ struct eth_pdata *pdata;
+ struct udevice *dev;
+ struct uclass *uc;
+ char path[256];
+ int offset;
+ int devfn;
+
+ uclass_get(UCLASS_ETH, );
+ uclass_foreach_dev(dev, uc) {
+ if (!dev->driver || !dev->driver->name ||
+ strcmp(dev->driver->name, ENETC_DRIVER_NAME))
+ continue;
+
+ pdata = dev_get_platdata(dev);
+ ppdata = dev_get_parent_platdata(dev);
+ devfn = PCI_FUNC(ppdata->devfn);
+
+ enetc_set_ierb_primary_mac(dev, devfn, pdata->enetaddr);
+
+ snprintf(path, 256, "/soc/pcie@1f000/ethernet@%x,%x",
+PCI_DEV(ppdata->devfn), PCI_FUNC(ppdata->devfn));
+ offset = fdt_path_offset(blob, path);
+ if (offset < 0)
+ continue;
+ fdt_setprop(blob, offset, "mac-address", pdata->enetaddr, 6);
The Linux Documentation/devicetree/bindings/net/ethernet.txt says:
- local-mac-address: array of 6 bytes, specifies the MAC address that was
assigned to the network device;
- mac-address: array of 6 bytes, specifies the MAC address that was last used by
the boot program; should be used in cases where the MAC address assigned to
the device by the boot program is different from the "local-mac-address"
property;
I'm not sure which property should be used here, but I think local-mac-address.
U-Boot does use (well, if needed) that address, if that is of any
importance.
of_get_mac_address in Linux has this comment:
/**
* Search the device tree for the best MAC address to use.
'mac-address' is
* checked first, because that is supposed to contain to "most recent" MAC
* address. If that isn't set, then 'local-mac-address' is checked next,
* because that is the default address. If that isn't set, then the
obsolete
* 'address' is checked, just in case we're using an old device tree.
If any
* of the above isn't set, then try to get MAC address from nvmem cell
named
* 'mac-address'.
I think mac-address is fine, we can leave local-mac-address to be used
with some default in Linux DT, in case mac-address is missing.
I don't have a strong opinion either way though.
Thanks!
Alex
+ }
+}
+
/*
* Bind the device:
* - set a more explicit name on the interface
@@ -551,7 +614,7 @@ static const struct eth_ops enetc_ops = {
};
U_BOOT_DRIVER(eth_enetc) = {
- .name = "enetc_eth",
+ .name = ENETC_DRIVER_NAME,
.id = UCLASS_ETH,
Re: [U-Boot] [ANNOUNCEMENT] Expect ML downtime at 2019-12-03 11:00 CET
Hi Claudius, On 12/3/2019 1:28 PM, Claudius Heine wrote: On 03/12/2019 11.25, Claudius Heine wrote: On 03/12/2019 10.05, Claudius Heine wrote: On 03/12/2019 08.44, Claudius Heine wrote: Hi everyone, because we are moving the mailing list to a different server, we have to take it down for hopefully only a few minutes. The DNS entries need to be updated, so it might take a while until those will stabilize. An update: We will also start sending mail over TLS only. That means that your smtp server needs to accept TLS connections. We will accept incoming unencrypted smtpd connections for now, but might disable that at some point as well. If you received this mail, then the migration to the new server was successful. If you encounter any issues then please message me and I will try to sort them out. The mailing will also no longer add the prefix to the subject line or the footer at the end of each message. This change should also now prevent invalidating the DKIM signature of messages. regards, Claudius I've sent 6 patches plus cover about one hour ago and so far patchwork picked up 4 of the patches and I've received 2 in my gmail account. I don't think this is caused by the smtp server I'm using to send, it the new mailing list server running OK? Thank you! Alex
driving Eth PHYs as udevices
Hi, does anyone have any strong feelings either way about the idea of driving Eth PHYs as udevices? I'm looking for a solution for a few problems: - There are a few Eth switches that are accessed over MDIO and could/should be driven as DSA switches instead of PHYs. This requires that MDIO does probing of child devices like any regular bus and doesn't assume they are all PHYs. Also it requires that MDIO enumerates (at least some) of its children instead of wating for an Eth device to come connect to a specific PHY/addr. - PHY scanning using empty 'reg' in PHY DT nodes works for anything more complicated than a single PHY on the bus only only if there is some state in the bus. In Linux child nodes with valid reg are bound first and then any others left are associated with PHYs found scanning. This doesn't work if there is no enumeration on the MDIO bus and it just waits for Eth drivers to connect to PHYs. - there are a few other peculiarities with current PHY support, mentioned by Grygorii on this list, like phy probe being called before its DT node was populated in phydev structure. Using the regular udevice infrastructure should fix that and make PHYs consistent with just about anything else. What I'm thinking to do is to enumerate all child DT nodes when MDIO is probed and bind drivers to them. If some are missing the 'reg' property the bus is scanned. The devices associated with DT child nodes are bound to drivers, probed, if they are PHYs then they have to wait for someone to come connect to them, but if they are switches or anything else they are set up as expected. Thoughts? Alex
Re: [U-Boot] [PATCH 2/5] board: fsl: ls2080a/ls2081a: remove empty arch_misc_init
On 12/3/2019 11:46 AM, Alex Marginean wrote:
> The arch_misc_init function is emtpy on LS2 SoCs/boards, remove it.
s/emtpy/empty/
>
> Signed-off-by: Alex Marginean
> ---
> arch/arm/Kconfig| 5 -
> board/freescale/ls2080a/ls2080a.c | 7 ---
> board/freescale/ls2080aqds/ls2080aqds.c | 7 ---
> board/freescale/ls2080ardb/ls2080ardb.c | 7 ---
> 4 files changed, 26 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3a3d77b04f..459876b543 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1096,7 +1096,6 @@ config TARGET_VEXPRESS64_JUNO
> config TARGET_LS2080A_EMU
> bool "Support ls2080a_emu"
> select ARCH_LS2080A
> - select ARCH_MISC_INIT
> select ARM64
> select ARMV8_MULTIENTRY
> select FSL_DDR_SYNC_REFRESH
> @@ -1109,7 +1108,6 @@ config TARGET_LS2080A_EMU
> config TARGET_LS2080A_SIMU
> bool "Support ls2080a_simu"
> select ARCH_LS2080A
> - select ARCH_MISC_INIT
> select ARM64
> select ARMV8_MULTIENTRY
> select BOARD_LATE_INIT
> @@ -1138,7 +1136,6 @@ config TARGET_LS1088AQDS
> config TARGET_LS2080AQDS
> bool "Support ls2080aqds"
> select ARCH_LS2080A
> - select ARCH_MISC_INIT
> select ARM64
> select ARMV8_MULTIENTRY
> select ARCH_SUPPORT_TFABOOT
> @@ -1157,7 +1154,6 @@ config TARGET_LS2080AQDS
> config TARGET_LS2080ARDB
> bool "Support ls2080ardb"
> select ARCH_LS2080A
> - select ARCH_MISC_INIT
> select ARM64
> select ARMV8_MULTIENTRY
> select ARCH_SUPPORT_TFABOOT
> @@ -1176,7 +1172,6 @@ config TARGET_LS2080ARDB
> config TARGET_LS2081ARDB
> bool "Support ls2081ardb"
> select ARCH_LS2080A
> - select ARCH_MISC_INIT
> select ARM64
> select ARMV8_MULTIENTRY
> select BOARD_LATE_INIT
> diff --git a/board/freescale/ls2080a/ls2080a.c
> b/board/freescale/ls2080a/ls2080a.c
> index 413a698511..bc68f99625 100644
> --- a/board/freescale/ls2080a/ls2080a.c
> +++ b/board/freescale/ls2080a/ls2080a.c
> @@ -48,13 +48,6 @@ void detail_board_ddr_info(void)
> #endif
> }
>
> -#if defined(CONFIG_ARCH_MISC_INIT)
> -int arch_misc_init(void)
> -{
> - return 0;
> -}
> -#endif
> -
> int board_eth_init(bd_t *bis)
> {
> int error = 0;
> diff --git a/board/freescale/ls2080aqds/ls2080aqds.c
> b/board/freescale/ls2080aqds/ls2080aqds.c
> index 25e80c8ac6..fce433461a 100644
> --- a/board/freescale/ls2080aqds/ls2080aqds.c
> +++ b/board/freescale/ls2080aqds/ls2080aqds.c
> @@ -289,13 +289,6 @@ void detail_board_ddr_info(void)
> #endif
> }
>
> -#if defined(CONFIG_ARCH_MISC_INIT)
> -int arch_misc_init(void)
> -{
> - return 0;
> -}
> -#endif
> -
> #if defined(CONFIG_FSL_MC_ENET) && !defined(CONFIG_SPL_BUILD)
> void fdt_fixup_board_enet(void *fdt)
> {
> diff --git a/board/freescale/ls2080ardb/ls2080ardb.c
> b/board/freescale/ls2080ardb/ls2080ardb.c
> index 6a1b8e3f53..282aaf47fb 100644
> --- a/board/freescale/ls2080ardb/ls2080ardb.c
> +++ b/board/freescale/ls2080ardb/ls2080ardb.c
> @@ -318,13 +318,6 @@ void detail_board_ddr_info(void)
> #endif
> }
>
> -#if defined(CONFIG_ARCH_MISC_INIT)
> -int arch_misc_init(void)
> -{
> - return 0;
> -}
> -#endif
> -
> #ifdef CONFIG_FSL_MC_ENET
> void fdt_fixup_board_enet(void *fdt)
> {
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/6] net: introduce DSA class for Ethernet switches
On 12/1/2019 5:17 AM, Florian Fainelli wrote: On 11/30/2019 6:21 PM, Alexandru Marginean wrote: Hi Joe, On 11/30/2019 1:56 AM, Joe Hershberger wrote: Hi Alex, On Mon, Nov 25, 2019 at 9:54 AM Alex Marginean wrote: DSA stands for Distributed Switch Architecture and it covers switches that are connected to the CPU through an Ethernet link and generally use frame tags to pass information about the source/destination ports to/from CPU. Front panel ports are presented as regular ethernet devices in U-Boot and they are expected to support the typical networking commands. DSA switches may be cascaded, DSA class code does not currently support this. Is it expected to eventually retrofit the other switch drivers that we have in U-Boot to use this? For now I would like to at least make sure that the uclass code allows that to happen. Longer term yes, it would be nice to get existing drivers converted. This is applicable to switches that rely on a master Eth device for I/O. The list should include switches that use DSA in kernel, of course: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa Some of these are under PHYs in U-Boot right now (like b53, mv88exxx), hooked up to the master Eth. This works, but comes with limitations, like this in b53 driver: * The configuration determines which PHY ports to activate using the * CONFIG_B53_PHY_PORTS bitmask. Set bit N will active port N and so on. Having these drivers converted should come with some benefits: - their DT nodes would be in sync with the kernel DSA bindings, - users would have some run-time control over the switch ports used for I/O, - driving PHYs of front panel ports comes more naturally with these ports being registered as eth devices. There are other switches which don't fall under DSA, those that present some sort of direct I/O interface to the CPU and don't rely on a master Eth device. These switched may not follow DSA bindings, since they are not technically DSA. For them we could either have a more generic Eth switch class or just let the drivers register the switch device or the ports on the switch as Eth devices. The Device Tree binding for DSA is actually fairly generic within the 'ports' container node, if you omit the ethernet phandle, this still allows you to describe a multi-port Ethernet switch with the data path being contained solely within the switch node and not spread across a DSA master and a discrete switch. At least, this could be a starting point. It is, I don't disagree with that. My argument is DSA-like binding isn't enforced in kernel tree for non-DSA switches (is it?) and allowing U-Boot to use existing kernel DTs for non-DSA switches is a good thing. Binding aside I still think DSA should be a class of its own in U-Boot. There are differences in the API between DSA ports and Eth devices and DSA uclass has the code dealing with master Eth, which is useless for non-DSA. I think this way the code and the interface to the drivers is simpler and more clear. Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] net: eth-uclass: ignore unavailable devices
On 12/1/2019 5:43 PM, Michael Walle wrote:
Am 2019-12-01 01:55, schrieb Alexandru Marginean:
Hi Michael,
On 10/22/2019 1:03 AM, Michael Walle wrote:
device_probe() may fail in which case the seq_id will be -1. Don't
display these devices during startup. While this is only a cosmetic
change, the return value of eth_initialize() will also change to the
actual number of available devices. The return value is only used in
spl_net to decide whether there are any devices to boot from. So
returning only available devices is also more correct in that case.
Signed-off-by: Michael Walle
---
net/eth-uclass.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3bd98b01ad..acd59216e3 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -420,20 +420,25 @@ int eth_initialize(void)
bootstage_mark(BOOTSTAGE_ID_NET_ETH_INIT);
do {
- if (num_devices)
- printf(", ");
+ if (dev->seq != -1) {
+ if (num_devices)
+ printf(", ");
- printf("eth%d: %s", dev->seq, dev->name);
+ printf("eth%d: %s", dev->seq, dev->name);
- if (ethprime && dev == prime_dev)
- printf(" [PRIME]");
+ if (ethprime && dev == prime_dev)
+ printf(" [PRIME]");
+ }
eth_write_hwaddr(dev);
+ if (dev->seq != -1)
+ num_devices++;
uclass_next_device_check();
- num_devices++;
} while (dev);
+ if (!num_devices)
+ printf("No ethernet found.\n");
putc('\n');
}
What would you say about something like this instead? It's a bit more
compact and should be functionally equivalent:
sure, but I've noticed that this is only part of the fix (or isn't even
necessary). At least, it doesn't work if the first device is unavailable,
because eth_get_dev() will then fail. You should be able to reproduce
the issue if you disable the first enetc of the LS1028A. Then network
should only work if ethact is set.
I've traced that back to the pci enumeration which doesn't respect the
'status = "disabled"' property. But instead someone added that check to
the enetc driver, but that doesn't really work, because the device is
already included in the uclass list. See separate patch, I'll send in a
minute.
-michael
For disabled nodes the pci_find_and_bind_driver patch is sufficient.
We should probably deal with drivers failing to probe for other reasons
too, there could be other legitimate reasons for that to happen.
how about the eth_initialize patch plus:
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -69,8 +69,8 @@ struct udevice *eth_get_dev(void)
uc_priv = eth_get_uclass_priv();
if (!uc_priv->current)
- eth_errno = uclass_first_device(UCLASS_ETH,
- _priv->current);
+ uclass_get_device_by_seq(UCLASS_ETH, 0, _priv->current);
+
return uc_priv->current;
}
Since seq == -1 on devices that fail to probe this code will ignore them.
Or add uclass helpers that only deal with devices that probe
successfully: uclass_get_first_device_active,
uclass_get_next_device_active and uclass_foreach_dev_active.
Alex
net/eth-uclass.c | 54 ++--
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 1bc8947749..4d4eaeb371 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -390,10 +390,16 @@ int eth_rx(void)
int eth_initialize(void)
{
+ struct udevice *dev, *prime_dev = NULL;
+ char *ethprime = env_get("ethprime");
int num_devices = 0;
- struct udevice *dev;
+ struct uclass *uc;
eth_common_init();
+ eth_set_dev(NULL);
+
+ if (ethprime)
+ prime_dev = eth_get_dev_by_name(ethprime);
/*
* Devices need to write the hwaddr even if not started so that
Linux
@@ -401,40 +407,30 @@ int eth_initialize(void)
* This is accomplished by attempting to probe each device and
calling
* their write_hwaddr() operation.
*/
- uclass_first_device_check(UCLASS_ETH, );
- if (!dev) {
- printf("No ethernet found.\n");
- bootstage_error(BOOTSTAGE_ID_NET_ETH_START);
- } else {
- char *ethprime = env_get("ethprime");
- struct udevice *prime_dev = NULL;
+ uclass_get(UCLASS_ETH, );
+ uclass_foreach_dev(dev, uc) {
+ if (device_probe(dev))
+ continue;
+
+ eth_write_hwaddr(dev);
+
+ if (num_devices)
+ printf(", ");
+ printf("eth%d: %s", dev->seq, dev->name);
- if (ethprime)
- prime_de
Re: [U-Boot] [PATCH] drivers: pci: ignore disabled devices
On 12/1/2019 5:45 PM, Michael Walle wrote: PCI devices may be disabled in the device tree. Devices which are probed by the device tree handle the "status" property and are skipped if disabled. Devices which are probed by the PCI enumeration don't check that property. Fix it. Signed-off-by: Michael Walle --- drivers/pci/pci-uclass.c | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Alex Marginean Tested-by: Alex Marginean ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/6] net: introduce DSA class for Ethernet switches
Hi Joe,
On 11/30/2019 1:56 AM, Joe Hershberger wrote:
Hi Alex,
On Mon, Nov 25, 2019 at 9:54 AM Alex Marginean
wrote:
DSA stands for Distributed Switch Architecture and it covers switches that
are connected to the CPU through an Ethernet link and generally use frame
tags to pass information about the source/destination ports to/from CPU.
Front panel ports are presented as regular ethernet devices in U-Boot and
they are expected to support the typical networking commands.
DSA switches may be cascaded, DSA class code does not currently support
this.
Is it expected to eventually retrofit the other switch drivers that we
have in U-Boot to use this?
For now I would like to at least make sure that the uclass code allows
that to happen. Longer term yes, it would be nice to get existing
drivers converted. This is applicable to switches that rely on a master
Eth device for I/O. The list should include switches that use DSA in
kernel, of course:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa
Some of these are under PHYs in U-Boot right now (like b53, mv88exxx),
hooked up to the master Eth. This works, but comes with limitations,
like this in b53 driver:
* The configuration determines which PHY ports to activate using the
* CONFIG_B53_PHY_PORTS bitmask. Set bit N will active port N and so on.
Having these drivers converted should come with some benefits:
- their DT nodes would be in sync with the kernel DSA bindings,
- users would have some run-time control over the switch ports used for I/O,
- driving PHYs of front panel ports comes more naturally with these
ports being registered as eth devices.
There are other switches which don't fall under DSA, those that present
some sort of direct I/O interface to the CPU and don't rely on a master
Eth device. These switched may not follow DSA bindings, since they are
not technically DSA. For them we could either have a more generic Eth
switch class or just let the drivers register the switch device or the
ports on the switch as Eth devices.
I'll send a v3 with fixes for the comments below.
Thanks!
Alex
Signed-off-by: Alex Marginean
---
drivers/net/Kconfig| 13 ++
include/dm/uclass-id.h | 1 +
include/dsa.h | 140
Please use include/net/dsa.h
net/Makefile | 1 +
net/dsa-uclass.c | 369 +
5 files changed, 524 insertions(+)
create mode 100644 include/dsa.h
create mode 100644 net/dsa-uclass.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 4182897d89..a4157cb122 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -37,6 +37,19 @@ config DM_MDIO_MUX
This is currently implemented in net/mdio-mux-uclass.c
Look in include/miiphy.h for details.
+config DM_DSA
+ bool "Enable Driver Model for DSA switches"
+ depends on DM_ETH && DM_MDIO
+ help
+ Enable Driver Model for DSA switches
+
+ Adds UCLASS_DSA class supporting switches that follow the Distributed
+ Switch Architecture (DSA). These switches rely on the presence of a
+ management switch port connected to an Ethernet controller capable of
+ receiving frames from the switch. This host Ethernet controller is
+ called "master" and "cpu" in DSA terminology.
+ This is currently implemented in net/dsa-uclass.c
+
config MDIO_SANDBOX
depends on DM_MDIO && SANDBOX
default y
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0c563d898b..8f37a91488 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -42,6 +42,7 @@ enum uclass_id {
UCLASS_DISPLAY, /* Display (e.g. DisplayPort, HDMI) */
UCLASS_DSI_HOST,/* Display Serial Interface host */
UCLASS_DMA, /* Direct Memory Access */
+ UCLASS_DSA, /* Distributed (Ethernet) Switch Architecture */
UCLASS_EFI, /* EFI managed devices */
UCLASS_ETH, /* Ethernet device */
UCLASS_FIRMWARE,/* Firmware */
diff --git a/include/dsa.h b/include/dsa.h
new file mode 100644
index 00..707a1d7e6f
--- /dev/null
+++ b/include/dsa.h
@@ -0,0 +1,140 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2019 NXP
+ */
+
+#ifndef __DSA_H__
+#define __DSA_H__
+
+#include
+#include
+#include
+
+/**
+ * DSA stands for Distributed Switch Architecture and it is infrastructure
+ * intended to support drivers for Switches that rely on an intermediary
+ * Ethernet device for I/O. These switches may support cascading allowing
+ * them to be arranged as a tree.
+ * DSA is documented in detail in the Linux kernel documentation under
+ * Documentation/networking/dsa/dsa.txt
+ * The network layout of such a switch is shown below:
+ *
+ * |---
+ * | CPU network device (eth0)|
+ *
Re: [U-Boot] [PATCH] net: eth-uclass: ignore unavailable devices
Hi Michael,
On 10/22/2019 1:03 AM, Michael Walle wrote:
device_probe() may fail in which case the seq_id will be -1. Don't
display these devices during startup. While this is only a cosmetic
change, the return value of eth_initialize() will also change to the
actual number of available devices. The return value is only used in
spl_net to decide whether there are any devices to boot from. So
returning only available devices is also more correct in that case.
Signed-off-by: Michael Walle
---
net/eth-uclass.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3bd98b01ad..acd59216e3 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -420,20 +420,25 @@ int eth_initialize(void)
bootstage_mark(BOOTSTAGE_ID_NET_ETH_INIT);
do {
- if (num_devices)
- printf(", ");
+ if (dev->seq != -1) {
+ if (num_devices)
+ printf(", ");
- printf("eth%d: %s", dev->seq, dev->name);
+ printf("eth%d: %s", dev->seq, dev->name);
- if (ethprime && dev == prime_dev)
- printf(" [PRIME]");
+ if (ethprime && dev == prime_dev)
+ printf(" [PRIME]");
+ }
eth_write_hwaddr(dev);
+ if (dev->seq != -1)
+ num_devices++;
uclass_next_device_check();
- num_devices++;
} while (dev);
+ if (!num_devices)
+ printf("No ethernet found.\n");
putc('\n');
}
What would you say about something like this instead? It's a bit more
compact and should be functionally equivalent:
net/eth-uclass.c | 54 ++--
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 1bc8947749..4d4eaeb371 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -390,10 +390,16 @@ int eth_rx(void)
int eth_initialize(void)
{
+ struct udevice *dev, *prime_dev = NULL;
+ char *ethprime = env_get("ethprime");
int num_devices = 0;
- struct udevice *dev;
+ struct uclass *uc;
eth_common_init();
+ eth_set_dev(NULL);
+
+ if (ethprime)
+ prime_dev = eth_get_dev_by_name(ethprime);
/*
* Devices need to write the hwaddr even if not started so that Linux
@@ -401,40 +407,30 @@ int eth_initialize(void)
* This is accomplished by attempting to probe each device and calling
* their write_hwaddr() operation.
*/
- uclass_first_device_check(UCLASS_ETH, );
- if (!dev) {
- printf("No ethernet found.\n");
- bootstage_error(BOOTSTAGE_ID_NET_ETH_START);
- } else {
- char *ethprime = env_get("ethprime");
- struct udevice *prime_dev = NULL;
+ uclass_get(UCLASS_ETH, );
+ uclass_foreach_dev(dev, uc) {
+ if (device_probe(dev))
+ continue;
+
+ eth_write_hwaddr(dev);
+
+ if (num_devices)
+ printf(", ");
+ printf("eth%d: %s", dev->seq, dev->name);
- if (ethprime)
- prime_dev = eth_get_dev_by_name(ethprime);
- if (prime_dev) {
+ if (dev == prime_dev) {
eth_set_dev(prime_dev);
eth_current_changed();
- } else {
- eth_set_dev(NULL);
+ printf(" [PRIME]");
}
+ num_devices++;
+ }
+ if (!num_devices) {
+ bootstage_error(BOOTSTAGE_ID_NET_ETH_START);
+ printf("No ethernet found.\n");
+ } else {
bootstage_mark(BOOTSTAGE_ID_NET_ETH_INIT);
- do {
- if (device_active(dev)) {
- if (num_devices)
- printf(", ");
-
- printf("eth%d: %s", dev->seq, dev->name);
-
- if (ethprime && dev == prime_dev)
- printf(" [PRIME]");
- eth_write_hwaddr(dev);
- }
-
- uclass_next_device_check();
- num_devices++;
- } while (dev);
-
putc('\n');
}
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] armv8: fsl-layerscape: LS1044A/1048A: enable Only 1x 10GE port
Hi Pramod,
On 11/25/2019 11:42 AM, Pramod Kumar wrote:
LS1044A, LS1048A are LS1088A personalities, which support only one
1x 10GE port.
There's probably a mix-up, and unfortunately we have an issue in the
LS1088A reference manual available on the website too.
- LS1088A is documented as 2x 10G, no issue there.
- LS1044A is documented as 1x 10G, no AIOP, no issue there.
- LS1048A I thought is a LS1088A with 4 cores but same DPAA, is it not?
That's what the numbering scheme should mean at least.
Appending B of the RM has a nice picture of LS1048A which shows 4 cores
(as expected), 2x 10G ports, AIOP, so the same DPAA as LS1088A.
But the table below mentions '1x 10GE and 8x 1GE'. One of the two has
to be wrong. I place my bet on a copy paste error and on LS1048A having
2x 10G ports.
- LS1084A, which you did not mention, based on the numbering scheme
should be a 8 core device with the same DPAA as LS1044A, so 1x 10G.
Appendix C has a picture that shows a LS1084A device with no AIOP, but
2x 10G ports. The table below mentions no AIOP but '2x 10GE and 8x
1GE'. I would guess that if LS1044A has just one 10G port then LS1084A
has just one too.
Can you please double-check and confirm? It would be nice to open a
ticket for documentation too, there are definitely some inconsistencies
there.
And also make sure the patch works for all 4 personalities.
Alex
MAC1 and MAC2 are associated with 1G SGMII, 2.5G SGMII, and XFI.
Disable MAC1 to have only one 1x 10GE port for LS1044A, LS1048A.
Signed-off-by: Pramod Kumar
---
Changes for v2:
- incorporated review comment's
arch/arm/cpu/armv8/fsl-layerscape/ls1088a_serdes.c | 27 --
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1088a_serdes.c
b/arch/arm/cpu/armv8/fsl-layerscape/ls1088a_serdes.c
index 8e8b45a..7b465e1 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/ls1088a_serdes.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1088a_serdes.c
@@ -1,10 +1,12 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright 2017 NXP
+ * Copyright 2017-2019 NXP
*/
#include
#include
+#include
+#include
struct serdes_config {
u8 ip_protocol;
@@ -32,6 +34,7 @@ static struct serdes_config serdes1_cfg_tbl[] = {
{0x3A, {SGMII3, PCIE1, SGMII1, SGMII2 }, {3, 5, 3, 3 } },
{}
};
+ > static struct serdes_config serdes2_cfg_tbl[] = {
/* SerDes 2 */
{0x0C, {PCIE1, PCIE1, PCIE1, PCIE1 }, {8, 8, 8, 8 } },
@@ -48,6 +51,19 @@ static struct serdes_config *serdes_cfg_tbl[] = {
serdes2_cfg_tbl,
};
+bool soc_has_mac1(void)
+{
+ struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+ unsigned int svr, ver;
+
+ svr = gur_in32(>svr);
+ ver = SVR_SOC_VER(svr);
+ if (ver == SVR_LS1088A)
+ return true;
+ else
+ return false;
+}
+
int serdes_get_number(int serdes, int cfg)
{
struct serdes_config *ptr;
@@ -87,7 +103,14 @@ enum srds_prtcl serdes_get_prtcl(int serdes, int cfg, int
lane)
if (serdes >= ARRAY_SIZE(serdes_cfg_tbl))
return 0;
-
+ /*
+* LS1044A/1048A support only one XFI port
+* Disable MAC1 for LS1044A/1048A
+*/
+ if (!serdes && lane == 2) {
+ if (!soc_has_mac1())
+ return 0;
+ }
ptr = serdes_cfg_tbl[serdes];
while (ptr->ip_protocol) {
if (ptr->ip_protocol == cfg)
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] ls1028a: Configure stream IDs for integrated PCI and fix up Linux DT
Hi Michael,
On 11/27/2019 3:33 PM, Michael Walle wrote:
Hi Alex,
Am 2019-11-27 14:57, schrieb Alex Marginean:
Hardware comes out of reset with implicit values, but these are outside
the accepted range for Layerscape gen 3 chassis spec used on LS1028A.
Allocate different IDs and fix up Linux DT to use them.
Signed-off-by: Alex Marginean
---
arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 9 ++
.../asm/arch-fsl-layerscape/stream_id_lsch3.h | 8 ++
board/freescale/ls1028a/ls1028a.c | 106 ++
Doh :( is there no other place where to put this fixup? That would mean I
have to replicate this code for our custom board and so does every board
which uses the LS1028A. Shouldn't this be in the SoC LS1028A architecture
specific code?
Yeah, you're right about that.
It should probably go into armv8/fsl-layerscape/icid.c or somewhere
around there.
I'll send a v2.
Thanks!
Alex
3 files changed, 123 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
index e993209593..1e7e46e88a 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
@@ -421,6 +421,12 @@ static void fdt_disable_multimedia(void *blob,
unsigned int svr)
}
#endif
+#ifdef CONFIG_PCIE_ECAM_GENERIC
+__weak void fdt_fixup_ecam(void *blob)
+{
+}
+#endif
+
void ft_cpu_setup(void *blob, bd_t *bd)
{
struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
@@ -485,4 +491,7 @@ void ft_cpu_setup(void *blob, bd_t *bd)
#ifdef CONFIG_ARCH_LS1028A
fdt_disable_multimedia(blob, svr);
#endif
+#ifdef CONFIG_PCIE_ECAM_GENERIC
+ fdt_fixup_ecam(blob);
+#endif
}
diff --git
a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
index 94ea99a349..01d362d183 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch3.h
@@ -42,6 +42,10 @@
* -the MC is responsible for allocating and setting up
'isolation context
* IDs (ICIDs) based on the allocated stream IDs for all DPAA2
devices.
*
+ * - ECAM (integrated PCI)
+ * - U-Boot applies the value here to HW and does DT fix-up for both
+ * 'iommu-map' and 'msi-map'
+ *
mhh this is not entirely true, because it is the board code which
does the fixup, which may lead to some confusion. >
* On Chasis-3 SoCs stream IDs are programmed in AMQ registers
(32-bits) for
* each of the different bus masters. The relationship between
* the AMQ registers and stream IDs is defined in the table below:
@@ -98,6 +102,10 @@
#define FSL_DPAA2_STREAM_ID_START 23
#define FSL_DPAA2_STREAM_ID_END 63
+/* PCI IEPs, this overlaps DPAA2 but these two are exclusive at least
for now */
+#define FSL_ECAM_STREAM_ID_START 32
+#define FSL_ECAM_STREAM_ID_END 63
+
#define FSL_SEC_STREAM_ID 64
#define FSL_SEC_JR1_STREAM_ID 65
#define FSL_SEC_JR2_STREAM_ID 66
diff --git a/board/freescale/ls1028a/ls1028a.c
b/board/freescale/ls1028a/ls1028a.c
index a9606b8865..1f5dc0d0b2 100644
--- a/board/freescale/ls1028a/ls1028a.c
+++ b/board/freescale/ls1028a/ls1028a.c
@@ -28,6 +28,52 @@
DECLARE_GLOBAL_DATA_PTR;
+#ifdef CONFIG_PCIE_ECAM_GENERIC
+
+#define ECAM_IERB_BASE 0x1f080ULL
+#define ECAM_IERB_OFFSET_NA -1
+#define ECAM_IERB_FUNC_CNT ARRAY_SIZE(ierb_offset)
+/* cache related transaction attributes for PCIe functions */
+#define ECAM_IERB_MSICAR (ECAM_IERB_BASE + 0xa400)
+#define ECAM_IERB_MSICAR_VALUE 0x30
+
+/* offset of IERB config register per PCI function */
+static int ierb_offset[] = {
+ 0x0800,
+ 0x1800,
+ 0x2800,
+ 0x3800,
+ 0x4800,
+ 0x5800,
+ 0x6800,
+ ECAM_IERB_OFFSET_NA,
+ 0x0804,
+ 0x0808,
+ 0x1804,
+ 0x1808,
+};
+
+/*
+ * Use a custom function for LS1028A, for now this is the only SoC
with IERB
+ * and we're currently considering reorganizing IERB for future SoCs.
+ */
+static void set_ecam_icids(void)
+{
+ int i;
+
+ out_le32(ECAM_IERB_MSICAR, ECAM_IERB_MSICAR_VALUE);
+
+ for (i = 0; i < ECAM_IERB_FUNC_CNT; i++) {
+ if (ierb_offset[i] == ECAM_IERB_OFFSET_NA)
+ continue;
+
+ out_le32(ECAM_IERB_BASE + ierb_offset[i],
+ FSL_ECAM_STREAM_ID_START + i);
+ }
+}
+
+#endif /* CONFIG_PCIE_ECAM_GENERIC */
+
int config_board_mux(void)
{
#if defined(CONFIG_TARGET_LS1028AQDS) && defined(CONFIG_FSL_QIXIS)
@@ -88,6 +134,16 @@ int board_init(void)
#endif
#endif
+
+ /*
+ * ICIDs for other hardware blocks are set really early on,
before MMU
+ * is set up. For integrated PCI we need access to IERB which is
not
+ * part of CCSR, so we have to wait for MMU mappings to be applied
+ */
+#ifdef CONFIG_PCIE_ECAM_GENERIC
+ set_ecam_icids();
+#endif
+
return 0;
}
@@ -244,3 +300,53 @@ int checkboard(void)
return 0;
}
#endif
+
+#ifdef
Re: [U-Boot] [PATCH 5/6] arm: dts: ls1028a: adds Ethernet switch node and its dependencies
Hi Michael, On 11/24/2019 1:11 AM, Michael Walle wrote: > Am 2019-11-22 02:36, schrieb Alex Marginean: >> The definition follows the DSA binding in kernel and describes the >> switch, >> its ports and PHYs. >> ENETC PF6 is the 2nd Eth controller linked to the switch on LS1028, it is > > nitpicking.. LS1028A > > >> not used in U-Boot and was disabled. > > it should be checked that the connected enetc port should be enabled. > otherwise this driver goes awry. On LS1028A if mater Eth is disabled (ENETC PF2) there is a crash, I assume that's what you bumped into. From the looks of it that's caused by a hardware issue. For others reading this, the issue is making PCI PF5 (the switch) unable to use its internal MDIO registers if PF2 (master Eth) is disabled. I don't think I want to have switch driver code check on the ENETC functions, this should end up in a SoC erratum with the recommendation to have PF2 enabled when using the switch. On other platforms having a master Eth disabled means traffic through the switch won't work, but should not crash U-Boot either, uclass code seems to be fine in that regard. Thanks for the review and for the comments, I'll send a v2 for this series. Alex > > [snip] > ___ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/5] doc: bindings: add mdio-handle property to ethernet nodes
On 11/21/2019 1:30 PM, Grygorii Strashko wrote:
Some thought, which i think might help.
- u-boot allows phy node not to have "reg" property.
- phy_connect() will return first PHY discovered on the MDIO bus if
addr<=0
So, if MDIO assignment per ethernet interface/slot is fixed DT can
look like
mdioX {
mdiox_phy0: ethernet-phy@0 {
};
}
ethernetX {
phy-handle = <_phy0>;
}
and so on. driver's parsing code can ignore PHY "reg" prop in phy
node and pass addr<=0 to phy_connect().
Functionally that's what I'm looking for, yes. There is the problem of
not strictly following the kernel binding and at the end of the day I
will have the same problem with the kernel too, so I should take this
topic to the netdev list anyway.
Having 'reg' made optional is actually an interesting idea. I think
'mdio-handle' is a bit more clear what it is, but having an actual PHY
node allows passing DT properties on to the PHY driver which is
certainly useful for some PHYs. It's like saying I don't know what PHY
I'm going to find, but if it's a PHY that has these configurable
properties here is your configuration.
Another good question is "phy-connection-type"/"phy-mode".
Are all your cards works in the same mode? And how is this solved?
These SoCs generally support multiple protocols on the serdes lanes.
The protocol is selected at power on so just using a single static
DT isn't sufficient.
For Linux DT I tried the DT overlay which seems to work fine. I'm
using it to fix up phy-connection-type based on serdes configuration
that is known to u-boot. Of course it's possible to apply an
overlay for each specific plug-in card in a given slot too, but that's
not practical especially for mixes of board with many slots combined
with many types of cards. I'm not sure this can be done automatically
in U-Boot such that the user doesn't have to do any manual configuration
when swapping cards either.
U-Boot doesn't have the luxury of having overlays applied before it
starts, we don't use SPL. I'm considering ways to have platform init
code apply some sort of fix-up or overlay for U-Boot DT based on serdes
configuration, but I don't have something running yet. Live tree is
something I want to look into to see if it would work here.
Is there anything common with SFP? Linux: bindings/net/sff,sfp.txt
They are not SFPs, they are normal MDIO driven PHYs just that they are
mounted on plug-in cards.
It seems, current Linux approach is to have PHY "reg" property as
required, but your case might be the reason to change this. >
I'll have to do a better write-up for the kernel list and we'll see
how this will go.
In the meantime any feedback on the other patches, except the one
introducing mdio-handle? For instance I should deal with fixed links
in the dm_eth_phy_connect helper too (calling phy_connect_fixed),
my intent is to take some code that is otherwise pretty generic out of
the drivers.
From one side, it look nice. From another, in my case MDIO is not a
device, so can't try it.
But, any way, I'd like to share some notes I have. It may help you or may
save your time by reducing number of possible regressions.
To be honest, there is a little mess in PHY creation area :(
Common way is to do in drivers:
phy_connect(mii_bus, phy_addr, dev, interface)
|-phy_find_by_mask()
|-get_phy_device_by_mask()
|-create_phy_by_mask()
|-create_phy_by_mask()
|-phy_device_create()
|-phy_probe()
|-phy_connect_dev(phydev, dev);
..
#ifdef CONFIG_DM_ETH
if (ofnode_valid(slave->data->phy_of_handle))
phydev->node = slave->data->phy_of_handle;
#endif
As you can see from above - the PHY probe will be called when
both phy_device->dev and phydev->node have not been initialized yet,
so no way to perform DT parsing in PHY .probe().
That is certainly inconsistent with the way other kinds of devices work.
What I did for DT based configuration on aquantia PHYs was to read the
DT properties in _config. Just from a practical stand-point that's
fine, phydev->node is presumably set up by now and ethernet drivers have
to call _config anyway and that gives the PHY driver a chance to check
the DT properties. But conceptually it is messy, it makes phy drivers
special. I suppose the solution is to make phy drivers behave like
the other udevices, either actually making them udevices or at least
introducing phy_bind ahead of probe.
On the current code, do you see any issues with dealing with the
PHY DT properties in _config, other than this inconsistency with the
other devices?
Another, not obvious, thing is that DT node for fixed phy passed
through the int addr parameter in create_phy_by_mask().
This is a work-around for the previous issue I assume.
And finally, as phy as fixed-phy may be connected to DT node which is
*not" ethernet device node and describes Port. The ethernet device is
parent DT node in this case.
Are these switch ports or on a multi-port Ethernet
Re: [U-Boot] [PATCH v4 4/5] doc: bindings: add mdio-handle property to ethernet nodes
On 11/20/2019 10:51 AM, Grygorii Strashko wrote:
On 19/11/2019 20:58, Grygorii Strashko wrote:
On 19/11/2019 01:31, Alexandru Marginean wrote:
On 11/18/2019 8:11 PM, Grygorii Strashko wrote:
On 14/11/2019 17:04, Alex Marginean wrote:
Adds an optional mdio-handle property which identifies a MDIO bus
which can
be scanned to find the relevant PHY. The property is ignored if
phy-handle
is also present.
Signed-off-by: Alex Marginean
---
doc/device-tree-bindings/net/ethernet.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/doc/device-tree-bindings/net/ethernet.txt
b/doc/device-tree-bindings/net/ethernet.txt
index 3fc360523b..9f9629f8d6 100644
--- a/doc/device-tree-bindings/net/ethernet.txt
+++ b/doc/device-tree-bindings/net/ethernet.txt
@@ -9,6 +9,8 @@ The following properties are common to the Ethernet
controllers:
- max-speed: number, specifies maximum speed in Mbit/s supported
by the device;
- max-frame-size: number, maximum transfer unit (IEEE defined
MTU), rather than
the maximum frame size (there's contradiction in ePAPR).
+- mdio-handle: phandle, specifies a reference to a MDIO bus to be
scanned to
+ find the PHY device. Ignored if phy-handle is also present.
Sry, but it looks redundant. The Ethernet-controller bindings
expects to use phy-handle which, in turn, allows to get MDIO node.
The problem I'm trying to solve is not that I don't want to get the
parent MDIO bus of a PHY referenced though phy-handle, but rather that I
want to avoid having a specific U-Boot DT for each PHY plug-in card that
could be used in our systems. I'll explain that in more detail.
We have these QDS systems which support plug-in cards with PHYs on them.
The way it works is that a given ethernet interface is associated with
one of the QDS slots that should contain a plug-in card. Each of the
slots has a MDIO bus associated with it, this bus is accessed through a
MDIO MUX on the QDS board.
The slot may be populated with one of several types of cards, which
are different designs, have different types of PHYs and more importantly
use different PHY addresses on the MDIO bus.
So now the summary is that an Ethernet interface is associated with a
MDIO bus that could be scanned to find the relevant PHY, while using the
existing phy-handle binding requires that U-Boot DT is edited whenever
a different card is plugged in, which is not practical and avoidable.
Thank you for explanation. It's clear now.
I think above description is good to have as part of commit message.
So, if your platform is DT based and can use DT then it's reasonable
to follow standard binding,
which, in addition, allows to specify Ethernet PHY properties.
Sure, and we do that on boards and for PHYs which are at fixed addresses
on MDIO bus, but that is impractical with the swappable cards.
Some thought, which i think might help.
- u-boot allows phy node not to have "reg" property.
- phy_connect() will return first PHY discovered on the MDIO bus if addr<=0
So, if MDIO assignment per ethernet interface/slot is fixed DT can look
like
mdioX {
mdiox_phy0: ethernet-phy@0 {
};
}
ethernetX {
phy-handle = <_phy0>;
}
and so on. driver's parsing code can ignore PHY "reg" prop in phy
node and pass addr<=0 to phy_connect().
Functionally that's what I'm looking for, yes. There is the problem of
not strictly following the kernel binding and at the end of the day I
will have the same problem with the kernel too, so I should take this
topic to the netdev list anyway.
Having 'reg' made optional is actually an interesting idea. I think
'mdio-handle' is a bit more clear what it is, but having an actual PHY
node allows passing DT properties on to the PHY driver which is
certainly useful for some PHYs. It's like saying I don't know what PHY
I'm going to find, but if it's a PHY that has these configurable
properties here is your configuration.
It seems, current Linux approach is to have PHY "reg" property as
required, but your case might be the reason to change this. >
I'll have to do a better write-up for the kernel list and we'll see
how this will go.
In the meantime any feedback on the other patches, except the one
introducing mdio-handle? For instance I should deal with fixed links
in the dm_eth_phy_connect helper too (calling phy_connect_fixed),
my intent is to take some code that is otherwise pretty generic out of
the drivers.
Thanks!
Alex
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/5] doc: bindings: add mdio-handle property to ethernet nodes
On 11/18/2019 8:11 PM, Grygorii Strashko wrote: On 14/11/2019 17:04, Alex Marginean wrote: Adds an optional mdio-handle property which identifies a MDIO bus which can be scanned to find the relevant PHY. The property is ignored if phy-handle is also present. Signed-off-by: Alex Marginean --- doc/device-tree-bindings/net/ethernet.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/device-tree-bindings/net/ethernet.txt b/doc/device-tree-bindings/net/ethernet.txt index 3fc360523b..9f9629f8d6 100644 --- a/doc/device-tree-bindings/net/ethernet.txt +++ b/doc/device-tree-bindings/net/ethernet.txt @@ -9,6 +9,8 @@ The following properties are common to the Ethernet controllers: - max-speed: number, specifies maximum speed in Mbit/s supported by the device; - max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than the maximum frame size (there's contradiction in ePAPR). +- mdio-handle: phandle, specifies a reference to a MDIO bus to be scanned to + find the PHY device. Ignored if phy-handle is also present. Sry, but it looks redundant. The Ethernet-controller bindings expects to use phy-handle which, in turn, allows to get MDIO node. The problem I'm trying to solve is not that I don't want to get the parent MDIO bus of a PHY referenced though phy-handle, but rather that I want to avoid having a specific U-Boot DT for each PHY plug-in card that could be used in our systems. I'll explain that in more detail. We have these QDS systems which support plug-in cards with PHYs on them. The way it works is that a given ethernet interface is associated with one of the QDS slots that should contain a plug-in card. Each of the slots has a MDIO bus associated with it, this bus is accessed through a MDIO MUX on the QDS board. The slot may be populated with one of several types of cards, which are different designs, have different types of PHYs and more importantly use different PHY addresses on the MDIO bus. So now the summary is that an Ethernet interface is associated with a MDIO bus that could be scanned to find the relevant PHY, while using the existing phy-handle binding requires that U-Boot DT is edited whenever a different card is plugged in, which is not practical and avoidable. So, if your platform is DT based and can use DT then it's reasonable to follow standard binding, which, in addition, allows to specify Ethernet PHY properties. Sure, and we do that on boards and for PHYs which are at fixed addresses on MDIO bus, but that is impractical with the swappable cards. More over, your series does not provide user for this new property. That's true, I was planning to send the QDS DT after this was merged, but if it helps I can add it to this patch set. Alex Personally I do not see even reasons to have doc/device-tree-bindings/net/ethernet.txt in u-boot and think we should follow [1] - phy-mode: string, operation mode of the PHY interface; supported values are "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-controller.yaml ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v3 0/3] Felix Eth switch driver and questions on DSA switches
Hi Bin, On 11/15/2019 3:32 PM, Bin Meng wrote: Hi Alex, On Fri, Nov 15, 2019 at 8:57 PM Alex Marginean wrote: The driver sets up the switch during probe making external and internal ports available to use. It does not support direct I/O through these switch ports in this version, instead ENETC ethernet interfaces that are internally linked to the switch can be used after the switch is set up. Thanks for the efforts! This is where the RFC part comes in. Both the switch as a device and its ports are probed as ethernet devices. That's handy as accessors to connect to the PHY can be used on switch ports, but otherwise they are useless as the user can't ping to these interfaces directly. We're not running STP in U-Boot either, so turning on the switch is a problem if there are loops in the network. The Linux driver for this piece of HW is now moving under DSA and this leads to my question. Does anyone here think that DSA support is something useful in U-Boot? DSA is described here: https://www.kernel.org/doc/Documentation/networking/dsa/dsa.txt From the doc: Summarized, this is basically how DSA looks like from a network device perspective: |--- | CPU network device (eth0)| | | || | Switch driver | || |||| || |---| |---| |---| | sw0p0 | | sw0p1 | | sw0p2 | |---| |---| |---| If we do DSA in U-Boot we would use the same bindings as in Linux. The switch From the perspective of reusing the same bindings as Linux, yes, I would like to see the same DSA support in U-Boot :) Even without DSA and depending on the driver the Linux DT could work for U-Boot as well. It's just that that DSA specific properties would be ignored and the behavior in U-Boot could be different than the one in Linux, depending on what exactly the driver does. The behavior part is more likely to be a problem for users, not the binding itself. In this driver we enable hardware switching which is fine in most cases and allows tftp on any port, but can cause problems with network loops since we don't run STP. Another U-Boot switch driver I looked at doesn't have the network loop problem, but only works on port 0. I think I'll give it a shot at prototyping some basic DSA support for U-Boot, some of these differences in behavior can be quite annoying and sooner or later they could cause problems. Thanks! Alex would be associated with a master network device which is a regular ethernet, this is part of the DSA binding in Linux. Whenever the user pings through swp0pN in background that would _start swp0pN, the switch port connected to the master network device and the master network device (eth0 in the picture above). Any frames sent through a switch port would have the DSA tag inserted and then actually sent though the mater network device. Similarly for Rx, polling swp0pN would in fact poll on the master network device and for any frame received the DSA code would check and remove the DSA tag. Switching between switch ports would be by default disabled. This kind of switch drivers should go under a new class, DSA or ETH_DSA, or something along those lines. I'd like to get some feedback from networking people on this list, if adding DSA support in U-Boot is something that could be useful, or the existing support is good enough. Currently U-Boot does support a few switches either as PHYs or as ETH devices with various limitations. Feel free to share any thoughts on this topic. With these patches applied the switch on LS1028A looks like this: => dm tree Class Index Probed DriverName --- pci 2 [ + ] pci_generic_ecam |-- pcie@1f000 eth 1 [ + ] enetc_eth | |-- enetc-0 eth 2 [ + ] enetc_eth | |-- enetc-1 eth 3 [ + ] enetc_eth | |-- enetc-2 mdio 5 [ + ] enetc_mdio| |-- emdio-3 pci_generi 0 [ ] pci_generic_drv | |-- pci_3:0.4 eth 4 [ + ] felix_ethsw | |-- felix_ethsw eth 6 [ + ] felix-port| | |-- port@0 eth 7 [ + ] felix-port| | |-- port@1 eth 8 [ + ] felix-port| | |-- port@2 eth 9 [ + ] felix-port| | |-- port@3 eth 10 [ + ] felix-port| | |-- port@4 eth 11 [ + ] felix-port| | `-- port@5 eth 5 [ + ] enetc_eth | |--
Re: [U-Boot] [PATCH v4 0/5] Add helper function for linking a DM Eth device to a PHY
On 11/14/2019 7:19 PM, Alexandru Marginean wrote: Hi Michael, On 11/14/2019 6:41 PM, Michael Walle wrote: Hi, sorry for being so late.. We're going to try to get this binding accepted in Linux too. shouldn't we try to get it accepted to linux first? To avoid any incompatibilities? I can propagate any changes kernel crows asks us to do here, that that was a weird typo. I meant 'kernel crowd' :) shouldn't be too difficult. Of course any feedback from the U-Boot community is welcome too, in the meantime :) Alex -michael ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v4 0/5] Add helper function for linking a DM Eth device to a PHY
Hi Michael, On 11/14/2019 6:41 PM, Michael Walle wrote: Hi, sorry for being so late.. We're going to try to get this binding accepted in Linux too. shouldn't we try to get it accepted to linux first? To avoid any incompatibilities? I can propagate any changes kernel crows asks us to do here, that shouldn't be too difficult. Of course any feedback from the U-Boot community is welcome too, in the meantime :) Alex -michael ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller
Hi JJ, On 11/5/2019 12:50 PM, Jean-Jacques Hiblot wrote: This adds a driver for mmio-based syscon multiplexers controlled by bitfields in a syscon register range. This is heavily based on the linux mmio-mux driver. Signed-off-by: Jean-Jacques Hiblot --- Changes in v2: None drivers/mux/Kconfig | 15 + drivers/mux/Makefile | 1 + drivers/mux/mmio.c | 155 +++ 3 files changed, 171 insertions(+) create mode 100644 drivers/mux/mmio.c do you plan to add support for reg-mux too, for parent devices which are not syscon? Thanks! Alex ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] drivers: net: fsl_enetc: fix RGMII configuration
On 10/26/2019 2:39 AM, Michael Walle wrote: Add the missing RGMII PHY modes in which case the MAC has configure its RGMII settings. The only difference between these modes is the RX and TX delay configuration. A user might choose any RGMII mode in the device tree. Signed-off-by: Michael Walle --- drivers/net/fsl_enetc.c | 3 +++ 1 file changed, 3 insertions(+) Reviewed-by: Alex Marginean Thanks! ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] drivers: net: fsl_enetc: set phydev->node
On 10/26/2019 2:39 AM, Michael Walle wrote: The saved ofnode is used by some PHY drivers to access the device tree node of the PHY. Signed-off-by: Michael Walle --- drivers/net/fsl_enetc.c | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Alex Marginean ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] drivers: net: driver for MDIO muxes controlled over I2C
On 7/13/2019 7:02 AM, Bin Meng wrote:
> Hi Alex,
>
> On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean
> wrote:
>>
>> This driver is used for MDIO muxes driven over I2C. This is currently
>> used on Freescale LS1028A QDS board, on which the physical MDIO MUX is
>> controlled by an on-board FPGA which in turn is configured through I2C.
>>
>> Signed-off-by: Alex Marginean
>> ---
>>
>> Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
>>
>> drivers/net/Kconfig| 8 +++
>> drivers/net/Makefile | 1 +
>> drivers/net/mdio_mux_i2c_reg.c | 108 +
>> 3 files changed, 117 insertions(+)
>> create mode 100644 drivers/net/mdio_mux_i2c_reg.c
>>
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 4d85fb1716..93535f7acb 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -595,4 +595,12 @@ config FSL_ENETC
>>This driver supports the NXP ENETC Ethernet controller found on
>> some
>>of the NXP SoCs.
>>
>> +config MDIO_MUX_I2C_REG
>
> I don't see current Linux upstream has any I2CREG support. I believe
> you will upstream the Linux support too?
Linux support is somewhat different so we won't have the same driver and
binding there. The main difference is that Linux has the concept of a
mux controller, independent of the type of bus it muxes. I didn't add
this to U-Boot, not yet at least.
This specific MDIO mux would be driven in Linux by a pair of devices:
- a mux controller with bindings defined by reg-mux.txt [1], this one
implements the select function,
- MDIO mux as a client of the controller, defined by
mdio-mux-multiplexer.txt [2], which deals with the MDIO specific side of
things.
In U-Boot I picked up the relevant properties from the two bindings, the
MDIO mux device in U-Boot is supposed to implement select like a generic
Linux MUX, while the uclass code deals with MDIO.
I noticed U-Boot already has an I2C MUX class, along the lines of the
MDIO one, I'm not sure if it's useful enough to introduce a new class of
MUX controllers and then make all other kind of muxes depend on it. For
now at least the U-Boot mux drivers are simple enough and the extra
class wouldn't help much.
The other difference is that in Linux the mux controller is driven by
the 'MMIO register bitfield-controlled multiplexer driver' [3], which is
not I2C specific. It is built on top of regmap which I also think is a
bit too much for U-Boot. For the U-Boot driver I just called I2C API
directly and in that context it made sense to add I2C to the driver
name, but there won't be an equivalent I2C based driver for Linux.
I'd really appreciate some feedback on this. I think the fairly simple
approach in U-Boot is good enough, but I'm open to suggestions.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mux/reg-mux.txt
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/mmio.c
>
> So based on existing Linux kernel MMIOREG support, (see
> mdio-mux-mmioreg.txt in the Linux devicetree bindings direoctory),
> looks we need name this as MDIO_MUX_I2CREG for consistency.
>
>> + bool "MDIO MUX accessed as a register over I2C"
>> + depends on DM_MDIO_MUX && DM_I2C
>> + help
>> + This driver is used for MDIO muxes driven by writing to a register
>> of
>> + an I2C chip. The board it was developed for uses a mux controlled
>> by
>> + on-board FPGA which in turn is accessed as a chip over I2C.
>> +
>> endif # NETDEVICES
>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> index 97119cec7c..9470f02dc6 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -80,3 +80,4 @@ obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
>> obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
>> obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o
>> obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o
>> +obj-$(CONFIG_MDIO_MUX_I2C_REG) += mdio_mux_i2c_reg.o
>> diff --git a/drivers/net/mdio_mux_i2c_reg.c b/drivers/net/mdio_mux_i2c_reg.c
>> new file mode 100644
>> index 00..eb75c5e032
>> --- /dev/null
>> +++ b/drivers/net/mdio_mux_i2c_reg.c
>> @@ -0,0 +1,108 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2019
>> + * Alex Marginean, NXP
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +/*
>> + * This driver is used for MDIO muxes driven by writing to a register of an
>> I2C
>> + * chip. The board it was developed for uses a mux controlled by on-board
>> FPGA
>> + * which in turn is accessed as a chip over I2C.
>> + */
>> +
>> +struct mmux_i2c_reg_priv {
>> + struct udevice *chip;
>> + int reg;
>> + int mask;
>> +};
>> +
>> +static int
[U-Boot] [PATCH v3] drivers: net: phy: Use Aquantia driver for AQR112, AQR412
adds AQR112 and AQR412 to the list of supported PHYs using existing AQR
code.
Signed-off-by: Alex Marginean
Reviewed-By: Ramon Fried
---
Changes in v2:
- Numerical ordering of structure definition and function calls
Changes in v3:
- Resent, v2 was mangled up pretty bad by microsoft email server..
drivers/net/phy/aquantia.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 5c3298d612..465ec2d342 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -461,6 +461,19 @@ struct phy_driver aqr107_driver = {
.shutdown = _shutdown,
};
+struct phy_driver aqr112_driver = {
+ .name = "Aquantia AQR112",
+ .uid = 0x3a1b660,
+ .mask = 0xfff0,
+ .features = PHY_10G_FEATURES,
+ .mmds = (MDIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config = _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
struct phy_driver aqr405_driver = {
.name = "Aquantia AQR405",
.uid = 0x3a1b4b2,
@@ -474,6 +487,19 @@ struct phy_driver aqr405_driver = {
.shutdown = _shutdown,
};
+struct phy_driver aqr412_driver = {
+ .name = "Aquantia AQR412",
+ .uid = 0x3a1b710,
+ .mask = 0xfff0,
+ .features = PHY_10G_FEATURES,
+ .mmds = (MDIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config = _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
int phy_aquantia_init(void)
{
phy_register(_driver);
@@ -481,7 +507,9 @@ int phy_aquantia_init(void)
phy_register(_driver);
phy_register(_driver);
phy_register(_driver);
+ phy_register(_driver);
phy_register(_driver);
+ phy_register(_driver);
return 0;
}
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] drivers: net: phy: Use Aquantia driver for AQR112, AQR412
adds AQR112 and AQR412 to the list of supported PHYs using existing AQR
code.
Signed-off-by: Alex Marginean
---
Changes in v2:
- Numerical ordering of structure definition and function calls
drivers/net/phy/aquantia.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 5c3298d612..465ec2d342 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -461,6 +461,19 @@ struct phy_driver aqr107_driver =
.shutdown =en10g_shutdown,
};
+struct phy_driver aqr112_driver =
+ .name =quantia AQR112",
+ .uid =3a1b660,
+ .mask =fff0,
+ .features =Y_10G_FEATURES,
+ .mmds =DIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config =quantia_config,
+ .startup =quantia_startup,
+ .shutdown =en10g_shutdown,
+};
+
struct phy_driver aqr405_driver =
.name =quantia AQR405",
.uid =3a1b4b2,
@@ -474,6 +487,19 @@ struct phy_driver aqr405_driver =
.shutdown =en10g_shutdown,
};
+struct phy_driver aqr412_driver =
+ .name =quantia AQR412",
+ .uid =3a1b710,
+ .mask =fff0,
+ .features =Y_10G_FEATURES,
+ .mmds =DIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config =quantia_config,
+ .startup =quantia_startup,
+ .shutdown =en10g_shutdown,
+};
+
int phy_aquantia_init(void)
{
phy_register(_driver);
@@ -481,7 +507,9 @@ int phy_aquantia_init(void)
phy_register(_driver);
phy_register(_driver);
phy_register(_driver);
+ phy_register(_driver);
phy_register(_driver);
+ phy_register(_driver);
return 0;
}
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] drivers: net: phy: Ignore PHY ID 0 during PHY probing
Current code fails to probe some C45 PHYs that also respond to C22 reads.
This is the case for PHYs like Aquantia AQR112, Marvell 88X2242 (as
previously posted on the u-boot list).
If the PHY ID reads all 0s just ignore it and try the next devad.
Signed-off-by: Alex Marginean
---
drivers/net/phy/phy.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c1c1af9abd..7ccbc4d9da 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -727,6 +727,15 @@ static struct phy_device *create_phy_by_mask(struct
mii_dev *bus,
while (phy_mask) {
int addr = ffs(phy_mask) - 1;
int r = get_phy_id(bus, addr, devad, _id);
+
+ /* If the PHY ID is flat 0 we ignore it. There are C45 PHYs
+* that return all 0s for C22 reads (like Aquantia AQR112) and
+* there are C22 PHYs that return all 0s for C45 reads (like
+* Atheros AR8035).
+*/
+ if (phy_id == 0)
+ return NULL;
+
/* If the PHY ID is mostly f's, we didn't find anything */
if (r == 0 && (phy_id & 0x1fff) != 0x1fff) {
is_c45 = (devad == MDIO_DEVAD_NONE) ? false : true;
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] drivers: net: phy: Use Aquantia driver for AQR112, AQR412
adds AQR112 and AQR412 to the list of supported PHYs using existing AQR
code.
Signed-off-by: Alex Marginean
---
drivers/net/phy/aquantia.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 5c3298d612..4c0fd36ea7 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -474,6 +474,32 @@ struct phy_driver aqr405_driver = {
.shutdown = _shutdown,
};
+struct phy_driver aqr112_driver = {
+ .name = "Aquantia AQR112",
+ .uid = 0x3a1b660,
+ .mask = 0xfff0,
+ .features = PHY_10G_FEATURES,
+ .mmds = (MDIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config = _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
+struct phy_driver aqr412_driver = {
+ .name = "Aquantia AQR412",
+ .uid = 0x3a1b710,
+ .mask = 0xfff0,
+ .features = PHY_10G_FEATURES,
+ .mmds = (MDIO_MMD_PMAPMD | MDIO_MMD_PCS |
+MDIO_MMD_PHYXS | MDIO_MMD_AN |
+MDIO_MMD_VEND1),
+ .config = _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
int phy_aquantia_init(void)
{
phy_register(_driver);
@@ -482,6 +508,8 @@ int phy_aquantia_init(void)
phy_register(_driver);
phy_register(_driver);
phy_register(_driver);
+ phy_register(_driver);
+ phy_register(_driver);
return 0;
}
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/4 v3] net: add MDIO_MUX DM class
Adds a class for MDIO MUXes, which control access to a series of
downstream child MDIOs.
MDIO MUX drivers are required to implement a select function used to switch
between child buses.
MUX children are registered as MDIO buses and they can be used just like
regular MDIOs.
Signed-off-by: Alex Marginean
---
Changes in v2:
- no change
Changes in v3:
- no change, just fighting with the email server
drivers/net/Kconfig| 12 +++
include/dm/uclass-id.h | 1 +
include/miiphy.h | 20
net/Makefile | 1 +
net/mdio-mux-uclass.c | 232 +
5 files changed, 266 insertions(+)
create mode 100644 net/mdio-mux-uclass.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 635f8d72c2..0dc26ac254 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -24,6 +24,18 @@ config DM_MDIO
This is currently implemented in net/mdio-uclass.c
Look in include/miiphy.h for details.
+config DM_MDIO_MUX
+ bool "Enable Driver Model for MDIO MUX devices"
+ depends on DM_MDIO
+ help
+ Enable driver model for MDIO MUX devices
+
+ Adds UCLASS_MDIO_MUX DM class supporting MDIO MUXes. Useful for
+ systems that support DM_MDIO and integrate one or multiple muxes on
+ the MDIO bus.
+ This is currently implemented in net/mdio-mux-uclass.c
+ Look in include/miiphy.h for details.
+
config MDIO_SANDBOX
depends on DM_MDIO && SANDBOX
default y
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 90667e62cf..b859a9ec04 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -59,6 +59,7 @@ enum uclass_id {
UCLASS_MAILBOX, /* Mailbox controller */
UCLASS_MASS_STORAGE,/* Mass storage device */
UCLASS_MDIO,/* MDIO bus */
+ UCLASS_MDIO_MUX,/* MDIO MUX/switch */
UCLASS_MISC,/* Miscellaneous device */
UCLASS_MMC, /* SD / MMC card or chip */
UCLASS_MOD_EXP, /* RSA Mod Exp device */
diff --git a/include/miiphy.h b/include/miiphy.h
index e6dd441983..9b97d09f18 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -167,4 +167,24 @@ struct phy_device *dm_mdio_phy_connect(struct udevice
*dev, int addr,
#endif
+#ifdef CONFIG_DM_MDIO_MUX
+
+/* indicates none of the child buses is selected */
+#define MDIO_MUX_SELECT_NONE -1
+
+/**
+ * struct mdio_mux_ops - MDIO MUX operations
+ *
+ * @select: Selects a child bus
+ * @deselect: Clean up selection. Optional, can be NULL
+ */
+struct mdio_mux_ops {
+ int (*select)(struct udevice *mux, int cur, int sel);
+ int (*deselect)(struct udevice *mux, int sel);
+};
+
+#define mdio_mux_get_ops(dev) ((struct mdio_mux_ops *)(dev)->driver->ops)
+
+#endif
+
#endif
diff --git a/net/Makefile b/net/Makefile
index 6251ff3991..826544f05f 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -16,6 +16,7 @@ else
obj-$(CONFIG_NET) += eth_legacy.o
endif
obj-$(CONFIG_DM_MDIO) += mdio-uclass.o
+obj-$(CONFIG_DM_MDIO_MUX) += mdio-mux-uclass.o
obj-$(CONFIG_NET) += eth_common.o
obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
obj-$(CONFIG_NET) += net.o
diff --git a/net/mdio-mux-uclass.c b/net/mdio-mux-uclass.c
new file mode 100644
index 00..e3fe12a531
--- /dev/null
+++ b/net/mdio-mux-uclass.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MDIO_MUX_CHILD_DRV_NAME"mdio-mux-bus-drv"
+
+/**
+ * struct mdio_mux_perdev_priv - Per-device class data for MDIO MUX DM
+ *
+ * @parent_mdio: Parent DM MDIO device, this is called for actual MDIO I/O
after
+ * setting up the mux. Typically this is a real MDIO device,
+ * unless there are cascaded muxes.
+ * @selected:Current child bus selection. Defaults to -1
+ */
+struct mdio_mux_perdev_priv {
+ struct udevice *mdio_parent;
+ int selected;
+};
+
+/*
+ * This source file uses three types of devices, as follows:
+ * - mux is the hardware MDIO MUX which selects between the existing child MDIO
+ * buses, this is the device relevant for MDIO MUX class of drivers.
+ * - ch is a child MDIO bus, this is just a representation of an mux selection,
+ * not a real piece of hardware.
+ * - mdio_parent is the actual MDIO bus called to perform reads/writes after
+ * the MUX is configured. Typically this is a real MDIO device, unless there
+ * are cascaded muxes.
+ */
+
+/**
+ * struct mdio_mux_ch_data - Per-device data for child MDIOs
+ *
+ * @sel: Selection value used by the MDIO MUX to access this child MDIO bus
+ */
+struct mdio_mux_ch_data {
+ int sel;
+};
+
+static struct udevice *mmux_get_parent_mdio(struct udevice *mux)
+{
+ struct mdio_mux_perdev_priv *pdata = dev_get_uclass_priv(mux);
+
+ return
[U-Boot] [PATCH 2/4 v3] doc: bindings: Add description for MDIO MUX dts nodes
Adds a short bindings document describing the expected structure of a MDIO
MUX dts node. This is based on Linux binding and the example is in fact
copied from there.
Signed-off-by: Alex Marginean
---
Changes in v2:
- no change
Changes in v3:
- no change, just fighting with the email server
doc/device-tree-bindings/net/mdio-mux.txt | 61 +++
1 file changed, 61 insertions(+)
create mode 100644 doc/device-tree-bindings/net/mdio-mux.txt
diff --git a/doc/device-tree-bindings/net/mdio-mux.txt
b/doc/device-tree-bindings/net/mdio-mux.txt
new file mode 100644
index 00..fdf817fd93
--- /dev/null
+++ b/doc/device-tree-bindings/net/mdio-mux.txt
@@ -0,0 +1,61 @@
+The expected structure of an MDIO MUX device tree node is described here. This
+is heavily based on current Linux specification.
+The MDIO buses downstream of the MUX should be described in the device tree as
+child nodes as indicated below.
+
+Required properties:
+mdio-parent-bus = a phandle to the MDIO bus used to perform actual I/O. This
is
+ typically a real MDIO device, unless there are cascaded
MUXes.
+#address-cells = <1>, each MDIO group is identified by one 32b value.
+#size-cells = <0>
+
+Other properties:
+The properties described here are sufficient for MDIO MUX DM class code, but
+MUX drivers may define additional properties, either required or optional.
+
+Required properties in child nodes:
+reg = value to be configured on the MUX to select the respective downstream
+ MDIO.
+
+Child nodes should normally contain PHY nodes, referenced by phandle from
+ethernet nodes of the eth interfaces using these PHYs.
+
+Example structure, extracted from Linux bindings document:
+
+ /* The parent MDIO bus. */
+ smi1: mdio@118001900 {
+ compatible = "cavium,octeon-3860-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x11800 0x1900 0x0 0x40>;
+ };
+
+ /*
+ An NXP sn74cbtlv3253 dual 1-of-4 switch controlled by a
+ pair of GPIO lines. Child busses 2 and 3 populated with 4
+ PHYs each.
+*/
+ mdio-mux {
+ compatible = "mdio-mux-gpio";
+ gpios = < 3 0>, < 4 0>;
+ mdio-parent-bus = <>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mdio@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy11: ethernet-phy@1 {
+ reg = <1>;
+ marvell,reg-init = <3 0x10 0 0x5777>,
+ <3 0x11 0 0x00aa>,
+ <3 0x12 0 0x4105>,
+ <3 0x13 0 0x0a60>;
+ interrupt-parent = <>;
+ interrupts = <10 8>; /* Pin 10, active low */
+ };
+ };
+ };
+
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 3/4 v3] test: dm_mdio: add a 2nd register to the emulated PHY
This 2nd register is used by the follow-up MDIO MUX test.
Signed-off-by: Alex Marginean
---
Changes in v2:
- no change
Changes in v3:
- no change, just fighting with the email server
drivers/net/mdio_sandbox.c | 16 +---
test/dm/mdio.c | 3 +++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/mdio_sandbox.c b/drivers/net/mdio_sandbox.c
index 07515e078c..df053f5381 100644
--- a/drivers/net/mdio_sandbox.c
+++ b/drivers/net/mdio_sandbox.c
@@ -9,11 +9,11 @@
#include
#define SANDBOX_PHY_ADDR 5
-#define SANDBOX_PHY_REG0
+#define SANDBOX_PHY_REG_CNT2
struct mdio_sandbox_priv {
int enabled;
- u16 reg;
+ u16 reg[SANDBOX_PHY_REG_CNT];
};
static int mdio_sandbox_read(struct udevice *dev, int addr, int devad, int reg)
@@ -27,10 +27,10 @@ static int mdio_sandbox_read(struct udevice *dev, int addr,
int devad, int reg)
return -ENODEV;
if (devad != MDIO_DEVAD_NONE)
return -ENODEV;
- if (reg != SANDBOX_PHY_REG)
+ if (reg < 0 || reg > SANDBOX_PHY_REG_CNT)
return -ENODEV;
- return priv->reg;
+ return priv->reg[reg];
}
static int mdio_sandbox_write(struct udevice *dev, int addr, int devad, int
reg,
@@ -45,10 +45,10 @@ static int mdio_sandbox_write(struct udevice *dev, int
addr, int devad, int reg,
return -ENODEV;
if (devad != MDIO_DEVAD_NONE)
return -ENODEV;
- if (reg != SANDBOX_PHY_REG)
+ if (reg < 0 || reg > SANDBOX_PHY_REG_CNT)
return -ENODEV;
- priv->reg = val;
+ priv->reg[reg] = val;
return 0;
}
@@ -56,8 +56,10 @@ static int mdio_sandbox_write(struct udevice *dev, int addr,
int devad, int reg,
static int mdio_sandbox_reset(struct udevice *dev)
{
struct mdio_sandbox_priv *priv = dev_get_priv(dev);
+ int i;
- priv->reg = 0;
+ for (i = 0; i < SANDBOX_PHY_REG_CNT; i++)
+ priv->reg[i] = 0;
return 0;
}
diff --git a/test/dm/mdio.c b/test/dm/mdio.c
index 5b66255f7d..dc229aed6d 100644
--- a/test/dm/mdio.c
+++ b/test/dm/mdio.c
@@ -13,6 +13,9 @@
/* macros copied over from mdio_sandbox.c */
#define SANDBOX_PHY_ADDR 5
+#define SANDBOX_PHY_REG_CNT2
+
+/* test using 1st register, 0 */
#define SANDBOX_PHY_REG0
#define TEST_REG_VALUE 0xabcd
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 4/4 v3] test: dm: add a test for MDIO MUX DM uclass
Adds a test using a makeshift MDIO MUX. The test is based on the existing
MDIO test. It uses the last emulated PHY register to verify MUX selection.
Signed-off-by: Alex Marginean
---
Changes in v2:
- no change
Changes in v3:
- no change, just fighting with the email server
arch/Kconfig | 1 +
arch/sandbox/dts/test.dts | 21 +++-
drivers/net/Kconfig| 10
drivers/net/Makefile | 1 +
drivers/net/mdio_mux_sandbox.c | 97 ++
test/dm/Makefile | 1 +
test/dm/mdio_mux.c | 80
7 files changed, 210 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/mdio_mux_sandbox.c
create mode 100644 test/dm/mdio_mux.c
diff --git a/arch/Kconfig b/arch/Kconfig
index 1e62a7615d..1a0f1ab8a7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -122,6 +122,7 @@ config SANDBOX
imply PCH
imply PHYLIB
imply DM_MDIO
+ imply DM_MDIO_MUX
config SH
bool "SuperH architecture"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index dd50a951a8..a05e437abf 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -808,7 +808,26 @@
dma-names = "m2m", "tx0", "rx0";
};
- mdio-test {
+ /*
+* keep mdio-mux ahead of mdio, u-boot doesn't do reference count on
+* these devices and we don't want mdio-parent-bus to be released before
+* the mux.
+*/
+ mdio-mux-test {
+ compatible = "sandbox,mdio-mux";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-parent-bus = <>;
+
+ mdio-ch-test@0 {
+ reg = <0>;
+ };
+ mdio-ch-test@1 {
+ reg = <1>;
+ };
+ };
+
+ mdio: mdio-test {
compatible = "sandbox,mdio";
};
};
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0dc26ac254..403df5e960 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -46,6 +46,16 @@ config MDIO_SANDBOX
This driver is used in for testing in test/dm/mdio.c
+config MDIO_MUX_SANDBOX
+ depends on DM_MDIO_MUX && MDIO_SANDBOX
+ default y
+ bool "Sandbox: Mocked MDIO-MUX driver"
+ help
+ This driver implements dummy select/deselect ops mimicking a MUX on
+ the MDIO bux. It uses mdio_sandbox driver as parent MDIO.
+
+ This driver is used for testing in test/dm/mdio.c
+
menuconfig NETDEVICES
bool "Network device support"
depends on NET
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 40038427db..a3706ca27b 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -78,3 +78,4 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
obj-y += mscc_eswitch/
obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
+obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o
diff --git a/drivers/net/mdio_mux_sandbox.c b/drivers/net/mdio_mux_sandbox.c
new file mode 100644
index 00..3dba4d18a1
--- /dev/null
+++ b/drivers/net/mdio_mux_sandbox.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019
+ * Alex Marginean, NXP
+ */
+
+#include
+#include
+#include
+
+/* macros copied over from mdio_sandbox.c */
+#define SANDBOX_PHY_ADDR 5
+#define SANDBOX_PHY_REG_CNT2
+
+struct mdio_mux_sandbox_priv {
+ int enabled;
+ int sel;
+};
+
+static int mdio_mux_sandbox_mark_selection(struct udevice *dev, int sel)
+{
+ struct udevice *mdio;
+ struct mdio_ops *ops;
+ int err;
+
+ /*
+* find the sandbox parent mdio and write a register on the PHY there
+* so the mux test can verify selection.
+*/
+ err = uclass_get_device_by_name(UCLASS_MDIO, "mdio-test", );
+ if (err)
+ return err;
+ ops = mdio_get_ops(mdio);
+ return ops->write(mdio, SANDBOX_PHY_ADDR, MDIO_DEVAD_NONE,
+ SANDBOX_PHY_REG_CNT - 1, (u16)sel);
+}
+
+static int mdio_mux_sandbox_select(struct udevice *dev, int cur, int sel)
+{
+ struct mdio_mux_sandbox_priv *priv = dev_get_priv(dev);
+
+ if (!priv->enabled)
+ return -ENODEV;
+
+ if (cur != priv->sel)
+ return -EINVAL;
+
+ priv->sel = sel;
+ mdio_mux_sandbox_mark_selection(dev, priv->sel);
+
+ return 0;
+}
+
+static int mdio_mux_sandbox_deselect(struct udevice *dev, int sel)
+{
+ struct mdio_mux_sandbox_priv *priv = dev_get_priv(dev);
+
+ if (!priv->enabled)
+ return -ENODEV;
+
+ if (sel != priv->sel)
+ return -EINVAL;
+
+ priv->sel = -1;
+ mdio_mux_sandbox_mark_selection(dev, priv->sel);
+
+ return 0;
+}
+
+static const struct mdio_mux_ops mdio_mux_sandbox_ops = {
+ .select =
Re: [U-Boot] [PATCH 2/4 v2] doc: bindings: Add description for MDIO MUX dts nodes
More email trouble at my end, please ignore this.
I'll resend as v3 from the nxp account..
Alex
On 6/18/2019 5:47 PM, Alex Marginean wrote:
> Adds a short bindings document describing the expected structure of a MDIO
> MUX dts node. This is based on Linux binding and the example is in fact
> copied from there.
>
> Signed-off-by: Alex Marginean
> ---
>
> Changes in v2:
> - no change
>
> doc/device-tree-bindings/net/mdio-mux.txt | 61 +++
> 1 file changed, 61 insertions(+)
> create mode 100644 doc/device-tree-bindings/net/mdio-mux.txt
>
> diff --git a/doc/device-tree-bindings/net/mdio-mux.txt
> b/doc/device-tree-bindings/net/mdio-mux.txt
> new file mode 100644
> index 00..fdf817fd93
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/mdio-mux.txt
> @@ -0,0 +1,61 @@
> +The expected structure of an MDIO MUX device tree node is described here.
> This
> +is heavily based on current Linux specification.
> +The MDIO buses downstream of the MUX should be described in the device tree
> as
> +child nodes as indicated below.
> +
> +Required properties:
> +mdio-parent-bus = a phandle to the MDIO bus used to perform actual I/O.
> This is
> + typically a real MDIO device, unless there are cascaded
> MUXes.
> +#address-cells = <1>, each MDIO group is identified by one 32b value.
> +#size-cells = <0>
> +
> +Other properties:
> +The properties described here are sufficient for MDIO MUX DM class code, but
> +MUX drivers may define additional properties, either required or optional.
> +
> +Required properties in child nodes:
> +reg = value to be configured on the MUX to select the respective downstream
> + MDIO.
> +
> +Child nodes should normally contain PHY nodes, referenced by phandle from
> +ethernet nodes of the eth interfaces using these PHYs.
> +
> +Example structure, extracted from Linux bindings document:
> +
> + /* The parent MDIO bus. */
> + smi1: mdio@118001900 {
> + compatible = "cavium,octeon-3860-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x11800 0x1900 0x0 0x40>;
> + };
> +
> + /*
> +An NXP sn74cbtlv3253 dual 1-of-4 switch controlled by a
> +pair of GPIO lines. Child busses 2 and 3 populated with 4
> +PHYs each.
> + */
> + mdio-mux {
> + compatible = "mdio-mux-gpio";
> + gpios = < 3 0>, < 4 0>;
> + mdio-parent-bus = <>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy11: ethernet-phy@1 {
> + reg = <1>;
> + marvell,reg-init = <3 0x10 0 0x5777>,
> + <3 0x11 0 0x00aa>,
> + <3 0x12 0 0x4105>,
> + <3 0x13 0 0x0a60>;
> + interrupt-parent = <>;
> + interrupts = <10 8>; /* Pin 10, active low */
> + };
> + };
> + };
> +
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] doc: bindings: Add description for MDIO MUX dts nodes
It looks like the first patch didn't make it through, blocked by google.
Sorry about that I'll try to resend the series.
Does anyone else have issues with gmail blocking patches and emails?
Any suggestions on how to get them through safely?
Sorry for the spam.
Alex
On 6/18/2019 12:22 PM, Alex Marginean wrote:
> Adds a short bindings document describing the expected structure of a MDIO
> MUX dts node. This is based on Linux binding and the example is in fact
> copied from there.
>
> Signed-off-by: Alex Marginean
> ---
> doc/device-tree-bindings/net/mdio-mux.txt | 61 +++
> 1 file changed, 61 insertions(+)
> create mode 100644 doc/device-tree-bindings/net/mdio-mux.txt
>
> diff --git a/doc/device-tree-bindings/net/mdio-mux.txt
> b/doc/device-tree-bindings/net/mdio-mux.txt
> new file mode 100644
> index 00..fdf817fd93
> --- /dev/null
> +++ b/doc/device-tree-bindings/net/mdio-mux.txt
> @@ -0,0 +1,61 @@
> +The expected structure of an MDIO MUX device tree node is described here.
> This
> +is heavily based on current Linux specification.
> +The MDIO buses downstream of the MUX should be described in the device tree
> as
> +child nodes as indicated below.
> +
> +Required properties:
> +mdio-parent-bus = a phandle to the MDIO bus used to perform actual I/O.
> This is
> + typically a real MDIO device, unless there are cascaded
> MUXes.
> +#address-cells = <1>, each MDIO group is identified by one 32b value.
> +#size-cells = <0>
> +
> +Other properties:
> +The properties described here are sufficient for MDIO MUX DM class code, but
> +MUX drivers may define additional properties, either required or optional.
> +
> +Required properties in child nodes:
> +reg = value to be configured on the MUX to select the respective downstream
> + MDIO.
> +
> +Child nodes should normally contain PHY nodes, referenced by phandle from
> +ethernet nodes of the eth interfaces using these PHYs.
> +
> +Example structure, extracted from Linux bindings document:
> +
> + /* The parent MDIO bus. */
> + smi1: mdio@118001900 {
> + compatible = "cavium,octeon-3860-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x11800 0x1900 0x0 0x40>;
> + };
> +
> + /*
> +An NXP sn74cbtlv3253 dual 1-of-4 switch controlled by a
> +pair of GPIO lines. Child busses 2 and 3 populated with 4
> +PHYs each.
> + */
> + mdio-mux {
> + compatible = "mdio-mux-gpio";
> + gpios = < 3 0>, < 4 0>;
> + mdio-parent-bus = <>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy11: ethernet-phy@1 {
> + reg = <1>;
> + marvell,reg-init = <3 0x10 0 0x5777>,
> + <3 0x11 0 0x00aa>,
> + <3 0x12 0 0x4105>,
> + <3 0x13 0 0x0a60>;
> + interrupt-parent = <>;
> + interrupts = <10 8>; /* Pin 10, active low */
> + };
> + };
> + };
> +
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 v2] drivers: net: add NXP ENETC ethernet driver
Hi Bin,
On 6/11/2019 11:18 AM, Alexandru Marginean wrote:
> Hi Bin,
>
> On 6/10/2019 6:42 AM, Bin Meng wrote:
>> Hi Alex,
>>
>> On Sat, Jun 8, 2019 at 12:12 AM Alex Marginean
>> wrote:
>>>
>>> Adds a driver for NXP ENETC ethernet controller currently integrated in
>>> LS1028a. ENETC is a fairly straight-forward BD ring device and interfaces
>>> are presented as PCI EPs on the SoC ECAM.
>>>
>>> Signed-off-by: Catalin Horghidan
>>> Signed-off-by: Alex Marginean
>>> ---
>>>
>>> Changes in v2:
>>> - Added SXGMII MAC configuration
>>> - simplified naming code in _bind
>>> - renamed ENETC_DBG to enetc_dbg and make it use debug()
>>> - replaced a couple of magic numbers with macros
>>> - droped _V1 from PCI ID macro
>>> - several styling and cosmetic updates to the header file
>>>
>>>configs/ls1028aqds_tfa_defconfig | 1 +
>>>configs/ls1028ardb_tfa_defconfig | 1 +
>>>drivers/net/Kconfig | 7 +
>>>drivers/net/Makefile | 1 +
>>>drivers/net/fsl_enetc.c | 344 +++
>>>drivers/net/fsl_enetc.h | 172
>>>include/pci_ids.h| 1 +
>>>7 files changed, 527 insertions(+)
>>>create mode 100644 drivers/net/fsl_enetc.c
>>>create mode 100644 drivers/net/fsl_enetc.h
>>>
[snip]
>>> diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
>>> new file mode 100644
>>> index 00..325e032746
>>> --- /dev/null
>>> +++ b/drivers/net/fsl_enetc.c
>>> @@ -0,0 +1,344 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * ENETC ethernet controller driver
>>> + * Copyright 2017-2019 NXP
>>> + */
>>> +
>>> +#include "fsl_enetc.h"
>>
>> nits: this local include should come after all other includes.
>>
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +static int enetc_bind(struct udevice *dev)
>>> +{
>>> + char name[16];
>>> +
>>> + sprintf(name, "enetc#%u", PCI_FUNC(dm_pci_get_bdf(dev)));
>>> + device_set_name(dev, name);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * Probe ENETC driver:
>>> + * - initialize port and station interface BARs
>>> + */
>>> +static int enetc_probe(struct udevice *dev)
>>> +{
>>> + struct enetc_devfn *hw = dev_get_priv(dev);
>>> + int err = 0;
>>> +
>>> + if (ofnode_valid(dev->node) && !ofnode_is_available(dev->node)) {
>>> + enetc_dbg(dev, "interface disabled\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /* initialize register */
>>> + hw->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
>>> + if (!hw->regs_base) {
>>> + enetc_dbg(dev, "failed to map BAR0\n");
>>> + return -EINVAL;
>>> + }
>>> + hw->port_regs = hw->regs_base + ENETC_PORT_REGS_OFF;
>>> +
>>> + dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
>>> +
>>> + return err;
>>> +}
>>> +
>>> +/* ENETC Port MAC address registers accept big-endian format */
>>> +static void enetc_set_primary_mac_addr(struct enetc_devfn *hw, const u8
>>> *addr)
>>> +{
>>> + u16 lower = *(const u16 *)(addr + 4);
>>> + u32 upper = *(const u32 *)addr;
>>> +
>>> + enetc_write_port(hw, ENETC_PSIPMAR0, upper);
>>> + enetc_write_port(hw, ENETC_PSIPMAR1, lower);
>>> +}
>>> +
>>> +int enetc_enable_si_port(struct enetc_devfn *hw)
>>
>> nits: this should be static. Can we put a single line comment to
>> describe what this function does, like others?
>>
>>> +{
>>> + u32 val;
>>> +
>>> + /* set Rx/Tx BDR count */
>>> + val = ENETC_PSICFGR_SET_TXBDR(ENETC_TX_BDR_CNT);
>>> + val |= ENETC_PSICFGR_SET_RXBDR(ENETC_RX_BDR_CNT);
>>> + enetc_write_port(hw, ENETC_PSICFGR(0), val);
>>> + /* set Rx max frame si
Re: [U-Boot] [EXT] Re: [PATCH 1/2 v3] net: introduce MDIO DM class for MDIO devices
Hi Ken, On 6/11/2019 12:44 PM, Ken Ma wrote: > Hi Alex > > Thanks a lot for your information! > > I think our patches have no essential difference. > The 2 patches have only small implementation difference: > In my patch, mii bus ops functions(read/write/reset...) need to be > implemented while in your patch mdio bus functions need to be > implemented and then mii bus ops functions will call mdio bus ops > functions. > I had planned to reuse those existed mii ops functions such as > smc911x_miiphy_read/ smc911x_miiphy_write/ sun8i_mdio_read/ > sun8i_mdio_write... then it is easy for turning old mdio driver to > DM. > > Now I am not working on u-boot, so I am sorry that I will not do the pulling > work. > > Yours, > Ken OK, I think I get what you wanted to do. Either way it's not too difficult to convert existing MDIOs to DM, but they have to start using struct udevice. That's similar to what was done on DM_ETH and others. The helpers mapping eth/phy/mdio make sense and could be useful, that's something I'll try to look into. Thank you! Alex > > -Original Message- > From: Alex Marginean > Sent: Tuesday, June 11, 2019 9:18 AM > To: joe.hershber...@ni.com; Ken Ma > Cc: u-boot ; Joseph Hershberger > > Subject: [EXT] Re: [U-Boot] [PATCH 1/2 v3] net: introduce MDIO DM class for > MDIO devices > > External Email > > -- > +Ken, > > Hi Joe, > > On 6/10/2019 11:25 PM, Joe Hershberger wrote: >> On Mon, Jun 3, 2019 at 11:11 AM Alex Marginean >> wrote: >>> >>> Adds UCLASS_MDIO DM class supporting MDIO buses that are probed as >>> stand-alone devices. Useful in particular for systems that support >>> DM_ETH and have a stand-alone MDIO hardware block shared by multiple >>> Ethernet interfaces. >>> >>> Signed-off-by: Alex Marginean >>> --- >>> >>> Changes in v2: >>> - fixed several comments using wrong API names >>> - dropped dm_ from names of internal functions that don't use >>> udevice * >>> - fixed UCLASS driver name >>> - added missing mdio_unregister in dm_mdio_pre_remove >>> - added a comment on why spaces in names aren't ok >>> - added a comment on how static mdio_read/_write/_reset functions >>> are used >>> Changes in v3: >>> - none >> >> >> Not sure if you already noticed this [1] or not, but there may be >> something there that you want to incorporate or maybe not. >> >> Cheers, >> -Joe >> >> [1] - https://patchwork.ozlabs.org/patch/939726/ >> > > I didn't notice it, thanks for pointing it out! > Apart from the obvious overlap of adding UCLASS_MDIO and code like > _post_probe they seem to deal with different needs. > > Ken, can you please take a look at the patch I sent? It has a wrapper over > phy_connect, but provides no helpers on how the caller would get the PHY > ADDR. Do you want to try pulling the API you add on top of the patch I sent, > or do you want me to try? It looks like it would work with minimal effort. > > Thank you! > Alex > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 v2] drivers: net: add NXP ENETC ethernet driver
Hi Bin,
On 6/10/2019 6:42 AM, Bin Meng wrote:
> Hi Alex,
>
> On Sat, Jun 8, 2019 at 12:12 AM Alex Marginean
> wrote:
>>
>> Adds a driver for NXP ENETC ethernet controller currently integrated in
>> LS1028a. ENETC is a fairly straight-forward BD ring device and interfaces
>> are presented as PCI EPs on the SoC ECAM.
>>
>> Signed-off-by: Catalin Horghidan
>> Signed-off-by: Alex Marginean
>> ---
>>
>> Changes in v2:
>> - Added SXGMII MAC configuration
>> - simplified naming code in _bind
>> - renamed ENETC_DBG to enetc_dbg and make it use debug()
>> - replaced a couple of magic numbers with macros
>> - droped _V1 from PCI ID macro
>> - several styling and cosmetic updates to the header file
>>
>> configs/ls1028aqds_tfa_defconfig | 1 +
>> configs/ls1028ardb_tfa_defconfig | 1 +
>> drivers/net/Kconfig | 7 +
>> drivers/net/Makefile | 1 +
>> drivers/net/fsl_enetc.c | 344 +++
>> drivers/net/fsl_enetc.h | 172
>> include/pci_ids.h| 1 +
>> 7 files changed, 527 insertions(+)
>> create mode 100644 drivers/net/fsl_enetc.c
>> create mode 100644 drivers/net/fsl_enetc.h
>>
>> diff --git a/configs/ls1028aqds_tfa_defconfig
>> b/configs/ls1028aqds_tfa_defconfig
>> index 7982ce4157..11fe344b04 100644
>> --- a/configs/ls1028aqds_tfa_defconfig
>> +++ b/configs/ls1028aqds_tfa_defconfig
>> @@ -45,6 +45,7 @@ CONFIG_PHY_ATHEROS=y
>> CONFIG_DM_ETH=y
>> CONFIG_PHY_GIGE=y
>> CONFIG_E1000=y
>> +CONFIG_FSL_ENETC=y
>
> This, along with mdio changes to the defconfig files in the patch
> [2/2], should be in a separate patch, and come in the last commit in
> this series.
>
>> CONFIG_PCI=y
>> CONFIG_DM_PCI=y
>> CONFIG_DM_PCI_COMPAT=y
>> diff --git a/configs/ls1028ardb_tfa_defconfig
>> b/configs/ls1028ardb_tfa_defconfig
>> index c65e37df79..ab6f2a850c 100644
>> --- a/configs/ls1028ardb_tfa_defconfig
>> +++ b/configs/ls1028ardb_tfa_defconfig
>> @@ -45,6 +45,7 @@ CONFIG_PHY_ATHEROS=y
>> CONFIG_DM_ETH=y
>> CONFIG_PHY_GIGE=y
>> CONFIG_E1000=y
>> +CONFIG_FSL_ENETC=y
>
> ditto.
>
>> CONFIG_PCI=y
>> CONFIG_DM_PCI=y
>> CONFIG_DM_PCI_COMPAT=y
>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>> index 635f8d72c2..1fe038b859 100644
>> --- a/drivers/net/Kconfig
>> +++ b/drivers/net/Kconfig
>> @@ -566,4 +566,11 @@ config HIGMACV300_ETH
>>This driver supports HIGMACV300 Ethernet controller found on
>>HiSilicon SoCs.
>>
>> +config FSL_ENETC
>> + bool "NXP ENETC Ethernet controller"
>> + depends on DM_PCI && DM_ETH
>> + help
>> + This driver supports the NXP ENETC Ethernet controller found on
>> some
>> + of the NXP SoCs.
>> +
>> endif # NETDEVICES
>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> index 40038427db..c08dc2e20c 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -78,3 +78,4 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
>> obj-y += mscc_eswitch/
>> obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
>> obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o
>> +obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o
>> diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
>> new file mode 100644
>> index 00..325e032746
>> --- /dev/null
>> +++ b/drivers/net/fsl_enetc.c
>> @@ -0,0 +1,344 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * ENETC ethernet controller driver
>> + * Copyright 2017-2019 NXP
>> + */
>> +
>> +#include "fsl_enetc.h"
>
> nits: this local include should come after all other includes.
>
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +static int enetc_bind(struct udevice *dev)
>> +{
>> + char name[16];
>> +
>> + sprintf(name, "enetc#%u", PCI_FUNC(dm_pci_get_bdf(dev)));
>> + device_set_name(dev, name);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Probe ENETC driver:
>> + * - initialize port and station interface BARs
>> + */
>> +static int enetc_probe(struct udevice *dev)
>> +{
>> + struct enetc_devfn *hw = dev_get_priv(dev);
>> + int err = 0;
>> +
>> + if (ofnode_valid(dev->node) && !ofnode_is_available(dev->node)) {
>> + enetc_dbg(dev, "interface disabled\n");
>> + return -ENODEV;
>> + }
>> +
>> + /* initialize register */
>> + hw->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0);
>> + if (!hw->regs_base) {
>> + enetc_dbg(dev, "failed to map BAR0\n");
>> + return -EINVAL;
>> + }
>> + hw->port_regs = hw->regs_base + ENETC_PORT_REGS_OFF;
>> +
>> + dm_pci_clrset_config16(dev, PCI_COMMAND, 0, PCI_COMMAND_MEMORY);
>> +
>> + return err;
>> +}
>> +
>> +/* ENETC Port MAC address registers accept big-endian format */
>> +static void enetc_set_primary_mac_addr(struct enetc_devfn *hw, const u8
>> *addr)
>> +{
>> +
Re: [U-Boot] [PATCH 3/4 v2] test: dm: Add a test for PCI Enhanced Allocation
Hi Bin,
On 6/5/2019 1:05 PM, Bin Meng wrote:
> Hi Alex,
>
> On Tue, Jun 4, 2019 at 8:46 PM Alex Marginean wrote:
>>
>> This test is built on top of the existing swap_case driver. It adds EA
>> capability structure support to swap_case and uses that to map BARs.
>> BAR1 works as it used to, swapping upper/lower case. BARs 2,4 map to a
>> couple of magic values.
>>
>> Signed-off-by: Alex Marginean
>> ---
>>
>> Changes in v2:
>> - new patch, v1 didn't have a test
>>
>> arch/sandbox/dts/test.dts | 8 +++
>> arch/sandbox/include/asm/test.h | 13
>> drivers/misc/swap_case.c| 102 +++-
>> test/dm/pci.c | 50
>> 4 files changed, 172 insertions(+), 1 deletion(-)
>>
>
> Well done!
>
> Reviewed-by: Bin Meng
> Tested-by: Bin Meng
>
> But please see some nits below:
I'm replying from the nxp account, apparently google decided this is
just spam and it's not worth sending out through gmail.
I'll send a v3 with fixes for you comments, should I keep either of your
two tags on this patch?
Thank you!
Alex
>
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index 46d8a56d0f..dd50a951a8 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -434,6 +434,14 @@
>> compatible = "sandbox,swap-case";
>> };
>> };
>> + pci@1,0 {
>> + compatible = "pci-generic";
>> + reg = <0x0800 0 0 0 0>;
>> + emul@0,0 {
>> + compatible = "sandbox,swap-case";
>> + use-ea;
>> + };
>> + };
>> pci@1f,0 {
>> compatible = "pci-generic";
>> reg = <0xf800 0 0 0 0>;
>> diff --git a/arch/sandbox/include/asm/test.h
>> b/arch/sandbox/include/asm/test.h
>> index e956a05262..32125f3037 100644
>> --- a/arch/sandbox/include/asm/test.h
>> +++ b/arch/sandbox/include/asm/test.h
>> @@ -19,6 +19,7 @@
>> #define PCI_CAP_ID_PM_OFFSET 0x50
>> #define PCI_CAP_ID_EXP_OFFSET 0x60
>> #define PCI_CAP_ID_MSIX_OFFSET 0x70
>> +#define PCI_CAP_ID_EA_OFFSET 0x80
>>
>> #define PCI_EXT_CAP_ID_ERR_OFFSET 0x100
>> #define PCI_EXT_CAP_ID_VC_OFFSET 0x200
>> @@ -30,6 +31,18 @@
>>
>> #define SANDBOX_CLK_RATE 32768
>>
>> +/* Macros used to test PCI EA capability structure */
>> +#define PCI_CAP_EA_BASE_LO00x0010
>> +#define PCI_CAP_EA_BASE_LO10x0011
>> +#define PCI_CAP_EA_BASE_LO20x0012
>> +#define PCI_CAP_EA_BASE_LO40x0014
>> +#define PCI_CAP_EA_BASE_HI20x0002ULL
>> +#define PCI_CAP_EA_BASE_HI40x0004ULL
>> +#define PCI_CAP_EA_SIZE_LO 0x
>> +#define PCI_CAP_EA_SIZE_HI 0x0010ULL
>> +#define PCI_EA_BAR2_MAGIC 0x72727272
>> +#define PCI_EA_BAR4_MAGIC 0x74747474
>> +
>> /* System controller driver data */
>> enum {
>> SYSCON0 = 32,
>> diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c
>> index fa608cec1b..949ef0fdd7 100644
>> --- a/drivers/misc/swap_case.c
>> +++ b/drivers/misc/swap_case.c
>> @@ -61,11 +61,63 @@ static int sandbox_swap_case_get_devfn(struct udevice
>> *dev)
>> return plat->devfn;
>> }
>>
>> +static int sandbox_swap_use_ea(struct udevice *dev)
>
> nits: for consistency, name it as "sandbox_swap_case_use_ea"
>
>> +{
>> + return !!ofnode_get_property(dev->node, "use-ea", NULL);
>> +}
>> +
>> +/* Please keep these macros in sync with ea_regs below */
>> +#define PCI_CAP_ID_EA_SIZE (sizeof(ea_regs) + 4)
>> +#define PCI_CAP_ID_EA_ENTRY_CNT4
>> +/* Hardcoded EA structure, excluding 1st DW. */
>> +static const u32 ea_regs[] = {
>> + /* BEI=0, ES=2, BAR0 32b Base + 32b MaxOffset, I/O space */
>> + (2 << 8) | 2,
>> + PCI_CAP_EA_BASE_LO0,
>> + 0,
>> + /* BEI=1, ES=2, BAR1 32b Base + 32b MaxOffset */
>> + (1 << 4) | 2,
>> + PCI_CAP_EA_BASE_LO1,
>> + MEM_TEXT_SIZE - 1,
>> + /* BEI=2, ES=3, BAR2 64b Base + 32b MaxOffset */
>> + (2 << 4) | 3,
>> + PCI_CAP_EA_BASE_LO2 | PCI_EA_IS_64,
>> + PCI_CAP_EA_SIZE_LO,
>> + PCI_CAP_EA_BASE_HI2,
>> + /* BEI=4, ES=4, BAR4 63b Base + 64b MaxOffset */
>
> nits: typo of '63b'
>
>> + (4 << 4) | 4,
>> + PCI_CAP_EA_BASE_LO4 | PCI_EA_IS_64,
>> + PCI_CAP_EA_SIZE_LO | PCI_EA_IS_64,
>> + PCI_CAP_EA_BASE_HI4,
>> + PCI_CAP_EA_SIZE_HI,
>> +};
>> +
>> +static int sandbox_swap_case_read_ea(struct udevice *emul, uint offset,
>> +ulong *valuep, enum pci_size_t size)
>> +{
>> + u32 reg;
>> +
>> + offset = offset -
