[PATCH] net: phy: dp83867: Do not check sgmii if rgmii is already used

[Michal Simek]

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 <michal.simek at xilinx.com>
>>>> ---
>>>>
>>>>    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.

Can you please sync it if you want to keep it in the same was as is done
in Linux?

> - 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.

Thanks,
Michal