[U-Boot] [PATCH v2 1/3] Move CONFIG_PHY_ADDR to Kconfig
Joe Hershberger
joe.hershberger at ni.com
Fri Mar 30 16:55:46 UTC 2018
On Fri, Mar 30, 2018 at 2:49 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Thu, Mar 29, 2018 at 2:17 AM, Joe Hershberger <joe.hershberger at ni.com> wrote:
>> 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).
>>
>
> This cannot be done as CONFIG_PHY_ADDR is an "int", not a "bool". We
> cannot do "# CONFIG_PHY_ADDR is not set" in the galileo_defconfig.
I just sent an RFC.
>>> 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.
>>
>
> When will this be fixed? At present the CONFIG_PHY_ADDR concept is
> just wrong. It can only handle one eth and one PHY.
I'm not sure exactly when I'll have the DT sourced phy addr ready, but
hopefully the RFC will fix things in the meantime.
Cheers,
-Joe
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list