Re: create drivers/net/mdio and move mdio drivers into it
On Thu, Feb 23, 2017 at 09:51:17AM +, YUAN Linyu wrote: > > > > -Original Message- > > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > > On Behalf Of Andrew Lunn > > Sent: Thursday, February 23, 2017 5:30 PM > > To: YUAN Linyu > > Cc: Florian Fainelli; David S . Miller; netdev@vger.kernel.org; > > cug...@163.com > > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > > > Big picture is we can remove struct mii_bus, > > > > So if you remove this, how do you represent MII as a bus? It is a bus, > > clause 22 allows up to 32 devices on it, and i have boards with more > > than 8 devices on the bus. Clause 44 allows many more devices on the > > bus. > > > add a phy device list to mdio_device. Hi Yuan And where do you put the bus name? The bus operations? The mutex for accessing devices on the bus? The bus is a device, so you need a device structure somewhere, to make it part of the device hierarchy. Where do you put that? I think it is time to stop this discussion, and you need to submit patches we can review, how you think this would work. That will make you look at the details. Andrew
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] > On Behalf Of Andrew Lunn > Sent: Thursday, February 23, 2017 5:30 PM > To: YUAN Linyu > Cc: Florian Fainelli; David S . Miller; netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > Big picture is we can remove struct mii_bus, > > So if you remove this, how do you represent MII as a bus? It is a bus, > clause 22 allows up to 32 devices on it, and i have boards with more > than 8 devices on the bus. Clause 44 allows many more devices on the > bus. > add a phy device list to mdio_device. > Andrew
Re: create drivers/net/mdio and move mdio drivers into it
> Big picture is we can remove struct mii_bus, So if you remove this, how do you represent MII as a bus? It is a bus, clause 22 allows up to 32 devices on it, and i have boards with more than 8 devices on the bus. Clause 44 allows many more devices on the bus. Andrew
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: Andrew Lunn [mailto:and...@lunn.ch] > Sent: Wednesday, February 22, 2017 6:21 PM > To: YUAN Linyu > Cc: Florian Fainelli; David S . Miller; netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > On Wed, Feb 22, 2017 at 05:38:49AM +, YUAN Linyu wrote: > > Hi Florian, > > > > 1. > > Let's go back to original topic, > > Can we move all mdio dirvers into drivers/net/mdio ? > > Hi Yuan > > Please could you explain what benefit this brings. Please also list > all the downsides for such a move. As Florian said, we need to ensure > such a move adds more value than it removes. At beginning I think mdio and phy are two different things, mdio should have it's home. > > > Per may understanding, > > I don't know why create a struct mii_bus instance to represent a mdio device > in current mdio driver. > > Why not create a struct mdio_device instance, it's easy to understand. > > (We can move part of member of mii_bus to mdio_device). > > Please take a step back. What are you trying to achieve. What is the > big picture. What cannot you do with the current design? Big picture is we can remove struct mii_bus, and use struct mdio_device/driver for mdio controller. > > Andrew
Re: create drivers/net/mdio and move mdio drivers into it
On Wed, Feb 22, 2017 at 05:38:49AM +, YUAN Linyu wrote: > Hi Florian, > > 1. > Let's go back to original topic, > Can we move all mdio dirvers into drivers/net/mdio ? Hi Yuan Please could you explain what benefit this brings. Please also list all the downsides for such a move. As Florian said, we need to ensure such a move adds more value than it removes. > Per may understanding, > I don't know why create a struct mii_bus instance to represent a mdio device > in current mdio driver. > Why not create a struct mdio_device instance, it's easy to understand. > (We can move part of member of mii_bus to mdio_device). Please take a step back. What are you trying to achieve. What is the big picture. What cannot you do with the current design? Andrew
RE: create drivers/net/mdio and move mdio drivers into it
Hi Florian, 1. Let's go back to original topic, Can we move all mdio dirvers into drivers/net/mdio ? Last time you said half convinced. If you insist not do it, l will follow your decision. 2. Per may understanding, I don't know why create a struct mii_bus instance to represent a mdio device in current mdio driver. Why not create a struct mdio_device instance, it's easy to understand. (We can move part of member of mii_bus to mdio_device). Then in mdio layer, there will mdio_device and mdio_driver. Other module(dsa) base on mdio should not register mdio_driver directly. thanks > -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Tuesday, February 21, 2017 6:31 AM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > > On 02/19/2017 10:29 PM, YUAN Linyu wrote: > > > > > >> -Original Message- > >> From: Florian Fainelli [mailto:f.faine...@gmail.com] > >> Sent: Monday, February 20, 2017 1:42 PM > >> To: YUAN Linyu; David S . Miller; Andrew Lunn > >> Cc: netdev@vger.kernel.org; cug...@163.com > >> Subject: Re: create drivers/net/mdio and move mdio drivers into it > >>> 4. support mdio auto probe phy device. > >> > >> That's already the case, even in a Device Tree enabled system if you > >> omit to provide a "reg" property for child nodes, the bus is > >> automatically scanned. > >> > > I check of_mdiobus_registe() which not do auto scan. > > Which function should I refer? > > of_mdiobus_register() does this: > > /* Loop over the child nodes and register a phy_device for each > phy */ > for_each_available_child_of_node(np, child) { > addr = of_mdio_parse_addr(>dev, child); > if (addr < 0) { > scanphys = true; > continue; > } > > if (of_mdiobus_child_is_phy(child)) > of_mdiobus_register_phy(mdio, child, addr); > else > of_mdiobus_register_device(mdio, child, addr); > } > > if (!scanphys) > return 0; > > It does continue with scanning the PHY child nodes which don't have a > correct "reg" property set here. > -- > Florian
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Tuesday, February 21, 2017 6:31 AM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > > On 02/19/2017 10:29 PM, YUAN Linyu wrote: > > > > > >> -Original Message- > >> From: Florian Fainelli [mailto:f.faine...@gmail.com] > >> Sent: Monday, February 20, 2017 1:42 PM > >> To: YUAN Linyu; David S . Miller; Andrew Lunn > >> Cc: netdev@vger.kernel.org; cug...@163.com > >> Subject: Re: create drivers/net/mdio and move mdio drivers into it > >>> 4. support mdio auto probe phy device. > >> > >> That's already the case, even in a Device Tree enabled system if you > >> omit to provide a "reg" property for child nodes, the bus is > >> automatically scanned. > >> > > I check of_mdiobus_registe() which not do auto scan. > > Which function should I refer? > > of_mdiobus_register() does this: > > /* Loop over the child nodes and register a phy_device for each > phy */ > for_each_available_child_of_node(np, child) { > addr = of_mdio_parse_addr(>dev, child); > if (addr < 0) { > scanphys = true; > continue; > } > > if (of_mdiobus_child_is_phy(child)) > of_mdiobus_register_phy(mdio, child, addr); > else > of_mdiobus_register_device(mdio, child, addr); > } > > if (!scanphys) > return 0; > > It does continue with scanning the PHY child nodes which don't have a > correct "reg" property set here. >From this code, it parse sub-node of mdio node. mdio@fc000 { rgmii_phy1: ethernet-phy@1 { }; }; But I think we can auto detect the phy even if there is no sub-node of mdio node. mdio@fc000 { }; > -- > Florian
Re: create drivers/net/mdio and move mdio drivers into it
On 02/19/2017 10:29 PM, YUAN Linyu wrote: > > >> -Original Message- >> From: Florian Fainelli [mailto:f.faine...@gmail.com] >> Sent: Monday, February 20, 2017 1:42 PM >> To: YUAN Linyu; David S . Miller; Andrew Lunn >> Cc: netdev@vger.kernel.org; cug...@163.com >> Subject: Re: create drivers/net/mdio and move mdio drivers into it >>> 4. support mdio auto probe phy device. >> >> That's already the case, even in a Device Tree enabled system if you >> omit to provide a "reg" property for child nodes, the bus is >> automatically scanned. >> > I check of_mdiobus_registe() which not do auto scan. > Which function should I refer? of_mdiobus_register() does this: /* Loop over the child nodes and register a phy_device for each phy */ for_each_available_child_of_node(np, child) { addr = of_mdio_parse_addr(>dev, child); if (addr < 0) { scanphys = true; continue; } if (of_mdiobus_child_is_phy(child)) of_mdiobus_register_phy(mdio, child, addr); else of_mdiobus_register_device(mdio, child, addr); } if (!scanphys) return 0; It does continue with scanning the PHY child nodes which don't have a correct "reg" property set here. -- Florian
Re: create drivers/net/mdio and move mdio drivers into it
On 02/19/2017 10:26 PM, YUAN Linyu wrote: > > >> -Original Message- >> From: Florian Fainelli [mailto:f.faine...@gmail.com] >> Sent: Monday, February 20, 2017 2:16 PM >> To: YUAN Linyu; David S . Miller; Andrew Lunn >> Cc: netdev@vger.kernel.org; cug...@163.com >> Subject: Re: create drivers/net/mdio and move mdio drivers into it >> >> >> >> On 02/19/2017 10:10 PM, YUAN Linyu wrote: >>> >>> >>>> -Original Message- >>>> From: Florian Fainelli [mailto:f.faine...@gmail.com] >>>> Sent: Monday, February 20, 2017 1:42 PM >>>> To: YUAN Linyu; David S . Miller; Andrew Lunn >>>> Cc: netdev@vger.kernel.org; cug...@163.com >>>> Subject: Re: create drivers/net/mdio and move mdio drivers into it >>>>> 3. another idea is bind mdio device to network device >>>> >>>> You would have to be more specific about what you want to do here. If >>>> the MDIO device is e.g: a switch, what we recommend doing is provide a >>>> fixed-link node that describes how the Ethernet MAC and the switch's >>>> CPU/management ports are connected (that way the MAC always "sees" >> the >>>> link as UP, running with a specific speed and duplex). >>>> >>> Yes, some system will configured the phy to fixed speed/duplex at boot time, >>> no phy driver used in kernel at all. >> >> There is always a PHY driver used, either is a dedicated one, or its the >> generic PHY driver in drivers/net/phy/phy_device.c, even when fixed >> PHYs/link are used. >> >>> If network device know mdio device it used, we can do phy dump through this >>> mdio device driver. >> >> We are not going to accept MDIO device drivers whose only purpose is to >> allow PHY devices register dumps. Implement a proper PHY driver for >> these devices, if nothing needs to be done, you just need to call into >> genphy_* functions, and just override how to do register dumps. >> > No, we discuss mdio driver here, not phy driver. > I mean if a network device know mdio device it used, we use mdio driver to > dump any register of phy device even it's driver is not build. The network device is supposed to attach to a PHY, which in turns has a backing PHY driver, which is a superset of a MDIO device driver. If the thing you are after is to be able to do PHY device register dumps without a PHY driver to assist you with that, then we need to add whole lot of things: - have an ioctl() which allows stopping/freezing the PHY state machine so dumps don't interfere with the PHY state - have the ability to reliably perform PHY dumps that e.g: require page programming etc. etc. Then again, all of this could be done by a PHY driver reliably and in a way that is consistent and predictable. -- Florian
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Monday, February 20, 2017 1:42 PM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > 4. support mdio auto probe phy device. > > That's already the case, even in a Device Tree enabled system if you > omit to provide a "reg" property for child nodes, the bus is > automatically scanned. > I check of_mdiobus_registe() which not do auto scan. Which function should I refer?
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Monday, February 20, 2017 2:16 PM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > > On 02/19/2017 10:10 PM, YUAN Linyu wrote: > > > > > >> -Original Message- > >> From: Florian Fainelli [mailto:f.faine...@gmail.com] > >> Sent: Monday, February 20, 2017 1:42 PM > >> To: YUAN Linyu; David S . Miller; Andrew Lunn > >> Cc: netdev@vger.kernel.org; cug...@163.com > >> Subject: Re: create drivers/net/mdio and move mdio drivers into it > >>> 3. another idea is bind mdio device to network device > >> > >> You would have to be more specific about what you want to do here. If > >> the MDIO device is e.g: a switch, what we recommend doing is provide a > >> fixed-link node that describes how the Ethernet MAC and the switch's > >> CPU/management ports are connected (that way the MAC always "sees" > the > >> link as UP, running with a specific speed and duplex). > >> > > Yes, some system will configured the phy to fixed speed/duplex at boot time, > > no phy driver used in kernel at all. > > There is always a PHY driver used, either is a dedicated one, or its the > generic PHY driver in drivers/net/phy/phy_device.c, even when fixed > PHYs/link are used. > > > If network device know mdio device it used, we can do phy dump through this > > mdio device driver. > > We are not going to accept MDIO device drivers whose only purpose is to > allow PHY devices register dumps. Implement a proper PHY driver for > these devices, if nothing needs to be done, you just need to call into > genphy_* functions, and just override how to do register dumps. > No, we discuss mdio driver here, not phy driver. I mean if a network device know mdio device it used, we use mdio driver to dump any register of phy device even it's driver is not build. > > > >> If this is a different kind of MDIO device, e.g: an USB/PCIe/SATA PHY, > >> there is no network device associated with those. > >> > > mdio under drivers/net/, it will not cover these devices. > > > > -- > Florian
Re: create drivers/net/mdio and move mdio drivers into it
On 02/19/2017 10:10 PM, YUAN Linyu wrote: > > >> -Original Message- >> From: Florian Fainelli [mailto:f.faine...@gmail.com] >> Sent: Monday, February 20, 2017 1:42 PM >> To: YUAN Linyu; David S . Miller; Andrew Lunn >> Cc: netdev@vger.kernel.org; cug...@163.com >> Subject: Re: create drivers/net/mdio and move mdio drivers into it >>> 3. another idea is bind mdio device to network device >> >> You would have to be more specific about what you want to do here. If >> the MDIO device is e.g: a switch, what we recommend doing is provide a >> fixed-link node that describes how the Ethernet MAC and the switch's >> CPU/management ports are connected (that way the MAC always "sees" the >> link as UP, running with a specific speed and duplex). >> > Yes, some system will configured the phy to fixed speed/duplex at boot time, > no phy driver used in kernel at all. There is always a PHY driver used, either is a dedicated one, or its the generic PHY driver in drivers/net/phy/phy_device.c, even when fixed PHYs/link are used. > If network device know mdio device it used, we can do phy dump through this > mdio device driver. We are not going to accept MDIO device drivers whose only purpose is to allow PHY devices register dumps. Implement a proper PHY driver for these devices, if nothing needs to be done, you just need to call into genphy_* functions, and just override how to do register dumps. > >> If this is a different kind of MDIO device, e.g: an USB/PCIe/SATA PHY, >> there is no network device associated with those. >> > mdio under drivers/net/, it will not cover these devices. > -- Florian
RE: create drivers/net/mdio and move mdio drivers into it
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Monday, February 20, 2017 1:42 PM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > 3. another idea is bind mdio device to network device > > You would have to be more specific about what you want to do here. If > the MDIO device is e.g: a switch, what we recommend doing is provide a > fixed-link node that describes how the Ethernet MAC and the switch's > CPU/management ports are connected (that way the MAC always "sees" the > link as UP, running with a specific speed and duplex). > Yes, some system will configured the phy to fixed speed/duplex at boot time, no phy driver used in kernel at all. If network device know mdio device it used, we can do phy dump through this mdio device driver. > If this is a different kind of MDIO device, e.g: an USB/PCIe/SATA PHY, > there is no network device associated with those. > mdio under drivers/net/, it will not cover these devices.
Re: create drivers/net/mdio and move mdio drivers into it
On 02/19/2017 09:23 PM, YUAN Linyu wrote: > 1. the main issue is mdio driver mixed with phy driver/ethernet driver. You can have a standalone MDIO bus driver located anywhere you want in the kernel tree. See drivers/net/ethernet/marvell/mvmdio.c for an example. > 2. move them together, it's easy to maintain, add, delete or optimization(we > can do it step by step). It may be easier, but whenever you make API changes, you need to make sure they cover every user in tree. So half convinced about this argument here. > > 3. another idea is bind mdio device to network device You would have to be more specific about what you want to do here. If the MDIO device is e.g: a switch, what we recommend doing is provide a fixed-link node that describes how the Ethernet MAC and the switch's CPU/management ports are connected (that way the MAC always "sees" the link as UP, running with a specific speed and duplex). If this is a different kind of MDIO device, e.g: an USB/PCIe/SATA PHY, there is no network device associated with those. > 4. support mdio auto probe phy device. That's already the case, even in a Device Tree enabled system if you omit to provide a "reg" property for child nodes, the bus is automatically scanned. > For 3, 4 can take device tree as example, > Current, > ethernet@e4000 { > phy-handle = <_phy1>; > phy-connection-type = "rgmii"; > }; > > ethernet@e6000 { > phy-handle = <_phy2>; > phy-connection-type = "rgmii"; > }; > > mdio@fc000 { > rgmii_phy1: ethernet-phy@1 { > reg = <0x1>; > }; > rgmii_phy2: ethernet-phy@2 { > reg = <0x2>; > }; > }; > After change, > ethernet@e4000 { > mdio-handle = <>; > phy-reg = <0x1>; > phy-connection-type = "rgmii"; > }; This is not providing anything more than the current representation, and in fact, it's actually counter to the Device Tree representation where nodes should be relative to their parent and describe their address with standard properties (reg) which are to be interpreted relative to their parent's node #address-cells and #size-cells properties. You are also introducing two new properties (phy-reg) and (mdio-handle) which both introduce an incompatible binding... Seriously, what problem do you really want to solve that we can't do today? It may just be easier to send patches and have all of us review them. > > ethernet@e6000 { > mdio-handle = <>; > phy-handle = <_phy2>; > phy-connection-type = "rgmii"; > }; > > mdio: mdio@fc000 { > reg = ; > }; > > >> -Original Message----- >> From: Florian Fainelli [mailto:f.faine...@gmail.com] >> Sent: Monday, February 20, 2017 1:03 PM >> To: YUAN Linyu; David S . Miller; Andrew Lunn >> Cc: netdev@vger.kernel.org; cug...@163.com >> Subject: Re: create drivers/net/mdio and move mdio drivers into it >> >> >> >> On 02/19/2017 04:20 PM, YUAN Linyu wrote: >>> Hi, >>> >>> I have an idea to create drivers/net/mdio and move mdio >> drivers(drivers/net/phy/mdio-* | drivers/net/ethernet/xxx/mdio-yyy | ) into >> it. >>> >>> Do you think it is acceptable ? >> >> What problem are you trying to fix by doing such a move? Moving files >> around mean that making stable backports are going to be much more painful. >> >> So without further explanation, does not sound like such a great idea. >> -- >> Florian -- Florian
RE: create drivers/net/mdio and move mdio drivers into it
1. the main issue is mdio driver mixed with phy driver/ethernet driver. 2. move them together, it's easy to maintain, add, delete or optimization(we can do it step by step). 3. another idea is bind mdio device to network device 4. support mdio auto probe phy device. For 3, 4 can take device tree as example, Current, ethernet@e4000 { phy-handle = <_phy1>; phy-connection-type = "rgmii"; }; ethernet@e6000 { phy-handle = <_phy2>; phy-connection-type = "rgmii"; }; mdio@fc000 { rgmii_phy1: ethernet-phy@1 { reg = <0x1>; }; rgmii_phy2: ethernet-phy@2 { reg = <0x2>; }; }; After change, ethernet@e4000 { mdio-handle = <>; phy-reg = <0x1>; phy-connection-type = "rgmii"; }; ethernet@e6000 { mdio-handle = <>; phy-handle = <_phy2>; phy-connection-type = "rgmii"; }; mdio: mdio@fc000 { reg = ; }; > -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Monday, February 20, 2017 1:03 PM > To: YUAN Linyu; David S . Miller; Andrew Lunn > Cc: netdev@vger.kernel.org; cug...@163.com > Subject: Re: create drivers/net/mdio and move mdio drivers into it > > > > On 02/19/2017 04:20 PM, YUAN Linyu wrote: > > Hi, > > > > I have an idea to create drivers/net/mdio and move mdio > drivers(drivers/net/phy/mdio-* | drivers/net/ethernet/xxx/mdio-yyy | ) into > it. > > > > Do you think it is acceptable ? > > What problem are you trying to fix by doing such a move? Moving files > around mean that making stable backports are going to be much more painful. > > So without further explanation, does not sound like such a great idea. > -- > Florian
Re: create drivers/net/mdio and move mdio drivers into it
On 02/19/2017 04:20 PM, YUAN Linyu wrote: > Hi, > > I have an idea to create drivers/net/mdio and move mdio > drivers(drivers/net/phy/mdio-* | drivers/net/ethernet/xxx/mdio-yyy | ) into > it. > > Do you think it is acceptable ? What problem are you trying to fix by doing such a move? Moving files around mean that making stable backports are going to be much more painful. So without further explanation, does not sound like such a great idea. -- Florian