Re: [PATCH v1 2/6] net: mv88e61xx: Configure PHY ports to also pass packets between them
Hi Lukasz, On Thu, Jun 01, 2023 at 01:44:30PM +0200, Marek Vasut wrote: > I think after two years, it would be good to drop the RB tags and do another > round of reviews. To expand on Marek's point. In those past 2 years, Tim Harvey has put in a considerable amount of effort to add another driver for mv88e6xxx that uses DM_DSA. I believe the current "PHY" driver for the same hardware should be considered obsolete until all platforms are converted to DM_DSA, then it can be deleted. So, no new features for it. Then, there's also the question of the sanity of the proposed change itself. I believe that we need to be humble enough to recognize that the U-Boot network stack is not competent enough to handle the switching capabilities of a switch, not even enough for it to be safe. It doesn't handle STP (Spanning Tree Protocol), for one thing. So it will never be capable of detecting switching loops, such as to block one of its ports in order to not kill the network. In principle, I would say: as long as there is no plan to handle STP, there should be no plan to allow autonomous packet forwarding from U-Boot. The U-Boot network stack is there so that you can TFTP a kernel and boot it, which is also the only use case behind DM_DSA. But you may say: I'm never going to allow packet forwarding from U-Boot in a network with loops! Okay, but your patch suggests otherwise. Which ports allow forwarding is a compile-time option, which... is by definition contrary to any runtime network topology determinations. Maybe enabling forwarding between switch ports through a CLI command that communicates with DM_DSA would be tolerable - assuming that users are smart enough to not use it in a network with STP. But again, I'm not really sure what's the use case.
Re: [PATCH 01/41] net: phy: aquantia: Staticize PHY driver entries
On Tue, Mar 21, 2023 at 03:39:16PM +0100, Michal Simek wrote: > Would be good if you also create cover letter which I can reply. > I have tested this series on Microblaze which is also using MANUAL > relocation (but we are removing it from 2023.07 release) and it is working > fine. Agree that a cover letter giving the big picture would have been nice, not really sure what I'm looking at here :-/ and especially whether there's anything to test on platforms which don't have CONFIG_NEEDS_MANUAL_RELOC.
[PATCH] configs: convert NXP LS1028A RDB and QDS to DM_SERIAL
Since the device trees are more or less synchronized with Linux, the only necessary changes are to enable CONFIG_DM_SERIAL and the DM_SERIAL driver for ns16550 (ns16550.c rather than serial_ns16550.c). ls1028aqds_tfa_lpuart_defconfig already uses DM_SERIAL for the LPUART driver, so I didn't touch that. Signed-off-by: Vladimir Oltean --- 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 525c7df6d0f4..76607920e2b8 100644 --- a/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig +++ b/configs/ls1028aqds_tfa_SECURE_BOOT_defconfig @@ -86,7 +86,8 @@ CONFIG_DM_RTC=y CONFIG_RTC_PCF2127=y CONFIG_SCSI=y CONFIG_DM_SCSI=y -CONFIG_SYS_NS16550_SERIAL=y +CONFIG_DM_SERIAL=y +CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_FSL_DSPI=y diff --git a/configs/ls1028aqds_tfa_defconfig b/configs/ls1028aqds_tfa_defconfig index dde204eaa0df..e054e1781899 100644 --- a/configs/ls1028aqds_tfa_defconfig +++ b/configs/ls1028aqds_tfa_defconfig @@ -92,7 +92,8 @@ CONFIG_DM_RTC=y CONFIG_RTC_PCF2127=y CONFIG_SCSI=y CONFIG_DM_SCSI=y -CONFIG_SYS_NS16550_SERIAL=y +CONFIG_DM_SERIAL=y +CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_FSL_DSPI=y diff --git a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig index b8df245025f3..5fa1c936eef0 100644 --- a/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig +++ b/configs/ls1028ardb_tfa_SECURE_BOOT_defconfig @@ -80,7 +80,8 @@ CONFIG_DM_RTC=y CONFIG_RTC_PCF2127=y CONFIG_SCSI=y CONFIG_DM_SCSI=y -CONFIG_SYS_NS16550_SERIAL=y +CONFIG_DM_SERIAL=y +CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_FSL_DSPI=y diff --git a/configs/ls1028ardb_tfa_defconfig b/configs/ls1028ardb_tfa_defconfig index 5b1cf988cf07..a73a8092a3e8 100644 --- a/configs/ls1028ardb_tfa_defconfig +++ b/configs/ls1028ardb_tfa_defconfig @@ -86,7 +86,8 @@ CONFIG_DM_RTC=y CONFIG_RTC_PCF2127=y CONFIG_SCSI=y CONFIG_DM_SCSI=y -CONFIG_SYS_NS16550_SERIAL=y +CONFIG_DM_SERIAL=y +CONFIG_SYS_NS16550=y CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_FSL_DSPI=y -- 2.34.1
Re: [PATCH 5/5] configs: ls208x: enable DM_SERIAL
On Tue, Feb 28, 2023 at 06:32:12PM +0200, Ioana Ciornei wrote: > Now that the DT nodes for the serial devices are in place for these > boards, enable DM_SERIAL in the associated configs. > > Signed-off-by: Ioana Ciornei > --- > configs/ls2088aqds_tfa_defconfig | 5 +++-- > configs/ls2088ardb_tfa_SECURE_BOOT_defconfig | 4 +++- > configs/ls2088ardb_tfa_defconfig | 4 +++- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/configs/ls2088aqds_tfa_defconfig > b/configs/ls2088aqds_tfa_defconfig > index a9faa1525ac7..9f10dd23b283 100644 > --- a/configs/ls2088aqds_tfa_defconfig > +++ b/configs/ls2088aqds_tfa_defconfig > @@ -18,7 +18,6 @@ CONFIG_AHCI=y > CONFIG_FSL_USE_PCA9547_MUX=y > CONFIG_FSL_QIXIS=y > # CONFIG_QIXIS_I2C_ACCESS is not set > -# CONFIG_SYS_MALLOC_F is not set I found this part of the change to be unexpected, but an offline discussion with Ioana informed me that this line automatically goes away after a "make savedefconfig". That being said: Reviewed-by: Vladimir Oltean
Re: [PATCH 3/5] arch: arm: dst: fsl-ls2080a.dts: sync serial nodes with Linux
On Tue, Feb 28, 2023 at 06:32:10PM +0200, Ioana Ciornei wrote: > Sync the serial nodes of the LS208XA RDB/QDS boards with their > representation in Linux. We also imported the clockgen and sysclk nodes > which are dependencies. > > Signed-off-by: Ioana Ciornei > --- Reviewed-by: Vladimir Oltean
Re: [PATCH 2/5] arch: arm: dst: fsl-ls2080a.dtsi: move the serial nodes under soc
On Tue, Feb 28, 2023 at 06:32:09PM +0200, Ioana Ciornei wrote: > Move the serial nodes under the soc node. No changes are made to the > nodes, just their location is changed. > > Signed-off-by: Ioana Ciornei > --- Small typo in the commit message here and in other patches: "dst" instead of "dts". Otherwise: Reviewed-by: Vladimir Oltean
Re: [PATCH 1/5] arch: arm: dst: fsl-ls2080a.dtsi: add an 'soc' node
On Tue, Feb 28, 2023 at 06:32:08PM +0200, Ioana Ciornei wrote: > The u-boot dts for these boards do not have an soc node, unlike its > Linux counterpart. This patch just adds the soc node as seen in Linux, > the next patches will move some nodes under it. > > Signed-off-by: Ioana Ciornei > --- Reviewed-by: Vladimir Oltean
Re: [PATCH] ls1021atsn: Suggest the NXP RCW github repo
On Thu, Jan 12, 2023 at 10:00:06PM -0300, Fabio Estevam wrote: > As explained in the text at the bottom of the page > https://source.codeaurora.org/external/qoriq/qoriq-components/rcw: > > "QUIC repositories on this site will not receive any updates after > March 31, 2022, and will be deleted on March 31, 2023." > > Point to the NXP RCW github repo instead. > > Signed-off-by: Fabio Estevam > --- Reviewed-by: Vladimir Oltean
Re: [PATCH v9 0/8] Add MV88E6xxx DSA driver and use on gwventana
On Wed, Nov 30, 2022 at 03:27:04PM -0800, Tim Harvey wrote: > That's the same head that I based them off of and I just did the > following and it worked fine: > cd /tmp > git clone git://git.denx.de/u-boot.git > cd u-boot > wget https://patchwork.ozlabs.org/series/330704/mbox/ -O mbox > git am mbox > > Looks like only the last patch failed right? Yeah, only the last patch failed. I converted them again to mbox format using my super awkward procmail-based mail-to-mbox scripts, and now it seems that git-am eats them just fine. I had taken a look at the last patch context, and it looked identical to what was in tree, so I didn't understand what was wrong. Anyway, I just wanted to make sure that the dsa sandbox tests still pass, which they do. I'm away from my boards right now, so I won't be able to change U-Boot remotely to do a live check.
Re: [PATCH v9 0/8] Add MV88E6xxx DSA driver and use on gwventana
On Wed, Nov 30, 2022 at 09:42:43AM -0800, Tim Harvey wrote:
> This series adds a DSA driver for the MV88E6xxx based on
> drivers/net/phy/mv88e61xx and uses it in the gwventana_gw5904_defconfig.
I can't apply the patches on the current master, am I doing something wrong?
$ git reset --hard origin/master
$ git am ~/incoming/*
Applying: net: mdio-uclass: scan for dm mdio children on post-bind
Applying: net: dsa: move cpu port probe to dsa_post_probe
Applying: net: dsa: ensure dsa driver has proper ops
Applying: net: dsa: allow rcv() and xmit() to be optional
Applying: net: ksz9477: remove unnecessary xmit and recv functions
Applying: net: fec: add support for DM_MDIO
Applying: net: add MV88E6xxx DSA driver
Applying: board: gw_ventana: enable MV88E61XX DSA support
error: patch failed: arch/arm/dts/imx6qdl-gw5904.dtsi:212
error: arch/arm/dts/imx6qdl-gw5904.dtsi: patch does not apply
error: patch failed: board/gateworks/gw_ventana/gw_ventana.c:83
error: board/gateworks/gw_ventana/gw_ventana.c: patch does not apply
error: patch failed: configs/gwventana_gw5904_defconfig:110
error: configs/gwventana_gw5904_defconfig: patch does not apply
Patch failed at 0008 board: gw_ventana: enable MV88E61XX DSA support
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
$ git remote show origin
* remote origin
Fetch URL: https://source.denx.de/u-boot/u-boot.git
Push URL: https://source.denx.de/u-boot/u-boot.git
$ git show origin/master
commit 39b81955d38c11254b455322b9d98e07010049d6 (origin/master)
Merge: 597e7b784dbf 5e6c069b2c6b
Author: Tom Rini
Date: Mon Nov 28 13:12:40 2022 -0500
Merge branch '2022-11-28-networking-updates-and-improvements'
- LiteX Ethernet support, dwc_eth_qos fixes, re-work fixing
CVE-2022-{30790,30552}, macb race fix, Intel XWAY PHY support
and add wget command and TCP support.
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Tue, Nov 29, 2022 at 04:58:33PM -0800, Tim Harvey wrote: > Yes, it makes sense. How about the following patch instead: > > diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c > index 211a991cdd0d..1ae9adc66eda 100644 > --- a/net/dsa-uclass.c > +++ b/net/dsa-uclass.c > @@ -142,6 +142,9 @@ static int dsa_port_send(struct udevice *pdev, > void *packet, int length) > struct dsa_port_pdata *port_pdata; > int err; > > + if (!ops->xmit) > + return eth_get_ops(master)->send(master, packet, length); My 2 cents, I would avoid calling eth_get_ops(master)->send() twice. Either keep the mangling inside the "if" block, or pass the length argument by reference to your function, or use a goto skip_mangling. > + > if (length + head + tail > PKTSIZE_ALIGN) > return -EINVAL; > > @@ -172,7 +175,7 @@ static int dsa_port_recv(struct udevice *pdev, int > flags, uchar **packetp) > int length, port_index, err; > > length = eth_get_ops(master)->recv(master, flags, packetp); > - if (length <= 0) > + if (length <= 0 || !ops->rcv) > return length; > > /* > > Perhaps it's more elegant to wrap the bulk of dsa_port_send with an > 'if (ops->xmit)' but changing the indentation makes the patch more > difficult to follow. > > Best Regards, > > Tim
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Tue, Nov 29, 2022 at 02:53:15PM -0800, Tim Harvey wrote:
> On Mon, Nov 28, 2022 at 7:58 AM Tom Rini wrote:
> >
> > On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
> >
> > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > > does not care to mangle a packet as in U-Boot only one network port is
> > > enabled at a time and thus no packet mangling is necessary.
> > >
> > > Suggested-by: Vladimir Oltean
> > > Signed-off-by: Tim Harvey
> > > Reviewed-by: Vladimir Oltean
> > > Reviewed-by: Fabio Estevam
> >
> > This causes:
> > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] -
> > AssertionError: assert False
> >
> > In sandbox, and I don't know if the test or the code is wrong.
> >
>
> Tom,
>
> I'm not familiar at all with U-Boot's sandbox or unit test
> infrastructure and am trying to learn.
>
> I've figured out how to build sandbox and run the dm_test_dsa
> (./u-boot -T -c "ut dm dsa") and see the same failure as you but I
> don't understand how to debug this as it seems prints I add in
> dsa_port_send and dsa_port_recv (which is what this patch modifies)
> don't get print when run from the test infrastructure.
>
> When I boot u-boot sandbox (./u-boot) I don't see any network devices
> at all - perhaps I'm not booting sandbox with dm or something? I need
> to understand what devices/drivers sandbox is using and how to
> recreate the network environment that the dm_test_dsa function is
> using which calls 'net_loop':
>
> net_ping_ip = string_to_ip("1.2.3.5");
>
> env_set("ethact", "eth2");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "lan0");
> ut_assertok(net_loop(PING));
> env_set("ethact", "lan1");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "");
>
> Best Regards,
>
> Tim
I use the following steps:
export KBUILD_OUTPUT=output-sandbox
make sandbox_defconfig NO_SDL=1
make -j 8 NO_SDL=1
$KBUILD_OUTPUT/u-boot -d $KBUILD_OUTPUT/arch/sandbox/dts/test.dtb
setenv ethact lan1
ping 1.2.3.5
ut dm dsa
In this case the problem with the patch is simple, sorry for not
noticing during review.
dsa_port_mangle_packet() changes the "length" variable. But if ops->xmit
exists, eth_get_ops(master)->send() is called with a bad (old) length,
the one pre mangling.
Re: [PATCH v8 0/8] Add MV88E6xxx DSA driver and use on gwventana
On Thu, Oct 27, 2022 at 05:49:29PM -0700, Tim Harvey wrote: > This series adds a DSA driver for the MV88E6xxx based on > drivers/net/phy/mv88e61xx and uses it in the gwventana_gw5904_defconfig. Looks good, thanks. To me this is ready to go!
Re: [PATCH v7 7/8] net: add MV88E6xxx DSA driver
On Wed, Oct 26, 2022 at 01:59:19PM -0700, Tim Harvey wrote: > not sure honestly - it's in the original drivers/net/phy/mv88e61xxx.c > driver that I based this one off of. You keep saying this; it doesn't matter where it comes from, the point is whether it's useful to keep it in a new driver or not... > From the functional spec of the 88E6176: > - energy detect mode1 (what I'm doing here): > only the signal detection circuitry and the serial management > interface are active. If the PHY detects energy on the line, it starts > to Auto-Negotiate sending FLPs for 5 seconds. If at the end of 5 > seconds the Auto-Negotiation is not completed, then the PHY stops > sending FLPs and goes back to monitoring receive energy. If > Auto-Negotation is completed, then the PHY goes into normal > 10/100/1000 MBps operation. If during normal operation the link is > lost, the PHY will re-start Auto-Negotiation. If no energy is detected > after 5 seconds, the PHY goes back to monitoring receive energy. > - energy detect mode2 (appears to be the power-on default): > The PHY sends out a single 10Mbps NLP (Normal Link Pulse) every on > second. Except for this difference, Mode 2 is identical to Mode 1 > operation. If the device is in Mode 1, it cannot wake up a connected > device; therefore, the connected device must be transmitting NLPs, or > either device must be woken up through register access. If the device > is in Mode 2, then it can wake a connected device. > - energy detect disabled: > Normal 10/100/1000 mbps operation > > You bring up a great point regarding the Marvell PHY driver not doing > this... I'm happy to kill it off? Ah, ok, it's a low power mode. That being said, it seems like Marvell is saying: hey, we offer you the ability to save power (when the PHY should be up) by disabling much of the PHY circuitry, just leave on some signal detection circuit. We certainly won't be emitting any NLP such that the link partner will know the link is up. You go first and then we'll respond! I wonder what happens if you connect 2 Marvell PHY ports with Energy Detect to each other... the link shouldn't come up, right? I absolutely think that this setting has no purpose in a bootloader driver. Just a potential pain point. You want the bootloader networking to be bullet proof. Kill it!
Re: [PATCH v7 7/8] net: add MV88E6xxx DSA driver
This patch looks much better to me. Only one comment.
On Wed, Oct 26, 2022 at 09:28:58AM -0700, Tim Harvey wrote:
> +static int mv88e6xxx_port_enable(struct udevice *dev, int port, struct
> phy_device *phy)
> +{
> + struct mv88e6xxx_priv *priv = dev_get_priv(dev);
> + int val, ret;
> +
> + dev_dbg(dev, "%s P%d phy:0x%08x %s\n", __func__, port,
> + phy->phy_id, phy_string_for_interface(phy->interface));
> +
> + if (phy->phy_id == PHY_FIXED_ID) {
> + /* Physical Control register: Table 62 */
> + val = mv88e6xxx_port_read(dev, port, PORT_REG_PHYS_CTRL);
> +
> + /* configure RGMII delays for fixed link */
> + switch (phy->interface) {
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + dev_dbg(dev, "configure internal RGMII delays\n");
> +
> + /* RGMII delays */
> + val &= ~(PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK ||
> + PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK);
> + if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phy->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK;
> + if (phy->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> + phy->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> + val |= PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK;
> + break;
> + default:
> + break;
> + }
> +
> + /* Force Link */
> + val |= PORT_REG_PHYS_CTRL_LINK_VALUE |
> +PORT_REG_PHYS_CTRL_LINK_FORCE;
> +
> + ret = mv88e6xxx_port_write(dev, port, PORT_REG_PHYS_CTRL, val);
> + if (ret < 0)
> + return ret;
> +
> + if (mv88e6xxx_6352_family(dev)) {
> + /* validate interface type */
> + dev_dbg(dev, "validate interface type\n");
> + val = mv88e6xxx_get_cmode(dev, port);
> + if (val < 0)
> + return val;
> + switch (phy->interface) {
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + if (val != PORT_REG_STATUS_CMODE_RGMII)
> + goto mismatch;
> + break;
> + case PHY_INTERFACE_MODE_1000BASEX:
> + if (val != PORT_REG_STATUS_CMODE_1000BASE_X)
> + goto mismatch;
> + break;
> +mismatch:
> + default:
> + dev_err(dev, "Mismatched PHY mode %s on port
> %d!\n",
> +
> phy_string_for_interface(phy->interface), port);
> + break;
> + }
> + }
> + } else {
What I was saying was here. The phy_id is not PHY_FIXED_ID, okay, but
that doesn't necessarily imply this is an internal PHY port, as the code
below goes on to assume. Maybe this is the case for your setup, but I
would like to see some guarding in a generic driver such that internal
PHY configuration only gets done for ports where that PHY exists.
Which actually brings me to another, more important point. This
PHY_REG_CTRL1 access is the only PHY access that the DSA driver
performs. But we also have a driver for the internal PHY. Why doesn't
the Marvell PHY driver do this write? Also, what is the utility of
enabling energy-detect sense?
> + /* Enable energy-detect sensing on PHY */
> + dev_dbg(dev, "enabling energy-detect sense on PHY\n");
> + val = mv88e6xxx_phy_read(dev, port, PHY_REG_CTRL1);
> + if (val < 0)
> + return val;
> + switch (priv->id) {
> + case PORT_SWITCH_ID_6096:
> + case PORT_SWITCH_ID_6097:
> + case PORT_SWITCH_ID_6172:
> + case PORT_SWITCH_ID_6176:
> + case PORT_SWITCH_ID_6240:
> + case PORT_SWITCH_ID_6352:
> + val &= ~GENMASK(8, 2);
> + val |= (PHY_REG_CTRL1_ENERGY_DET_SENSE_XMIT << 8);
> + break;
> + default:
> + val &= ~GENMASK(14, 1);
> + val |= (PHY_REG_CTRL1_ENERGY_DET_SENSE_PULSE << 14);
> + break;
> + }
> + val = mv88e6xxx_phy_write(dev, port, PHY_REG_CTRL1, val);
> +
Re: [PATCH v7 8/8] board: gw_ventana: enable MV88E61XX DSA support
After the realization from the v6 review, can you please add to this:
On Wed, Oct 26, 2022 at 09:28:59AM -0700, Tim Harvey wrote:
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 612b6e068e28..c0790183c013 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -212,6 +212,27 @@
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sw_phy0: ethernet-phy@0 {
> + reg = <0x0>;
> + };
> +
> + sw_phy1: ethernet-phy@1 {
> + reg = <0x1>;
> + };
> +
> + sw_phy2: ethernet-phy@2 {
> + reg = <0x2>;
> + };
> +
> + sw_phy3: ethernet-phy@3 {
> + reg = <0x3>;
> + };
> + };
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -219,27 +240,36 @@
> port@0 {
> reg = <0>;
> label = "lan4";
> + phy-handle = <_phy0>;
phy-mode = "internal";
> };
>
> port@1 {
> reg = <1>;
> label = "lan3";
> + phy-handle = <_phy1>;
phy-mode = "internal";
> };
>
> port@2 {
> reg = <2>;
> label = "lan2";
> + phy-handle = <_phy2>;
phy-mode = "internal";
> };
>
> port@3 {
> reg = <3>;
> label = "lan1";
> + phy-handle = <_phy3>;
phy-mode = "internal";
> };
>
> port@5 {
> reg = <5>;
> - label = "cpu";
> ethernet = <>;
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> };
> };
> };
Re: [PATCH v6 7/8] net: add MV88E61xx DSA driver
Sorry for the late reply.
On Thu, Oct 13, 2022 at 11:09:32AM -0700, Tim Harvey wrote:
> The SERDES configuration on the 88E6352/88E6240/88E6176/88E6172 (which
> I have documentation for) is not in port specific registers so I think
> I should move that to the probe function for the switch.
It's not in port specific registers for this switch family, because
there's a single SERDES port. What you could do is you could do the
SERDES initialization once per (this) switch in the per-port procedure,
if the current port is a SERDES port according to the C_MODE. This would
have to be done anyway for other switches. Anyway, what you did in v7
works too, you can leave it like that and others can refactor if needed
(I think; I didn't look in too much detail yet).
> > > +static int mv88e6xxx_port_enable(struct udevice *dev, int port, struct
> > > phy_device *phy)
> > > +{
> > > + struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
> > > + struct mv88e6xxx_priv *priv = dev_get_priv(dev);
> > > + int val, ret;
> > > +
> > > + dev_dbg(dev, "%s P%d phy:0x%08x %s\n", __func__, port,
> > > + phy->phy_id, phy_string_for_interface(phy->interface));
> > > +
> > > + /*
> > > + * Enable energy-detect sensing on PHY, used to determine when a PHY
> > > + * port is physically connected
> > > + */
> > > + if (port != dsa_pdata->cpu_port) {
> >
> > Can you test based on phy->interface == PHY_INTERFACE_MODE_INTERNAL?
> > Support may come later for cascade ports, and those are ports without an
> > internal PHY which are not CPU ports.
>
> In my config there are no ports with interface ==
> PHY_INTERFACE_MODE_INTERNAL so I'm not sure what you mean here. What
> situations does that come up in?
Ugh. Again the nasty Linux dt-binding legacy for this switch
So normally, an Ethernet port has a phy-handle/fixed-link, and a phy-mode.
For a port connected to an internal PHY, you should have phy-handle = <>
(which you do), and phy-mode = "internal" (which you don't). Linux
supports skipping both the phy-handle and the phy-mode.
Sorry for not realizing this in time. This is again a thing so that
U-Boot can know that you're operating on a port with an internal PHY,
rather than hardcoding this based on the assumption that the SERDES port
is the CPU port, and everything else has internal PHY (which assumes
RGMII does not exist).
Re: [PATCH v6 7/8] net: add MV88E61xx DSA driver
On Mon, Oct 10, 2022 at 09:39:43AM -0700, Tim Harvey wrote:
> Add a DSA driver for the MV88E6xxx compatible Ethernet switches.
>
> Cc: Marek BehĂșn
> Cc: Vladimir Oltean
> Signed-off-by: Tim Harvey
> Reviewed-by: Vladimir Oltean
> Reviewed-by: Fabio Estevam
> ---
> v6:
> - update commit msg (s/MV88E61xx/MV88E6xxx)
really?
> - remove GbE from commit msg and Kconfig desc
> - squash accidental change from patch 7 into patch 6
> - added error print on failure to read switch id
> - mv88e6xxx_probe:
> - check for switch enabled
> - remove unused variable enabled
> - remove unnecessary call to dsa_set_tagging
> - port_probe:
> - new function to configure phy features by calling phy_config
> - port_enable:
> - only enable energy-detect sensing on phy ports
> - add phy/cmode verification mistmatch warning
> - remove mv88e6xxx_fixed_port_setup()
> - always force link up for fixed ports
> - always set SERDES mode regardless of cpu port
really?
> +struct mv88e6xxx_priv {
> + int smi_addr;
> + int id;
> + int port_count; /* Number of switch ports */
> + int port_reg_base; /* Base of the switch port registers */
> + u8 global1; /* Offset of Switch Global 1 registers */
> + u8 global2; /* Offset of Switch Global 2 registers */
> + u8 phy_ctrl1_en_det_shift; /* 'EDet' bit field offset */
> + u8 phy_ctrl1_en_det_width; /* Width of 'EDet' bit field */
> + u8 phy_ctrl1_en_det_ctrl; /* 'EDet' control value */
> +};
> +
> +static inline int smi_cmd(int cmd, int addr, int reg)
If the convention in U-Boot is the same as in Linux (which it well may be,
except for the fact that more things escape review), then you should
avoid manually defining functions as inline in C files. The compiler
will decide whether to inline or not regardless of what you specify here.
Although I wonder if these couldn't be in fact expressed more clearly
using plain, boring old logical | and << operators as part of a macro.
> +{
> + cmd = bitfield_replace(cmd, SMI_CMD_ADDR_SHIFT, SMI_CMD_ADDR_WIDTH,
> +addr);
> + cmd = bitfield_replace(cmd, SMI_CMD_REG_SHIFT, SMI_CMD_REG_WIDTH, reg);
> + return cmd;
> +}
> +
> +static inline int smi_cmd_read(int addr, int reg)
> +{
> + return smi_cmd(SMI_CMD_READ, addr, reg);
> +}
> +
> +static inline int smi_cmd_write(int addr, int reg)
> +{
> + return smi_cmd(SMI_CMD_WRITE, addr, reg);
> +}
> +
> +/* Wait for the current SMI indirect command to complete */
> +static int mv88e6xxx_smi_wait(struct udevice *dev, int smi_addr)
> +{
> + int val;
> + u32 timeout = 100;
> +
> + do {
> + val = dm_mdio_read(dev->parent, smi_addr, MDIO_DEVAD_NONE,
> SMI_CMD_REG);
> + if (val >= 0 && (val & SMI_BUSY) == 0)
> + return 0;
> +
> + mdelay(1);
> + } while (--timeout);
> +
> + dev_err(dev, "SMI busy timeout\n");
> + return -ETIMEDOUT;
> +}
> +
> +static int mv88e6xxx_serdes_init(struct udevice *dev)
> +{
> + int val;
> +
> + val = mv88e6xxx_set_page(dev, DEVADDR_SERDES, PHY_PAGE_SERDES);
> + if (val < 0)
> + return val;
> +
> + /* Power up serdes module */
> + val = mv88e6xxx_phy_read(dev, DEVADDR_SERDES, MII_BMCR);
> + if (val < 0)
> + return val;
> + val &= ~(BMCR_PDOWN);
> + val = mv88e6xxx_phy_write(dev, DEVADDR_SERDES, MII_BMCR, val);
> + if (val < 0)
> + return val;
> +
> + return 0;
> +}
> +
> +static int mv88e6xxx_port_enable(struct udevice *dev, int port, struct
> phy_device *phy)
> +{
> + struct dsa_pdata *dsa_pdata = dev_get_uclass_plat(dev);
> + struct mv88e6xxx_priv *priv = dev_get_priv(dev);
> + int val, ret;
> +
> + dev_dbg(dev, "%s P%d phy:0x%08x %s\n", __func__, port,
> + phy->phy_id, phy_string_for_interface(phy->interface));
> +
> + /*
> + * Enable energy-detect sensing on PHY, used to determine when a PHY
> + * port is physically connected
> + */
> + if (port != dsa_pdata->cpu_port) {
Can you test based on phy->interface == PHY_INTERFACE_MODE_INTERNAL?
Support may come later for cascade ports, and those are ports without an
internal PHY which are not CPU ports.
> + val = mv88e6xxx_phy_read(dev, port, PHY_REG_CTRL1);
> + if (val < 0)
> + return val;
> + val = bitfield_replace(val, priv->phy_ctrl1_en_det_shift,
> +priv->phy_ctrl1_en_det_width,
Re: [PATCH v6 8/8] board: gw_ventana: enable MV88E6XXX DSA support
On Mon, Oct 10, 2022 at 09:39:44AM -0700, Tim Harvey wrote: > Add MV88E6XXX DSA support: > - update dt to provide internal MDIO bus and port handles. >U-Boot requires a more restrictive subset of the dt bindings >required by Linux for the sake of simplifying code > - update defconfig to remove old driver and enable new one > - replace mv88e61xx_hw_reset weak override with board_phy_config support >for register configuration that is outside the scope of the DSA driver > > Signed-off-by: Tim Harvey > Reviewed-by: Fabio Estevam > --- Reviewed-by: Vladimir Oltean
Re: [PATCH v6 4/8] net: dsa: allow rcv() and xmit() to be optional
Hi Simon, On Mon, Oct 10, 2022 at 04:25:29PM -0600, Simon Glass wrote: > Hi Tim, > > On Mon, 10 Oct 2022 at 10:40, Tim Harvey wrote: > > > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver > > does not care to mangle a packet as in U-Boot only one network port is > > enabled at a time and thus no packet mangling is necessary. > > Should the tests be updated for this? Patch 3/8 serves as a runtime validation for this change. I don't believe that adding something more to test/dm/dsa.c is warranted, and I don't see exactly what can be added, either.
Re: [PATCH v5 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Tue, Oct 04, 2022 at 09:49:18AM -0700, Tim Harvey wrote:
> Add MV88E61XX DSA support:
> - update dt: U-Boot dsa driver requires different device-tree syntax
>than the linux driver in order to link the dsa ports to the mdio bus.
Not really correct. Better said, U-Boot requires a more restrictive
subset of what DT bindings Linux supports, for sake of simplicity of the
code. It is equally valid to link ports to their internal PHY by
phy-handle in the Linux mv88e6xxx driver.
> - update defconfig
> - replace mv88e61xx_hw_reset weak override with board_phy_config support
>for mv88e61xx configuration that is outside the scope of the DSA driver
>
> Note that the dt changes were not placed in a u-boot.dtsi as it is
> intended that these changes get merged with the Linux dt's.
>
> Signed-off-by: Tim Harvey
> Reviewed-by: Fabio Estevam
> ---
> v5:
> - fix typo in defconfig s/MV88E61XX/MV88E6XXX
> - added Fabio's rb tag
> - added note to commit log
> v4:
> - use standard Linux internal MDIO dt structure
> - use PHY_FIXED_ID define
> v3:
> - move mdio's mdio@0 subnode
> v2: no changes
> ---
> arch/arm/dts/imx6qdl-gw5904.dtsi| 31 +++
> board/gateworks/gw_ventana/gw_ventana.c | 50 +
> configs/gwventana_gw5904_defconfig | 7 ++--
> drivers/net/mv88e6xxx.c | 29 ++
> 4 files changed, 64 insertions(+), 53 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 612b6e068e28..487dd7648274 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -212,6 +212,27 @@
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sw_phy0: ethernet-phy@0 {
> + reg = <0x0>;
> + };
> +
> + sw_phy1: ethernet-phy@1 {
> + reg = <0x1>;
> + };
> +
> + sw_phy2: ethernet-phy@2 {
> + reg = <0x2>;
> + };
> +
> + sw_phy3: ethernet-phy@3 {
> + reg = <0x3>;
> + };
> + };
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -219,27 +240,37 @@
> port@0 {
> reg = <0>;
> label = "lan4";
> + phy-handle = <_phy0>;
> };
>
> port@1 {
> reg = <1>;
> label = "lan3";
> + phy-handle = <_phy1>;
> };
>
> port@2 {
> reg = <2>;
> label = "lan2";
> + phy-handle = <_phy2>;
> };
>
> port@3 {
> reg = <3>;
> label = "lan1";
> + phy-handle = <_phy3>;
> };
>
> port@5 {
> reg = <5>;
> label = "cpu";
This is not used, please remove it.
> ethernet = <>;
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> };
> };
> };
Binding looks good, thanks.
> diff --git a/board/gateworks/gw_ventana/gw_ventana.c
> b/board/gateworks/gw_ventana/gw_ventana.c
> index 99f52b9953e2..1c3da2c82707 100644
> --- a/board/gateworks/gw_ventana/gw_ventana.c
> +++ b/board/gateworks/gw_ventana/gw_ventana.c
> @@ -83,44 +83,30 @@ int board_phy_config(struct phy_device *phydev)
> break;
> }
>
> + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */
> + if (phydev->phy_id == PHY_FIXED_ID &&
> + (board_type == GW5904 || board_type == GW5909)) {
> + struct mii_dev *bus = miiphy_get_dev_by_name("mdio");
> +
> + puts("MV88E61XX ");
> + /* GPIO[0]
Re: [PATCH v5 7/8] net: add MV88E61xx DSA driver
Please also rewrite "61xx" from the commit message into "6xxx".
On Tue, Oct 04, 2022 at 09:49:17AM -0700, Tim Harvey wrote:
> Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches.
>
> Cc: Marek BehĂșn
> Cc: Vladimir Oltean
> Signed-off-by: Tim Harvey
> Reviewed-by: Vladimir Oltean
> Reviewed-by: Fabio Estevam
> ---
> v5:
> - added support for MV88E6320
> - added Fabio's rb tag
> v4:
> - rename to mv88e6xxx
> - sort includes alphabetically
> - remove dsa term from function names
> - reduce indentation level and remove unecessary code in of probe_mdio
> function
> - rename pdev to mdev to represent mdio device
> v3:
> - Added Vladimir's rb tag
> v2:
> - rebase on v2022.07-rc1 (use ofnode_get_phy_node)
> - remove unused commented out fields from struct
> - remove unused PORT_MASK macro
> - remove phy from priv struct name
> - refactor code from original drivers/net/phy/mv88e61xx with
>suggestions from review to consolidate some functions
>into mv88e61xx_dsa_port_enable
> - remove unecessary skiping of disabling of CPU port
> - remove unecessary dev_set_parent_priv
> - remove unnecessary static init flag
> - replace debug with a dev_warn if switch device-id unsupported
> - remove unnecessary xmit/recv functions as we rely on the fact that
>only a single port is active instead of mangling packets
> ---
> drivers/net/Kconfig | 7 +
> drivers/net/Makefile| 1 +
> drivers/net/mv88e6xxx.c | 833
> 3 files changed, 841 insertions(+)
> create mode 100644 drivers/net/mv88e6xxx.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 6bbbadc5eef3..5508dacbcc49 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -438,6 +438,13 @@ config KSZ9477
> This driver implements a DSA switch driver for the KSZ9477 family
> of GbE switches using the I2C interface.
>
> +config MV88E6XXX
> + bool "Marvell MV88E6xxx GbE switch DSA driver"
> + depends on DM_DSA && DM_MDIO
> + help
> + This driver implements a DSA switch driver for the MV88E6xxx family
> + of GbE switches using the MDIO interface
I don't think they are all gigabit.
> +
> config MVGBE
> bool "Marvell Orion5x/Kirkwood network interface support"
> depends on ARCH_KIRKWOOD || ARCH_ORION5X
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 96b7678e9882..f05fd58e3fa7 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o
> obj-$(CONFIG_MPC8XX_FEC) += mpc8xx_fec.o
> obj-$(CONFIG_MT7620_ETH) += mt7620-eth.o
> obj-$(CONFIG_MT7628_ETH) += mt7628-eth.o
> +obj-$(CONFIG_MV88E6XXX) += mv88e6xxx.o
> obj-$(CONFIG_MVGBE) += mvgbe.o
> obj-$(CONFIG_MVMDIO) += mvmdio.o
> obj-$(CONFIG_MVNETA) += mvneta.o
> diff --git a/drivers/net/mv88e6xxx.c b/drivers/net/mv88e6xxx.c
> new file mode 100644
> index ..e59e2464c641
> --- /dev/null
> +++ b/drivers/net/mv88e6xxx.c
> +static int mv88e6xxx_switch_reset(struct udevice *dev)
> +{
> + struct mv88e6xxx_priv *priv = dev_get_priv(dev);
> + int time;
> + int val;
> + u8 port;
> +
> + /* Disable all ports */
> + for (port = 0; port < priv->port_count; port++) {
> + val = mv88e6xxx_port_read(dev, port, PORT_REG_CTRL);
> + if (val < 0)
> + return val;
> + val = bitfield_replace(val, PORT_REG_CTRL_PSTATE_SHIFT,
> +PORT_REG_CTRL_PSTATE_WIDTH,
> +PORT_REG_CTRL_PSTATE_DISABLED);
> + val = mv88e6xxx_port_write(dev, port, PORT_REG_CTRL, val);
> + if (val < 0)
> + return val;
> + }
> +
> + /* Wait 2 ms for queues to drain */
> + udelay(2000);
> +
> + /* Reset switch */
> + val = mv88e6xxx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
> + if (val < 0)
> + return val;
> + val |= GLOBAL1_CTRL_SWRESET;
> + val = mv88e6xxx_reg_write(dev, priv->global1, GLOBAL1_CTRL, val);
> + if (val < 0)
> + return val;
> +
> + /* Wait up to 1 second for switch reset complete */
for switch reset *to* complete
> + for (time = 1000; time; time--) {
Maybe "time_ms"? Or "retries"?
> + val = mv88e6xxx_reg_read(dev, priv->global1, GLOBAL1_CTRL);
> + if (val >= 0 && ((val & GLOBAL1_CTRL_SWRESET) == 0))
> + break;
> + udelay(1000);
Re: [PATCH v5 2/8] net: dsa: move cpu port probe to dsa_post_probe
On Tue, Oct 04, 2022 at 09:49:12AM -0700, Tim Harvey wrote:
> diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
> index 5b7046432ff3..a37e76e25a8f 100644
> --- a/net/dsa-uclass.c
> +++ b/net/dsa-uclass.c
> @@ -466,7 +466,6 @@ static int dsa_pre_probe(struct udevice *dev)
> +static int dsa_post_probe(struct udevice *dev)
> +{
> + struct dsa_priv *priv = dev_get_uclass_priv(dev);
> + struct dsa_ops *ops = dsa_get_ops(dev);
> + int err;
> +
> /* Simulate a probing event for the CPU port */
> if (ops->port_probe) {
> err = ops->port_probe(dev, priv->cpu_port,
> @@ -491,13 +499,14 @@ static int dsa_pre_probe(struct udevice *dev)
> }
>
> return 0;
> -}
> +};
Semicolons aren't needed at the end of functions.
Re: [PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Fri, Jun 24, 2022 at 04:16:34PM -0700, Tim Harvey wrote:
> On Fri, Jun 24, 2022 at 3:25 AM Vladimir Oltean
> wrote:
> >
> > On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote:
> > > Add MV88E61XX DSA support:
> > > - update dt: U-Boot dsa driver requires different device-tree syntax
> > >than the linux driver in order to link the dsa ports to the mdio bus.
> > > - update defconfig
> > > - replace mv88e61xx_hw_reset weak override with board_phy_config support
> > >for mv88e61xx configuration that is outside the scope of the DSA driver
> > >
> > > Signed-off-by: Tim Harvey
> > > ---
> > > v3:
> > > - move mdio's mdio@0 subnode
> > > v2: no changes
> > > ---
> > > arch/arm/dts/imx6qdl-gw5904.dtsi| 41
> > > board/gateworks/gw_ventana/gw_ventana.c | 50 +
> > > configs/gwventana_gw5904_defconfig | 7 ++--
> > > 3 files changed, 62 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > index 286c7a9924c2..1b2f70d1ccb2 100644
> > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > @@ -219,6 +219,33 @@
> > > compatible = "marvell,mv88e6085";
> > > reg = <0>;
> > >
> > > + mdios {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + mdio@0 {
> >
> > If you are going to follow this new model with a dedicated "mdios" subnode,
> > I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't
> > be a problem to later make Linux accept this alternate binding format.
> > But in that case, please match this OF node by a dedicated compatible
> > string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a
> > way to differentiate this from the existing
> > "marvell,mv88e6xxx-mdio-external"
> > when support for that is added in U-Boot.
> >
> > Alternatively, to repeat myself, you can always follow the de-facto
> > bindings for Linux mv88e6xxx which have:
> >
> > switch0: switch0@0 {
> > compatible = "marvell,mv88e6190";
> >
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > ...
> > };
> >
> > mdio { // internal
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > ...
> > };
> >
> > mdio1 {
> > compatible =
> > "marvell,mv88e6xxx-mdio-external";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > ...
> > };
> > };
> >
>
> Documentation/devicetree/bindings/net/dsa/marvell.txt shows en example
> with just one child node under the internal mdio node:
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> switch1phy0: switch1phy0@0 {
> reg = <0>;
> interrupt-parent = <>;
> interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> };
> };
>
> Am I to assume I can add additional nodes there for the other ports
> such as the following?
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> switch1phy0: switch1phy0@0 {
> reg = <0>;
> };
>
> switch1phy1: switch1phy1@1 {
> reg = <1>;
> };
>
> switch1phy2: switch1phy2@2 {
> reg = <2>;
> };
>
> switch1phy3: switch1phy3@3 {
> reg = <3>;
> };
>
> ...
> };
Sure, but name those PHY nodes "ethernet-phy@N" rather than "switchMphyN",
as Documentation/devicetree/bindings/net/ethernet-phy.yaml requires.
Many mistakes were made in writing mv88e6xxx device trees, we don't need
to follow each and every one of them, only the important ones.
Re: [PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote:
> Add MV88E61XX DSA support:
> - update dt: U-Boot dsa driver requires different device-tree syntax
>than the linux driver in order to link the dsa ports to the mdio bus.
> - update defconfig
> - replace mv88e61xx_hw_reset weak override with board_phy_config support
>for mv88e61xx configuration that is outside the scope of the DSA driver
>
> Signed-off-by: Tim Harvey
> ---
> v3:
> - move mdio's mdio@0 subnode
> v2: no changes
> ---
> arch/arm/dts/imx6qdl-gw5904.dtsi| 41
> board/gateworks/gw_ventana/gw_ventana.c | 50 +
> configs/gwventana_gw5904_defconfig | 7 ++--
> 3 files changed, 62 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 286c7a9924c2..1b2f70d1ccb2 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -219,6 +219,33 @@
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> + mdios {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio@0 {
If you are going to follow this new model with a dedicated "mdios" subnode,
I've consulted with Andrew Lunn and Florian Fainelli and there shouldn't
be a problem to later make Linux accept this alternate binding format.
But in that case, please match this OF node by a dedicated compatible
string, like "marvell,mv88e6xxx-mdio-internal", so that there will be a
way to differentiate this from the existing "marvell,mv88e6xxx-mdio-external"
when support for that is added in U-Boot.
Alternatively, to repeat myself, you can always follow the de-facto
bindings for Linux mv88e6xxx which have:
switch0: switch0@0 {
compatible = "marvell,mv88e6190";
ports {
#address-cells = <1>;
#size-cells = <0>;
...
};
mdio { // internal
#address-cells = <1>;
#size-cells = <0>;
...
};
mdio1 {
compatible = "marvell,mv88e6xxx-mdio-external";
#address-cells = <1>;
#size-cells = <0>;
...
};
};
and the associated parsing code:
static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
struct device_node *np)
{
struct device_node *child;
int err;
/* Always register one mdio bus for the internal/default mdio
* bus. This maybe represented in the device tree, but is
* optional.
*/
child = of_get_child_by_name(np, "mdio");
err = mv88e6xxx_mdio_register(chip, child, false);
of_node_put(child);
if (err)
return err;
/* Walk the device tree, and see if there are any other nodes
* which say they are compatible with the external mdio
* bus.
*/
for_each_available_child_of_node(np, child) {
if (of_device_is_compatible(
child, "marvell,mv88e6xxx-mdio-external")) {
err = mv88e6xxx_mdio_register(chip, child, true);
if (err) {
mv88e6xxx_mdios_unregister(chip);
of_node_put(child);
return err;
}
}
}
return 0;
}
Personally I believe that if you have limited amount of time to spend on
U-Boot DM_DSA and DT bindings in general, you should just follow the
format already accepted by Linux ("mdio" node is for internal MDIO,
doesn't have compatible string, is placed directly under "switch" node",
while external MDIO is matched by compatible string and its node can
have any name).
What we should try to accomplish is that the DT blob that U-Boot creates
for itself here to be coherent enough that Linux is able to understand
and use it, in case we decide to pass it to the operating system. With
your approach you'd have work to do in Linux as well to accept the
"mdios" subnode and parse controllers by compatible string inside, and
I'm not exactly sure you're willing to do that.
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sw_phy0: ethernet-phy@0 {
> + reg =
Re: [PATCH v3 7/8] net: add MV88E61xx DSA driver
On Tue, Jun 21, 2022 at 08:11:06AM -0700, Tim Harvey wrote:
> On Tue, Jun 21, 2022 at 12:21 AM Vladimir Oltean
> wrote:
> >
> > On Mon, Jun 20, 2022 at 04:37:45PM -0700, Tim Harvey wrote:
> > > On Mon, Jun 20, 2022 at 4:58 AM Vladimir Oltean
> > > wrote:
> > > >
> > > > On Mon, May 23, 2022 at 11:25:48AM -0700, Tim Harvey wrote:
> > > > > +/* bind and probe the switch mdios */
> > > > > +static int mv88e61xx_dsa_probe_mdio(struct udevice *dev)
> > > > > +{
> > > > > + struct udevice *pdev;
> > > > > + ofnode node, mdios;
> > > > > + const char *name;
> > > > > + int ret;
> > > > > +
> > > > > + /* bind phy ports of mdios child node to mv88e61xx_mdio device
> > > > > */
> > > > > + mdios = dev_read_subnode(dev, "mdios");
> > > > > + if (ofnode_valid(mdios)) {
> > > > > + ofnode_for_each_subnode(node, mdios) {
> > > > > + name = ofnode_get_name(node);
> > > > > + ret = device_bind_driver_to_node(dev,
> > > > > +
> > > > > "mv88e61xx_mdio",
> > > > > + name, node,
> > > > > );
> > > > > + if (ret) {
> > > > > + dev_err(dev, "failed to bind %s: %d\n",
> > > > > name, ret);
> > > > > + continue;
> > > > > + }
> > > > > +
> > > > > + /* need to probe it as there is no compatible
> > > > > to do so */
> > > > > + ret = uclass_get_device_by_ofnode(UCLASS_MDIO,
> > > > > node, );
> > > > > + if (ret) {
> > > > > + dev_err(dev, "failed to probe %s:
> > > > > %d\n", name, ret);
> > > > > + continue;
> > > > > + }
> > > >
> > > > What do you do with this pdev once you get it? Are you missing a
> > > > device_probe() call?
> > > > Also, why "pdev" and not "dev"? What does the "p" stand for?
> > >
> > > struct udevice *dev is passed into the function so I use pdev to
> > > iterate over the ports in the mdios node so 'pdev' means 'port' here.
> >
> > Yes, but those under the mdios node aren't ports, they're MDIO
> > controllers, hence my comment.
>
> how about devp (dev pointer) or subdev or mdio?
The terminology problem I have with "mdio" is that it would be a
struct udevice *, whereas the plural variable, "mdios", is an ofnode.
How about mdev, for mdio device?
Re: [PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Tue, Jun 21, 2022 at 09:57:35AM -0700, Tim Harvey wrote:
> > > diff --git a/board/gateworks/gw_ventana/gw_ventana.c
> > > b/board/gateworks/gw_ventana/gw_ventana.c
> > > index c06630a66b66..bef3f7ef0d2b 100644
> > > --- a/board/gateworks/gw_ventana/gw_ventana.c
> > > +++ b/board/gateworks/gw_ventana/gw_ventana.c
> > > @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev)
> > > phy_write(phydev, MDIO_DEVAD_NONE, 14, val);
> > > }
> > >
> > > + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch
> > > */
> > > + else if (phydev->phy_id == 0xa5a55a5a &&
> > > + ((board_type == GW5904) || (board_type == GW5909))) {
> > > + struct mii_dev *bus = miiphy_get_dev_by_name("mdio");
> > > +
> > > + puts("MV88E61XX ");
> > > + /* GPIO[0] output CLK125 for RGMII_REFCLK */
> > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x62 << 8) |
> > > 0xfe);
> > > + bus->write(bus, 0x1c, 0, 0x1a, (1 << 15) | (0x68 << 8) | 7);
> > > +
> > > + /* Port 0-3 LED configuration: Table 80/82 */
> > > + /* LED configuration: 7:4-green (8=Activity) 3:0 amber
> > > (8=Link) */
> > > + bus->write(bus, 0x10, 0, 0x16, 0x8088);
> > > + bus->write(bus, 0x11, 0, 0x16, 0x8088);
> > > + bus->write(bus, 0x12, 0, 0x16, 0x8088);
> > > + bus->write(bus, 0x13, 0, 0x16, 0x8088);
> > > + }
> > > +
> >
> > There's nothing too board specific about this configuration, why do you
> > feel it does not belong to the DSA driver?
> >
> > Some macros hiding away magic register addresses and values would be
> > good either way.
> >
>
> I don't much care for MAC/PHY drivers configuring GPIO's and LED's due
> to the lack of consistent dt bindings for such a thing and I wasn't
> planning on trying to enhance the capabilities of the mv88e6xxx
> driver.
>
> There are 7 functions each of the 15 GPIO's can be set to:
> 0 - GPIO
> 1 - PTP_TRIG - Precise Timing Protocol Trigger Generate Output
> 2 - PTP_EVREQ - Precise Timing Protocol Event Request Input
> 3 - PTP_EXTCLK - Precise Timing Protocol ExternalClock Input
> 4 - SE_RCLK0 - SyncE Receive Clock 0 Output
> 5 - SE_RCLK1 - SyncE Receive Clock 1 Output
> 6 - Reserved
> 7 - CLK125 - Free running 125 MHz Clock Output
>
> There are two LED's per port each of which can be set to 16 different
> functions also and these functions take a lot of words to describe
> thus probably wouldn't lend well to #define names.
>
> Are there dt bindings that you would suggest I look at for per-port
> LED config on a switch, or GPIO config on a switch? If I add
> dt-bindings I'll have to update the kernel driver as well which again,
> was not my goal here. Maybe moving these into the mv88e6xxx driver per
> dt bindings could be a 'todo'.
>
> This patch isn't making what is already in the board file more
> obscure, it is just updating it to work with the new dsa driver. The
> following was what this patch replaced:
> -#ifdef CONFIG_MV88E61XX_SWITCH
> -int mv88e61xx_hw_reset(struct phy_device *phydev)
> -{
> - struct mii_dev *bus = phydev->bus;
> -
> - /* GPIO[0] output, CLK125 */
> - debug("enabling RGMII_REFCLK\n");
> - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0,
> - 0x1a /*MV_SCRATCH_MISC*/,
> - (1 << 15) | (0x62 /*MV_GPIO_DIR*/ << 8) | 0xfe);
> - bus->write(bus, 0x1c /*MV_GLOBAL2*/, 0,
> - 0x1a /*MV_SCRATCH_MISC*/,
> - (1 << 15) | (0x68 /*MV_GPIO01_CNTL*/ << 8) | 7);
> -
> - /* RGMII delay - Physical Control register bit[15:14] */
> - debug("setting port%d RGMII rx/tx delay\n",
> CONFIG_MV88E61XX_CPU_PORT);
> - /* forced 1000mbps full-duplex link */
> - bus->write(bus, 0x10 + CONFIG_MV88E61XX_CPU_PORT, 0, 1, 0xc0fe);
> - phydev->autoneg = AUTONEG_DISABLE;
> - phydev->speed = SPEED_1000;
> - phydev->duplex = DUPLEX_FULL;
> -
> - /* LED configuration: 7:4-green (8=Activity) 3:0 amber (8=Link) */
> - bus->write(bus, 0x10, 0, 0x16, 0x8088);
> - bus->write(bus, 0x11, 0, 0x16, 0x8088);
> - bus->write(bus, 0x12, 0, 0x16, 0x8088);
> - bus->write(bus, 0x13, 0, 0x16, 0x8088);
> -
> - return 0;
> -}
> -#endif // CONFIG_MV88E61XX_SWITCH
>
> Best Regards,
>
> Tim
Ok, I was thinking PHY LED configuration could be hardcoded in the
driver itself (no DT bindings) and nobody would probably even notice.
For pin configuration as RGMII 125 MHz clock or GPIO or otherwise,
there would probably need to be a "pinctrl" DT subnode with a specific
pinctrl driver attached. It's best if the development for that would be
done in concert with the Linux community, perhaps even in Linux first.
In any case, from my side it's ok if you leave a pinctrl sub-driver as TODO.
Re: [PATCH v3 7/8] net: add MV88E61xx DSA driver
On Mon, Jun 20, 2022 at 04:37:45PM -0700, Tim Harvey wrote:
> On Mon, Jun 20, 2022 at 4:58 AM Vladimir Oltean
> wrote:
> >
> > On Mon, May 23, 2022 at 11:25:48AM -0700, Tim Harvey wrote:
> > > +/* bind and probe the switch mdios */
> > > +static int mv88e61xx_dsa_probe_mdio(struct udevice *dev)
> > > +{
> > > + struct udevice *pdev;
> > > + ofnode node, mdios;
> > > + const char *name;
> > > + int ret;
> > > +
> > > + /* bind phy ports of mdios child node to mv88e61xx_mdio device */
> > > + mdios = dev_read_subnode(dev, "mdios");
> > > + if (ofnode_valid(mdios)) {
> > > + ofnode_for_each_subnode(node, mdios) {
> > > + name = ofnode_get_name(node);
> > > + ret = device_bind_driver_to_node(dev,
> > > + "mv88e61xx_mdio",
> > > + name, node, );
> > > + if (ret) {
> > > + dev_err(dev, "failed to bind %s: %d\n",
> > > name, ret);
> > > + continue;
> > > + }
> > > +
> > > + /* need to probe it as there is no compatible to do
> > > so */
> > > + ret = uclass_get_device_by_ofnode(UCLASS_MDIO,
> > > node, );
> > > + if (ret) {
> > > + dev_err(dev, "failed to probe %s: %d\n",
> > > name, ret);
> > > + continue;
> > > + }
> >
> > What do you do with this pdev once you get it? Are you missing a
> > device_probe() call?
> > Also, why "pdev" and not "dev"? What does the "p" stand for?
>
> struct udevice *dev is passed into the function so I use pdev to
> iterate over the ports in the mdios node so 'pdev' means 'port' here.
Yes, but those under the mdios node aren't ports, they're MDIO
controllers, hence my comment.
> I do not need to do anything with pdev but I must use
> uclass_get_device_by_ofnode() to probe it and that function requires
> it. I don't need to call device_probe because
> uclass_get_device_by_ofnode does it for me
Ok, I didn't notice they all call uclass_get_device_tail() which calls
device_probe().
Re: [PATCH v3 7/8] net: add MV88E61xx DSA driver
On Mon, May 23, 2022 at 11:25:48AM -0700, Tim Harvey wrote: > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches. > > Cc: Marek BehĂșn > Cc: Vladimir Oltean > Signed-off-by: Tim Harvey > Reviewed-by: Vladimir Oltean > --- > v3: > - Added Vladimir's rb tag > v2: > - rebase on v2022.07-rc1 (use ofnode_get_phy_node) > - remove unused commented out fields from struct > - remove unused PORT_MASK macro > - remove phy from priv struct name > - refactor code from original drivers/net/phy/mv88e61xx with >suggestions from review to consolidate some functions >into mv88e61xx_dsa_port_enable > - remove unecessary skiping of disabling of CPU port > - remove unecessary dev_set_parent_priv > - remove unnecessary static init flag > - replace debug with a dev_warn if switch device-id unsupported > - remove unnecessary xmit/recv functions as we rely on the fact that >only a single port is active instead of mangling packets This looks good, but my opinion remains that we can rename mv88e61xx to mv88e6xxx for consistency with Linux. Users will know that the drivers are expected to support the same hardware models (even if the compatible list is now incomplete and does not cover an actual 61xx device). > --- > drivers/net/Kconfig | 7 + > drivers/net/Makefile| 1 + > drivers/net/mv88e61xx.c | 843 > 3 files changed, 851 insertions(+) > create mode 100644 drivers/net/mv88e61xx.c > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index 7fe0e00649cf..edb1a0898986 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -433,6 +433,13 @@ config LPC32XX_ETH > depends on ARCH_LPC32XX > default y > > +config MV88E61XX > + bool "Marvell MV88E61xx GbE switch DSA driver" > + depends on DM_DSA && DM_MDIO > + help > + This driver implements a DSA switch driver for the MV88E61xx family > + of GbE switches using the MDIO interface > + > config MVGBE > bool "Marvell Orion5x/Kirkwood network interface support" > depends on ARCH_KIRKWOOD || ARCH_ORION5X > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index 69fb3bbbf7cb..36b4c279430a 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -59,6 +59,7 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o > obj-$(CONFIG_MPC8XX_FEC) += mpc8xx_fec.o > obj-$(CONFIG_MT7620_ETH) += mt7620-eth.o > obj-$(CONFIG_MT7628_ETH) += mt7628-eth.o > +obj-$(CONFIG_MV88E61XX) += mv88e61xx.o > obj-$(CONFIG_MVGBE) += mvgbe.o > obj-$(CONFIG_MVMDIO) += mvmdio.o > obj-$(CONFIG_MVNETA) += mvneta.o > diff --git a/drivers/net/mv88e61xx.c b/drivers/net/mv88e61xx.c > new file mode 100644 > index ..514835bf03b9 > --- /dev/null > +++ b/drivers/net/mv88e61xx.c > @@ -0,0 +1,843 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2022 > + * Gateworks Corporation <http://www.gateworks.com/> > + * Tim Harvey > + * > + * (C) Copyright 2015 > + * Elecsys Corporation <http://www.elecsyscorp.com/> > + * Kevin Smith > + * > + * Original driver: > + * (C) Copyright 2009 > + * Marvell Semiconductor <http://www.marvell.com/> > + * Prafulla Wadaskar > + */ > + > +/* > + * DSA driver for mv88e61xx ethernet switches. > + * > + * This driver configures the mv88e61xx for basic use as a DSA switch. > + * > + * This driver was adapted from drivers/net/phy/mv88e61xx and tested > + * on the mv88e6176 via an SGMII interface. > + */ > + > +#include > +#include > +#include Alphabetic ordering please. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* Device addresses */ > +#define DEVADDR_PHY(p) (p) > +#define DEVADDR_SERDES 0x0F > + > +/* SMI indirection registers for multichip addressing mode */ > +#define SMI_CMD_REG 0x00 > +#define SMI_DATA_REG 0x01 > + > +/* Global registers */ > +#define GLOBAL1_STATUS 0x00 > +#define GLOBAL1_CTRL 0x04 > +#define GLOBAL1_MON_CTRL 0x1A > + > +/* Global 2 registers */ > +#define GLOBAL2_REG_PHY_CMD 0x18 > +#define GLOBAL2_REG_PHY_DATA 0x19 > +#define GLOBAL2_REG_SCRATCH 0x1A > + > +/* Port registers */ > +#define PORT_REG_STATUS 0x00 > +#define PORT_REG_PHYS_CTRL 0x01 > +#define PORT_REG_SWITCH_ID 0x03 > +#define PORT_REG_CTRL0x04 > +#define PORT_REG_VLAN_MAP0x06 > +#define PORT_REG_VL
Re: [PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Mon, May 23, 2022 at 11:25:49AM -0700, Tim Harvey wrote:
> Add MV88E61XX DSA support:
> - update dt: U-Boot dsa driver requires different device-tree syntax
>than the linux driver in order to link the dsa ports to the mdio bus.
> - update defconfig
> - replace mv88e61xx_hw_reset weak override with board_phy_config support
>for mv88e61xx configuration that is outside the scope of the DSA driver
>
> Signed-off-by: Tim Harvey
> ---
> v3:
> - move mdio's mdio@0 subnode
> v2: no changes
> ---
> arch/arm/dts/imx6qdl-gw5904.dtsi| 41
> board/gateworks/gw_ventana/gw_ventana.c | 50 +
> configs/gwventana_gw5904_defconfig | 7 ++--
> 3 files changed, 62 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 286c7a9924c2..1b2f70d1ccb2 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -219,6 +219,33 @@
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> + mdios {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mdio@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sw_phy0: ethernet-phy@0 {
> + reg = <0x0>;
> + };
> +
> + sw_phy1: ethernet-phy@1 {
> + reg = <0x1>;
> + };
> +
> + sw_phy2: ethernet-phy@2 {
> + reg = <0x2>;
> + };
> +
> + sw_phy3: ethernet-phy@3 {
> + reg = <0x3>;
> + };
> + };
> + };
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -226,27 +253,41 @@
> port@0 {
> reg = <0>;
> label = "lan4";
> + phy-mode = "internal";
> + phy-handle = <_phy0>;
> };
>
> port@1 {
> reg = <1>;
> label = "lan3";
> + phy-mode = "internal";
> + phy-handle = <_phy1>;
> };
>
> port@2 {
> reg = <2>;
> label = "lan2";
> + phy-mode = "internal";
> + phy-handle = <_phy2>;
> };
>
> port@3 {
> reg = <3>;
> label = "lan1";
> + phy-mode = "internal";
> + phy-handle = <_phy3>;
> };
>
> port@5 {
> reg = <5>;
> label = "cpu";
> ethernet = <>;
> + phy-mode = "rgmii-id";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> };
> };
> };
> diff --git a/board/gateworks/gw_ventana/gw_ventana.c
> b/board/gateworks/gw_ventana/gw_ventana.c
> index c06630a66b66..bef3f7ef0d2b 100644
> --- a/board/gateworks/gw_ventana/gw_ventana.c
> +++ b/board/gateworks/gw_ventana/gw_ventana.c
> @@ -68,44 +68,30 @@ int board_phy_config(struct phy_device *phydev)
> phy_write(phydev, MDIO_DEVAD_NONE, 14, val);
> }
>
> + /* Fixed PHY: for GW5904/GW5909 this is Marvell 88E6176 GbE Switch */
> + else if (phydev->phy_id == 0xa5a55a5a &&
PHY_FIXED_ID, but see below.
> + ((board_type == GW5904) || (board_type == GW5909))) {
> + struct mii_dev *bus = miiphy_get_dev_by_name("mdio");
> +
> + puts("MV88E61XX ");
> + /* GPIO[0] output CLK125 for RGMII_REFCLK */
> +
Re: [PATCH v3 8/8] board: gw_ventana: enable MV88E61XX DSA support
Hi Tim, On Tue, Jun 14, 2022 at 10:00:57AM -0700, Tim Harvey wrote: > Vladimir, > > I'm not sure if you've had a chance to look at this latest patch > revision yet. I believe above is what you were describing as the > correct way to do this (without modifying the Linux driver and > improving bindings) > > Best Regards, > > Tim I'm currently on vacation but I'm slowly working through my inbox. This is certainly one of the reasonable ways of doing things, but it will require modifying the mv88e6xxx Linux driver to support an "mdios" subnode (something which nobody should object against, I believe). I'll follow up with more comments when I get to these patches.
Re: [PATCH] net: phy: Remove inline definitions from convinience functions
On Sun, Jun 05, 2022 at 03:44:15AM +0300, Ramon Fried wrote: > The convinience functions are not that small and they caused convenience > bloated text segments because of their usage. > There was no need to inline them in the first place, as > they're not part of a fastpath. > > Signed-off-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH] phy: dp83867: add dp83867_{read,write}_mmd helpers
On Tue, May 24, 2022 at 01:31:41PM +0200, Rasmus Villemoes wrote:
> On 19/05/2022 16.38, Vladimir Oltean wrote:
> > Hi Rasmus,
> >
> > On Tue, May 17, 2022 at 04:27:06PM +0200, Rasmus Villemoes wrote:
> >> Since the phy_{read,write}_mmd functions are static inlines using
> >> other static inline functions, they cause code using them to explode.
> >>
> >> Defining local wrappers cuts the size of the generated code by 50%:
> >>
> >> $ size drivers/net/phy/dp83867.o.{before,after}
> >>textdata bss dec hex filename
> >>4873 112 049851379 drivers/net/phy/dp83867.o.before
> >>2413 112 02525 9dd drivers/net/phy/dp83867.o.after
> >>
> >> Of course, most of that improvement could also be had by making the
> >> phy_*_mmd functions out-of-line, and they probably should be, but this
>
> >> still has the advantage of avoiding passing the DP83867_DEVADDR
> >> argument at all call sites, which allows lines to be unwrapped (and
> >> probably also gives a little .text reduction by itself).
> >>
> >> Signed-off-by: Rasmus Villemoes
> >> ---
> >
> > Have you considered making phy_read_mmd() and phy_write_mmd() non-inline?
> > There are few users, but it looks like they would all benefit from this.
>
> Yes, I wrote precisely that in the commit message. But the problem with
> that is finding some TU to put them in which is guaranteed to be built
> and included by all current users. This localized change gives just
> about the same benefit in .text, plus the line unwrapping. And nothing
> prevents somebody later from figuring out a proper place to put
> out-of-line versions of these phy_*_mmd functions.
I don't see why the translation unit you mention cannot be
drivers/net/phy/phy.c.
For phy_read_mmd() and phy_write_mmd() this is even pretty easy to see,
as all the callers are either PHY drivers or cmd/mdio.c which itself
depends on CONFIG_PHYLIB.
The phy_read() and phy_write() calls themselves can also be probably be
uninlined as a further exercise, but I didn't request that.
But yeah, ok, whatever.
Re: [PATCH] phy: dp83867: add dp83867_{read,write}_mmd helpers
Hi Rasmus,
On Tue, May 17, 2022 at 04:27:06PM +0200, Rasmus Villemoes wrote:
> Since the phy_{read,write}_mmd functions are static inlines using
> other static inline functions, they cause code using them to explode.
>
> Defining local wrappers cuts the size of the generated code by 50%:
>
> $ size drivers/net/phy/dp83867.o.{before,after}
>textdata bss dec hex filename
>4873 112 049851379 drivers/net/phy/dp83867.o.before
>2413 112 02525 9dd drivers/net/phy/dp83867.o.after
>
> Of course, most of that improvement could also be had by making the
> phy_*_mmd functions out-of-line, and they probably should be, but this
> still has the advantage of avoiding passing the DP83867_DEVADDR
> argument at all call sites, which allows lines to be unwrapped (and
> probably also gives a little .text reduction by itself).
>
> Signed-off-by: Rasmus Villemoes
> ---
Have you considered making phy_read_mmd() and phy_write_mmd() non-inline?
There are few users, but it looks like they would all benefit from this.
Re: [PATCH 8/8] board: gw_ventana: enable MV88E61XX DSA support
Hi Tim,
On Wed, May 18, 2022 at 09:15:26AM -0700, Tim Harvey wrote:
> On Wed, May 18, 2022 at 7:41 AM Vladimir Oltean
> wrote:
> >
> > On Wed, May 11, 2022 at 05:20:03PM -0700, Tim Harvey wrote:
> > > Add MV88E61XX DSA support:
> > > - update dt: U-Boot dsa driver requires different device-tree syntax
> > >than the linux driver in order to link the dsa ports to the mdio bus.
> > > - update defconfig
> > > - replace mv88e61xx_hw_reset weak override with board_phy_config support
> > >for mv88e61xx configuration that is outside the scope of the DSA driver
> > >
> > > Signed-off-by: Tim Harvey
> > > ---
> > > v2: no changes
> > > ---
> > > arch/arm/dts/imx6qdl-gw5904.dtsi| 35 +
> > > board/gateworks/gw_ventana/gw_ventana.c | 50 +
> > > configs/gwventana_gw5904_defconfig | 7 ++--
> > > 3 files changed, 56 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > index 286c7a9924c2..63590a2debc7 100644
> > > --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> > > @@ -219,6 +219,27 @@
> > > compatible = "marvell,mv88e6085";
> > > reg = <0>;
> > >
> > > + mdios {
> >
> > I'm thinking either "mdios" is a container node for other "mdio" nodes,
> > or you have the "mdio" node directly. Are you sure you want this
> > structure? We might have problems if we pass this DT as-is up to Linux.
> > Whereas the support for an "mdios" subnode could be upstreamed, I think.
> > The current DT bindings document says:
> >
> > - mdio : Container of PHY and devices on the switches MDIO
> > bus.
> > - mdio? : Container of PHYs and devices on the external MDIO
> > bus. The node must contains a compatible string of
> > "marvell,mv88e6xxx-mdio-external"
> >
> > You have an "mdios" which theoretically matches "mdio?" but only
> > unintendedly so, since it is the internal MDIO bus and not the external
> > one.
> >
> > Easiest way out right now is to not introduce something which isn't
> > parsed by the current Linux driver. So the internal MDIO bus would be
> > named "mdio", and the external one would be named whatever, and would be
> > identified by compatible string.
> >
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + sw_phy0: ethernet-phy@0 {
> > > + reg = <0x0>;
> > > + };
> > > +
> > > + sw_phy1: ethernet-phy@1 {
> > > + reg = <0x1>;
> > > + };
> > > +
> > > + sw_phy2: ethernet-phy@2 {
> > > + reg = <0x2>;
> > > + };
> > > +
> > > + sw_phy3: ethernet-phy@3 {
> > > + reg = <0x3>;
> > > + };
> > > + };
> > > +
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
>
> Vladimir,
>
> I'm not sure I follow your suggestion correctly. I agree this is not
> ideal and not at all what I wanted. The dt prior to this patch is what
> exists and works in Linux but U-Boot dsa (more correctly DM_MDIO
> dm_eth_phy_connect()) requires the phy-mode and phy-handle in the
> ports and I'm not clear how to define the mdio's for those to point to
> here.
>
> Recall I did the same thing for gw7901 in commit 1cb87b929e1e ("arm:
> dts: imx8mm-venice-gw7901.dts: fix dsa switch configuration") which I
> didn't feel was correct either as the previous dt there is what is in
> Linux and works.
>
> Can you give me an example?
My considerations are the following.
The Linux Documentation/devicetree/bindings/net/mdio.yaml file says that
the following are valid node names for an MDI
Re: [PATCH] phy: Fix phy_string_for_interface() function
On Thu, May 19, 2022 at 02:49:12PM +0200, Pali Rohár wrote:
> Commit c677fb1e3196 ("phy: Move PHY_INTERFACE_MODE_NA to the beginning of
> the enum definition") broke function phy_string_for_interface(). And
> therefore completely broke support for 2500base-x mode in Armada 3720
> comphy driver.
>
> Since that commit function phy_string_for_interface() returns constant
> value PHY_INTERFACE_MODE_NA because PHY_INTERFACE_MODE_NA from moved from
> end to the beginning.
>
> Previous value of PHY_INTERFACE_MODE_NA was PHY_INTERFACE_MODE_MAX-1. So
> change phy_string_for_interface() code to check upper bound via previous
> value.
>
> This patch fixes 2500base-x mode on Armada 3720
>
> Fixes: c677fb1e3196 ("phy: Move PHY_INTERFACE_MODE_NA to the beginning of the
> enum definition")
> Signed-off-by: Pali Rohár
> ---
Ramon, can you please apply this patch, since people are hitting the issue?
https://patchwork.ozlabs.org/project/uboot/patch/20220510224910.12553-1-thar...@gateworks.com/
Re: [PATCH 8/8] board: gw_ventana: enable MV88E61XX DSA support
On Wed, May 11, 2022 at 05:20:03PM -0700, Tim Harvey wrote:
> Add MV88E61XX DSA support:
> - update dt: U-Boot dsa driver requires different device-tree syntax
>than the linux driver in order to link the dsa ports to the mdio bus.
> - update defconfig
> - replace mv88e61xx_hw_reset weak override with board_phy_config support
>for mv88e61xx configuration that is outside the scope of the DSA driver
>
> Signed-off-by: Tim Harvey
> ---
> v2: no changes
> ---
> arch/arm/dts/imx6qdl-gw5904.dtsi| 35 +
> board/gateworks/gw_ventana/gw_ventana.c | 50 +
> configs/gwventana_gw5904_defconfig | 7 ++--
> 3 files changed, 56 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-gw5904.dtsi
> b/arch/arm/dts/imx6qdl-gw5904.dtsi
> index 286c7a9924c2..63590a2debc7 100644
> --- a/arch/arm/dts/imx6qdl-gw5904.dtsi
> +++ b/arch/arm/dts/imx6qdl-gw5904.dtsi
> @@ -219,6 +219,27 @@
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> + mdios {
I'm thinking either "mdios" is a container node for other "mdio" nodes,
or you have the "mdio" node directly. Are you sure you want this
structure? We might have problems if we pass this DT as-is up to Linux.
Whereas the support for an "mdios" subnode could be upstreamed, I think.
The current DT bindings document says:
- mdio : Container of PHY and devices on the switches MDIO
bus.
- mdio? : Container of PHYs and devices on the external MDIO
bus. The node must contains a compatible string of
"marvell,mv88e6xxx-mdio-external"
You have an "mdios" which theoretically matches "mdio?" but only
unintendedly so, since it is the internal MDIO bus and not the external
one.
Easiest way out right now is to not introduce something which isn't
parsed by the current Linux driver. So the internal MDIO bus would be
named "mdio", and the external one would be named whatever, and would be
identified by compatible string.
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sw_phy0: ethernet-phy@0 {
> + reg = <0x0>;
> + };
> +
> + sw_phy1: ethernet-phy@1 {
> + reg = <0x1>;
> + };
> +
> + sw_phy2: ethernet-phy@2 {
> + reg = <0x2>;
> + };
> +
> + sw_phy3: ethernet-phy@3 {
> + reg = <0x3>;
> + };
> + };
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
Re: [PATCH 7/8] net: add MV88E61xx DSA driver
On Wed, May 11, 2022 at 05:20:02PM -0700, Tim Harvey wrote: > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches. > > Cc: Marek BehĂșn > Cc: Vladimir Oltean > Signed-off-by: Tim Harvey > --- > v2: > - rebase on v2022.07-rc1 (use ofnode_get_phy_node) > - remove unused commented out fields from struct > - remove unused PORT_MASK macro > - remove phy from priv struct name > - refactor code from original drivers/net/phy/mv88e61xx with >suggestions from review to consolidate some functions >into mv88e61xx_dsa_port_enable > - remove unecessary skiping of disabling of CPU port > - remove unecessary dev_set_parent_priv > - remove unnecessary static init flag > - replace debug with a dev_warn if switch device-id unsupported > - remove unnecessary xmit/recv functions as we rely on the fact that >only a single port is active instead of mangling packets > --- Some may prefer the driver be named mv88e6xxx, for more perceived similarity with Linux. That can be done later anyway. Thanks for the submission, IMO it looks good enough and is certainly a great start. Reviewed-by: Vladimir Oltean
Re: [PATCH 3/8] net: dsa: ensure dsa driver has proper ops
On Wed, May 11, 2022 at 05:19:58PM -0700, Tim Harvey wrote: > Add a function to sanity check a dsa driver having proper ops. > > Suggested-by: Vladimir Oltean > Signed-off-by: Tim Harvey > --- Reviewed-by: Vladimir Oltean
Re: [PATCH 4/8] net: dsa: allow rcv() and xmit() to be optional
On Wed, May 11, 2022 at 05:19:59PM -0700, Tim Harvey wrote: > Allow rcv() and xmit() dsa driver ops to be optional in case a driver > does not care to mangle a packet as in U-Boot only one network port is > enabled at a time and thus no packet mangling is necessary. > > Suggested-by: Vladimir Oltean > Signed-off-by: Tim Harvey > --- Reviewed-by: Vladimir Oltean
Re: [PATCH 5/8] net: ksz9477: remove unnecessary xmit and recv functions
On Wed, May 11, 2022 at 05:20:00PM -0700, Tim Harvey wrote: > Remove the unnecessary xmit and recv functions. > > Signed-off-by: Tim Harvey > --- Reviewed-by: Vladimir Oltean
Re: [PATCH 2/8] net: dsa: move cpu port probe to dsa_post_probe
On Wed, May 11, 2022 at 05:19:57PM -0700, Tim Harvey wrote: > In order to ensure that a DSA driver probe gets called before > dsa_ops->port_probe move the port_probe of the cpu_port to > a post-probe function. > > Signed-off-by: Tim Harvey > Reviewed-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH v4 07/16] net: dsa: Fix segmentation fault if master fails to probe
On Thu, May 05, 2022 at 01:11:36PM -0400, Sean Anderson wrote:
> If the DSA master fails to probe for whatever reason, then DSA devices
> will continue on as if nothing is wrong. This can cause incorrect
> behavior. In particular, on sandbox, dsa_sandbox_probe attempts to
> access the master's private data. This is only safe to do if the master
> has been probed first. Fix this by probing the master after we look it
> up, and bailing out if we get an error.
>
> Fixes: fc054d563b ("net: Introduce DSA class for Ethernet switches")
> Signed-off-by: Sean Anderson
> ---
Reviewed-by: Vladimir Oltean
Re: [PATCH] ls1028a: hdp: Add config support for HDP firmware loading
On Tue, May 10, 2022 at 04:18:32PM +0200, Michael Walle wrote: > > From: Alison Wang > > > > This patch adds config support for HDP firmware loading on LS1028A. > > FWIW, I really don't like this as this is just for a downstream kernel. This => what? I only see a config option. I searched the list and the source code for CONFIG_VIDEO_LS_HDP_LOAD and I don't see any user... > For the upstream kernel the firmware should be loaded by the kernel itself. In principle I agree. > But unfortunately, there is no development there. (I have some half > baked patches though and can confirm, that firmware loading is working > from linux). Instead of this I'd like to see some work to support the > DP PHY in upstream linux as this is really the last missing piece to > get graphics working. It is? What did I miss? > Now this is purely board related code, so it is probably fine. But in the > past the handling of purly downstream stuff was removed from upstream > u-boot anyway in favor of the upstream method. i.e. fixup of the > ls1028a-gpu node. > > -michael
Re: [PATCH 5/6] net: add MV88E61xx DSA driver
On Tue, Mar 29, 2022 at 03:52:39PM -0700, Tim Harvey wrote: > Add a DSA driver for the MV88E61xx compatible GbE Ethernet switches. > > Signed-off-by: Tim Harvey > --- > drivers/net/Kconfig | 7 + > drivers/net/Makefile| 1 + > drivers/net/mv88e61xx.c | 982 > 3 files changed, 990 insertions(+) > create mode 100644 drivers/net/mv88e61xx.c > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index a6171a7c7ffd..fc018f5ba47f 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -428,6 +428,13 @@ config LPC32XX_ETH > depends on ARCH_LPC32XX > default y > > +config MV88E61XX > + bool "Marvell MV88E61xx GbE switch DSA driver" > + depends on DM_DSA && DM_MDIO > + help > + This driver implements a DSA switch driver for the MV88E61xx family > + of GbE switches using the MDIO interface > + > config MVGBE > bool "Marvell Orion5x/Kirkwood network interface support" > depends on ARCH_KIRKWOOD || ARCH_ORION5X > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index a6d0c23f02d3..11ada73658e9 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -66,6 +66,7 @@ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o > obj-$(CONFIG_MPC8XX_FEC) += mpc8xx_fec.o > obj-$(CONFIG_MT7620_ETH) += mt7620-eth.o > obj-$(CONFIG_MT7628_ETH) += mt7628-eth.o > +obj-$(CONFIG_MV88E61XX) += mv88e61xx.o > obj-$(CONFIG_MVGBE) += mvgbe.o > obj-$(CONFIG_MVMDIO) += mvmdio.o > obj-$(CONFIG_MVNETA) += mvneta.o > diff --git a/drivers/net/mv88e61xx.c b/drivers/net/mv88e61xx.c > new file mode 100644 > index ..9dd7a0c7f42e > --- /dev/null > +++ b/drivers/net/mv88e61xx.c > @@ -0,0 +1,982 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2022 > + * Gateworks Corporation > + * Tim Harvey > + * > + * (C) Copyright 2015 > + * Elecsys Corporation > + * Kevin Smith > + * > + * Original driver: > + * (C) Copyright 2009 > + * Marvell Semiconductor > + * Prafulla Wadaskar > + */ > + > +/* > + * DSA driver for mv88e61xx ethernet switches. > + * > + * This driver configures the mv88e61xx for basic use as a DSA switch. > + * > + * This driver was adapted from drivers/net/phy/mv88e61xx and tested > + * on the mv88e6176 via an SGMII interface. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PORT_MASK(port_count)((1 << (port_count)) - 1) Not used. > + > +/* Device addresses */ > +#define DEVADDR_PHY(p) (p) > +#define DEVADDR_SERDES 0x0F > + > +/* SMI indirection registers for multichip addressing mode */ > +#define SMI_CMD_REG 0x00 > +#define SMI_DATA_REG 0x01 > + > +/* Global registers */ > +#define GLOBAL1_STATUS 0x00 > +#define GLOBAL1_CTRL 0x04 > +#define GLOBAL1_MON_CTRL 0x1A > + > +/* Global 2 registers */ > +#define GLOBAL2_REG_PHY_CMD 0x18 > +#define GLOBAL2_REG_PHY_DATA 0x19 > +#define GLOBAL2_REG_SCRATCH 0x1A > + > +/* Port registers */ > +#define PORT_REG_STATUS 0x00 > +#define PORT_REG_PHYS_CTRL 0x01 > +#define PORT_REG_SWITCH_ID 0x03 > +#define PORT_REG_CTRL0x04 > +#define PORT_REG_VLAN_MAP0x06 > +#define PORT_REG_VLAN_ID 0x07 > +#define PORT_REG_LED_CTRL0x16 > + > +/* Phy registers */ > +#define PHY_REG_CTRL10x10 > +#define PHY_REG_STATUS1 0x11 > +#define PHY_REG_PAGE 0x16 > + > +/* Serdes registers */ > +#define SERDES_REG_CTRL_10x10 > + > +/* Phy page numbers */ > +#define PHY_PAGE_COPPER 0 > +#define PHY_PAGE_SERDES 1 > + > +/* Register fields */ > +#define GLOBAL1_CTRL_SWRESET BIT(15) > + > +#define GLOBAL1_MON_CTRL_CPUDEST_SHIFT 4 > +#define GLOBAL1_MON_CTRL_CPUDEST_WIDTH 4 > + > +#define PORT_REG_STATUS_SPEED_SHIFT 8 > +#define PORT_REG_STATUS_SPEED_10 0 > +#define PORT_REG_STATUS_SPEED_1001 > +#define PORT_REG_STATUS_SPEED_1000 2 > + > +#define PORT_REG_STATUS_CMODE_MASK 0xF > +#define PORT_REG_STATUS_CMODE_100BASE_X 0x8 > +#define PORT_REG_STATUS_CMODE_1000BASE_X 0x9 > +#define PORT_REG_STATUS_CMODE_SGMII 0xa > + > +#define PORT_REG_PHYS_CTRL_RGMII_DELAY_RXCLK BIT(15) > +#define PORT_REG_PHYS_CTRL_RGMII_DELAY_TXCLK BIT(14) > +#define PORT_REG_PHYS_CTRL_PCS_AN_EN BIT(10) > +#define PORT_REG_PHYS_CTRL_PCS_AN_RSTBIT(9) > +#define PORT_REG_PHYS_CTRL_FC_VALUE BIT(7) > +#define PORT_REG_PHYS_CTRL_FC_FORCE BIT(6) > +#define PORT_REG_PHYS_CTRL_LINK_VALUEBIT(5) > +#define PORT_REG_PHYS_CTRL_LINK_FORCEBIT(4) > +#define PORT_REG_PHYS_CTRL_DUPLEX_VALUE BIT(3) > +#define
Re: [PATCH 2/2] net: phy: atheros: avoid error in ar803x_of_init() when PHY has no OF node
Hi Ramon, On Fri, Apr 01, 2022 at 10:11:13PM +0300, Ramon Fried wrote: > On Thu, Mar 3, 2022 at 9:53 AM Ramon Fried wrote: > > Reviewed-by: Ramon Fried > Applied to u-boot-net/next > Thanks, > Ramon Did you push this commit anywhere? I don't see it here https://source.denx.de/u-boot/custodians/u-boot-net/-/commits/next/
Re: [PATCH 5/6] net: add MV88E61xx DSA driver
On Thu, Apr 07, 2022 at 01:33:58PM -0700, Tim Harvey wrote: > I guess I'll have to invest in tagging packets if you won't accept the > simplistic approach of not having to tag frames knowing that only one > port is active at a time. I genuinely don't know where you got the impression from that I don't accept the simplistic approach. I gave you an option to make the xmit and receive ops actually optional at the DSA uclass level so you don't have to come up with a make-believe tag parsing function. In the end it goes towards the simplification of the Marvell driver. Let's let them battle it out for a while and if tag insertion/parsing won't be necessary even for multi-switch systems we can discuss about removing that logic completely. > That said, I have no idea if or when I will re-visit this. Adding a > DSA version of this driver was something on my personal wish list and > not something that was necessary by any means by my employer so I may > have to just drop it as I don't have the personal time to work through > this part of it or unravelling the mii bus mess in the fec_mxc driver. > It seems to me there is an issue with trying to create DM_MDIO drivers > in general as most dt's I've seen wouldn't support the requirements > yet configure DM_MDIO anyway (meaning if you implemented it you would > break those boards as I found). I don't know why there are boards which set CONFIG_DM_MDIO and then fight against the current trying to survive that config being set. Maybe you can look into disabling that config option on boards that aren't prepared to handle it?
Re: [PATCH u-boot-net v3 14/14] net: phy: don't require PHY interface mode during PHY creation
On Tue, Mar 29, 2022 at 10:08:45PM +0200, Marek Behún wrote: > From: Marek Behún > > Currently we require PHY interface mode to be known when > finding/creating the PHY - the functions > * phy_device_create() > * create_phy_by_mask() > * search_for_existing_phy() > * get_phy_device_by_mask() > * phy_find_by_mask() (this is the only one global) > all require the interface parameter, but the only thing done with it is > that it is assigned to phydev->interface. > > This makes it impossible to find a PHY device without overwriting the > set mode. > > Since the interface mode is not used during .probe() and should be used > at first in .config(), drop the interface parameter from these > functions. Make the default value of phydev->interface (in > phy_device_create()) to be PHY_INTERFACE_MODE_NA. Move the interface > parameter to phy_connect_dev(), where it should be. > > Change all occurrences treewide. In occurrences where we don't call > phy_connect_dev() for some reason (they only configure the PHY without > connecting it to an ethernet controller), set > phydev->interface = value from phy_find_by_mask call. > > Signed-off-by: Marek Behún > Reviewed-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH u-boot-net v3 09/14] treewide: Rename PHY_INTERFACE_MODE_NONE to PHY_INTERFACE_MODE_NA
On Tue, Mar 29, 2022 at 10:08:40PM +0200, Marek Behún wrote: > From: Marek Behún > > Rename constant PHY_INTERFACE_MODE_NONE to PHY_INTERFACE_MODE_NA to make > it compatible with Linux' naming. > > Signed-off-by: Marek Behún > Reviewed-by: Stefan Roese > Reviewed-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH u-boot-net v3 08/14] treewide: Rename PHY_INTERFACE_MODE_COUNT to PHY_INTERFACE_MODE_MAX
On Tue, Mar 29, 2022 at 10:08:39PM +0200, Marek Behún wrote: > From: Marek Behún > > Rename constant PHY_INTERFACE_MODE_COUNT to PHY_INTERFACE_MODE_MAX to > make it compatible with Linux' naming. > > Signed-off-by: Marek Behún > Reviewed-by: Stefan Roese > --- Reviewed-by: Vladimir Oltean
Re: [PATCH u-boot-net v3 06/14] net: phy: fix parsing wrong property
On Tue, Mar 29, 2022 at 10:08:37PM +0200, Marek Behún wrote:
> From: Marek Behún
>
> The "phy-interface-type" property should be "phy-connection-type".
>
> Signed-off-by: Marek Behún
> Reviewed-by: Ramon Fried
> ---
Reviewed-by: Vladimir Oltean
> drivers/net/phy/phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index c9fc20855b..fe6dbdaee4 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -989,7 +989,7 @@ struct phy_device *fixed_phy_create(ofnode node)
>
> if_str = ofnode_read_string(node, "phy-mode");
> if (!if_str) {
> - if_str = ofnode_read_string(node, "phy-interface-type");
> + if_str = ofnode_read_string(node, "phy-connection-type");
> }
> if (if_str) {
> interface = phy_get_interface_by_name(if_str);
> --
> 2.34.1
>
Re: [PATCH u-boot-net v3 05/14] treewide: use dm_mdio_read/write/reset() wrappers
On Tue, Mar 29, 2022 at 10:08:36PM +0200, Marek Behún wrote: > From: Marek Behún > > Use the new dm_mdio_read/write/reset() wrappers treewide, instead of > always getting and dereferencing MDIO operations structure pointer. > > Signed-off-by: Marek Behún > Reviewed-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH u-boot-net v3 04/14] net: mdio-uclass: add wrappers for read/write/reset operations
On Tue, Mar 29, 2022 at 10:08:35PM +0200, Marek Behún wrote: > From: Marek Behún > > Add wrappers dm_mdio_read(), dm_mdio_write() and dm_mdio_reset() for > DM MDIO's .read(), .write() and .reset() operations. > > Signed-off-by: Marek Behún > Reviewed-by: Ramon Fried > --- Reviewed-by: Vladimir Oltean
Re: [PATCH u-boot-net v3 02/14] net: mdio-uclass: use ARRAY_SIZE()
On Tue, Mar 29, 2022 at 10:08:33PM +0200, Marek Behún wrote:
> From: Marek Behún
>
> Use the ARRAY_SIZE() macro instead of hardcoding sizes of arrays in
> macros.
>
> Signed-off-by: Marek Behún
> ---
Reviewed-by: Vladimir Oltean
> net/mdio-uclass.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> index 5735afe49e..649dc60f73 100644
> --- a/net/mdio-uclass.c
> +++ b/net/mdio-uclass.c
> @@ -16,13 +16,12 @@
> #include
>
> /* DT node properties for MAC-PHY interface */
> -#define PHY_MODE_STR_CNT 2
> -static const char * const phy_mode_str[PHY_MODE_STR_CNT] = {
> +static const char * const phy_mode_str[] = {
> "phy-mode", "phy-connection-type"
> };
> +
> /* DT node properties that reference a PHY node */
> -#define PHY_HANDLE_STR_CNT 3
> -static const char * const phy_handle_str[PHY_HANDLE_STR_CNT] = {
> +static const char * const phy_handle_str[] = {
> "phy-handle", "phy", "phy-device"
> };
>
> @@ -149,7 +148,7 @@ static struct phy_device
> *dm_eth_connect_phy_handle(struct udevice *ethdev,
> goto out;
> }
>
> - for (i = 0; i < PHY_HANDLE_STR_CNT; i++)
> + for (i = 0; i < ARRAY_SIZE(phy_handle_str); i++)
> if (!dev_read_phandle_with_args(ethdev, phy_handle_str[i], NULL,
> 0, 0, ))
> break;
> @@ -199,7 +198,7 @@ struct phy_device *dm_eth_phy_connect(struct udevice
> *ethdev)
> }
>
> interface = PHY_INTERFACE_MODE_NONE;
> - for (i = 0; i < PHY_MODE_STR_CNT; i++) {
> + for (i = 0; i < ARRAY_SIZE(phy_mode_str); i++) {
> if_str = dev_read_string(ethdev, phy_mode_str[i]);
> if (if_str) {
> interface = phy_get_interface_by_name(if_str);
> --
> 2.34.1
>
Re: [PATCH u-boot-net v3 01/14] net: mdio-uclass: fix type for phy_mode_str and phy_handle_str
On Tue, Mar 29, 2022 at 10:08:32PM +0200, Marek Behún wrote:
> From: Marek Behún
>
> These global variables should both have type
> static const char * const
>
> Signed-off-by: Marek Behún
> Reviewed-by: Ramon Fried
> ---
Reviewed-by: Vladimir Oltean
> net/mdio-uclass.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> index e74e34f78f..5735afe49e 100644
> --- a/net/mdio-uclass.c
> +++ b/net/mdio-uclass.c
> @@ -17,13 +17,14 @@
>
> /* DT node properties for MAC-PHY interface */
> #define PHY_MODE_STR_CNT 2
> -static const char *phy_mode_str[PHY_MODE_STR_CNT] = { "phy-mode",
> - "phy-connection-type" };
> +static const char * const phy_mode_str[PHY_MODE_STR_CNT] = {
> + "phy-mode", "phy-connection-type"
> +};
> /* DT node properties that reference a PHY node */
> #define PHY_HANDLE_STR_CNT 3
> -const char *phy_handle_str[PHY_HANDLE_STR_CNT] = { "phy-handle",
> -"phy",
> -"phy-device" };
> +static const char * const phy_handle_str[PHY_HANDLE_STR_CNT] = {
> + "phy-handle", "phy", "phy-device"
> +};
>
> void dm_mdio_probe_devices(void)
> {
> --
> 2.34.1
>
Re: [PATCH 5/6] net: add MV88E61xx DSA driver
On Fri, Apr 01, 2022 at 01:24:48PM -0700, Tim Harvey wrote:
> > > > Why is mv88e61xx_dsa_xmit() no-op?
> > >
> > > For DSA dsa-uclass calls the switch master eth device send function
> > > after calling the dsa_ops->xmit function so that a dsa driver can add
> > > any header/footer if needed. The function is required but in my case I
> > > don't care about header/footer tagging or vlan as only 1 port is
> > > active at a time in U-Boot so I just return success.
> >
> > So if I make one port active, the other are completely disabled? They
> > won't even switch? Is that how DSA uclass is supposed to work in U-Boot?
> >
> > I would think that it should be somehow configurable instead.
>
> Marek,
>
> I'll let Vladimir correct me if I'm wrong but my understanding is DSA
> in U-Boot is not intended to allow switches to forward packets on
> their own from port to port but instead just for forwarding packets
> between the active port and the MAC connected to the CPU (at least
> that's what I intended when I wrote the ksz9477 dsa driver
> previously).
>
> In my opinion what a DSA driver provides is avoidance of putting
> switches in forwarding mode having the potential of easily creating
> bridge loops. With the existing mv88e61xx driver I've had users create
> bridge loops often.
DM_DSA follows the guiding principle that the user doesn't care about
the switch more than being able to transfer a boot image through TFTP
without causing switching loops in the network. The way this is achieved
is by following the "port extender" model which is exactly the way in
which Linux DSA was first introduced, prior to having support for L2
bridging offloads via switchdev. For U-Boot, packet forwarding across
front ports is effectively a non-goal. In fact, what we want to avoid is
the proliferation of bloatware such as cmd/ethsw.c, which can and will
be abused to preconfigure a switch from the bootloader so you won't have
to manage it from Linux.
Tim, I accept that U-Boot's Ethernet usage model (a single device active
at a time, with RX in synchronous poll mode) simplifies DM_DSA to the
point that creating/parsing DSA tags (which serve the purpose of
multiplexing/demultiplexing packets to/from switch ports) on the CPU is
more or less redundant when said multiplexing can be achieved on a time
basis by disabling all the ports except the required source/destination
switch port and the CPU port. This holds true even when DM_DSA gains
support for multi-switch setups.
What I would like, however, is to avoid driver code putting pressure on
the DSA uclass code. I don't really like the tracking of the priv->active_port
as being the port on which ->port_enable was last called. The only reason
this is done is to appease the "port_index != port_pdata->index" check
from dsa_port_recv(). In turn, this is because the ->xmit() and ->rcv()
DSA ops are not optional. Let's make them optional instead, instead of
adding code to a C file that indirectly depends on the code structure
from another C file.
But ->port_enable() and ->port_disable() are optional too, and that's
not okay, because if the driver doesn't provide a way to enable/disable
ports, packets can flow anywhere.
I suggest making it clear to driver writers that it has to be one or the
other by sanitizing the ops:
diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 9ff55a02fb23..ea860fa715bb 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -342,6 +342,19 @@ U_BOOT_DRIVER(dsa_port) = {
.plat_auto = sizeof(struct eth_pdata),
};
+static int dsa_sanitize_ops(struct udevice *dev)
+{
+ struct dsa_ops *ops = dsa_get_ops(dev);
+
+ if ((!ops->xmit || !ops->rcv) &&
+ (!ops->port_enable && !ops->port_disable)) {
+ dev_err(dev, "Packets cannot be steered to ports\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/*
* This function mostly deals with pulling information out of the device tree
* into the pdata structure.
@@ -358,6 +371,10 @@ static int dsa_post_bind(struct udevice *dev)
if (!ofnode_valid(node))
return -ENODEV;
+ err = dsa_sanitize_ops(dev);
+ if (err)
+ return err;
+
pdata->master_node = ofnode_null();
node = ofnode_find_subnode(node, "ports");
Then we can properly make rcv() and xmit() optional at the level of the
DSA uclass. This way we avoid the tail chasing approach of pretending
that "return priv->active_port" _is_ the way to decode a packet for a
certain driver. It isn't, and it should just be DSA's fallback if no
such method is known. I'd like to preserve the option to implement a
rcv() and xmit() procedure, because it offers one extra layer of safety
that the CPU is really talking to the switch port it thinks it's
talking, plus inter-switch traffic is reduced in cross-chip topologies
(which are not supported now, but maybe they will be).
I think that fallback could look like this:
diff --git a/net/dsa-uclass.c
Re: [PATCH 4/6] net: fec: add support for DM_MDIO
On Fri, Apr 01, 2022 at 10:53:14AM -0700, Tim Harvey wrote: > Can you review 'net: add MV88E61xx DSA driver' for me? I will. I've been thinking all day today about what to say that isn't stupid. Give me some time and I'll provide feedback.
Re: [PATCH 4/6] net: fec: add support for DM_MDIO
On Thu, Mar 31, 2022 at 10:48:55AM -0700, Tim Harvey wrote:
> > On which branch does this apply? The context above fecmxc_read_rom_hwaddr()
> > is different in the branches I've checked:
> > https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1276
> > https://source.denx.de/u-boot/custodians/u-boot-net/-/blob/next/drivers/net/fec_mxc.c#L1276
> >
>
> Sorry, I should have specified in my cover letter that this is on top
> of next which removes the non dm-eth support from the driver which is
> quite the cleanup.
Ok, but now the next patch fails to apply to the 'next' branch:
Applying: net: dsa: move cpu port probe to dsa_post_probe
Applying: net: mdio-uclass: add wrappers for read/write/reset operations
Applying: net: fec: add support for DM_MDIO
Applying: net: add MV88E61xx DSA driver
error: patch failed: drivers/net/Kconfig:428
error: drivers/net/Kconfig: patch does not apply
error: patch failed: drivers/net/Makefile:66
error: drivers/net/Makefile: patch does not apply
Patch failed at 0005 net: add MV88E61xx DSA driver
> Also, after more testing I believe the dm-mdio driver code 'above' is
> correct but the business of trying to use it and fallback 'below' is
> wrong and needs work. It doesn't break current users of fec_mxc from
> what I can tell but it also doesn't properly connect the dm_mdio
> driver.
Can you spell out what is wrong about the fallback logic?
The whole thing with eth_phy_get_mdio_bus()/eth_phy_set_mdio_bus() has
me so confused that I am not really following along anymore.
> > > static int fecmxc_read_rom_hwaddr(struct udevice *dev)
> > > {
> > > struct fec_priv *priv = dev_get_priv(dev);
> > > @@ -1088,7 +1164,7 @@ static int device_get_phy_addr(struct fec_priv
> > > *priv, struct udevice *dev)
> > >
> > > static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
> > > {
> > > - struct phy_device *phydev;
> > > + struct phy_device *phydev = NULL;
> > > int addr;
> > >
> > > addr = device_get_phy_addr(priv, dev);
> > > @@ -1096,7 +1172,12 @@ static int fec_phy_init(struct fec_priv *priv,
> > > struct udevice *dev)
> > > addr = CONFIG_FEC_MXC_PHYADDR;
> > > #endif
> > >
> > > - phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> > > +#ifdef CONFIG_DM_MDIO
> > > + if (priv->dm_mdio)
> > > + phydev = dm_eth_phy_connect(dev);
> > > +#endif
> > > + if (!phydev)
> > > + phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> > > if (!phydev)
> > > return -ENODEV;
> > >
> > > @@ -1227,11 +1308,19 @@ static int fecmxc_probe(struct udevice *dev)
> > >
> > > priv->dev_id = dev_seq(dev);
> > >
> > > +#ifdef CONFIG_DM_MDIO
> > > + ret = dm_fec_bind_mdio(dev);
> > > + if (!ret) {
> > > + ret = fec_phy_init(priv, dev);
> > > + if (!ret)
> > > + priv->dm_mdio = true;
> > > + }
> > > +#endif
> > > #ifdef CONFIG_DM_ETH_PHY
> > > bus = eth_phy_get_mdio_bus(dev);
> > > #endif
> > >
> > > - if (!bus) {
> > > + if (!bus && !priv->dm_mdio) {
> > > dm_mii_bus = false;
> > > #ifdef CONFIG_FEC_MXC_MDIO_BASE
> > > bus = fec_get_miibus((ulong)CONFIG_FEC_MXC_MDIO_BASE,
> > > @@ -1240,7 +1329,7 @@ static int fecmxc_probe(struct udevice *dev)
> > > bus = fec_get_miibus((ulong)priv->eth, dev_seq(dev));
> > > #endif
> > > }
> > > - if (!bus) {
> > > + if (!bus && !priv->dm_mdio) {
> > > ret = -ENOMEM;
> > > goto err_mii;
> > > }
> > > @@ -1271,14 +1360,16 @@ static int fecmxc_probe(struct udevice *dev)
> > > break;
> > > }
> > >
> > > - ret = fec_phy_init(priv, dev);
> > > - if (ret)
> > > - goto err_phy;
> > > + if (!priv->dm_mdio) {
> > > + ret = fec_phy_init(priv, dev);
> > > + if (ret)
> > > + goto err_phy;
> > > + }
> > >
> > > return 0;
> > >
> > > err_phy:
> > > - if (!dm_mii_bus) {
> > > + if (!dm_mii_bus && !priv->dm_mdio) {
> > > mdio_unregister(bus);
> > > free(bus);
> > > }
> > > @@ -1294,8 +1385,10 @@ static int fecmxc_remove(struct udevice *dev)
> > >
> > > free(priv->phydev);
> > > fec_free_descs(priv);
> > > - mdio_unregister(priv->bus);
> > > - mdio_free(priv->bus);
> > > + if (priv->bus) {
> > > + mdio_unregister(priv->bus);
> > > + mdio_free(priv->bus);
> > > + }
> > >
> > > #ifdef CONFIG_DM_REGULATOR
> > > if (priv->phy_supply)
> > > diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h
> > > index 48faa33d66ec..c297880ccc54 100644
> > > --- a/drivers/net/fec_mxc.h
> > > +++ b/drivers/net/fec_mxc.h
> > > @@ -248,6 +248,7 @@ struct fec_priv {
> > > uint8_t *tdb_ptr;
> > > int dev_id;
> > > struct mii_dev *bus;
> > > + bool dm_mdio;
> > > #ifdef CONFIG_PHYLIB
> >
Re: [PATCH 4/6] net: fec: add support for DM_MDIO
On Tue, Mar 29, 2022 at 03:52:38PM -0700, Tim Harvey wrote:
> Add support for DM_MDIO by registering a UCLASS_MDIO driver and
> attempting to use it. This is necessary if wanting to use a DSA
> driver for example hanging off of the FEC MAC.
>
> Care is taken to fallback to non DM_MDIO as several boards define
> DM_MDIO without having the proper device-tree configuration necessary
> such as an mdio subnode, a phy-mode prop, and either a valid phy-handle
> prop or fixed-phy subnode.
>
> Signed-off-by: Tim Harvey
> ---
> drivers/net/fec_mxc.c | 113 ++
> drivers/net/fec_mxc.h | 1 +
> 2 files changed, 104 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index e8ebef09032a..374ef9d67d5e 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -30,6 +30,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "fec_mxc.h"
> #include
> @@ -1025,6 +1027,80 @@ struct mii_dev *fec_get_miibus(ulong base_addr, int
> dev_id)
> return bus;
> }
>
> +#ifdef CONFIG_DM_MDIO
> +struct dm_fec_mdio_priv {
> + struct ethernet_regs *regs;
> +};
> +
> +static int dm_fec_mdio_read(struct udevice *dev, int addr, int devad, int
> reg)
> +{
> + struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> + return fec_mdio_read(priv->regs, addr, reg);
> +}
> +
> +static int dm_fec_mdio_write(struct udevice *dev, int addr, int devad, int
> reg, u16 data)
> +{
> + struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> + return fec_mdio_write(priv->regs, addr, reg, data);
> +}
> +
> +static const struct mdio_ops dm_fec_mdio_ops = {
> + .read = dm_fec_mdio_read,
> + .write = dm_fec_mdio_write,
> +};
> +
> +static int dm_fec_mdio_probe(struct udevice *dev)
> +{
> + struct dm_fec_mdio_priv *priv = dev_get_priv(dev);
> +
> + priv->regs = (struct ethernet_regs
> *)ofnode_get_addr(dev_ofnode(dev->parent));
> +
> + return 0;
> +}
> +
> +U_BOOT_DRIVER(fec_mdio) = {
> + .name = "fec_mdio",
> + .id = UCLASS_MDIO,
> + .probe = dm_fec_mdio_probe,
> + .ops= _fec_mdio_ops,
> + .priv_auto = sizeof(struct dm_fec_mdio_priv),
> +};
> +
> +static int dm_fec_bind_mdio(struct udevice *dev)
> +{
> + struct udevice *mdiodev;
> + const char *name;
> + ofnode mdio;
> + int ret;
> +
> + /* for a UCLASS_MDIO driver we need to bind and probe manually
> + * for an internal MDIO bus that has no dt compatible of its own
> + */
> + ofnode_for_each_subnode(mdio, dev_ofnode(dev)) {
> + name = ofnode_get_name(mdio);
> +
> + if (strcmp(name, "mdio"))
> + continue;
> +
> + ret = device_bind_driver_to_node(dev, "fec_mdio",
> + name, mdio, );
> + if (ret)
> + return ret;
> +
> + /* need to probe it as there is no compatible to do so */
> + ret = uclass_get_device_by_ofnode(UCLASS_MDIO, mdio, );
> + if (!ret)
> + return 0;
> +
> + break;
> + }
> +
> + return ret;
> +}
> +#endif
> +
On which branch does this apply? The context above fecmxc_read_rom_hwaddr()
is different in the branches I've checked:
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L1276
https://source.denx.de/u-boot/custodians/u-boot-net/-/blob/next/drivers/net/fec_mxc.c#L1276
> static int fecmxc_read_rom_hwaddr(struct udevice *dev)
> {
> struct fec_priv *priv = dev_get_priv(dev);
> @@ -1088,7 +1164,7 @@ static int device_get_phy_addr(struct fec_priv *priv,
> struct udevice *dev)
>
> static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
> {
> - struct phy_device *phydev;
> + struct phy_device *phydev = NULL;
> int addr;
>
> addr = device_get_phy_addr(priv, dev);
> @@ -1096,7 +1172,12 @@ static int fec_phy_init(struct fec_priv *priv, struct
> udevice *dev)
> addr = CONFIG_FEC_MXC_PHYADDR;
> #endif
>
> - phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> +#ifdef CONFIG_DM_MDIO
> + if (priv->dm_mdio)
> + phydev = dm_eth_phy_connect(dev);
> +#endif
> + if (!phydev)
> + phydev = phy_connect(priv->bus, addr, dev, priv->interface);
> if (!phydev)
> return -ENODEV;
>
> @@ -1227,11 +1308,19 @@ static int fecmxc_probe(struct udevice *dev)
>
> priv->dev_id = dev_seq(dev);
>
> +#ifdef CONFIG_DM_MDIO
> + ret = dm_fec_bind_mdio(dev);
> + if (!ret) {
> + ret = fec_phy_init(priv, dev);
> + if (!ret)
> + priv->dm_mdio = true;
> + }
> +#endif
> #ifdef CONFIG_DM_ETH_PHY
> bus = eth_phy_get_mdio_bus(dev);
> #endif
>
> - if (!bus) {
> + if (!bus && !priv->dm_mdio) {
> dm_mii_bus = false;
>
Re: [PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY
On Mon, Mar 28, 2022 at 03:23:02PM -0700, Tim Harvey wrote: > On Mon, Mar 28, 2022 at 2:26 AM Vladimir Oltean > wrote: > > > > On Fri, Mar 25, 2022 at 02:03:56PM -0700, Tim Harvey wrote: > > > On Fri, Mar 25, 2022 at 11:07 AM Vladimir Oltean > > > wrote: > > > > > > > > Hi Tim, > > > > > > > > On Fri, Mar 25, 2022 at 09:53:20AM -0700, Tim Harvey wrote: > > > > > Vladimir, > > > > > > > > > > I came across this while looking for the best place to configure cpu > > > > > port interface mode (ie rgmii id) for the mv88e61xx dsa driver I'm > > > > > working on. Note that this patch causes port_probe to be called on the > > > > > cpu port 'before' the master device has been probed. I'm not sure if > > > > > this is intended or not? > > > > > > > > > > In my case I was looking to configure the cpu port interface mode when > > > > > the port was probed but I can't do that because it happens before the > > > > > switch is probed because of some things that need to happen there. > > > > > Best Regards, > > > > > > > > > > Tim > > > > > > > > You're past the DM_MDIO probing issues now? Glad to hear that. > > > > Probing the DSA CPU port before the master wasn't necessarily the > > > > intention, but then again, I'm not sure that there's a strict ordering > > > > guarantee between the two that needs to be satisfied? > > > > > > > > What do you need exactly? We could probably always reverse the > > > > device_probe(master) call with the probing of the CPU port if the > > > > ordering turns out to be a real problem. I can regression-test the > > > > change on my boards, but I'd like to understand the need you have, maybe > > > > even document it so that future changes are aware of it. > > > > > > Yes, I've got the mdio probing working now. The mv88e61xx driver > > > supports several chips that have different register offsets that need > > > to be known before indirect read/write and port read/write can be > > > used. That detection happens early on so I have it in the dsa_probe. I > > > would prefer to configure the cpu port interface mode (mac mode, link > > > speed/duplex etc) once instead of doing it every time the cpu port > > > gets enabled so I want to put that in the dsa_probe but at that time I > > > don't have the phy_device to determine interface mode (I suppose I > > > could get it from dt?). I noticed dsa_class calls ops->port_probe for > > > the cpu port early so that seemed like a great place to do all that, > > > but then I found my switch dsa_probe hadn't been called yet so I > > > haven't identified the switch and set the register offsets yet. > > > > > > I have worked around the fact that the port_probe is called for the > > > cpu port before the switch is probed I just wasn't sure if something > > > in this patch should be changed in case others fall into this trap in > > > the future. dsa_port_probe probes the master before calling > > > ops->port_probe with the comment 'We depend on the master device for > > > proper operation' so I just figured the same should be done for the > > > pre_probe. > > > > Sorry, that was quite a blunder, we should definitely ensure that > > U_BOOT_DRIVER :: probe gets called before dsa_ops :: port_probe. > > I've made a change moving the port_probe call for the CPU port to > > dsa_post_probe(), tested it in the sandbox, and it appears to work. > > > > Ok, sounds good - did you submit this someplace? I haven't seen it. No, I didn't submit it anywhere. My thinking is that since the existing DSA drivers aren't critically affected by the calling order, you could include a change along those lines in your work for Marvell switches. > > > I hope to send a series in the next few days but I do have a few items > > > I still need to fix: > > > > > > 1. my board currently uses the mv88e61xx_hw_reset weak override to > > > configure LEDs, GPIO's using direct register writes as I need one of > > > the GPIO's to be configured as a 125MHz RGMII_RCLK and configure MAC > > > interface mode(rgmii delays). I've moved the mac interface config into > > > the driver based on the dt props (phy-mode and fixed-link subnode) but > > > am still not sure how to go about dealing with the 'very custom' LED > > > and GPIO config without the hassle
Re: [PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY
On Fri, Mar 25, 2022 at 02:03:56PM -0700, Tim Harvey wrote:
> On Fri, Mar 25, 2022 at 11:07 AM Vladimir Oltean
> wrote:
> >
> > Hi Tim,
> >
> > On Fri, Mar 25, 2022 at 09:53:20AM -0700, Tim Harvey wrote:
> > > Vladimir,
> > >
> > > I came across this while looking for the best place to configure cpu
> > > port interface mode (ie rgmii id) for the mv88e61xx dsa driver I'm
> > > working on. Note that this patch causes port_probe to be called on the
> > > cpu port 'before' the master device has been probed. I'm not sure if
> > > this is intended or not?
> > >
> > > In my case I was looking to configure the cpu port interface mode when
> > > the port was probed but I can't do that because it happens before the
> > > switch is probed because of some things that need to happen there.
> > > Best Regards,
> > >
> > > Tim
> >
> > You're past the DM_MDIO probing issues now? Glad to hear that.
> > Probing the DSA CPU port before the master wasn't necessarily the
> > intention, but then again, I'm not sure that there's a strict ordering
> > guarantee between the two that needs to be satisfied?
> >
> > What do you need exactly? We could probably always reverse the
> > device_probe(master) call with the probing of the CPU port if the
> > ordering turns out to be a real problem. I can regression-test the
> > change on my boards, but I'd like to understand the need you have, maybe
> > even document it so that future changes are aware of it.
>
> Yes, I've got the mdio probing working now. The mv88e61xx driver
> supports several chips that have different register offsets that need
> to be known before indirect read/write and port read/write can be
> used. That detection happens early on so I have it in the dsa_probe. I
> would prefer to configure the cpu port interface mode (mac mode, link
> speed/duplex etc) once instead of doing it every time the cpu port
> gets enabled so I want to put that in the dsa_probe but at that time I
> don't have the phy_device to determine interface mode (I suppose I
> could get it from dt?). I noticed dsa_class calls ops->port_probe for
> the cpu port early so that seemed like a great place to do all that,
> but then I found my switch dsa_probe hadn't been called yet so I
> haven't identified the switch and set the register offsets yet.
>
> I have worked around the fact that the port_probe is called for the
> cpu port before the switch is probed I just wasn't sure if something
> in this patch should be changed in case others fall into this trap in
> the future. dsa_port_probe probes the master before calling
> ops->port_probe with the comment 'We depend on the master device for
> proper operation' so I just figured the same should be done for the
> pre_probe.
Sorry, that was quite a blunder, we should definitely ensure that
U_BOOT_DRIVER :: probe gets called before dsa_ops :: port_probe.
I've made a change moving the port_probe call for the CPU port to
dsa_post_probe(), tested it in the sandbox, and it appears to work.
> I hope to send a series in the next few days but I do have a few items
> I still need to fix:
>
> 1. my board currently uses the mv88e61xx_hw_reset weak override to
> configure LEDs, GPIO's using direct register writes as I need one of
> the GPIO's to be configured as a 125MHz RGMII_RCLK and configure MAC
> interface mode(rgmii delays). I've moved the mac interface config into
> the driver based on the dt props (phy-mode and fixed-link subnode) but
> am still not sure how to go about dealing with the 'very custom' LED
> and GPIO config without the hassle of defining new dt bindings. I was
> hoping to use board_phy_config() but when that is called for the
> switch the phy_id is a generic PHY_FIXED_ID and when called for the
> ports the phydev->bus uses indirect register writes which can't be
> used to configure the gpios.
How are these configs handled in Linux?
> 2. While my switch mdio bus is probing now as well as my fec_dm_mdio
> bus I'm not clear how to properly get the struct mii_dev * associated
> with the fec_dm_mdio from the dsa switch. Currently I'm using
> miiphy_get_dev_by_name("mdio") which is horrible. How do I get the
> mii_bus or even the udevice of an dm_mdio bus associated with a dm_eth
> device?
Hmm, isn't the MDIO bus udevice the dev->parent of the switch?
Assuming that the reason you need this is for MDIO input/output towards
the switch. Although my suggestion would be to wrap this I/O behind
dm_mdio_read(struct udevice *dev /* MDIO child device */) and
dm_mdio_write(struct udevice *dev), rather than poking inside struct
udevice from the mv88e61xx driver.
Re: [PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY
Hi Tim, On Fri, Mar 25, 2022 at 09:53:20AM -0700, Tim Harvey wrote: > Vladimir, > > I came across this while looking for the best place to configure cpu > port interface mode (ie rgmii id) for the mv88e61xx dsa driver I'm > working on. Note that this patch causes port_probe to be called on the > cpu port 'before' the master device has been probed. I'm not sure if > this is intended or not? > > In my case I was looking to configure the cpu port interface mode when > the port was probed but I can't do that because it happens before the > switch is probed because of some things that need to happen there. > Best Regards, > > Tim You're past the DM_MDIO probing issues now? Glad to hear that. Probing the DSA CPU port before the master wasn't necessarily the intention, but then again, I'm not sure that there's a strict ordering guarantee between the two that needs to be satisfied? What do you need exactly? We could probably always reverse the device_probe(master) call with the probing of the CPU port if the ordering turns out to be a real problem. I can regression-test the change on my boards, but I'd like to understand the need you have, maybe even document it so that future changes are aware of it.
[PATCH] video: sandbox: fix missing shim definition of sandbox_sdl_remove_display()
When CONFIG_SANDBOX_SDL=n, sandbox_sdl_set_bpp() from
drivers/video/sandbox_sdl.c
calls sandbox_sdl_remove_display() in arch/sandbox/cpu/sdl.c, but this
isn't compiled in. A shim definition is missing, leading to a
compilation warning (missing function prototype) and a linkage bug.
Fixes: 8657ad43f353 ("sandbox: video: Add BMP tests for 32bpp and 8bpp modes")
Signed-off-by: Vladimir Oltean
---
I see Simon's email on the list from January 10 saying:
| I see that I broke it...it needs a static inline for
| sandbox_sdl_remove_display().
https://lore.kernel.org/all/CAPnjgZ0yqn_MjZ3uTLCrRp1-ifskKJgTmqmA4x31JZqVYn=q...@mail.gmail.com/
but I'm looking at the current 'master' and 'next' branches and that bug
is still there, huh. Am I missing something?
arch/sandbox/include/asm/sdl.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h
index e271a849af10..56dcb84803d3 100644
--- a/arch/sandbox/include/asm/sdl.h
+++ b/arch/sandbox/include/asm/sdl.h
@@ -94,6 +94,11 @@ static inline int sandbox_sdl_init_display(int width, int
height, int log2_bpp,
return -ENODEV;
}
+static inline int sandbox_sdl_remove_display(void)
+{
+ return -ENODEV;
+}
+
static inline int sandbox_sdl_sync(void *lcd_base)
{
return -ENODEV;
--
2.25.1
Re: dsa driver for mv88e61xx
Hello Tim,
On Fri, Mar 11, 2022 at 08:41:48AM -0800, Tim Harvey wrote:
> On Thu, Mar 10, 2022 at 3:18 PM Vladimir Oltean wrote:
> >
> > On Thu, Mar 10, 2022 at 02:35:21PM -0800, Tim Harvey wrote:
> > > On Thu, Mar 10, 2022 at 8:50 AM Vladimir Oltean wrote:
> > > >
> > > > Hello Tim,
> > > >
> > > > On Thu, Mar 10, 2022 at 08:16:13AM -0800, Tim Harvey wrote:
> > > > > Greetings,
> > > > >
> > > > > I wanted to take a stab at adding dsa support for the mv88e61xx which
> > > > > currently has a driver in drivers/net/phy [1]. The board I have
> > > > > available to me is the gw5904 which has a mv88e6085 with the upstream
> > > > > port connected to the IMX6 FEC MAC over RGMII. This currently works
> > > > > with the existing mv88e61xx driver with the following defined in
> > > > > gwventana_gw5904_defconfig:
> > > > >
> > > > > CONFIG_MV88E61XX_SWITCH=y
> > > > > CONFIG_MV88E61XX_CPU_PORT=5
> > > > > CONFIG_MV88E61XX_PHY_PORTS=0xf
> > > > > CONFIG_MV88E61XX_FIXED_PORTS=0x0
> > > > >
> > > > > The device-tree is arch/arm/dts/imx6qdl-gw5904.dtsi [2] which I
> > > > > believe is proper and works in Linux with the Linux driver in
> > > > > drivers/net/dsa/mv88e6xxx [3].
> > > > >
> > > > > {
> > > > > pinctrl-names = "default";
> > > > > pinctrl-0 = <_enet>;
> > > > > phy-mode = "rgmii-id";
> > > > > phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> > > > > phy-reset-duration = <10>;
> > > > > phy-reset-post-delay = <100>;
> > > > > status = "okay";
> > > > >
> > > > > fixed-link {
> > > > > speed = <1000>;
> > > > > full-duplex;
> > > > > };
> > > > >
> > > > > mdio {
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > switch@0 {
> > > > > compatible = "marvell,mv88e6085";
> > > > > reg = <0>;
> > > > >
> > > > > ports {
> > > > > #address-cells = <1>;
> > > > > #size-cells = <0>;
> > > > >
> > > > > port@0 {
> > > > > reg = <0>;
> > > > > label = "lan4";
> > > > > };
> > > > >
> > > > > port@1 {
> > > > > reg = <1>;
> > > > > label = "lan3";
> > > > > };
> > > > >
> > > > > port@2 {
> > > > > reg = <2>;
> > > > > label = "lan2";
> > > > > };
> > > > >
> > > > > port@3 {
> > > > > reg = <3>;
> > > > > label = "lan1";
> > > > > };
> > > > >
> > > > > port@5 {
> > > > > reg = <5>;
> > > > > label = "cpu";
> > > > > ethernet = <>;
> > > > > };
> > > > > };
> > > > > };
> > > > > };
> > > > > };
> > > > >
> > > > > My motivation for doing this is to be able to drop
> > > > > gwventana_gw5904_defconfig as it is the same defconfig as
Re: dsa driver for mv88e61xx
On Thu, Mar 10, 2022 at 02:35:21PM -0800, Tim Harvey wrote:
> On Thu, Mar 10, 2022 at 8:50 AM Vladimir Oltean wrote:
> >
> > Hello Tim,
> >
> > On Thu, Mar 10, 2022 at 08:16:13AM -0800, Tim Harvey wrote:
> > > Greetings,
> > >
> > > I wanted to take a stab at adding dsa support for the mv88e61xx which
> > > currently has a driver in drivers/net/phy [1]. The board I have
> > > available to me is the gw5904 which has a mv88e6085 with the upstream
> > > port connected to the IMX6 FEC MAC over RGMII. This currently works
> > > with the existing mv88e61xx driver with the following defined in
> > > gwventana_gw5904_defconfig:
> > >
> > > CONFIG_MV88E61XX_SWITCH=y
> > > CONFIG_MV88E61XX_CPU_PORT=5
> > > CONFIG_MV88E61XX_PHY_PORTS=0xf
> > > CONFIG_MV88E61XX_FIXED_PORTS=0x0
> > >
> > > The device-tree is arch/arm/dts/imx6qdl-gw5904.dtsi [2] which I
> > > believe is proper and works in Linux with the Linux driver in
> > > drivers/net/dsa/mv88e6xxx [3].
> > >
> > > {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <_enet>;
> > > phy-mode = "rgmii-id";
> > > phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> > > phy-reset-duration = <10>;
> > > phy-reset-post-delay = <100>;
> > > status = "okay";
> > >
> > > fixed-link {
> > > speed = <1000>;
> > > full-duplex;
> > > };
> > >
> > > mdio {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > switch@0 {
> > > compatible = "marvell,mv88e6085";
> > > reg = <0>;
> > >
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > port@0 {
> > > reg = <0>;
> > > label = "lan4";
> > > };
> > >
> > > port@1 {
> > > reg = <1>;
> > > label = "lan3";
> > > };
> > >
> > > port@2 {
> > > reg = <2>;
> > > label = "lan2";
> > > };
> > >
> > > port@3 {
> > > reg = <3>;
> > > label = "lan1";
> > > };
> > >
> > > port@5 {
> > > reg = <5>;
> > > label = "cpu";
> > > ethernet = <>;
> > > };
> > > };
> > > };
> > > };
> > > };
> > >
> > > My motivation for doing this is to be able to drop
> > > gwventana_gw5904_defconfig as it is the same defconfig as
> > > gwventana_emmc_defconfig with the switch added and with get_phy_id
> > > overridden by the current mv88e61xx driver that config won't work with
> > > boards that lack the switch.
> > >
> > > My first approach was to just put a #if !defined(CONFIG_DM_DSA) around
> > > mv88e61xx get_phy_id and add a skeleton driver with an of_match of
> > > compatible = "marvell,mv88e6085" but the driver does not probe with
> > > the above dt fragment.
> > >
> > > Any ideas why the driver won't probe and advise on how I should
> > > proceed with this? I'm not clear yet if I can just modify the existing
> > > driver or if I should create a new one.
> > >
> > > Best Regards,
> > >
> > > Tim
> > > [1]
> > > https://elixir.bootlin.com/u-boot/latest/source/drivers/net/phy/mv88e61xx.c
> >
Re: dsa driver for mv88e61xx
Hello Tim,
On Thu, Mar 10, 2022 at 08:16:13AM -0800, Tim Harvey wrote:
> Greetings,
>
> I wanted to take a stab at adding dsa support for the mv88e61xx which
> currently has a driver in drivers/net/phy [1]. The board I have
> available to me is the gw5904 which has a mv88e6085 with the upstream
> port connected to the IMX6 FEC MAC over RGMII. This currently works
> with the existing mv88e61xx driver with the following defined in
> gwventana_gw5904_defconfig:
>
> CONFIG_MV88E61XX_SWITCH=y
> CONFIG_MV88E61XX_CPU_PORT=5
> CONFIG_MV88E61XX_PHY_PORTS=0xf
> CONFIG_MV88E61XX_FIXED_PORTS=0x0
>
> The device-tree is arch/arm/dts/imx6qdl-gw5904.dtsi [2] which I
> believe is proper and works in Linux with the Linux driver in
> drivers/net/dsa/mv88e6xxx [3].
>
> {
> pinctrl-names = "default";
> pinctrl-0 = <_enet>;
> phy-mode = "rgmii-id";
> phy-reset-gpios = < 30 GPIO_ACTIVE_LOW>;
> phy-reset-duration = <10>;
> phy-reset-post-delay = <100>;
> status = "okay";
>
> fixed-link {
> speed = <1000>;
> full-duplex;
> };
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> switch@0 {
> compatible = "marvell,mv88e6085";
> reg = <0>;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> label = "lan4";
> };
>
> port@1 {
> reg = <1>;
> label = "lan3";
> };
>
> port@2 {
> reg = <2>;
> label = "lan2";
> };
>
> port@3 {
> reg = <3>;
> label = "lan1";
> };
>
> port@5 {
> reg = <5>;
> label = "cpu";
> ethernet = <>;
> };
> };
> };
> };
> };
>
> My motivation for doing this is to be able to drop
> gwventana_gw5904_defconfig as it is the same defconfig as
> gwventana_emmc_defconfig with the switch added and with get_phy_id
> overridden by the current mv88e61xx driver that config won't work with
> boards that lack the switch.
>
> My first approach was to just put a #if !defined(CONFIG_DM_DSA) around
> mv88e61xx get_phy_id and add a skeleton driver with an of_match of
> compatible = "marvell,mv88e6085" but the driver does not probe with
> the above dt fragment.
>
> Any ideas why the driver won't probe and advise on how I should
> proceed with this? I'm not clear yet if I can just modify the existing
> driver or if I should create a new one.
>
> Best Regards,
>
> Tim
> [1]
> https://elixir.bootlin.com/u-boot/latest/source/drivers/net/phy/mv88e61xx.c
> [2]
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/imx6qdl-gw5904.dtsi
> [3]
> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c
I spent about 3 minutes looking at mv88e61xx.c, so take what I'm about
to say with a grain of salt.
The biggest issue with reusing that code seems to be that it uses struct
phy_device throughout. A DM_DSA driver would need to work around a
struct udevice. I'd probably create a new driver, make it easy for
others to use, then delete old driver. I see there are 5 occurrences of
CONFIG_MV88E61XX_PHY_PORTS in defconfigs, all added within the past 4
years. One is yours. Could have been a lot worse.
As to why your driver is not probing. I think the fec_mxc parent MDIO
bus must be a udevice as well, registered using DM_MDIO?
[PATCH 2/2] net: phy: atheros: avoid error in ar803x_of_init() when PHY has no OF node
A DM_ETH driver may use phy_connect() towards a PHY address on an MDIO
bus which is not specified in the device tree, as evidenced by:
pfe_eth_probe
-> pfe_phy_configure
-> phy_connect
When this happens, the PHY will have an invalid OF node.
When ar803x_config() runs, it silently fails at ar803x_of_init(), and
therefore, fails to run the rest of the initialization.
This makes MII_BMCR contain what it had after BMCR_RESET (0x8000) has
been written into it by phy_reset(). Since BMCR_RESET is volatile and
self-clearing, the MII_BMCR ends up having a value of 0x0. The further
configuration of this register, which is supposed to be handled by
genphy_config_aneg() lower in ar803x_config(), never gets a chance to
run due to this early error from ar803x_of_init().
As a result of having MII_BMCR as 0, the following symptom appears:
=> setenv ethact pfe_eth0
=> setenv ipaddr 10.0.0.1
=> ping 10.0.0.2
pfe_eth0 Waiting for PHY auto negotiation to complete. TIMEOUT !
Could not initialize PHY pfe_eth0
Manually writing 0x1140 into register 0 of the PHY makes the connection
work, but it is rather desirable that the port works without any manual
intervention.
Fixes: fe6293a80959 ("phy: atheros: add device tree bindings and config")
Signed-off-by: Vladimir Oltean
---
drivers/net/phy/atheros.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index f922fecd6b5d..fa1fe08518f4 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -199,7 +199,7 @@ static int ar803x_of_init(struct phy_device *phydev)
node = phy_get_ofnode(phydev);
if (!ofnode_valid(node))
- return -EINVAL;
+ return 0;
priv = malloc(sizeof(*priv));
if (!priv)
--
2.25.1
[PATCH 1/2] net: phy: dp83867: avoid error in dp83867_of_init() when PHY has no OF node
A DM_ETH driver may use phy_connect() towards a PHY address on an MDIO
bus which is not specified in the device tree, as evidenced by:
pfe_eth_probe
-> pfe_phy_configure
-> phy_connect
When this happens, the PHY will have an invalid OF node.
The dp83867_config() method has extra initialization steps which are
bypassed when the PHY lacks an OF node, which is undesirable because it
will lead to broken networking. Allow the rest of the code to run.
Fixes: 085445ca4104 ("net: phy: ti: Allow the driver to be more configurable")
Signed-off-by: Vladimir Oltean
---
drivers/net/phy/dp83867.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index eada4541c9c3..49978d0f25f3 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -158,7 +158,7 @@ static int dp83867_of_init(struct phy_device *phydev)
node = phy_get_ofnode(phydev);
if (!ofnode_valid(node))
- return -EINVAL;
+ return 0;
/* Optional configuration */
ret = ofnode_read_u32(node, "ti,clk-output-sel",
--
2.25.1
[PATCH 0/2] PHY of_init breakage with non-OF
Investigating an issue with the NXP LS1012A-FRWY board and the AR8031 PHY not negotiating, I found that the introduction of the node = phy_get_ofnode(phydev); if (!ofnode_valid(node)) return -EINVAL; pattern in PHY drivers causes silent breakage for users of phy_connect(). I've fixed this in the Atheros driver, and also saw the same issue in the TI PHY driver, so I fixed it there as well. Vladimir Oltean (2): net: phy: dp83867: avoid error in dp83867_of_init() when PHY has no OF node net: phy: atheros: avoid error in ar803x_of_init() when PHY has no OF node drivers/net/phy/atheros.c | 2 +- drivers/net/phy/dp83867.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.25.1
Re: [PATCH 2/2] scsi: ceva: Enable PHY and reset support
On Wed, Jan 19, 2022 at 03:18:54PM +0100, Michal Simek wrote:
> Add phy and reset support for ceva sata IP. Phy and reset are optional
> properties that's why detect if description is available. If not just
> continue with operation.
> This code was tested on Xilinx Kria SOM kv260-revA with sata connector
> populated.
>
> Signed-off-by: Michal Simek
> ---
Reviewed-by: Vladimir Oltean
>
> drivers/ata/sata_ceva.c | 44 +
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/ata/sata_ceva.c b/drivers/ata/sata_ceva.c
> index b71f10223da8..43bcc59cd282 100644
> --- a/drivers/ata/sata_ceva.c
> +++ b/drivers/ata/sata_ceva.c
> @@ -6,9 +6,12 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
> #include
> #include
> +#include
> #include
>
> /* Vendor Specific Register Offsets */
> @@ -181,6 +184,47 @@ static int sata_ceva_bind(struct udevice *dev)
> static int sata_ceva_probe(struct udevice *dev)
> {
> struct ceva_sata_priv *priv = dev_get_priv(dev);
> + struct phy phy;
> + int ret;
> + struct reset_ctl_bulk resets;
> +
> + ret = generic_phy_get_by_index(dev, 0, );
> + if (!ret) {
> + dev_dbg(dev, "Perform PHY initialization\n");
> + ret = generic_phy_init();
> + if (ret)
> + return ret;
> + } else if (ret != -ENOENT) {
> + dev_dbg(dev, "could not get phy (err %d)\n", ret);
> + return ret;
> + }
> +
> + /* reset is optional */
> + ret = reset_get_bulk(dev, );
> + if (ret && ret != -ENOTSUPP && ret != -ENOENT) {
> + dev_dbg(dev, "Getting reset fails (err %d)\n", ret);
> + return ret;
> + }
> +
> + /* Just trigger reset when reset is specified */
> + if (!ret) {
> + dev_dbg(dev, "Perform IP reset\n");
> + ret = reset_deassert_bulk();
> + if (ret) {
> + dev_dbg(dev, "Reset fails (err %d)\n", ret);
> + reset_release_bulk();
> + return ret;
> + }
> + }
> +
> + if (phy.dev) {
> + dev_dbg(dev, "Perform PHY power on\n");
> + ret = generic_phy_power_on();
> + if (ret) {
> + dev_dbg(dev, "PHY power on failed (err %d)\n", ret);
> + return ret;
> + }
> + }
>
> ceva_init_sata(priv);
>
> --
> 2.34.1
>
[PATCH v3 16/16] arm: dts: ls1028a-qds: declare in-band autoneg for Ethernet ports
The commit in the Fixes: tag below broke traffic through switch ports
where the SERDES protocol requires in-band autoneg and this requirement
isn't described in the device tree: SGMII, QSGMII, USXGMII (with
2500Base-X, in-band autoneg isn't supported).
The LS1028A-QDS boards are not yet ready for syncing their device trees
with Linux, since Ethernet is missing there (but has been submitted):
https://lore.kernel.org/lkml/2022223457.10599-11-leoyang...@nxp.com/
When agreement is reached for the Ethernet support in Linux, there will
be a sync for these boards as well. For now, just enable in-band autoneg
to fix the breakage.
Fixes: e3789a726269 ("net: dsa: felix: configure the in-band autoneg property
based on OF node info")
Cc: Ramon Fried
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi | 1 +
arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi | 1 +
arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds-x5xx-sch-28021-LBRW.dtsi | 4
6 files changed, 18 insertions(+)
diff --git a/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
index f4c557e69e6e..f208a02721e3 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
@@ -15,6 +15,7 @@
_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "usxgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@02}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
index 7d197c31814a..0a0926473541 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
@@ -14,6 +14,7 @@
_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
b/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
index 992092ec7838..94b5e765aedb 100644
--- a/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
@@ -44,24 +44,28 @@
_felix_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
_felix_port1 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@50/phy@1c}>;
};
_felix_port2 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1e}>;
};
_felix_port3 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1f}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
b/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
index a905d77a9a71..bd46adfd2928 100644
--- a/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
@@ -29,24 +29,28 @@
_felix_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
_felix_port1 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1d}>;
};
_felix_port2 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1e}>;
};
_felix_port3 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1f}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
index 62e818f099ca..5909e7635a1a 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
@@ -29
[PATCH v3 15/16] arm: dts: ls1028a-rdb: sync device tree with Linux
Allow device trees to be reused between Linux and U-Boot.
The source for these device trees is linux-next as of commit
bd8a9cd624c6 ("arm64: dts: ls1028a-rdb: update copyright"), which was
chosen because some changes needed to be done to the Linux DTs too,
before they could be shared:
https://lore.kernel.org/linux-arm-kernel/20211202141528.2450169-5-vladimir.olt...@nxp.com/T/#m6f63c92e75fa79a01144b2c2c6dc4776e7971395
There are two more commits on the RDB device tree which haven't been
picked up yet, because they have dependencies on the SoC device tree:
dd3d936a1b17 ("arm64: dts: ls1028a: add ftm_alarm1 node to be used as wakeup
source")
b2e2d3e02fb6 ("arm64: dts: ls1028a-rdb: enable pwm0")
These will be picked up on the next resync.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v2: RTC compatible string change broken out into separate patch
v2->v3: update commit message to point out that the Linux dependency
patches were merged
arch/arm/dts/fsl-ls1028a-rdb.dts | 158 ---
1 file changed, 146 insertions(+), 12 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 5a35258fa97f..639f40740d56 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -1,19 +1,27 @@
-// SPDX-License-Identifier: GPL-2.0+ OR X11
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
- * NXP ls1028ARDB device tree source
+ * Device Tree file for NXP LS1028A RDB Board.
*
- * Copyright 2019 NXP
+ * Copyright 2018-2021 NXP
+ *
+ * Harninder Rai
*
*/
/dts-v1/;
-
#include "fsl-ls1028a.dtsi"
/ {
- model = "NXP Layerscape 1028a RDB Board";
+ model = "LS1028A RDB Board";
compatible = "fsl,ls1028a-rdb", "fsl,ls1028a";
+
aliases {
+ crypto =
+ serial0 =
+ serial1 =
+ mmc0 =
+ mmc1 =
+ rtc1 = _alarm0;
spi0 =
ethernet0 = _port0;
ethernet1 = _port2;
@@ -22,6 +30,83 @@
ethernet4 = _felix_port2;
ethernet5 = _felix_port3;
};
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@8000 {
+ device_type = "memory";
+ reg = <0x0 0x8000 0x1 0x000>;
+ };
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <2500>;
+ };
+
+ reg_1p8v: regulator-1p8v {
+ compatible = "regulator-fixed";
+ regulator-name = "1P8V";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+ };
+
+ sb_3v3: regulator-sb3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "3v3_vbus";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <2500>;
+ };
+ };
+};
+
+ {
+ status = "okay";
+
+ can-transceiver {
+ max-bitrate = <500>;
+ };
+};
+
+ {
+ status = "okay";
+
+ can-transceiver {
+ max-bitrate = <500>;
+ };
};
{
@@ -67,43 +152,83 @@
};
{
+ sd-
[PATCH v3 13/16] arm: dts: ls1028a-rdb: disable I2C buses 1 through 7
There is no I2C peripheral on these buses on the reference design board,
and the Linux device tree does not enable them either.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-rdb.dts | 28
1 file changed, 28 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index c5b95e169f5c..10070eab6e61 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -112,34 +112,6 @@
};
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
_felix {
status = "okay";
};
--
2.25.1
[PATCH v3 12/16] arm: dts: ls1028a-rdb: disable DSPI nodes
There is no SPI peripheral on the LS1028A-RDB, therefore no reason to
enable these nodes in the U-Boot device tree (and Linux does not enable
them either).
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-rdb.dts | 12
1 file changed, 12 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index b5e56b1c1f15..c5b95e169f5c 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -24,18 +24,6 @@
};
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
{
status = "okay";
};
--
2.25.1
[PATCH v3 14/16] arm: dts: ls1028a-rdb: enable PCIe controllers from U-Boot dtsi
Reuse the scheme implemented by the Kontron SL28 boards in commit
d08011d7f9b4 ("arm: dts: ls1028a: disable the PCIe controller by
default") and move the 'status = "okay"' lines for the PCIe controllers
inside a separate U-Boot dtsi for the LS1028A-RDB board. This way, the
existing Linux device tree can simply be dropped in.
Signed-off-by: Vladimir Oltean
---
v2->v3: stop including the -u-boot.dtsi by hand, it gets included
automatically
arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi | 15 +++
arch/arm/dts/fsl-ls1028a-rdb.dts | 8
2 files changed, 15 insertions(+), 8 deletions(-)
create mode 100644 arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
diff --git a/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
b/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
new file mode 100644
index ..a72b57305dc3
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright 2021 NXP */
+
+/*
+ * u-boot will enable the device in the linux device tree in place. Because
+ * we are using the linux device tree, we have to enable the PCI controller
+ * ourselves.
+ */
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 10070eab6e61..5a35258fa97f 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -153,14 +153,6 @@
status = "okay";
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
{
status = "okay";
};
--
2.25.1
[PATCH v3 10/16] arm: dts: ls1028a-rdb: sort nodes alphabetically
The nodes in the NXP LS1028A-RDB device tree are out of order, regroup
them alphabetically to have a simple delta when the Linux device tree is
brought in.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-rdb.dts | 110 +++
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index ddb01db73f8e..11bf7e5f627d 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -36,6 +36,48 @@
status = "okay";
};
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+_mdio_pf3 {
+ status = "okay";
+ rdb_phy0: phy@2 {
+ reg = <2>;
+ };
+
+ /* VSC8514 QSGMII PHY */
+ sw_phy0: phy@10 {
+ reg = <0x10>;
+ };
+
+ sw_phy1: phy@11 {
+ reg = <0x11>;
+ };
+
+ sw_phy2: phy@12 {
+ reg = <0x12>;
+ };
+
+ sw_phy3: phy@13 {
+ reg = <0x13>;
+ };
+};
+
+_port0 {
+ status = "okay";
+ phy-mode = "sgmii";
+ phy-handle = <_phy0>;
+};
+
+_port2 {
+ status = "okay";
+};
+
{
status = "okay";
};
@@ -110,44 +152,6 @@
status = "okay";
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
-_port0 {
- status = "okay";
- phy-mode = "sgmii";
- phy-handle = <_phy0>;
-};
-
-_port2 {
- status = "okay";
-};
-
_felix {
status = "okay";
};
@@ -185,26 +189,22 @@
status = "okay";
};
-_mdio_pf3 {
+ {
status = "okay";
- rdb_phy0: phy@2 {
- reg = <2>;
- };
+};
- /* VSC8514 QSGMII PHY */
- sw_phy0: phy@10 {
- reg = <0x10>;
- };
+ {
+ status = "okay";
+};
- sw_phy1: phy@11 {
- reg = <0x11>;
- };
+ {
+ status = "okay";
+};
- sw_phy2: phy@12 {
- reg = <0x12>;
- };
+ {
+ status = "okay";
+};
- sw_phy3: phy@13 {
- reg = <0x13>;
- };
+ {
+ status = "okay";
};
--
2.25.1
[PATCH v3 11/16] arm: dts: ls1028a-rdb: sync Ethernet device tree nodes with Linux
In a bit of a blunder, the blamed commit in the Fixes: tag below made
the mscc_felix switch driver look at the 'managed = "in-band-status"'
device tree property, forgetting that the U-Boot device tree had not
been updated to include that property, whereas the Linux one does.
The switch is therefore described in the device tree as not requiring
in-band autoneg, but the PHY driver for VSC8514 (drivers/net/phy/mscc.c)
still enables that feature. This results in a mismatch => no traffic.
This patch is a copy-paste of the Ethernet device tree nodes from Linux,
which resolves that issue. The device tree update also renames the
Ethernet PHY labels.
Fixes: e3789a726269 ("net: dsa: felix: configure the in-band autoneg property
based on OF node info")
Cc: Ramon Fried
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-rdb.dts | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 11bf7e5f627d..b5e56b1c1f15 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -45,33 +45,33 @@
};
_mdio_pf3 {
- status = "okay";
- rdb_phy0: phy@2 {
- reg = <2>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
};
- /* VSC8514 QSGMII PHY */
- sw_phy0: phy@10 {
+ /* VSC8514 QSGMII quad PHY */
+ qsgmii_phy0: ethernet-phy@10 {
reg = <0x10>;
};
- sw_phy1: phy@11 {
+ qsgmii_phy1: ethernet-phy@11 {
reg = <0x11>;
};
- sw_phy2: phy@12 {
+ qsgmii_phy2: ethernet-phy@12 {
reg = <0x12>;
};
- sw_phy3: phy@13 {
+ qsgmii_phy3: ethernet-phy@13 {
reg = <0x13>;
};
};
_port0 {
- status = "okay";
+ phy-handle = <_phy0>;
phy-mode = "sgmii";
- phy-handle = <_phy0>;
+ managed = "in-band-status";
+ status = "okay";
};
_port2 {
@@ -158,28 +158,32 @@
_felix_port0 {
label = "swp0";
- phy-handle = <_phy0>;
+ managed = "in-band-status";
+ phy-handle = <_phy0>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port1 {
label = "swp1";
- phy-handle = <_phy1>;
+ managed = "in-band-status";
+ phy-handle = <_phy1>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port2 {
label = "swp2";
- phy-handle = <_phy2>;
+ managed = "in-band-status";
+ phy-handle = <_phy2>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port3 {
label = "swp3";
- phy-handle = <_phy3>;
+ managed = "in-band-status";
+ phy-handle = <_phy3>;
phy-mode = "qsgmii";
status = "okay";
};
--
2.25.1
[PATCH v3 08/16] arm: dts: lx2160a-rdb: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LX2160A-RDB to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-lx2160a-rdb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-lx2160a-rdb.dts b/arch/arm/dts/fsl-lx2160a-rdb.dts
index 5fbdd907017c..8ca4afa7eaea 100644
--- a/arch/arm/dts/fsl-lx2160a-rdb.dts
+++ b/arch/arm/dts/fsl-lx2160a-rdb.dts
@@ -117,7 +117,7 @@
status = "okay";
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 09/16] rtc: pcf2127: remove U-Boot specific compatible string
Now that all in-tree boards have been converted to the compatible
strings from Linux, delete the support for the ad-hoc "pcf2127-rtc" one.
Cc: Simon Glass
Signed-off-by: Vladimir Oltean
Reviewed-by: Simon Glass
---
v1->v3: none
drivers/rtc/pcf2127.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
index 291ef0329a3e..2f3fafb4968f 100644
--- a/drivers/rtc/pcf2127.c
+++ b/drivers/rtc/pcf2127.c
@@ -120,7 +120,6 @@ static const struct rtc_ops pcf2127_rtc_ops = {
};
static const struct udevice_id pcf2127_rtc_ids[] = {
- { .compatible = "pcf2127-rtc" },
{ .compatible = "nxp,pcf2127" },
{ .compatible = "nxp,pcf2129" },
{ .compatible = "nxp,pca2129" },
--
2.25.1
[PATCH v3 07/16] arm: dts: ls1028a-rdb: use Linux compatible string for RTC
During this board's sync with Linux device trees, it was observed that
it doesn't use the same compatible string for the RTC node as in U-Boot.
This change makes the RTC compatible strings match, for a smoother sync.
Signed-off-by: Vladimir Oltean
---
v1->v2: patch is new
v2->v3: none
arch/arm/dts/fsl-ls1028a-rdb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 537ebbc697cb..ddb01db73f8e 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -75,7 +75,7 @@
reg = <0x3>;
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 06/16] arm: dts: ls1028a-qds: use Linux compatible string for RTC
The LS1028A-QDS board won't be synced with the Linux device trees right
now, since those are currently still in progress (Ethernet is missing).
However, while we're at converting the RDB, it can be observed that the
same RTC is present on the two boards, and the wrong compatible string
is used in both places. This change updates the RTC from the QDS to use
the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1028a-qds.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-ls1028a-qds.dtsi
b/arch/arm/dts/fsl-ls1028a-qds.dtsi
index 0da0a7bc5db8..3b063d0257de 100644
--- a/arch/arm/dts/fsl-ls1028a-qds.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds.dtsi
@@ -195,7 +195,7 @@
status = "okay";
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 05/16] arm: dts: lx2160a-qds: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LX2160A-QDS to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-lx2160a-qds.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-lx2160a-qds.dtsi
b/arch/arm/dts/fsl-lx2160a-qds.dtsi
index 288607c0347b..69e11cca2da1 100644
--- a/arch/arm/dts/fsl-lx2160a-qds.dtsi
+++ b/arch/arm/dts/fsl-lx2160a-qds.dtsi
@@ -250,7 +250,7 @@
reg = <0x3>;
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 04/16] arm: dts: ls1088a-rdb: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LS1088A-RDB to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1088a-rdb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-ls1088a-rdb.dts b/arch/arm/dts/fsl-ls1088a-rdb.dts
index de92bf22e203..5cdd59815234 100644
--- a/arch/arm/dts/fsl-ls1088a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1088a-rdb.dts
@@ -135,7 +135,7 @@
reg = <0x3>;
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 02/16] rtc: pcf2127: sync with Linux compatible strings
Allow this driver to be used by boards which inherit their device trees
from Linux. Compatibility is temporarily retained with the old
compatible string which is U-Boot specific, and will be removed after a
few changes.
Cc: Simon Glass
Signed-off-by: Vladimir Oltean
Reviewed-by: Simon Glass
---
v1->v3: none
drivers/rtc/pcf2127.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
index 57f86401d371..291ef0329a3e 100644
--- a/drivers/rtc/pcf2127.c
+++ b/drivers/rtc/pcf2127.c
@@ -121,6 +121,9 @@ static const struct rtc_ops pcf2127_rtc_ops = {
static const struct udevice_id pcf2127_rtc_ids[] = {
{ .compatible = "pcf2127-rtc" },
+ { .compatible = "nxp,pcf2127" },
+ { .compatible = "nxp,pcf2129" },
+ { .compatible = "nxp,pca2129" },
{ }
};
--
2.25.1
[PATCH v3 00/16] Sync NXP LS1028A-RDB device trees between U-Boot and Linux
The changes were intended to be minimal, but unfortunately I discovered some other stuff as well: - we need to make some changes to the compatible strings of RTC devices and I2C muxes. This has ramifications to other NXP boards which were also updated. - I broke Ethernet on LS1028A boards through a patch that is currently in Ramon's tree. Therefore this patch set is a bit larger than would be otherwise expected. The Linux device tree changes have just been posted by me here and are currently in flight, but they are rather small so I don't expect too much pushback on them: https://lore.kernel.org/linux-arm-kernel/20211202141528.2450169-5-vladimir.olt...@nxp.com/T/#m6f63c92e75fa79a01144b2c2c6dc4776e7971395 I've also triggered an Azure CI build with these changes: https://github.com/u-boot/u-boot/pull/102 and it appears that 2 tests fail due to external causes: 1. https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3283=logs=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=c864b9c4-48aa-5e04-3916-54070f85e156 2. https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3283=logs=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=c864b9c4-48aa-5e04-3916-54070f85e156=27 Unable to find image 'trini/u-boot-gitlab-ci-runner:focal-20211006-14Nov2021' locally docker: Error response from daemon: Head "https://registry-1.docker.io/v2/trini/u-boot-gitlab-ci-runner/manifests/focal-20211006-14Nov2021": received unexpected HTTP status: 502 Bad Gateway. The other tests seem to pass. Cc: Heiko Schocher Cc: Simon Glass Cc: Ramon Fried Vladimir Oltean (16): i2c: muxes: pca954x: add PCA9847 variant rtc: pcf2127: sync with Linux compatible strings arm: dts: ls1088a-qds: use Linux compatible string for RTC arm: dts: ls1088a-rdb: use Linux compatible string for RTC arm: dts: lx2160a-qds: use Linux compatible string for RTC arm: dts: ls1028a-qds: use Linux compatible string for RTC arm: dts: ls1028a-rdb: use Linux compatible string for RTC arm: dts: lx2160a-rdb: use Linux compatible string for RTC rtc: pcf2127: remove U-Boot specific compatible string arm: dts: ls1028a-rdb: sort nodes alphabetically arm: dts: ls1028a-rdb: sync Ethernet device tree nodes with Linux arm: dts: ls1028a-rdb: disable DSPI nodes arm: dts: ls1028a-rdb: disable I2C buses 1 through 7 arm: dts: ls1028a-rdb: enable PCIe controllers from U-Boot dtsi arm: dts: ls1028a-rdb: sync device tree with Linux arm: dts: ls1028a-qds: declare in-band autoneg for Ethernet ports .../dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi | 1 + .../dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi | 1 + .../fsl-ls1028a-qds--sch-24801-LBRW.dtsi | 4 + .../dts/fsl-ls1028a-qds--sch-24801.dtsi | 4 + .../fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi | 4 + .../fsl-ls1028a-qds-x5xx-sch-28021-LBRW.dtsi | 4 + arch/arm/dts/fsl-ls1028a-qds.dtsi | 2 +- arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi | 15 + arch/arm/dts/fsl-ls1028a-rdb.dts | 294 -- arch/arm/dts/fsl-ls1088a-qds.dtsi | 2 +- arch/arm/dts/fsl-ls1088a-rdb.dts | 2 +- arch/arm/dts/fsl-lx2160a-qds.dtsi | 2 +- arch/arm/dts/fsl-lx2160a-rdb.dts | 2 +- drivers/i2c/muxes/pca954x.c | 9 +- drivers/rtc/pcf2127.c | 4 +- 15 files changed, 241 insertions(+), 109 deletions(-) create mode 100644 arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi -- 2.25.1
[PATCH v3 03/16] arm: dts: ls1088a-qds: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LS1088A-QDS to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
v1->v3: none
arch/arm/dts/fsl-ls1088a-qds.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-ls1088a-qds.dtsi
b/arch/arm/dts/fsl-ls1088a-qds.dtsi
index a7d0edcf0aa9..21c50078c3a4 100644
--- a/arch/arm/dts/fsl-ls1088a-qds.dtsi
+++ b/arch/arm/dts/fsl-ls1088a-qds.dtsi
@@ -88,7 +88,7 @@
reg = <0x3>;
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v3 01/16] i2c: muxes: pca954x: add PCA9847 variant
This seems to be very similar to the already existing PCA9547, save for
the fact that it supports 0.8V and doesn't support 5V. In fact, it is so
similar to the PCA9547 that the NXP LS1028A-RDB board has been driving
this chip using a "nxp,pca9547" compatible string.
Create a new compatible for the PCA9847 (which is the same as in Linux)
and define the same operating parameters for it as for PCA9547.
Cc: Heiko Schocher
Signed-off-by: Vladimir Oltean
Reviewed-by: Heiko Schocher
Reviewed-by: Priyanka Jain
---
v1->v3: none
drivers/i2c/muxes/pca954x.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c
index 55858cf653f2..0034dfbf6daf 100644
--- a/drivers/i2c/muxes/pca954x.c
+++ b/drivers/i2c/muxes/pca954x.c
@@ -23,7 +23,8 @@ enum pca_type {
PCA9546,
PCA9547,
PCA9548,
- PCA9646
+ PCA9646,
+ PCA9847,
};
struct chip_desc {
@@ -68,6 +69,11 @@ static const struct chip_desc chips[] = {
.muxtype = pca954x_isswi,
.width = 4,
},
+ [PCA9847] = {
+ .enable = 0x8,
+ .muxtype = pca954x_ismux,
+ .width = 8,
+ },
};
static int pca954x_deselect(struct udevice *mux, struct udevice *bus,
@@ -106,6 +112,7 @@ static const struct udevice_id pca954x_ids[] = {
{ .compatible = "nxp,pca9547", .data = PCA9547 },
{ .compatible = "nxp,pca9548", .data = PCA9548 },
{ .compatible = "nxp,pca9646", .data = PCA9646 },
+ { .compatible = "nxp,pca9847", .data = PCA9847 },
{ }
};
--
2.25.1
Re: [PATCH v2 00/16] Sync NXP LS1028A-RDB device trees between U-Boot and Linux
On Thu, Dec 23, 2021 at 01:47:48PM +0100, Michael Walle wrote: > Am 2021-12-23 13:44, schrieb Vladimir Oltean: > > On Thu, Dec 23, 2021 at 06:34:13AM +, Priyanka Jain wrote: > > > >The Linux side of device tree patches were merged today, and I see you've > > > >reviewed the U-Boot side of changes too. Could you please pick them up? > > > > > > Yes, I will pick the series as part of next pull-request for 2022.04 > > > > Thanks. Could you also delete the '#include > > "fsl-ls1028a-rdb-u-boot.dtsi"' > > line from patch 14/16 like Michael suggested, or should I resend for > > that? > > Patch 15/16 is also affected? I guess you took the linux dts and added > that include line afterwards? that shouldn't be necessary. Alright, I'll send v3 later today or tomorrow.
Re: [PATCH v2 00/16] Sync NXP LS1028A-RDB device trees between U-Boot and Linux
On Thu, Dec 23, 2021 at 06:34:13AM +, Priyanka Jain wrote: > >The Linux side of device tree patches were merged today, and I see you've > >reviewed the U-Boot side of changes too. Could you please pick them up? > > Yes, I will pick the series as part of next pull-request for 2022.04 Thanks. Could you also delete the '#include "fsl-ls1028a-rdb-u-boot.dtsi"' line from patch 14/16 like Michael suggested, or should I resend for that?
Re: [PATCH v2 00/16] Sync NXP LS1028A-RDB device trees between U-Boot and Linux
Hi Priyanka, On Tue, Dec 07, 2021 at 10:20:07PM +0200, Vladimir Oltean wrote: > The changes were intended to be minimal, but unfortunately I discovered > some other stuff as well: > - we need to make some changes to the compatible strings of RTC devices > and I2C muxes. This has ramifications to other NXP boards which were > also updated. > - I broke Ethernet on LS1028A boards through a patch that is currently > in Ramon's tree. > > Therefore this patch set is a bit larger than would be otherwise > expected. > > The Linux device tree changes have just been posted by me here and are > currently in flight, but they are rather small so I don't expect too > much pushback on them: > https://lore.kernel.org/linux-arm-kernel/20211202141528.2450169-5-vladimir.olt...@nxp.com/T/#m6f63c92e75fa79a01144b2c2c6dc4776e7971395 > > I've also triggered an Azure CI build with these changes: > https://github.com/u-boot/u-boot/pull/102 > and it appears that 2 tests fail due to external causes: > 1. > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3283=logs=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=c864b9c4-48aa-5e04-3916-54070f85e156 > 2. > https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3283=logs=5fafc5b9-a417-5c75-4c48-15c7f941e4ee=c864b9c4-48aa-5e04-3916-54070f85e156=27 > > Unable to find image 'trini/u-boot-gitlab-ci-runner:focal-20211006-14Nov2021' > locally > docker: Error response from daemon: Head > "https://registry-1.docker.io/v2/trini/u-boot-gitlab-ci-runner/manifests/focal-20211006-14Nov2021": > received unexpected HTTP status: 502 Bad Gateway. > > The other tests seem to pass. The Linux side of device tree patches were merged today, and I see you've reviewed the U-Boot side of changes too. Could you please pick them up?
[PATCH v2 15/16] arm: dts: ls1028a-rdb: sync device tree with Linux
Allow device trees to be reused between Linux and U-Boot.
The source for these device trees is today's linux-next plus these
changes that have been made so that the sources can be shared. These
other patches are currently in flight:
https://lore.kernel.org/linux-arm-kernel/20211202141528.2450169-5-vladimir.olt...@nxp.com/T/#m6f63c92e75fa79a01144b2c2c6dc4776e7971395
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-rdb.dts | 158 ---
1 file changed, 146 insertions(+), 12 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 70fcf71dbd0e..09c38ecaf95c 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -1,20 +1,28 @@
-// SPDX-License-Identifier: GPL-2.0+ OR X11
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
- * NXP ls1028ARDB device tree source
+ * Device Tree file for NXP LS1028A RDB Board.
*
- * Copyright 2019 NXP
+ * Copyright 2018-2021 NXP
+ *
+ * Harninder Rai
*
*/
/dts-v1/;
-
#include "fsl-ls1028a.dtsi"
#include "fsl-ls1028a-rdb-u-boot.dtsi"
/ {
- model = "NXP Layerscape 1028a RDB Board";
+ model = "LS1028A RDB Board";
compatible = "fsl,ls1028a-rdb", "fsl,ls1028a";
+
aliases {
+ crypto =
+ serial0 =
+ serial1 =
+ mmc0 =
+ mmc1 =
+ rtc1 = _alarm0;
spi0 =
ethernet0 = _port0;
ethernet1 = _port2;
@@ -23,6 +31,83 @@
ethernet4 = _felix_port2;
ethernet5 = _felix_port3;
};
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@8000 {
+ device_type = "memory";
+ reg = <0x0 0x8000 0x1 0x000>;
+ };
+
+ sys_mclk: clock-mclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <2500>;
+ };
+
+ reg_1p8v: regulator-1p8v {
+ compatible = "regulator-fixed";
+ regulator-name = "1P8V";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ regulator-always-on;
+ };
+
+ sb_3v3: regulator-sb3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "3v3_vbus";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ sound {
+ compatible = "simple-audio-card";
+ simple-audio-card,format = "i2s";
+ simple-audio-card,widgets =
+ "Microphone", "Microphone Jack",
+ "Headphone", "Headphone Jack",
+ "Speaker", "Speaker Ext",
+ "Line", "Line In Jack";
+ simple-audio-card,routing =
+ "MIC_IN", "Microphone Jack",
+ "Microphone Jack", "Mic Bias",
+ "LINE_IN", "Line In Jack",
+ "Headphone Jack", "HP_OUT",
+ "Speaker Ext", "LINE_OUT";
+
+ simple-audio-card,cpu {
+ sound-dai = <>;
+ frame-master;
+ bitclock-master;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <>;
+ frame-master;
+ bitclock-master;
+ system-clock-frequency = <2500>;
+ };
+ };
+};
+
+ {
+ status = "okay";
+
+ can-transceiver {
+ max-bitrate = <500>;
+ };
+};
+
+ {
+ status = "okay";
+
+ can-transceiver {
+ max-bitrate = <500>;
+ };
};
{
@@ -68,43 +153,83 @@
};
{
+ sd-uhs-sdr104;
+ sd-uhs-sdr50;
+ sd-uhs-sdr25;
+ sd-uhs-sdr12;
status = "okay";
};
{
- status = "okay";
mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ bus-width = <8>;
+ status = "okay";
};
{
status = "okay";
mt35xu02g0: flash@0 {
+ compatible = "jedec,spi-nor";
#address-cells = <1>;
#size-cells = <1>;
- compatible = "jedec,s
[PATCH v2 16/16] arm: dts: ls1028a-qds: declare in-band autoneg for Ethernet ports
The commit in the Fixes: tag below broke traffic through switch ports
where the SERDES protocol requires in-band autoneg and this requirement
isn't described in the device tree: SGMII, QSGMII, USXGMII (with
2500Base-X, in-band autoneg isn't supported).
The LS1028A-QDS boards are not yet ready for syncing their device trees
with Linux, since Ethernet is missing there (but has been submitted):
https://lore.kernel.org/lkml/2022223457.10599-11-leoyang...@nxp.com/
When agreement is reached for the Ethernet support in Linux, there will
be a sync for these boards as well. For now, just enable in-band autoneg
to fix the breakage.
Fixes: e3789a726269 ("net: dsa: felix: configure the in-band autoneg property
based on OF node info")
Cc: Ramon Fried
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi | 1 +
arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi | 1 +
arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi | 4
arch/arm/dts/fsl-ls1028a-qds-x5xx-sch-28021-LBRW.dtsi | 4
6 files changed, 18 insertions(+)
diff --git a/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
index f4c557e69e6e..f208a02721e3 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-1xxx-sch-30842.dtsi
@@ -15,6 +15,7 @@
_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "usxgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@02}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
index 7d197c31814a..0a0926473541 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-8xxx-sch-24801.dtsi
@@ -14,6 +14,7 @@
_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
b/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
index 992092ec7838..94b5e765aedb 100644
--- a/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds--sch-24801-LBRW.dtsi
@@ -44,24 +44,28 @@
_felix_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
_felix_port1 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@50/phy@1c}>;
};
_felix_port2 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1e}>;
};
_felix_port3 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1f}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
b/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
index a905d77a9a71..bd46adfd2928 100644
--- a/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds--sch-24801.dtsi
@@ -29,24 +29,28 @@
_felix_port0 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1c}>;
};
_felix_port1 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1d}>;
};
_felix_port2 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1e}>;
};
_felix_port3 {
status = "okay";
+ managed = "in-band-status";
phy-mode = "sgmii";
phy-handle = <&{/soc/i2c@200/fpga@66/mux-mdio@54/mdio@40/phy@1f}>;
};
diff --git a/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
b/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
index 62e818f099ca..5909e7635a1a 100644
--- a/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds-x3xx-sch-30841-LBRW.dtsi
@@ -29,24 +29,28 @@
_fe
[PATCH v2 14/16] arm: dts: ls1028a-rdb: enable PCIe controllers from U-Boot dtsi
Reuse the scheme implemented by the Kontron SL28 boards in commit
d08011d7f9b4 ("arm: dts: ls1028a: disable the PCIe controller by
default") and move the 'status = "okay"' lines for the PCIe controllers
inside a separate U-Boot dtsi for the LS1028A-RDB board. This way, the
existing Linux device tree can simply be dropped in.
Signed-off-by: Vladimir Oltean
---
arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi | 15 +++
arch/arm/dts/fsl-ls1028a-rdb.dts | 9 +
2 files changed, 16 insertions(+), 8 deletions(-)
create mode 100644 arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
diff --git a/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
b/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
new file mode 100644
index ..a72b57305dc3
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-rdb-u-boot.dtsi
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright 2021 NXP */
+
+/*
+ * u-boot will enable the device in the linux device tree in place. Because
+ * we are using the linux device tree, we have to enable the PCI controller
+ * ourselves.
+ */
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 10070eab6e61..70fcf71dbd0e 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -9,6 +9,7 @@
/dts-v1/;
#include "fsl-ls1028a.dtsi"
+#include "fsl-ls1028a-rdb-u-boot.dtsi"
/ {
model = "NXP Layerscape 1028a RDB Board";
@@ -153,14 +154,6 @@
status = "okay";
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
{
status = "okay";
};
--
2.25.1
[PATCH v2 12/16] arm: dts: ls1028a-rdb: disable DSPI nodes
There is no SPI peripheral on the LS1028A-RDB, therefore no reason to
enable these nodes in the U-Boot device tree (and Linux does not enable
them either).
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-rdb.dts | 12
1 file changed, 12 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index b5e56b1c1f15..c5b95e169f5c 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -24,18 +24,6 @@
};
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
{
status = "okay";
};
--
2.25.1
[PATCH v2 11/16] arm: dts: ls1028a-rdb: sync Ethernet device tree nodes with Linux
In a bit of a blunder, the blamed commit in the Fixes: tag below made
the mscc_felix switch driver look at the 'managed = "in-band-status"'
device tree property, forgetting that the U-Boot device tree had not
been updated to include that property, whereas the Linux one does.
The switch is therefore described in the device tree as not requiring
in-band autoneg, but the PHY driver for VSC8514 (drivers/net/phy/mscc.c)
still enables that feature. This results in a mismatch => no traffic.
This patch is a copy-paste of the Ethernet device tree nodes from Linux,
which resolves that issue. The device tree update also renames the
Ethernet PHY labels.
Fixes: e3789a726269 ("net: dsa: felix: configure the in-band autoneg property
based on OF node info")
Cc: Ramon Fried
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-rdb.dts | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index 11bf7e5f627d..b5e56b1c1f15 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -45,33 +45,33 @@
};
_mdio_pf3 {
- status = "okay";
- rdb_phy0: phy@2 {
- reg = <2>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
};
- /* VSC8514 QSGMII PHY */
- sw_phy0: phy@10 {
+ /* VSC8514 QSGMII quad PHY */
+ qsgmii_phy0: ethernet-phy@10 {
reg = <0x10>;
};
- sw_phy1: phy@11 {
+ qsgmii_phy1: ethernet-phy@11 {
reg = <0x11>;
};
- sw_phy2: phy@12 {
+ qsgmii_phy2: ethernet-phy@12 {
reg = <0x12>;
};
- sw_phy3: phy@13 {
+ qsgmii_phy3: ethernet-phy@13 {
reg = <0x13>;
};
};
_port0 {
- status = "okay";
+ phy-handle = <_phy0>;
phy-mode = "sgmii";
- phy-handle = <_phy0>;
+ managed = "in-band-status";
+ status = "okay";
};
_port2 {
@@ -158,28 +158,32 @@
_felix_port0 {
label = "swp0";
- phy-handle = <_phy0>;
+ managed = "in-band-status";
+ phy-handle = <_phy0>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port1 {
label = "swp1";
- phy-handle = <_phy1>;
+ managed = "in-band-status";
+ phy-handle = <_phy1>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port2 {
label = "swp2";
- phy-handle = <_phy2>;
+ managed = "in-band-status";
+ phy-handle = <_phy2>;
phy-mode = "qsgmii";
status = "okay";
};
_felix_port3 {
label = "swp3";
- phy-handle = <_phy3>;
+ managed = "in-band-status";
+ phy-handle = <_phy3>;
phy-mode = "qsgmii";
status = "okay";
};
--
2.25.1
[PATCH v2 13/16] arm: dts: ls1028a-rdb: disable I2C buses 1 through 7
There is no I2C peripheral on these buses on the reference design board,
and the Linux device tree does not enable them either.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-rdb.dts | 28
1 file changed, 28 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index c5b95e169f5c..10070eab6e61 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -112,34 +112,6 @@
};
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
_felix {
status = "okay";
};
--
2.25.1
[PATCH v2 10/16] arm: dts: ls1028a-rdb: sort nodes alphabetically
The nodes in the NXP LS1028A-RDB device tree are out of order, regroup
them alphabetically to have a simple delta when the Linux device tree is
brought in.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-rdb.dts | 110 +++
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/arch/arm/dts/fsl-ls1028a-rdb.dts b/arch/arm/dts/fsl-ls1028a-rdb.dts
index ddb01db73f8e..11bf7e5f627d 100644
--- a/arch/arm/dts/fsl-ls1028a-rdb.dts
+++ b/arch/arm/dts/fsl-ls1028a-rdb.dts
@@ -36,6 +36,48 @@
status = "okay";
};
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+_mdio_pf3 {
+ status = "okay";
+ rdb_phy0: phy@2 {
+ reg = <2>;
+ };
+
+ /* VSC8514 QSGMII PHY */
+ sw_phy0: phy@10 {
+ reg = <0x10>;
+ };
+
+ sw_phy1: phy@11 {
+ reg = <0x11>;
+ };
+
+ sw_phy2: phy@12 {
+ reg = <0x12>;
+ };
+
+ sw_phy3: phy@13 {
+ reg = <0x13>;
+ };
+};
+
+_port0 {
+ status = "okay";
+ phy-mode = "sgmii";
+ phy-handle = <_phy0>;
+};
+
+_port2 {
+ status = "okay";
+};
+
{
status = "okay";
};
@@ -110,44 +152,6 @@
status = "okay";
};
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
- {
- status = "okay";
-};
-
-_port0 {
- status = "okay";
- phy-mode = "sgmii";
- phy-handle = <_phy0>;
-};
-
-_port2 {
- status = "okay";
-};
-
_felix {
status = "okay";
};
@@ -185,26 +189,22 @@
status = "okay";
};
-_mdio_pf3 {
+ {
status = "okay";
- rdb_phy0: phy@2 {
- reg = <2>;
- };
+};
- /* VSC8514 QSGMII PHY */
- sw_phy0: phy@10 {
- reg = <0x10>;
- };
+ {
+ status = "okay";
+};
- sw_phy1: phy@11 {
- reg = <0x11>;
- };
+ {
+ status = "okay";
+};
- sw_phy2: phy@12 {
- reg = <0x12>;
- };
+ {
+ status = "okay";
+};
- sw_phy3: phy@13 {
- reg = <0x13>;
- };
+ {
+ status = "okay";
};
--
2.25.1
[PATCH v2 08/16] arm: dts: lx2160a-rdb: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LX2160A-RDB to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-lx2160a-rdb.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-lx2160a-rdb.dts b/arch/arm/dts/fsl-lx2160a-rdb.dts
index 5fbdd907017c..8ca4afa7eaea 100644
--- a/arch/arm/dts/fsl-lx2160a-rdb.dts
+++ b/arch/arm/dts/fsl-lx2160a-rdb.dts
@@ -117,7 +117,7 @@
status = "okay";
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v2 05/16] arm: dts: lx2160a-qds: use Linux compatible string for RTC
During the LS1028A-RDB sync with Linux device trees, it was observed
that the same RTC is present on the two boards, and the wrong compatible
string is used in both places. This change updates the RTC from the
LX2160A-QDS to use the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-lx2160a-qds.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-lx2160a-qds.dtsi
b/arch/arm/dts/fsl-lx2160a-qds.dtsi
index 288607c0347b..69e11cca2da1 100644
--- a/arch/arm/dts/fsl-lx2160a-qds.dtsi
+++ b/arch/arm/dts/fsl-lx2160a-qds.dtsi
@@ -250,7 +250,7 @@
reg = <0x3>;
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
[PATCH v2 06/16] arm: dts: ls1028a-qds: use Linux compatible string for RTC
The LS1028A-QDS board won't be synced with the Linux device trees right
now, since those are currently still in progress (Ethernet is missing).
However, while we're at converting the RDB, it can be observed that the
same RTC is present on the two boards, and the wrong compatible string
is used in both places. This change updates the RTC from the QDS to use
the compatible string that was established in Linux.
Signed-off-by: Vladimir Oltean
Reviewed-by: Priyanka Jain
---
arch/arm/dts/fsl-ls1028a-qds.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/fsl-ls1028a-qds.dtsi
b/arch/arm/dts/fsl-ls1028a-qds.dtsi
index 0da0a7bc5db8..3b063d0257de 100644
--- a/arch/arm/dts/fsl-ls1028a-qds.dtsi
+++ b/arch/arm/dts/fsl-ls1028a-qds.dtsi
@@ -195,7 +195,7 @@
status = "okay";
rtc@51 {
- compatible = "pcf2127-rtc";
+ compatible = "nxp,pcf2129";
reg = <0x51>;
};
};
--
2.25.1
