Hi Joe,
On Thu, Mar 29, 2018 at 2:17 AM, Joe Hershberger <joe.hershber...@ni.com> wrote:
> On Sun, Mar 25, 2018 at 8:40 PM, Bin Meng <bmeng...@gmail.com> wrote:
>> Hi Joe,
>>
>> On Sat, Mar 24, 2018 at 1:11 AM, Joe Hershberger <joe.hershber...@ni.com>
>> wrote:
>>> On Thu, Mar 22, 2018 at 9:46 AM, Bin Meng <bmeng...@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> On Fri, Feb 2, 2018 at 9:53 PM, Stefan Mavrodiev <ste...@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 <ste...@olimex.com>
>>>>> Acked-by: Maxime Ripard <maxime.rip...@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.
>> 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.
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
