[U-Boot] [PATCH v2 1/3] Move CONFIG_PHY_ADDR to Kconfig

[Joe Hershberger]

On Sun, Mar 25, 2018 at 8:40 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Sat, Mar 24, 2018 at 1:11 AM, Joe Hershberger <joe.hershberger at ni.com> wrote:
>> On Thu, Mar 22, 2018 at 9:46 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi,
>>>
>>> On Fri, Feb 2, 2018 at 9:53 PM, Stefan Mavrodiev <stefan at olimex.com> wrote:
>>>> CONFIG_PHY_ADDR is used for old-style configuration. This makes
>>>> impossible changing the PHY address, if multiple boards share a same
>>>> config header file (for example include/configs/sunxi-common.h).
>>>>
>>>> Moving this to Kconfig helps overcoming this issue. It's defined
>>>> as entry inside PHYLIB section.
>>>>
>>>> After the implemention, moveconfig was run. The issues are:
>>>>         - edb9315a      - CONFIG_PHYLIB is not enabled. Entry is
>>>>                           deleted.
>>>>
>>>>         - ds414         - CONFIG_PHYLIB is in incompatible format:
>>>>                           { 0x1, 0x0 }. This entry is also deleted.
>>>>
>>>>         - devkit3250    - The PHY_ADDR is in hex format (0x1F).
>>>>                           Manually CONFIG_PHY_ADDR=31 is added in
>>>>                           the defconfig.
>>>>
>>>> After the changes the suspicious defconfigs passes building.
>>>>
>>>> Signed-off-by: Stefan Mavrodiev <stefan at olimex.com>
>>>> Acked-by: Maxime Ripard <maxime.ripard at free-electrons.com>
>>>> ---
>>>>  Changes for v2:
>>>>    - Replaced CONFIG_SUNXI_PHY_ADDR with a common one
>>>>      CONFIG_PHY_ADDR, using moveconfig.
>>>>
>>>>  README                         | 4 ----
>>>>  configs/devkit3250_defconfig   | 1 +
>>>>  configs/khadas-vim_defconfig   | 1 +
>>>>  configs/libretech-cc_defconfig | 1 +
>>>>  configs/p212_defconfig         | 1 +
>>>>  drivers/net/phy/Kconfig        | 7 +++++++
>>>>  include/configs/am335x_shc.h   | 1 -
>>>>  include/configs/baltos.h       | 1 -
>>>>  include/configs/devkit3250.h   | 1 -
>>>>  include/configs/ds414.h        | 1 -
>>>>  include/configs/edb93xx.h      | 1 -
>>>>  include/configs/khadas-vim.h   | 2 --
>>>>  include/configs/libretech-cc.h | 2 --
>>>>  include/configs/p212.h         | 2 --
>>>>  include/configs/pepper.h       | 1 -
>>>>  include/configs/sunxi-common.h | 2 --
>>>>  include/configs/work_92105.h   | 1 -
>>>>  include/configs/x600.h         | 1 -
>>>>  scripts/config_whitelist.txt   | 1 -
>>>>  19 files changed, 11 insertions(+), 21 deletions(-)
>>>>
>>>
>>> [snip]
>>>
>>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>>> index 95b7534..c934aed 100644
>>>> --- a/drivers/net/phy/Kconfig
>>>> +++ b/drivers/net/phy/Kconfig
>>>> @@ -12,6 +12,13 @@ menuconfig PHYLIB
>>>>
>>>>  if PHYLIB
>>>>
>>>> +config PHY_ADDR
>>>> +       int "PHY address"
>>>> +       default 1 if ARCH_SUNXI
>>>> +       default 0
>>>> +       help
>>>> +         The address of PHY on MII bus. Usually in range of 0 to 31.
>>>> +
>>>
>>> Sorry for jumping out so late, but this commit breaks Intel Galileo
>>> ethernet. Previously the board boots with the following log:
>>>
>>> Net: eth0: eth_designware#0, eth1: eth_designware#1
>>>
>>> With this commit it becomes:
>>>
>>> Net:   No ethernet found.
>>>
>>> The reason is that the board has two designware ethernet controllers,
>>> and PHY_ADDR has been set to zero for both. A simple fix is to:
>>>
>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>>> index 43670a7..1394119 100644
>>> --- a/drivers/net/designware.c
>>> +++ b/drivers/net/designware.c
>>> @@ -471,10 +471,6 @@ static int dw_phy_init(struct dw_eth_dev *priv, void *dev)
>>>         struct phy_device *phydev;
>>>         int mask = 0xffffffff, ret;
>>>
>>> -#ifdef CONFIG_PHY_ADDR
>>> -       mask = 1 << CONFIG_PHY_ADDR;
>>> -#endif
>>> -
>>>         phydev = phy_find_by_mask(priv->bus, mask, priv->interface);
>>>         if (!phydev)
>>>                 return -ENODEV;
>>>
>>> But the real question is that: why do we introduce this PHY_ADDR
>>> Kconfig? It for sure won't work for multiple ethernet controllers.This
>>> should be eliminated IMHO. Comments?
>>
>> This should be able to come from the device tree, ultimately. Can you
>> undefine the phy addr for the Galileo board?
>>
>>> [snip]
>>>
>
> #undf the PHY_ADDR in Galileo board looks weird. This to me is a workaround.

I didn't mean to add a #undef. I was just saying that if the "default
0" in the Kconfig were instead "default 0 if !X86" or something (or
maybe if the board defconfig explicitly does unselects it).

> Since the designware ethernet controller driver supports finding any
> PHY attached to its mdio bus, the changes suggested above can be a
> proper fix.

That is good for your board, but some board may have more than one phy
and want to specify which to use in U-Boot.

Ultimately it should be possible to read it from the DT.

Cheers,
-Joe

> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot