Re: broonie-regmap/for-next bisection: boot on ox820-cloudengines-pogoplug-series-3

From: Florian Fainelli
Date: Fri Jan 03 2020 - 12:46:21 EST

On 1/3/20 5:30 AM, Sriram Dash wrote:
>> From: kernelci.org bot <bot@xxxxxxxxxxxx>
>> Subject: broonie-regmap/for-next bisection: boot on ox820-cloudengines-
>> pogoplug-series-3
>>
>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>> * This automated bisection report was sent to you on the basis *
>> * that you may be involved with the breaking commit it has *
>> * found. No manual investigation has been done to verify it, *
>> * and the root cause of the problem may be somewhere else. *
>> * *
>> * If you do send a fix, please include this trailer: *
>> * Reported-by: "kernelci.org bot" <bot@xxxxxxxxxxxx> *
>> * *
>> * Hope this helps! *
>> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
>>
>> broonie-regmap/for-next bisection: boot on ox820-cloudengines-pogoplug-
>> series-3
>>
>> Summary:
>> Start: 46cf053efec6 Linux 5.5-rc3
>> Details: https://protect2.fireeye.com/url?k=36fb52ed-6b2b5a21-36fad9a2-
>> 000babff3793-
>> f64e7c227e0a8b34&u=https://kernelci.org/boot/id/5e02ce65451524462f9731
>> 4f
>> Plain log: https://protect2.fireeye.com/url?k=58f5fc3b-0525f4f7-58f47774-
>> 000babff3793-f96a18481add0d7f&u=https://storage.kernelci.org//broonie-
>> regmap/for-next/v5.5-rc3/arm/oxnas_v6_defconfig/gcc-8/lab-
>> baylibre/boot-ox820-cloudengines-pogoplug-series-3.txt
>> HTML log: https://protect2.fireeye.com/url?k=eaed2629-b73d2ee5-
>> eaecad66-000babff3793-
>> 84ba1e41025b4f73&u=https://storage.kernelci.org//broonie-regmap/for-
>> next/v5.5-rc3/arm/oxnas_v6_defconfig/gcc-8/lab-baylibre/boot-ox820-
>> cloudengines-pogoplug-series-3.html
>> Result: d3e014ec7d5e net: stmmac: platform: Fix MDIO init for platforms
>> without PHY
>>
>> Checks:
>> revert: PASS
>> verify: PASS
>>
>> Parameters:
>> Tree: broonie-regmap
>> URL:
>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
>> Branch: for-next
>> Target: ox820-cloudengines-pogoplug-series-3
>> CPU arch: arm
>> Lab: lab-baylibre
>> Compiler: gcc-8
>> Config: oxnas_v6_defconfig
>> Test suite: boot
>>
>> Breaking commit found:
>>
>> -------------------------------------------------------------------------------
>> commit d3e014ec7d5ebe9644b5486bc530b91e62bbf624
>> Author: Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx>
>> Date: Thu Dec 19 15:47:01 2019 +0530
>>
>> net: stmmac: platform: Fix MDIO init for platforms without PHY
>>
>> The current implementation of "stmmac_dt_phy" function initializes
>> the MDIO platform bus data, even in the absence of PHY. This fix
>> will skip MDIO initialization if there is no PHY present.
>>
>> Fixes: 7437127 ("net: stmmac: Convert to phylink and remove phylib logic")
>> Acked-by: Jayati Sahu <jayati.sahu@xxxxxxxxxxx>
>> Signed-off-by: Sriram Dash <sriram.dash@xxxxxxxxxxx>
>> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx>
>> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index bedaff0c13bd..cc8d7e7bf9ac 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -320,7 +320,7 @@ static int stmmac_mtl_setup(struct platform_device
>> *pdev, static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
>> struct device_node *np, struct device *dev) {
>> - bool mdio = true;
>> + bool mdio = false;
>> static const struct of_device_id need_mdio_ids[] = {
>> { .compatible = "snps,dwc-qos-ethernet-4.10" },
>> {},
>> -------------------------------------------------------------------------------
>>
>>
>> Git bisection log:
>>
>> -------------------------------------------------------------------------------
>> git bisect start
>> # good: [e42617b825f8073569da76dc4510bfa019b1c35a] Linux 5.5-rc1 git
>> bisect good e42617b825f8073569da76dc4510bfa019b1c35a
>> # bad: [46cf053efec6a3a5f343fead837777efe8252a46] Linux 5.5-rc3 git bisect
>> bad 46cf053efec6a3a5f343fead837777efe8252a46
>> # good: [2187f215ebaac73ddbd814696d7c7fa34f0c3de0] Merge tag 'for-5.5-
>> rc2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
>> git bisect good 2187f215ebaac73ddbd814696d7c7fa34f0c3de0
>> # good: [0dd1e3773ae8afc4bfdce782bdeffc10f9cae6ec] pipe: fix empty pipe
>> check in pipe_write() git bisect good
>> 0dd1e3773ae8afc4bfdce782bdeffc10f9cae6ec
>> # good: [040cda8a15210f19da7e29232c897ca6ca6cc950] Merge tag 'wireless-
>> drivers-2019-12-17' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers
>> git bisect good 040cda8a15210f19da7e29232c897ca6ca6cc950
>> # bad: [4bfeadfc0712bbc8a6556eef6d47cbae1099dea3] Merge branch 'sfc-
>> fix-bugs-introduced-by-XDP-patches'
>> git bisect bad 4bfeadfc0712bbc8a6556eef6d47cbae1099dea3
>> # good: [0fd260056ef84ede8f444c66a3820811691fe884] Merge
>> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf
>> git bisect good 0fd260056ef84ede8f444c66a3820811691fe884
>> # good: [90b3b339364c76baa2436445401ea9ade040c216] net: hisilicon: Fix a
>> BUG trigered by wrong bytes_compl git bisect good
>> 90b3b339364c76baa2436445401ea9ade040c216
>> # bad: [4c8dc00503db24deaf0b89dddfa84b7cba7cd4ce] qede: Disable
>> hardware gro when xdp prog is installed git bisect bad
>> 4c8dc00503db24deaf0b89dddfa84b7cba7cd4ce
>> # bad: [28a3b8408f70b646e78880a7eb0a97c22ace98d1] net/smc: unregister
>> ib devices in reboot_event git bisect bad
>> 28a3b8408f70b646e78880a7eb0a97c22ace98d1
>> # bad: [d3e014ec7d5ebe9644b5486bc530b91e62bbf624] net: stmmac:
>> platform: Fix MDIO init for platforms without PHY git bisect bad
>> d3e014ec7d5ebe9644b5486bc530b91e62bbf624
>> # good: [af1c0e4e00f3cc76cb136ebf2e2c04e8b6446285] llc2: Fix return
>> statement of llc_stat_ev_rx_null_dsap_xid_c (and _test_c) git bisect good
>> af1c0e4e00f3cc76cb136ebf2e2c04e8b6446285
>> # first bad commit: [d3e014ec7d5ebe9644b5486bc530b91e62bbf624] net:
>> stmmac: platform: Fix MDIO init for platforms without PHY
>> -------------------------------------------------------------------------------
>
>
> The mdio bus will be allocated in case of a phy transceiver is on board, but if
> fixed-link is configured, it will be NULL and of_mdiobus_register will
> not take effect.

There appears to be another possible flaw in the code here:

for_each_child_of_node(np, plat->mdio_node) {
if (of_device_is_compatible(plat->mdio_node,
"snps,dwmac-mdio"))
break;
}

the loop should use for_each_available_child_of_node() such that if a
platform has a Device Tree definition where the MDIO bus node was
provided but it was not disabled by default (a mistake, it should be
disabled by default), and a "fixed-link" property ended up being used at
the board level, we should not end-up with an invalid plat->mdio_node
reference. Then the code could possibly eliminate the use of 'mdio' as a
boolean and rely exclusively on plat->mdio_node. What do you think?

And an alternative to your fix would be to scan even further the MDIO
bus node for available child nodes, if there are none, do not perform
the MDIO initialization at all since we have no MDIO devices beneath.


> The commit d3e014ec7d5e fixes the code for fixed-link configuration.
> However, some platforms like oxnas820 which have phy
> transceivers (rgmii), fail. This is because the platforms expect the allocation
> of mdio_bus_data during stmmac_dt_phy.
>
> Proper solution to this is adding the mdio node in the device tree of the
> platform which can be fetched by stmmac_dt_phy.

That sounds reasonable, but we should also not break existing platforms
with existing Device Trees out there, as much as possible.
--
Florian