On 13/02/2020 08:23, Michal Simek wrote:
On 12. 02. 20 21:24, Grygorii Strashko wrote:
On 11/02/2020 10:11, Michal Simek wrote:
On 10. 02. 20 13:07, Grygorii Strashko wrote:
On 07/02/2020 13:31, Michal Simek wrote:
There is no reason to check sgmii branch again when it is clear that
phy
interface is rgmii.
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/phy/dp83867.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 4d796e289c45..3178787ff1c7 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -327,9 +327,7 @@ static int dp83867_config(struct phy_device
*phydev)
phy_write_mmd(phydev, DP83867_DEVADDR,
DP83867_RGMIIDCTL, delay);
- }
-
- if (phy_interface_is_sgmii(phydev)) {
+ } else if (phy_interface_is_sgmii(phydev)) {
phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
(BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000));
From one side I have no objections, but from another - I'd prefer to
keep as is.
Can you please be elaborate on this one more?
- keep the same way as in the Kernel
If kernel does it in the same way it should be also fixed.
I have been checking yesterday dt binding docs in u-boot and in Linux
and surprisingly they are different.
ti,dp83867-rxctrl-strap-quirk is supported in u-boot but not described
ti,clk-output-sel is supported but even in code is said that it is
optional property.
ti,min-output-impedance, ti,max-output-impedance and ti,fifo-depth are
not documented in dt binding doc
ti,sgmii-ref-clock-output-enable is not supported in u-boot but it is in
Linux and we are using this feature.
My understanding is that u-boot goal is to have uboot-dt == kernel-dt and
current
approach add DT+bindings to the kernel first.
So, if you check most of u-boot bindings are missed or obsolete.
Can you please sync it if you want to keep it in the same was as is done
in Linux?
So, may be it can be just deleted.
- code readability
I don't think this is really changing code readability. For improving
readability would be the best to move bodies of these ifs to separate
functions and not have dp83867_config() ~140 lines long.
It really too minor change to fight for. But if you wish you can update kernel
driver as per-above your proposal and then port it in u-boot.
--
Best regards,
grygorii
