| Message ID | E1iTo7N-0005Sj-Nk@rmk-PC.armlinux.org.uk |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add support for SFPs behind PHYs | expand |
On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > Add core phylib help for supporting SFP sockets on PHYs. This provides > a mechanism to inform the SFP layer about PHY up/down events, and also > unregister the SFP bus when the PHY is going away. Hi Russell What does the device tree binding look like? I think you have SFP proprieties in the PHYs node? Thanks Andrew
On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > a mechanism to inform the SFP layer about PHY up/down events, and also > > unregister the SFP bus when the PHY is going away. > > Hi Russell > > What does the device tree binding look like? I think you have SFP > proprieties in the PHYs node? Correct, just the same as network devices. Hmm, however, neither are documented... oh dear, it looks like I need to figure out how this yaml stuff works. :(
On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > unregister the SFP bus when the PHY is going away. > > > > Hi Russell > > > > What does the device tree binding look like? I think you have SFP > > proprieties in the PHYs node? > > Correct, just the same as network devices. Hmm, however, neither are > documented... oh dear, it looks like I need to figure out how this > yaml stuff works. :( Yes, that would be good. I also assume you have at least one DT patch for one of the Marvell boards? Seeing that would also help. FYI: It is good to see this feature added. It has been blocked for a long time, but this implementation is actually nice and simple. Thanks Andrew
On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > unregister the SFP bus when the PHY is going away. > > > > > > Hi Russell > > > > > > What does the device tree binding look like? I think you have SFP > > > proprieties in the PHYs node? > > > > Correct, just the same as network devices. Hmm, however, neither are > > documented... oh dear, it looks like I need to figure out how this > > yaml stuff works. :( > > Yes, that would be good. I also assume you have at least one DT patch > for one of the Marvell boards? Seeing that would also help. There's no need, it was already added by bootlin back in May 2018. arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
On Sun, Nov 10, 2019 at 05:08:34PM +0000, Russell King - ARM Linux admin wrote: > On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > > unregister the SFP bus when the PHY is going away. > > > > > > > > Hi Russell > > > > > > > > What does the device tree binding look like? I think you have SFP > > > > proprieties in the PHYs node? > > > > > > Correct, just the same as network devices. Hmm, however, neither are > > > documented... oh dear, it looks like I need to figure out how this > > > yaml stuff works. :( > > > > Yes, that would be good. I also assume you have at least one DT patch > > for one of the Marvell boards? Seeing that would also help. > > There's no need, it was already added by bootlin back in May 2018. Ah. Lucky for them the binding works :-) Andrew
On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > unregister the SFP bus when the PHY is going away. > > > > > > Hi Russell > > > > > > What does the device tree binding look like? I think you have SFP > > > proprieties in the PHYs node? > > > > Correct, just the same as network devices. Hmm, however, neither are > > documented... oh dear, it looks like I need to figure out how this > > yaml stuff works. :( > > Yes, that would be good. I also assume you have at least one DT patch > for one of the Marvell boards? Seeing that would also help. So, how does one make sure that the .yaml files are correct? The obvious thing is to use the "dtbs_check" target, described by Documentation/devicetree/writing-schema.md, but that's far from trivial. First it complained about lack of libyaml development, which is easy to solve. Having given it that, "dtbs_check" now complains: /bin/sh: 1: dt-doc-validate: not found /bin/sh: 1: dt-mk-schema: not foundmake[2]: *** [...Documentation/devicetree/bindings/Makefile:12: Documentation/devicetree/bindings/arm/stm32/stm32.example.dts] Error 127 (spot the lack of newline character - which is obviously an entirely different problem...) # apt search dt-doc-validate Sorting... Done Full Text Search... Done # apt search dt-mk-schema Sorting... Done Full Text Search... Done Searching google, it appears it needs another git repository (https://github.com/robherring/dt-schema/) from Rob Herring to use this stuff, which is totally undocumented in the kernel tree. Rob, can we have some _decent_ documentation on how to use this stuff please, particularly something that describes what the external dependencies are and where to get the necessary code from - it's rather unfair to make documentation of DT properties a requirement for integration into the kernel, convert the documentation to .yaml format, and not tell people how they can sensibly validate it. Thanks.
On Mon, Nov 11, 2019 at 02:01:14PM +0000, Russell King - ARM Linux admin wrote: > So, how does one make sure that the .yaml files are correct? > > The obvious thing is to use the "dtbs_check" target, described by > Documentation/devicetree/writing-schema.md, but that's far from > trivial. > > First it complained about lack of libyaml development, which is easy > to solve. Having given it that, "dtbs_check" now complains: > > /bin/sh: 1: dt-doc-validate: not found > /bin/sh: 1: dt-mk-schema: not foundmake[2]: *** > [...Documentation/devicetree/bindings/Makefile:12: > Documentation/devicetree/bindings/arm/stm32/stm32.example.dts] Error 127 > > (spot the lack of newline character - which is obviously an entirely > different problem...) > > # apt search dt-doc-validate > Sorting... Done > Full Text Search... Done > # apt search dt-mk-schema > Sorting... Done > Full Text Search... Done > > Searching google, it appears it needs another git repository > (https://github.com/robherring/dt-schema/) from Rob Herring to use > this stuff, which is totally undocumented in the kernel tree. Okay, that just seems to be "examples" rather than something which is installable to provide the necessary commands. So, I'm afraid I've no idea how I can change any of the .yaml files and know that the changes are in fact correct. Given that, I think we need to drop the rule that DT bindings are documented prior to being merged into the kernel until there is some decent and full documentation.
On Mon, Nov 11, 2019 at 8:01 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > > unregister the SFP bus when the PHY is going away. > > > > > > > > Hi Russell > > > > > > > > What does the device tree binding look like? I think you have SFP > > > > proprieties in the PHYs node? > > > > > > Correct, just the same as network devices. Hmm, however, neither are > > > documented... oh dear, it looks like I need to figure out how this > > > yaml stuff works. :( > > > > Yes, that would be good. I also assume you have at least one DT patch > > for one of the Marvell boards? Seeing that would also help. > > So, how does one make sure that the .yaml files are correct? > > The obvious thing is to use the "dtbs_check" target, described by > Documentation/devicetree/writing-schema.md, but that's far from > trivial. 'dt_binding_check' will check just the bindings. 'dtbs_check' is pretty slow given we have so many dts files and it generates lots of warnings. > First it complained about lack of libyaml development, which is easy > to solve. Having given it that, "dtbs_check" now complains: > > /bin/sh: 1: dt-doc-validate: not found > /bin/sh: 1: dt-mk-schema: not foundmake[2]: *** > [...Documentation/devicetree/bindings/Makefile:12: > Documentation/devicetree/bindings/arm/stm32/stm32.example.dts] Error 127 > > (spot the lack of newline character - which is obviously an entirely > different problem...) > > # apt search dt-doc-validate > Sorting... Done > Full Text Search... Done > # apt search dt-mk-schema > Sorting... Done > Full Text Search... Done > > Searching google, it appears it needs another git repository > (https://github.com/robherring/dt-schema/) from Rob Herring to use > this stuff, which is totally undocumented in the kernel tree. The dependencies are all documented in writing-schema.rst (formerly .md) in the 'Dependencies' section. TL;DR: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master Rob
On Mon, Nov 11, 2019 at 09:01:22AM -0600, Rob Herring wrote: > On Mon, Nov 11, 2019 at 8:01 AM Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > > > On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > > > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > > > unregister the SFP bus when the PHY is going away. > > > > > > > > > > Hi Russell > > > > > > > > > > What does the device tree binding look like? I think you have SFP > > > > > proprieties in the PHYs node? > > > > > > > > Correct, just the same as network devices. Hmm, however, neither are > > > > documented... oh dear, it looks like I need to figure out how this > > > > yaml stuff works. :( > > > > > > Yes, that would be good. I also assume you have at least one DT patch > > > for one of the Marvell boards? Seeing that would also help. > > > > So, how does one make sure that the .yaml files are correct? > > > > The obvious thing is to use the "dtbs_check" target, described by > > Documentation/devicetree/writing-schema.md, but that's far from > > trivial. > > 'dt_binding_check' will check just the bindings. 'dtbs_check' is > pretty slow given we have so many dts files and it generates lots of > warnings. > > > First it complained about lack of libyaml development, which is easy > > to solve. Having given it that, "dtbs_check" now complains: > > > > /bin/sh: 1: dt-doc-validate: not found > > /bin/sh: 1: dt-mk-schema: not foundmake[2]: *** > > [...Documentation/devicetree/bindings/Makefile:12: > > Documentation/devicetree/bindings/arm/stm32/stm32.example.dts] Error 127 > > > > (spot the lack of newline character - which is obviously an entirely > > different problem...) > > > > # apt search dt-doc-validate > > Sorting... Done > > Full Text Search... Done > > # apt search dt-mk-schema > > Sorting... Done > > Full Text Search... Done > > > > Searching google, it appears it needs another git repository > > (https://github.com/robherring/dt-schema/) from Rob Herring to use > > this stuff, which is totally undocumented in the kernel tree. > > The dependencies are all documented in writing-schema.rst (formerly > .md) in the 'Dependencies' section. > > TL;DR: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master What is pip3, and where do I get it from (I'm running Debian stable). # apt search pip3 Sorting... Done Full Text Search... Done Don't expect people know python.
On Mon, Nov 11, 2019 at 9:08 AM Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Mon, Nov 11, 2019 at 09:01:22AM -0600, Rob Herring wrote: > > On Mon, Nov 11, 2019 at 8:01 AM Russell King - ARM Linux admin > > <linux@armlinux.org.uk> wrote: > > > > > > On Sun, Nov 10, 2019 at 06:00:40PM +0100, Andrew Lunn wrote: > > > > On Sun, Nov 10, 2019 at 04:40:07PM +0000, Russell King - ARM Linux admin wrote: > > > > > On Sun, Nov 10, 2019 at 05:13:07PM +0100, Andrew Lunn wrote: > > > > > > On Sun, Nov 10, 2019 at 02:23:05PM +0000, Russell King wrote: > > > > > > > Add core phylib help for supporting SFP sockets on PHYs. This provides > > > > > > > a mechanism to inform the SFP layer about PHY up/down events, and also > > > > > > > unregister the SFP bus when the PHY is going away. > > > > > > > > > > > > Hi Russell > > > > > > > > > > > > What does the device tree binding look like? I think you have SFP > > > > > > proprieties in the PHYs node? > > > > > > > > > > Correct, just the same as network devices. Hmm, however, neither are > > > > > documented... oh dear, it looks like I need to figure out how this > > > > > yaml stuff works. :( > > > > > > > > Yes, that would be good. I also assume you have at least one DT patch > > > > for one of the Marvell boards? Seeing that would also help. > > > > > > So, how does one make sure that the .yaml files are correct? > > > > > > The obvious thing is to use the "dtbs_check" target, described by > > > Documentation/devicetree/writing-schema.md, but that's far from > > > trivial. > > > > 'dt_binding_check' will check just the bindings. 'dtbs_check' is > > pretty slow given we have so many dts files and it generates lots of > > warnings. > > > > > First it complained about lack of libyaml development, which is easy > > > to solve. Having given it that, "dtbs_check" now complains: > > > > > > /bin/sh: 1: dt-doc-validate: not found > > > /bin/sh: 1: dt-mk-schema: not foundmake[2]: *** > > > [...Documentation/devicetree/bindings/Makefile:12: > > > Documentation/devicetree/bindings/arm/stm32/stm32.example.dts] Error 127 > > > > > > (spot the lack of newline character - which is obviously an entirely > > > different problem...) > > > > > > # apt search dt-doc-validate > > > Sorting... Done > > > Full Text Search... Done > > > # apt search dt-mk-schema > > > Sorting... Done > > > Full Text Search... Done > > > > > > Searching google, it appears it needs another git repository > > > (https://github.com/robherring/dt-schema/) from Rob Herring to use > > > this stuff, which is totally undocumented in the kernel tree. > > > > The dependencies are all documented in writing-schema.rst (formerly > > .md) in the 'Dependencies' section. > > > > TL;DR: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master > > What is pip3, and where do I get it from (I'm running Debian stable). apt install python3-pip This is not in the documentation because it is distro specific. > # apt search pip3 > Sorting... Done > Full Text Search... Done > > Don't expect people know python. How about rewording: 'The DT schema project can be installed with pip:' to this: 'The DT schema project is a Python project and is most easily installed using the Python package manager "pip": pip3 install git+https://github.com/devicetree-org/dt-schema.git@master It is also possible to install by cloning the git repository and running "setup.py". ' Googling 'python pip' or 'setup.py' should get folks the rest of the way if 'pip' or 'setup.py' is new to them. Rob
On Mon, Nov 11, 2019 at 09:01:22AM -0600, Rob Herring wrote: > The dependencies are all documented in writing-schema.rst (formerly > .md) in the 'Dependencies' section. > > TL;DR: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master I've mentioned it in a few IRC forums, and it seems that I'm not the only one who has trouble with this - so, this DT schema stuff totally fails the "keep it simple" approach, which is bad news if you want people to use it. It's a blocker when you have to do something with these .yaml files - or take the approach that you'd write something that looks write and hope that someone else checks it. So, to help people, please consider adding to that section information such as: 8<==== For debian stable (buster), you will need to install: - python3-pip for the pip3 tool. You do not need other python3 packages, as the pip3 tool will install the latest versions of any dependent packages into your ~/.local directory. However, note that these will not be kept up to date by your distribution. 8<==== It may be useful if the instructions for other distros are also added.
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 0a314cf45408..cb9ab24ee898 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -23,6 +23,7 @@ #include <linux/ethtool.h> #include <linux/phy.h> #include <linux/phy_led_triggers.h> +#include <linux/sfp.h> #include <linux/workqueue.h> #include <linux/mdio.h> #include <linux/io.h> @@ -866,6 +867,9 @@ void phy_stop(struct phy_device *phydev) mutex_lock(&phydev->lock); + if (phydev->sfp_bus) + sfp_upstream_stop(phydev->sfp_bus); + phydev->state = PHY_HALTED; mutex_unlock(&phydev->lock); @@ -900,6 +904,9 @@ void phy_start(struct phy_device *phydev) goto out; } + if (phydev->sfp_bus) + sfp_upstream_start(phydev->sfp_bus); + /* if phy was suspended, bring the physical link up again */ __phy_resume(phydev); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 9ddfb8b0ce04..ff47ec245100 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -27,6 +27,7 @@ #include <linux/bitmap.h> #include <linux/phy.h> #include <linux/phy_led_triggers.h> +#include <linux/sfp.h> #include <linux/mdio.h> #include <linux/io.h> #include <linux/uaccess.h> @@ -1174,6 +1175,65 @@ phy_standalone_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(phy_standalone); +/** + * phy_sfp_attach - attach the SFP bus to the PHY upstream network device + * @upstream: pointer to the phy device + * @bus: sfp bus representing cage being attached + * + * This is used to fill in the sfp_upstream_ops .attach member. + */ +void phy_sfp_attach(void *upstream, struct sfp_bus *bus) +{ + struct phy_device *phydev = upstream; + + if (phydev->attached_dev) + phydev->attached_dev->sfp_bus = bus; + phydev->sfp_bus_attached = true; +} +EXPORT_SYMBOL(phy_sfp_attach); + +/** + * phy_sfp_detach - detach the SFP bus from the PHY upstream network device + * @upstream: pointer to the phy device + * @bus: sfp bus representing cage being attached + * + * This is used to fill in the sfp_upstream_ops .detach member. + */ +void phy_sfp_detach(void *upstream, struct sfp_bus *bus) +{ + struct phy_device *phydev = upstream; + + if (phydev->attached_dev) + phydev->attached_dev->sfp_bus = NULL; + phydev->sfp_bus_attached = false; +} +EXPORT_SYMBOL(phy_sfp_detach); + +/** + * phy_sfp_probe - probe for a SFP cage attached to this PHY device + * @phydev: Pointer to phy_device + * @ops: SFP's upstream operations + */ +int phy_sfp_probe(struct phy_device *phydev, + const struct sfp_upstream_ops *ops) +{ + struct sfp_bus *bus; + int ret; + + if (phydev->mdio.dev.fwnode) { + bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode); + if (IS_ERR(bus)) + return PTR_ERR(bus); + + phydev->sfp_bus = bus; + + ret = sfp_bus_add_upstream(bus, phydev, ops); + sfp_bus_put(bus); + } + return 0; +} +EXPORT_SYMBOL(phy_sfp_probe); + /** * phy_attach_direct - attach a network device to a given PHY device pointer * @dev: network device to attach @@ -1249,6 +1309,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, if (dev) { phydev->attached_dev = dev; dev->phydev = phydev; + + if (phydev->sfp_bus_attached) + dev->sfp_bus = phydev->sfp_bus; } /* Some Ethernet drivers try to connect to a PHY device before @@ -2333,6 +2396,9 @@ static int phy_remove(struct device *dev) phydev->state = PHY_DOWN; mutex_unlock(&phydev->lock); + sfp_bus_del_upstream(phydev->sfp_bus); + phydev->sfp_bus = NULL; + if (phydev->drv && phydev->drv->remove) { phydev->drv->remove(phydev); diff --git a/include/linux/phy.h b/include/linux/phy.h index 1f9a141ccfb4..9753d9f28d73 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -203,6 +203,8 @@ static inline const char *phy_modes(phy_interface_t interface) struct device; struct phylink; +struct sfp_bus; +struct sfp_upstream_ops; struct sk_buff; /* @@ -342,6 +344,8 @@ struct phy_c45_device_ids { * dev_flags: Device-specific flags used by the PHY driver. * irq: IRQ number of the PHY's interrupt (-1 if none) * phy_timer: The timer for handling the state machine + * sfp_bus_attached: flag indicating whether the SFP bus has been attached + * sfp_bus: SFP bus attached to this PHY's fiber port * attached_dev: The attached enet driver's device instance ptr * adjust_link: Callback for the enet controller to respond to * changes in the link state. @@ -430,6 +434,9 @@ struct phy_device { struct mutex lock; + /* This may be modified under the rtnl lock */ + bool sfp_bus_attached; + struct sfp_bus *sfp_bus; struct phylink *phylink; struct net_device *attached_dev; @@ -1015,6 +1022,10 @@ int phy_suspend(struct phy_device *phydev); int phy_resume(struct phy_device *phydev); int __phy_resume(struct phy_device *phydev); int phy_loopback(struct phy_device *phydev, bool enable); +void phy_sfp_attach(void *upstream, struct sfp_bus *bus); +void phy_sfp_detach(void *upstream, struct sfp_bus *bus); +int phy_sfp_probe(struct phy_device *phydev, + const struct sfp_upstream_ops *ops); struct phy_device *phy_attach(struct net_device *dev, const char *bus_id, phy_interface_t interface); struct phy_device *phy_find_first(struct mii_bus *bus);
Add core phylib help for supporting SFP sockets on PHYs. This provides a mechanism to inform the SFP layer about PHY up/down events, and also unregister the SFP bus when the PHY is going away. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/phy.c | 7 ++++ drivers/net/phy/phy_device.c | 66 ++++++++++++++++++++++++++++++++++++ include/linux/phy.h | 11 ++++++ 3 files changed, 84 insertions(+)