| Message ID | 20201129220000.16550-1-adrien.grassein@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] dt-bindings: net: fsl-fec add mdc/mdio bitbang option | expand |
On Sun, Nov 29, 2020 at 11:51:43PM +0100, Adrien Grassein wrote: > Hi Andrew, > > Please find my answers below. > > Le dim. 29 nov. 2020 à 23:41, Andrew Lunn <andrew@lunn.ch> a écrit : > > On Sun, Nov 29, 2020 at 10:59:58PM +0100, Adrien Grassein wrote: > > Add dt-bindings explanation for the two new gpios > > (mdio and mdc) used for bitbanging. > > Hi Adrien > > What is missing is an explanation of why! > > I'm sorry, it's my first upstreaming attempt. Hi Adrien Please take a look at https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html It is normal to have a patch 0/X which explains the big picture. Then the commit message for each patch should explain why you are doing something. That is much more important than what you are doing, i can see that from the patch itself. > I am currently upstreaming the "Nitrogen 8m Mini board" that seems to not use a > "normal" mdio bus but a "bitbanged" one with the fsl fec driver. Any idea why? Anyway, you should not replicate code, don't copy bitbanging code into the FEC. Just use the existing bit-banger MDIO bus master driver. Andrew
On 11/29/2020 3:04 PM, Andrew Lunn wrote: > On Sun, Nov 29, 2020 at 11:51:43PM +0100, Adrien Grassein wrote: >> Hi Andrew, >> >> Please find my answers below. >> >> Le dim. 29 nov. 2020 à 23:41, Andrew Lunn <andrew@lunn.ch> a écrit : >> >> On Sun, Nov 29, 2020 at 10:59:58PM +0100, Adrien Grassein wrote: >> > Add dt-bindings explanation for the two new gpios >> > (mdio and mdc) used for bitbanging. >> >> Hi Adrien >> >> What is missing is an explanation of why! >> >> I'm sorry, it's my first upstreaming attempt. > > Hi Adrien > > Please take a look at > > https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html > > It is normal to have a patch 0/X which explains the big picture. > > Then the commit message for each patch should explain why you are > doing something. That is much more important than what you are doing, > i can see that from the patch itself. > >> I am currently upstreaming the "Nitrogen 8m Mini board" that seems to not use a >> "normal" mdio bus but a "bitbanged" one with the fsl fec driver. > > Any idea why? > > Anyway, you should not replicate code, don't copy bitbanging code into > the FEC. Just use the existing bit-banger MDIO bus master driver. Right there should be no need for you to modify the FEC driver at all, there is an existing generic bitbanged MDIO bus driver here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/mdio/mdio-gpio.c with its binding here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mdio-gpio.txt so all you should need to do is make sure that you place a "virtual,mdio-gpio" node, declare the PHY devices that are present on that bus, and have your FEC nodes point to those PHY devices with an appropriate 'phy-handle' property.
> >> I am currently upstreaming the "Nitrogen 8m Mini board" that seems to not use a > >> "normal" mdio bus but a "bitbanged" one with the fsl fec driver. > > > > Any idea why? > > > > Anyway, you should not replicate code, don't copy bitbanging code into > > the FEC. Just use the existing bit-banger MDIO bus master driver. > > Right there should be no need for you to modify the FEC driver at all, > there is an existing generic bitbanged MDIO bus driver here: Hi Florian Speculation on my part, until i hear back on the Why? question, but i'm guessing the board has a wrong pullup on the MDIO line. It takes too long for the PHY/FEC to pull the line low at the default 2.5MHz. bit-banging is much slower, so it works. If i'm right, there is a much simpler fix for this. Use the clock-frequency property for the MDIO bus to slow the clock down. Andrew
Hi all, I'm not a kernel expert, but I try to find why these patches were needed in the FSL/Boundary kernel. I found that the pin muxing in the original kernel was not good for FEC. It's now working well with the mainline code. You can delete these patches. Thanks a lot, Regards, Adrien Le lun. 30 nov. 2020 à 23:26, Andrew Lunn <andrew@lunn.ch> a écrit : > > > >> I am currently upstreaming the "Nitrogen 8m Mini board" that seems to not use a > > >> "normal" mdio bus but a "bitbanged" one with the fsl fec driver. > > > > > > Any idea why? > > > > > > Anyway, you should not replicate code, don't copy bitbanging code into > > > the FEC. Just use the existing bit-banger MDIO bus master driver. > > > > Right there should be no need for you to modify the FEC driver at all, > > there is an existing generic bitbanged MDIO bus driver here: > > Hi Florian > > Speculation on my part, until i hear back on the Why? question, but > i'm guessing the board has a wrong pullup on the MDIO line. It takes > too long for the PHY/FEC to pull the line low at the default > 2.5MHz. bit-banging is much slower, so it works. > > If i'm right, there is a much simpler fix for this. Use the > clock-frequency property for the MDIO bus to slow the clock down. > > Andrew
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt index 9b543789cd52..e9fa992354b7 100644 --- a/Documentation/devicetree/bindings/net/fsl-fec.txt +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt @@ -22,6 +22,10 @@ Optional properties: - fsl,err006687-workaround-present: If present indicates that the system has the hardware workaround for ERR006687 applied and does not need a software workaround. +- mdc-gpios: Bitbanged MDIO Management Data Clock GPIO. If specified, +mdio-gpios should be specified too. +- mdio-gpios: Bitbanged MDIO Management Data I/O GPIO. If specified, +mdc-gpios should be specified too. - fsl,stop-mode: register bits of stop mode control, the format is <&gpr req_gpr req_bit>. gpr is the phandle to general purpose register node.
Add dt-bindings explanation for the two new gpios (mdio and mdc) used for bitbanging. Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com> --- Documentation/devicetree/bindings/net/fsl-fec.txt | 4 ++++ 1 file changed, 4 insertions(+)