| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200717081852.GA23732@laureti-dev>
Date: Fri, 17 Jul 2020 10:18:52 +0200
From: Helmut Grohne <helmut.grohne@...enta.de>
To: Andrew Lunn <andrew@...n.ch>
CC: Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Woojung Huh <woojung.huh@...rochip.com>,
Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
Vivien Didelot <vivien.didelot@...il.com>,
Tristram Ha <Tristram.Ha@...rochip.com>
Subject: Re: [PATCH v2] net: dsa: microchip: call phy_remove_link_mode during
probe
On Thu, Jul 16, 2020 at 04:10:44PM +0200, Andrew Lunn wrote:
> However, i'm having trouble understanding how PHYs actually work in
> this driver.
>
> We have:
>
> struct ksz_port {
> u16 member;
> u16 vid_member;
> int stp_state;
> struct phy_device phydev;
>
> with an instance of this structure per port of the switch.
>
> And it is this phydev which you are manipulating.
>
> > + for (i = 0; i < dev->phy_port_cnt; ++i) {
> > + /* The MAC actually cannot run in 1000 half-duplex mode. */
> > + phy_remove_link_mode(&dev->ports[i].phydev,
> > + ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
> > +
> > + /* PHY does not support gigabit. */
> > + if (!(dev->features & GBIT_SUPPORT))
> > + phy_remove_link_mode(&dev->ports[i].phydev,
> > + ETHTOOL_LINK_MODE_1000baseT_Full_BIT);
> > + }
> > +
> > return 0;
>
> But how is this phydev associated with the netdev? I don't see how
> phylink_connect_phy() is called using this phydev structure?
The ksz* drivers are implemented using the DSA framework. The relevant
phylink_connect_phy call is issued by the DSA infrastructure. We can see
this (and its ordering relative to phy_remove_link_mode after my patch)
using ftrace by adding the following to the kernel command line:
trace_options=func_stack_trace ftrace=function ftrace_filter=phylink_connect_phy,phy_remove_link_mode
I've added the trace buffer to the end of this mail to avoid cluttering
it. The key takeaways are:
* phylink_connect_phy is called by dsa_slave_create. A few inlined
functions later we arrive at dsa_register_switch, which is called
during driver probe.
* All phy_remove_link_mode calls now happen before any
phylink_connect_phy calls as requested.
Helmut
----8<-----8<------
# tracer: function
#
# entries-in-buffer/entries-written: 10/10 #P:2
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
swapper/0-1 [000] .... 7.166645: phy_remove_link_mode <-macb_probe
swapper/0-1 [000] .... 7.166654: <stack trace>
=> macb_probe
=> platform_drv_probe
=> really_probe
=> driver_probe_device
=> device_driver_attach
=> __driver_attach
=> bus_for_each_dev
=> driver_attach
=> bus_add_driver
=> driver_register
=> __platform_driver_register
=> macb_driver_init
=> do_one_initcall
=> kernel_init_freeable
=> kernel_init
=> ret_from_fork
=> 0
kworker/1:1-28 [001] .... 7.592479: phy_remove_link_mode <-ksz9477_setup
kworker/1:1-28 [001] .... 7.592489: <stack trace>
=> ksz9477_setup
=> dsa_register_switch
=> ksz_switch_register
=> ksz9477_switch_register
=> ksz9477_i2c_probe
=> i2c_device_probe
=> really_probe
=> driver_probe_device
=> __device_attach_driver
=> bus_for_each_drv
=> __device_attach
=> device_initial_probe
=> bus_probe_device
=> deferred_probe_work_func
=> process_one_work
=> worker_thread
=> kthread
=> ret_from_fork
=> 0
kworker/1:1-28 [001] .... 7.592494: phy_remove_link_mode <-ksz9477_setup
kworker/1:1-28 [001] .... 7.592498: <stack trace>
=> ksz9477_setup
=> dsa_register_switch
=> ksz_switch_register
=> ksz9477_switch_register
=> ksz9477_i2c_probe
=> i2c_device_probe
=> really_probe
=> driver_probe_device
=> __device_attach_driver
=> bus_for_each_drv
=> __device_attach
=> device_initial_probe
=> bus_probe_device
=> deferred_probe_work_func
=> process_one_work
=> worker_thread
=> kthread
=> ret_from_fork
=> 0
kworker/1:1-28 [001] .... 7.604375: phylink_connect_phy <-dsa_slave_create
kworker/1:1-28 [001] .... 7.604383: <stack trace>
=> dsa_slave_create
=> dsa_register_switch
=> ksz_switch_register
=> ksz9477_switch_register
=> ksz9477_i2c_probe
=> i2c_device_probe
=> really_probe
=> driver_probe_device
=> __device_attach_driver
=> bus_for_each_drv
=> __device_attach
=> device_initial_probe
=> bus_probe_device
=> deferred_probe_work_func
=> process_one_work
=> worker_thread
=> kthread
=> ret_from_fork
=> 0
kworker/1:1-28 [001] .n.. 7.623675: phylink_connect_phy <-dsa_slave_create
kworker/1:1-28 [001] .n.. 7.623685: <stack trace>
=> dsa_slave_create
=> dsa_register_switch
=> ksz_switch_register
=> ksz9477_switch_register
=> ksz9477_i2c_probe
=> i2c_device_probe
=> really_probe
=> driver_probe_device
=> __device_attach_driver
=> bus_for_each_drv
=> __device_attach
=> device_initial_probe
=> bus_probe_device
=> deferred_probe_work_func
=> process_one_work
=> worker_thread
=> kthread
=> ret_from_fork
=> 0
---->8----->8------
Powered by blists - more mailing lists