On 13. 02. 20 16:49, Grygorii Strashko wrote: > > > 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.
I am ok with that. > >> >>> - 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. I was looking at latest kernel code and it is designed differently there. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/net/phy/dp83867.c?h=next-20200213#n479 there is if rgmii or sgmii if rgmii io impedance if sgmii in u-boot you have if rgmii if sgmii io impedance I am ok with having this in sync but that's not what we have today. Thanks, Michal
