| Message ID | 20221201003109.448693-1-tharvey@gateworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ARM: dts: imx6qdl-gw5904: add internal mdio nodes | expand |
Hi Tim, [Adding Andrew] On Wed, Nov 30, 2022 at 9:31 PM Tim Harvey <tharvey@gateworks.com> wrote: > > Complete the switch definition by adding the internal mdio nodes. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > arch/arm/boot/dts/imx6qdl-gw5904.dtsi | 35 +++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > index 612b6e068e28..08463e65dca3 100644 > --- a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > @@ -212,6 +212,27 @@ switch@0 { > 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,41 @@ ports { > port@0 { > reg = <0>; > label = "lan4"; > + phy-handle = <&sw_phy0>; > + phy-mode = "internal"; > }; > > port@1 { > reg = <1>; > label = "lan3"; > + phy-handle = <&sw_phy1>; > + phy-mode = "internal"; > }; > > port@2 { > reg = <2>; > label = "lan2"; > + phy-handle = <&sw_phy2>; > + phy-mode = "internal"; > }; > > port@3 { > reg = <3>; > label = "lan1"; > + phy-handle = <&sw_phy3>; > + phy-mode = "internal"; > }; > > port@5 { > reg = <5>; > label = "cpu"; > ethernet = <&fec>; > + phy-mode = "rgmii-id"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > }; > }; > }; > -- > 2.25.1 >
On Thu, Dec 01, 2022 at 10:02:08PM -0300, Fabio Estevam wrote: > Hi Tim, > > [Adding Andrew] It is not wrong, but it should also mostly not be needed. The switch driver can link internal PHYs to ports. > > port@5 { > > reg = <5>; > > label = "cpu"; > > ethernet = <&fec>; > > + phy-mode = "rgmii-id"; > > + > > + fixed-link { > > + speed = <1000>; > > + full-duplex; > > + }; > > }; This part is needed to make a warning go away. Does the SoC network interface have phy-mode = "rgmii"; ? Andrew
On Fri, Dec 2, 2022 at 5:08 AM Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, Dec 01, 2022 at 10:02:08PM -0300, Fabio Estevam wrote: > > Hi Tim, > > > > [Adding Andrew] > > It is not wrong, but it should also mostly not be needed. The switch > driver can link internal PHYs to ports. Andrew, I should have mentioned in the commit log that this does not change behavior on Linux but is required for boot firmware. Specifically U-Boot requires the internal PHY ports to be defined for its DSA architecture and they share dt's as much as possible. > > > > port@5 { > > > reg = <5>; > > > label = "cpu"; > > > ethernet = <&fec>; > > > + phy-mode = "rgmii-id"; > > > + > > > + fixed-link { > > > + speed = <1000>; > > > + full-duplex; > > > + }; > > > }; > > This part is needed to make a warning go away. Does the SoC network interface > have phy-mode = "rgmii"; ? No, it looks like this: &fec { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_enet>; phy-mode = "rgmii-id"; status = "okay"; fixed-link { speed = <1000>; full-duplex; }; mdio { #address-cells = <1>; #size-cells = <0>; switch@0 { compatible = "marvell,mv88e6085"; reg = <0>; ... Is something here wrong? Best Regards, Tim
> > > > port@5 { > > > > reg = <5>; > > > > label = "cpu"; > > > > ethernet = <&fec>; > > > > + phy-mode = "rgmii-id"; > > > > + > > > > + fixed-link { > > > > + speed = <1000>; > > > > + full-duplex; > > > > + }; > > > > }; > > > > This part is needed to make a warning go away. Does the SoC network interface > > have phy-mode = "rgmii"; ? > > No, it looks like this: > > &fec { > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_enet>; > phy-mode = "rgmii-id"; > Is something here wrong? It suggests both ends should insert RGMII delays. So you will end up with double delays. Have one end say plain "rgmii" and the other "rgmii-id". I would probably go with the FEC being "rgmii". Andrew
On Fri, Dec 2, 2022 at 9:31 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > port@5 { > > > > > reg = <5>; > > > > > label = "cpu"; > > > > > ethernet = <&fec>; > > > > > + phy-mode = "rgmii-id"; > > > > > + > > > > > + fixed-link { > > > > > + speed = <1000>; > > > > > + full-duplex; > > > > > + }; > > > > > }; > > > > > > This part is needed to make a warning go away. Does the SoC network interface > > > have phy-mode = "rgmii"; ? > > > > No, it looks like this: > > > > &fec { > > pinctrl-names = "default"; > > pinctrl-0 = <&pinctrl_enet>; > > phy-mode = "rgmii-id"; > > > Is something here wrong? > > It suggests both ends should insert RGMII delays. So you will end up > with double delays. Have one end say plain "rgmii" and the other > "rgmii-id". I would probably go with the FEC being "rgmii". > > Andrew Andrew, That makes sense - I will change the fec node to rgmii. Upon further testing I find there is something else wrong with this patch however that I don't yet understand. Without it the switch works fine (due to RGMII delay being configured via boot firmware) but I do get the warning you had mentioned due to the phy-mode/phy-handle props missing: mv88e6085 2188000.ethernet-1:00: switch 0x1760 detected: Marvell 88E6176, revision 1 mv88e6085 2188000.ethernet-1:00: OF node /soc/bus@2100000/ethernet@2188000/mdio/switch@0/ports/port@5 of CPU port 5 lacks the required "phy-mode" property mv88e6085 2188000.ethernet-1:00: OF node /soc/bus@2100000/ethernet@2188000/mdio/switch@0/ports/port@5 of CPU port 5 lacks the required "phy-handle", "fixed-link" or "managed" properties mv88e6085 2188000.ethernet-1:00: Skipping phylink registration for CPU port 5 mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): PHY [mv88e6xxx-1:00] driver [Generic PHY] (irq=POLL) mv88e6085 2188000.ethernet-1:00 lan3 (uninitialized): PHY [mv88e6xxx-1:01] driver [Generic PHY] (irq=POLL) mv88e6085 2188000.ethernet-1:00 lan2 (uninitialized): PHY [mv88e6xxx-1:02] driver [Generic PHY] (irq=POLL) mv88e6085 2188000.ethernet-1:00 lan1 (uninitialized): PHY [mv88e6xxx-1:03] driver [Generic PHY] (irq=POLL) When I add the phy-mode/phy-handle props with this patch I get the following failure: mv88e6085 2188000.ethernet-1:00: switch 0x1760 detected: Marvell 88E6176, revision 1 mv88e6085 2188000.ethernet-1:00: switch 0x1760 detected: Marvell 88E6176, revision 1 mv88e6085 2188000.ethernet-1:00: configuring for fixed/rgmii-id link mode mv88e6085 2188000.ethernet-1:00: p5: delay RXCLK yes, TXCLK yes mv88e6085 2188000.ethernet-1:00: p5: delay RXCLK yes, TXCLK yes mv88e6085 2188000.ethernet-1:00: Link is Up - 1Gbps/Full - flow control off mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): failed to connect to PHY: -EINVAL mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 0 mv88e6085 2188000.ethernet-1:00 lan3 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL mv88e6085 2188000.ethernet-1:00 lan3 (uninitialized): failed to connect to PHY: -EINVAL mv88e6085 2188000.ethernet-1:00 lan3 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 1 mv88e6085 2188000.ethernet-1:00 lan2 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL mv88e6085 2188000.ethernet-1:00 lan2 (uninitialized): failed to connect to PHY: -EINVAL mv88e6085 2188000.ethernet-1:00 lan2 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 2 mv88e6085 2188000.ethernet-1:00 lan1 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL mv88e6085 2188000.ethernet-1:00 lan1 (uninitialized): failed to connect to PHY: -EINVAL mv88e6085 2188000.ethernet-1:00 lan1 (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 3 I've run into this message before and had a hard time understanding the issue from the message - it seems to indicate the phy status matches advertisement but that its an invalid mode? Thanks, Tim
> mv88e6085 2188000.ethernet-1:00: Skipping phylink registration for CPU port 5 > mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): PHY > [mv88e6xxx-1:00] driver [Generic PHY] (irq=POLL) > mv88e6085 2188000.ethernet-1:00 lan3 (uninitialized): PHY > [mv88e6xxx-1:01] driver [Generic PHY] (irq=POLL) > mv88e6085 2188000.ethernet-1:00 lan2 (uninitialized): PHY > [mv88e6xxx-1:02] driver [Generic PHY] (irq=POLL) > mv88e6085 2188000.ethernet-1:00 lan1 (uninitialized): PHY > [mv88e6xxx-1:03] driver [Generic PHY] (irq=POLL) Hi Tim You are using the generic PHY driver, not the Marvell driver. I suggest you fix that first. See if that causes the problem to go away. Andrew
Hi Tim, On Fri, Dec 02, 2022 at 02:29:20PM -0800, Tim Harvey wrote: > When I add the phy-mode/phy-handle props with this patch I get the > following failure: > mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL > mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): failed to connect to PHY: -EINVAL > > I've run into this message before and had a hard time understanding > the issue from the message - it seems to indicate the phy status > matches advertisement but that its an invalid mode? Does this patch help? From 6bd79d9b9994fcb7762301fe297f963c272d9210 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Mon, 5 Dec 2022 19:05:24 +0200 Subject: [PATCH] net: dsa: mv88e6xxx: accept phy-mode = "internal" for internal PHY ports The ethernet-controller dt-schema, mostly evolved by Linux, has the "internal" PHY mode for connections between a MAC and internal PHY. U-Boot may provide device tree blobs where this phy-mode is specified, so make the Linux driver accept them. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index ccfa4751d3b7..ba4fff8690aa 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -833,10 +833,13 @@ static void mv88e6xxx_get_caps(struct dsa_switch *ds, int port, chip->info->ops->phylink_get_caps(chip, port, config); - /* Internal ports need GMII for PHYLIB */ - if (mv88e6xxx_phy_is_internal(ds, port)) + if (mv88e6xxx_phy_is_internal(ds, port)) { + __set_bit(PHY_INTERFACE_MODE_INTERNAL, + config->supported_interfaces); + /* Internal ports with no phy-mode need GMII for PHYLIB */ __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces); + } } static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
On Wed, Nov 30, 2022 at 04:31:08PM -0800, Tim Harvey wrote: > Complete the switch definition by adding the internal mdio nodes. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> > arch/arm/boot/dts/imx6qdl-gw5904.dtsi | 35 +++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > index 612b6e068e28..08463e65dca3 100644 > --- a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > +++ b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi > @@ -212,6 +212,27 @@ switch@0 { > 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,41 @@ ports { > port@0 { > reg = <0>; > label = "lan4"; > + phy-handle = <&sw_phy0>; > + phy-mode = "internal"; > }; > > port@1 { > reg = <1>; > label = "lan3"; > + phy-handle = <&sw_phy1>; > + phy-mode = "internal"; > }; > > port@2 { > reg = <2>; > label = "lan2"; > + phy-handle = <&sw_phy2>; > + phy-mode = "internal"; > }; > > port@3 { > reg = <3>; > label = "lan1"; > + phy-handle = <&sw_phy3>; > + phy-mode = "internal"; > }; > > port@5 { > reg = <5>; > label = "cpu"; The patch context here might conflict with Arınç Ünal's effort to remove label = "cpu" from everywhere. https://patchwork.kernel.org/project/netdevbpf/cover/20221130141040.32447-1-arinc.unal@arinc9.com/ > ethernet = <&fec>; > + phy-mode = "rgmii-id"; > + > + fixed-link { > + speed = <1000>; > + full-duplex; > + }; > }; > }; > }; > -- > 2.25.1 >
On Mon, Dec 5, 2022 at 9:10 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > Hi Tim, > > On Fri, Dec 02, 2022 at 02:29:20PM -0800, Tim Harvey wrote: > > When I add the phy-mode/phy-handle props with this patch I get the > > following failure: > > mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): validation of internal with support 00000000,00000000,000062ff and advertisement 00000000,00000000,000062ff failed: -EINVAL > > mv88e6085 2188000.ethernet-1:00 lan4 (uninitialized): failed to connect to PHY: -EINVAL > > > > I've run into this message before and had a hard time understanding > > the issue from the message - it seems to indicate the phy status > > matches advertisement but that its an invalid mode? > > Does this patch help? > > From 6bd79d9b9994fcb7762301fe297f963c272d9210 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Mon, 5 Dec 2022 19:05:24 +0200 > Subject: [PATCH] net: dsa: mv88e6xxx: accept phy-mode = "internal" for > internal PHY ports > > The ethernet-controller dt-schema, mostly evolved by Linux, has the > "internal" PHY mode for connections between a MAC and internal PHY. > > U-Boot may provide device tree blobs where this phy-mode is specified, > so make the Linux driver accept them. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index ccfa4751d3b7..ba4fff8690aa 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -833,10 +833,13 @@ static void mv88e6xxx_get_caps(struct dsa_switch *ds, int port, > > chip->info->ops->phylink_get_caps(chip, port, config); > > - /* Internal ports need GMII for PHYLIB */ > - if (mv88e6xxx_phy_is_internal(ds, port)) > + if (mv88e6xxx_phy_is_internal(ds, port)) { > + __set_bit(PHY_INTERFACE_MODE_INTERNAL, > + config->supported_interfaces); > + /* Internal ports with no phy-mode need GMII for PHYLIB */ > __set_bit(PHY_INTERFACE_MODE_GMII, > config->supported_interfaces); > + } > } > > static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port, > -- > 2.34.1 > Vladimir, Yes, this patch resolves the issue. Enabling CONFIG_MARVELL_PHY without this patch still shows the issue. Thanks - please cc me on that and I'll respond (unless you want me to send it). I'll submit a v2 of my dt patch with fec phy-mode = 'rgmii' after this patch lands. Best Regards, Tim
On Mon, Dec 05, 2022 at 10:01:35AM -0800, Tim Harvey wrote: > Vladimir, > > Yes, this patch resolves the issue. Enabling CONFIG_MARVELL_PHY > without this patch still shows the issue. Yeah, that makes sense considering what the issue is. > Thanks - please cc me on that and I'll respond (unless you want me to send it). > > I'll submit a v2 of my dt patch with fec phy-mode = 'rgmii' after this > patch lands. The question is how far to backport this change. I see that the GW5904 board was introduced in 2017. Which kernels do you need to support the complete/unambiguous dt-binding? I'm tempted to submit the patch to the "net" tree (making it a candidate for stable releases) but without a Fixes: tag, making it go as far as it will. That would be down to kernel 5.18, where the mv88e6xxx_get_caps() function was introduced. Further down, and we'd need different patches for older stable trees.
On Mon, Dec 5, 2022 at 11:02 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Mon, Dec 05, 2022 at 10:01:35AM -0800, Tim Harvey wrote: > > Vladimir, > > > > Yes, this patch resolves the issue. Enabling CONFIG_MARVELL_PHY > > without this patch still shows the issue. > > Yeah, that makes sense considering what the issue is. > > > Thanks - please cc me on that and I'll respond (unless you want me to send it). > > > > I'll submit a v2 of my dt patch with fec phy-mode = 'rgmii' after this > > patch lands. > > The question is how far to backport this change. > > I see that the GW5904 board was introduced in 2017. Which kernels do you > need to support the complete/unambiguous dt-binding? before adding the internal PHY nodes it works fine so no backporting needed. I just don't want to add the complete/unambiguous dt-bindings until your patch lands as at that point it would otherwise be broken. Best Regards, Tim > > I'm tempted to submit the patch to the "net" tree (making it a candidate > for stable releases) but without a Fixes: tag, making it go as far as it > will. That would be down to kernel 5.18, where the mv88e6xxx_get_caps() > function was introduced. Further down, and we'd need different patches > for older stable trees.
On Mon, Dec 05, 2022 at 11:10:45AM -0800, Tim Harvey wrote: > before adding the internal PHY nodes it works fine so no backporting > needed. I just don't want to add the complete/unambiguous dt-bindings > until your patch lands as at that point it would otherwise be broken. Right, but there is some debate on whether device tree updates should require new kernel or not. Since in this case, kernels 5.18 and newer can trivially support the updated device trees with this change, I think it would be silly not to let it be backported. As for older kernels, it really depends on what the board ships with.
On Mon, Dec 5, 2022 at 11:15 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On Mon, Dec 05, 2022 at 11:10:45AM -0800, Tim Harvey wrote: > > before adding the internal PHY nodes it works fine so no backporting > > needed. I just don't want to add the complete/unambiguous dt-bindings > > until your patch lands as at that point it would otherwise be broken. > > Right, but there is some debate on whether device tree updates should > require new kernel or not. Since in this case, kernels 5.18 and newer > can trivially support the updated device trees with this change, I think > it would be silly not to let it be backported. As for older kernels, it > really depends on what the board ships with. I see. So then your patch should be backported to when the phy validation was added which would be d4ebf12bcec4 ("net: dsa: mv88e6xxx: populate supported_interfaces and mac_capabilities") then right? Best Regards, Tim
On Mon, Dec 05, 2022 at 11:24:39AM -0800, Tim Harvey wrote: > I see. So then your patch should be backported to when the phy > validation was added which would be d4ebf12bcec4 ("net: dsa: > mv88e6xxx: populate supported_interfaces and mac_capabilities") then > right? Yup. Actually I suspect that before the commit you're indicating, the mv88e6xxx driver was actually accepting the "internal" phy-mode.
diff --git a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi index 612b6e068e28..08463e65dca3 100644 --- a/arch/arm/boot/dts/imx6qdl-gw5904.dtsi +++ b/arch/arm/boot/dts/imx6qdl-gw5904.dtsi @@ -212,6 +212,27 @@ switch@0 { 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,41 @@ ports { port@0 { reg = <0>; label = "lan4"; + phy-handle = <&sw_phy0>; + phy-mode = "internal"; }; port@1 { reg = <1>; label = "lan3"; + phy-handle = <&sw_phy1>; + phy-mode = "internal"; }; port@2 { reg = <2>; label = "lan2"; + phy-handle = <&sw_phy2>; + phy-mode = "internal"; }; port@3 { reg = <3>; label = "lan1"; + phy-handle = <&sw_phy3>; + phy-mode = "internal"; }; port@5 { reg = <5>; label = "cpu"; ethernet = <&fec>; + phy-mode = "rgmii-id"; + + fixed-link { + speed = <1000>; + full-duplex; + }; }; }; };
Complete the switch definition by adding the internal mdio nodes. Signed-off-by: Tim Harvey <tharvey@gateworks.com> --- arch/arm/boot/dts/imx6qdl-gw5904.dtsi | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)