All of lore.kernel.org
 help / color / mirror / Atom feed
* SoCFPGA ethernet broken
@ 2015-10-15 19:09 Dinh Nguyen
  2015-10-15 20:03 ` Florian Fainelli
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-15 19:09 UTC (permalink / raw)
  To: David S. Miller; +Cc: david.daney, Florian Fainelli, netdev, linux-kernel

Hi,

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." seems to have broken ethernet support for the SoCFPGA
platform which is using the stmmac ethernet driver.

It appears that during DHCP, it cannot get an IP address. This only
happens if ethernet was not used by the bootloader to tftp an kernel
image. If I use the bootloader to tftp an image then ethernet is working
fine. So I think the PHY is not getting enabled properly.

If I revert this patch, then ethernet is back to working on the platform.

I just tested this in the v4.3-rc5.

Will continue to debug, but was wondering if anyone has seen this issue.

Thanks for any help/comments.

Dinh

Bootlog below.
----
[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Linux version 4.3.0-rc5-00037-g5b5f145
(dinguyen@linux-builds1) (gcc version 4.7.3 20130226 (prerelease) (crosstool
-NG linaro-1.13.1-4.7-2013.03-20130313 - Linaro GCC 2013.03) ) #1 SMP
Thu Oct 15 13:55:48 CDT 2015
[    0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7),
cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[    0.000000] Machine model: Altera SOCFPGA Cyclone V SoC Development Kit
[    0.000000] Truncating RAM at 0x00000000-0x40000000 to -0x2f800000
[    0.000000] Consider using a HIGHMEM enabled kernel.
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] On node 0 totalpages: 194560
[    0.000000] free_area_init_node: node 0, pgdat c067c080, node_mem_map
ef20b000
[    0.000000]   Normal zone: 1520 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 194560 pages, LIFO batch:31
[    0.000000] PERCPU: Embedded 13 pages/cpu @ef1df000 s21760 r8192
d23296 u53248
[    0.000000] pcpu-alloc: s21760 r8192 d23296 u53248 alloc=13*4096
[    0.000000] pcpu-alloc: [0] 0 [0] 1
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 193040
[    0.000000] Kernel command line: console=ttyS0,115200 root=/dev/nfs
rw nfsroot=137.57.160.210:/home/dinguyen/rootfs_yocto ip=dh
cp debug
[    0.000000] PID hash table entries: 4096 (order: 2, 16384 bytes)
[    0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288
bytes)
[    0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144
bytes)
[    0.000000] Memory: 764368K/778240K available (4696K kernel code,
274K rwdata, 1328K rodata, 324K init, 131K bss, 13872K reserv
ed, 0K cma-reserved)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
[    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
[    0.000000]       .text : 0xc0008000 - 0xc05ea5f4   (6026 kB)
[    0.000000]       .init : 0xc05eb000 - 0xc063c000   ( 324 kB)
[    0.000000]       .data : 0xc063c000 - 0xc0680b30   ( 275 kB)
[    0.000000]        .bss : 0xc0680b30 - 0xc06a1a38   ( 132 kB)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[    0.000000] Hierarchical RCU implementation.
[    0.000000]  Build-time adjustment of leaf fanout to 32.
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] L2C-310 enabling early BRESP for Cortex-A9
[    0.000000] L2C-310 full line of zeros enabled for Cortex-A9
[    0.000000] L2C-310 ID prefetch enabled, offset 1 lines
[    0.000000] L2C-310 dynamic clock gating enabled, standby mode enabled
[    0.000000] L2C-310 cache controller enabled, 8 ways, 512 kB
[    0.000000] L2C-310: CACHE_ID 0x410030c9, AUX_CTRL 0x76060001
[    0.000000] clocksource: timer1: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 19112604467 ns
[    0.000005] sched_clock: 32 bits at 100MHz, resolution 10ns, wraps
every 21474836475ns
[    0.000134] Console: colour dummy device 80x30
[    0.000151] Calibrating delay loop... 1836.64 BogoMIPS (lpj=9183232)
[    0.059867] pid_max: default: 32768 minimum: 301
[    0.059938] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.059947] Mountpoint-cache hash table entries: 2048 (order: 1, 8192
bytes)
[    0.060411] CPU: Testing write buffer coherency: ok
[    0.060586] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.060735] Setting up static identity map for 0x8280 - 0x82d8
[    0.119895] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.119954] Brought up 2 CPUs
[    0.119968] SMP: Total of 2 processors activated (3679.84 BogoMIPS).
[    0.119974] CPU: All CPU(s) started in SVC mode.
[    0.120594] devtmpfs: initialized
[    0.124057] VFP support v0.3: implementor 41 architecture 3 part 30
variant 9 rev 4
[    0.124296] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.125249] NET: Registered protocol family 16
[    0.125991] DMA: preallocated 256 KiB pool for atomic coherent
allocations
[    0.129791] hw-breakpoint: found 5 (+1 reserved) breakpoint and 1
watchpoint registers.
[    0.129803] hw-breakpoint: maximum watchpoint size is 4 bytes.
[    0.163465] SCSI subsystem initialized
[    0.163736] usbcore: registered new interface driver usbfs
[    0.163793] usbcore: registered new interface driver hub
[    0.163848] usbcore: registered new device driver usb
[    0.164469] pps_core: LinuxPPS API ver. 1 registered
[    0.164478] pps_core: Software ver. 5.3.6 - Copyright 2005-2007
Rodolfo Giometti <giometti@linux.it>
[    0.164508] PTP clock support registered
[    0.165340] clocksource: Switched to clocksource timer1
[    0.188264] NET: Registered protocol family 2
[    0.188761] TCP established hash table entries: 8192 (order: 3, 32768
bytes)
[    0.188839] TCP bind hash table entries: 8192 (order: 4, 65536 bytes)
[    0.188961] TCP: Hash tables configured (established 8192 bind 8192)
[    0.189040] UDP hash table entries: 512 (order: 2, 16384 bytes)
[    0.189083] UDP-Lite hash table entries: 512 (order: 2, 16384 bytes)
[    0.189250] NET: Registered protocol family 1
[    0.189570] RPC: Registered named UNIX socket transport module.
[    0.189579] RPC: Registered udp transport module.
[    0.189584] RPC: Registered tcp transport module.
[    0.189589] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    0.190622] futex hash table entries: 512 (order: 3, 32768 bytes)
[    0.199426] ntfs: driver 2.1.32 [Flags: R/W].
[    0.200257] io scheduler noop registered (default)
[    0.204012] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    0.204920] console [ttyS0] disabled
[    0.204954] ffc02000.serial0: ttyS0 at MMIO 0xffc02000 (irq = 34,
base_baud = 6250000) is a 16550A
[    0.740840] console [ttyS0] enabled
[    0.744835] ffc03000.serial1: ttyS1 at MMIO 0xffc03000 (irq = 35,
base_baud = 6250000) is a 16550A
[    0.755333] brd: module loaded
[    0.758903] CAN device driver interface
[    0.763108] stmmac - user ID: 0x10, Synopsys ID: 0x37
[    0.768216]  Ring mode enabled
[    0.771259]  DMA HW capability register supported
[    0.775776]  Enhanced/Alternate descriptors
[    0.780122]  Enabled extended descriptors
[    0.784112]  RX Checksum Offload Engine supported (type 2)
[    0.789585]  TX Checksum insertion supported
[    0.793835]  Enable RX Mitigation via HW Watchdog Timer
[    0.804992] libphy: stmmac: probed
[    0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
[    1.665377] dwc2 ffb40000.usb: DWC OTG Controller
[    1.670086] dwc2 ffb40000.usb: new USB bus registered, assigned bus
number 1
[    1.677138] dwc2 ffb40000.usb: irq 36, io mem 0x00000000
[    1.683139] hub 1-0:1.0: USB hub found
[    1.686917] hub 1-0:1.0: 1 port detected
[    1.691477] mousedev: PS/2 mouse device common for all mice
[    1.697271] i2c /dev entries driver
[    1.701325] Synopsys Designware Multimedia Card Interface Driver
[    1.707550] dw_mmc ff704000.dwmmc0: IDMAC supports 32-bit address mode.
[    1.714163] dw_mmc ff704000.dwmmc0: Using PIO mode.
[    1.719035] dw_mmc ff704000.dwmmc0: Version ID is 240a
[    1.724185] dw_mmc ff704000.dwmmc0: DW MMC controller at irq 30,32
bit host data width,1024 deep fifo
[    1.733502] dw_mmc ff704000.dwmmc0: Got CD GPIO
[    1.775329] dw_mmc ff704000.dwmmc0: 1 slots initialized
[    1.780769] usbcore: registered new interface driver usbhid
[    1.786338] usbhid: USB HID core driver
[    1.790327] oprofile: no performance counters
[    1.794750] oprofile: using timer interrupt.
[    1.799920] NET: Registered protocol family 10
[    1.805084] sit: IPv6 over IPv4 tunneling driver
[    1.810277] NET: Registered protocol family 17
[    1.814726] NET: Registered protocol family 15
[    1.819173] can: controller area network core (rev 20120528 abi 9)
[    1.825384] NET: Registered protocol family 29
[    1.829817] can: raw protocol (rev 20120528)
[    1.834073] can: broadcast manager protocol (rev 20120528 t)
[    1.839727] can: netlink gateway (rev 20130117) max_hops=1
[    1.845361] 8021q: 802.1Q VLAN Support v1.8
[    1.849630] ThumbEE CPU extension supported.
[    1.853900] Registering SWP/SWPB emulation handler
[    1.854029] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req
50000000Hz, actual 50000000HZ div = 0)
[    1.854079] mmc0: new high speed SDHC card at address aaaa
[    1.854496] mmcblk0: mmc0:aaaa SU32G 29.7 GiB
[    1.855953]  mmcblk0: p1 p2 p3
[    1.985343] Sending DHCP requests ..
[    5.955860] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx
[    9.105279] .... timed out!
[   85.166664] IP-Config: Retrying forever (NFS root)...
[   85.256666] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[   89.245860] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx
[   89.255301] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   89.275329] Sending DHCP requests ...... timed out!
[  161.306641] IP-Config: Retrying forever (NFS root)...
[  161.396643] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[  165.385861] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx
[  165.395301] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 20:03 ` Florian Fainelli
@ 2015-10-15 19:59   ` Dinh Nguyen
  2015-10-15 20:25     ` Florian Fainelli
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-15 19:59 UTC (permalink / raw)
  To: Florian Fainelli, David S. Miller; +Cc: david.daney, netdev, linux-kernel

On 10/15/2015 03:03 PM, Florian Fainelli wrote:
> On 15/10/15 12:09, Dinh Nguyen wrote:
>> Hi,
>>
>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>> platform which is using the stmmac ethernet driver.
> 
> It is not clear to me how this relates to what you are seeing yet.
> 
>>
>> It appears that during DHCP, it cannot get an IP address. This only
>> happens if ethernet was not used by the bootloader to tftp an kernel
>> image. If I use the bootloader to tftp an image then ethernet is working
>> fine. So I think the PHY is not getting enabled properly.
>>
>> If I revert this patch, then ethernet is back to working on the platform.
> 
> Is the Device Tree source for this platform available somewhere to look at?
> 

Yes, I'm using the DTS that is in the mainline:

arch/arm/boot/dts/socfpga.dtsi
arch/arm/boot/dts/socfpga_cyclone5.dtsi
arch/arm/boot/dts/socfpga_cyclone5_socdk.dts

Dinh



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 19:09 SoCFPGA ethernet broken Dinh Nguyen
@ 2015-10-15 20:03 ` Florian Fainelli
  2015-10-15 19:59   ` Dinh Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Florian Fainelli @ 2015-10-15 20:03 UTC (permalink / raw)
  To: Dinh Nguyen, David S. Miller; +Cc: david.daney, netdev, linux-kernel

On 15/10/15 12:09, Dinh Nguyen wrote:
> Hi,
> 
> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
> the bus' parent." seems to have broken ethernet support for the SoCFPGA
> platform which is using the stmmac ethernet driver.

It is not clear to me how this relates to what you are seeing yet.

> 
> It appears that during DHCP, it cannot get an IP address. This only
> happens if ethernet was not used by the bootloader to tftp an kernel
> image. If I use the bootloader to tftp an image then ethernet is working
> fine. So I think the PHY is not getting enabled properly.
> 
> If I revert this patch, then ethernet is back to working on the platform.

Is the Device Tree source for this platform available somewhere to look at?

> 
> I just tested this in the v4.3-rc5.
> 
> Will continue to debug, but was wondering if anyone has seen this issue.
> 
> Thanks for any help/comments.
> 
> Dinh
> 
> Bootlog below.
> ----
> [    0.000000] Booting Linux on physical CPU 0x0
> [    0.000000] Initializing cgroup subsys cpuset
> [    0.000000] Linux version 4.3.0-rc5-00037-g5b5f145
> (dinguyen@linux-builds1) (gcc version 4.7.3 20130226 (prerelease) (crosstool
> -NG linaro-1.13.1-4.7-2013.03-20130313 - Linaro GCC 2013.03) ) #1 SMP
> Thu Oct 15 13:55:48 CDT 2015
> [    0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7),
> cr=10c5387d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
> instruction cache
> [    0.000000] Machine model: Altera SOCFPGA Cyclone V SoC Development Kit
> [    0.000000] Truncating RAM at 0x00000000-0x40000000 to -0x2f800000
> [    0.000000] Consider using a HIGHMEM enabled kernel.
> [    0.000000] Memory policy: Data cache writealloc
> [    0.000000] On node 0 totalpages: 194560
> [    0.000000] free_area_init_node: node 0, pgdat c067c080, node_mem_map
> ef20b000
> [    0.000000]   Normal zone: 1520 pages used for memmap
> [    0.000000]   Normal zone: 0 pages reserved
> [    0.000000]   Normal zone: 194560 pages, LIFO batch:31
> [    0.000000] PERCPU: Embedded 13 pages/cpu @ef1df000 s21760 r8192
> d23296 u53248
> [    0.000000] pcpu-alloc: s21760 r8192 d23296 u53248 alloc=13*4096
> [    0.000000] pcpu-alloc: [0] 0 [0] 1
> [    0.000000] Built 1 zonelists in Zone order, mobility grouping on.
> Total pages: 193040
> [    0.000000] Kernel command line: console=ttyS0,115200 root=/dev/nfs
> rw nfsroot=137.57.160.210:/home/dinguyen/rootfs_yocto ip=dh
> cp debug
> [    0.000000] PID hash table entries: 4096 (order: 2, 16384 bytes)
> [    0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288
> bytes)
> [    0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144
> bytes)
> [    0.000000] Memory: 764368K/778240K available (4696K kernel code,
> 274K rwdata, 1328K rodata, 324K init, 131K bss, 13872K reserv
> ed, 0K cma-reserved)
> [    0.000000] Virtual kernel memory layout:
> [    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
> [    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
> [    0.000000]     vmalloc : 0xf0000000 - 0xff000000   ( 240 MB)
> [    0.000000]     lowmem  : 0xc0000000 - 0xef800000   ( 760 MB)
> [    0.000000]     modules : 0xbf000000 - 0xc0000000   (  16 MB)
> [    0.000000]       .text : 0xc0008000 - 0xc05ea5f4   (6026 kB)
> [    0.000000]       .init : 0xc05eb000 - 0xc063c000   ( 324 kB)
> [    0.000000]       .data : 0xc063c000 - 0xc0680b30   ( 275 kB)
> [    0.000000]        .bss : 0xc0680b30 - 0xc06a1a38   ( 132 kB)
> [    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
> [    0.000000] Hierarchical RCU implementation.
> [    0.000000]  Build-time adjustment of leaf fanout to 32.
> [    0.000000] NR_IRQS:16 nr_irqs:16 16
> [    0.000000] L2C-310 enabling early BRESP for Cortex-A9
> [    0.000000] L2C-310 full line of zeros enabled for Cortex-A9
> [    0.000000] L2C-310 ID prefetch enabled, offset 1 lines
> [    0.000000] L2C-310 dynamic clock gating enabled, standby mode enabled
> [    0.000000] L2C-310 cache controller enabled, 8 ways, 512 kB
> [    0.000000] L2C-310: CACHE_ID 0x410030c9, AUX_CTRL 0x76060001
> [    0.000000] clocksource: timer1: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 19112604467 ns
> [    0.000005] sched_clock: 32 bits at 100MHz, resolution 10ns, wraps
> every 21474836475ns
> [    0.000134] Console: colour dummy device 80x30
> [    0.000151] Calibrating delay loop... 1836.64 BogoMIPS (lpj=9183232)
> [    0.059867] pid_max: default: 32768 minimum: 301
> [    0.059938] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
> [    0.059947] Mountpoint-cache hash table entries: 2048 (order: 1, 8192
> bytes)
> [    0.060411] CPU: Testing write buffer coherency: ok
> [    0.060586] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> [    0.060735] Setting up static identity map for 0x8280 - 0x82d8
> [    0.119895] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
> [    0.119954] Brought up 2 CPUs
> [    0.119968] SMP: Total of 2 processors activated (3679.84 BogoMIPS).
> [    0.119974] CPU: All CPU(s) started in SVC mode.
> [    0.120594] devtmpfs: initialized
> [    0.124057] VFP support v0.3: implementor 41 architecture 3 part 30
> variant 9 rev 4
> [    0.124296] clocksource: jiffies: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 19112604462750000 ns
> [    0.125249] NET: Registered protocol family 16
> [    0.125991] DMA: preallocated 256 KiB pool for atomic coherent
> allocations
> [    0.129791] hw-breakpoint: found 5 (+1 reserved) breakpoint and 1
> watchpoint registers.
> [    0.129803] hw-breakpoint: maximum watchpoint size is 4 bytes.
> [    0.163465] SCSI subsystem initialized
> [    0.163736] usbcore: registered new interface driver usbfs
> [    0.163793] usbcore: registered new interface driver hub
> [    0.163848] usbcore: registered new device driver usb
> [    0.164469] pps_core: LinuxPPS API ver. 1 registered
> [    0.164478] pps_core: Software ver. 5.3.6 - Copyright 2005-2007
> Rodolfo Giometti <giometti@linux.it>
> [    0.164508] PTP clock support registered
> [    0.165340] clocksource: Switched to clocksource timer1
> [    0.188264] NET: Registered protocol family 2
> [    0.188761] TCP established hash table entries: 8192 (order: 3, 32768
> bytes)
> [    0.188839] TCP bind hash table entries: 8192 (order: 4, 65536 bytes)
> [    0.188961] TCP: Hash tables configured (established 8192 bind 8192)
> [    0.189040] UDP hash table entries: 512 (order: 2, 16384 bytes)
> [    0.189083] UDP-Lite hash table entries: 512 (order: 2, 16384 bytes)
> [    0.189250] NET: Registered protocol family 1
> [    0.189570] RPC: Registered named UNIX socket transport module.
> [    0.189579] RPC: Registered udp transport module.
> [    0.189584] RPC: Registered tcp transport module.
> [    0.189589] RPC: Registered tcp NFSv4.1 backchannel transport module.
> [    0.190622] futex hash table entries: 512 (order: 3, 32768 bytes)
> [    0.199426] ntfs: driver 2.1.32 [Flags: R/W].
> [    0.200257] io scheduler noop registered (default)
> [    0.204012] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
> [    0.204920] console [ttyS0] disabled
> [    0.204954] ffc02000.serial0: ttyS0 at MMIO 0xffc02000 (irq = 34,
> base_baud = 6250000) is a 16550A
> [    0.740840] console [ttyS0] enabled
> [    0.744835] ffc03000.serial1: ttyS1 at MMIO 0xffc03000 (irq = 35,
> base_baud = 6250000) is a 16550A
> [    0.755333] brd: module loaded
> [    0.758903] CAN device driver interface
> [    0.763108] stmmac - user ID: 0x10, Synopsys ID: 0x37
> [    0.768216]  Ring mode enabled
> [    0.771259]  DMA HW capability register supported
> [    0.775776]  Enhanced/Alternate descriptors
> [    0.780122]  Enabled extended descriptors
> [    0.784112]  RX Checksum Offload Engine supported (type 2)
> [    0.789585]  TX Checksum insertion supported
> [    0.793835]  Enable RX Mitigation via HW Watchdog Timer
> [    0.804992] libphy: stmmac: probed
> [    0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
> [    1.665377] dwc2 ffb40000.usb: DWC OTG Controller
> [    1.670086] dwc2 ffb40000.usb: new USB bus registered, assigned bus
> number 1
> [    1.677138] dwc2 ffb40000.usb: irq 36, io mem 0x00000000
> [    1.683139] hub 1-0:1.0: USB hub found
> [    1.686917] hub 1-0:1.0: 1 port detected
> [    1.691477] mousedev: PS/2 mouse device common for all mice
> [    1.697271] i2c /dev entries driver
> [    1.701325] Synopsys Designware Multimedia Card Interface Driver
> [    1.707550] dw_mmc ff704000.dwmmc0: IDMAC supports 32-bit address mode.
> [    1.714163] dw_mmc ff704000.dwmmc0: Using PIO mode.
> [    1.719035] dw_mmc ff704000.dwmmc0: Version ID is 240a
> [    1.724185] dw_mmc ff704000.dwmmc0: DW MMC controller at irq 30,32
> bit host data width,1024 deep fifo
> [    1.733502] dw_mmc ff704000.dwmmc0: Got CD GPIO
> [    1.775329] dw_mmc ff704000.dwmmc0: 1 slots initialized
> [    1.780769] usbcore: registered new interface driver usbhid
> [    1.786338] usbhid: USB HID core driver
> [    1.790327] oprofile: no performance counters
> [    1.794750] oprofile: using timer interrupt.
> [    1.799920] NET: Registered protocol family 10
> [    1.805084] sit: IPv6 over IPv4 tunneling driver
> [    1.810277] NET: Registered protocol family 17
> [    1.814726] NET: Registered protocol family 15
> [    1.819173] can: controller area network core (rev 20120528 abi 9)
> [    1.825384] NET: Registered protocol family 29
> [    1.829817] can: raw protocol (rev 20120528)
> [    1.834073] can: broadcast manager protocol (rev 20120528 t)
> [    1.839727] can: netlink gateway (rev 20130117) max_hops=1
> [    1.845361] 8021q: 802.1Q VLAN Support v1.8
> [    1.849630] ThumbEE CPU extension supported.
> [    1.853900] Registering SWP/SWPB emulation handler
> [    1.854029] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req
> 50000000Hz, actual 50000000HZ div = 0)
> [    1.854079] mmc0: new high speed SDHC card at address aaaa
> [    1.854496] mmcblk0: mmc0:aaaa SU32G 29.7 GiB
> [    1.855953]  mmcblk0: p1 p2 p3
> [    1.985343] Sending DHCP requests ..
> [    5.955860] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx
> [    9.105279] .... timed out!
> [   85.166664] IP-Config: Retrying forever (NFS root)...
> [   85.256666] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [   89.245860] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx
> [   89.255301] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [   89.275329] Sending DHCP requests ...... timed out!
> [  161.306641] IP-Config: Retrying forever (NFS root)...
> [  161.396643] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [  165.385861] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx
> [  165.395301] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 19:59   ` Dinh Nguyen
@ 2015-10-15 20:25     ` Florian Fainelli
  2015-10-15 20:35       ` David Daney
  2015-12-03 20:48       ` Pavel Machek
  0 siblings, 2 replies; 30+ messages in thread
From: Florian Fainelli @ 2015-10-15 20:25 UTC (permalink / raw)
  To: Dinh Nguyen, David S. Miller; +Cc: david.daney, netdev, linux-kernel

On 15/10/15 12:59, Dinh Nguyen wrote:
> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>> Hi,
>>>
>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>>> platform which is using the stmmac ethernet driver.
>>
>> It is not clear to me how this relates to what you are seeing yet.
>>
>>>
>>> It appears that during DHCP, it cannot get an IP address. This only
>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>> image. If I use the bootloader to tftp an image then ethernet is working
>>> fine. So I think the PHY is not getting enabled properly.
>>>
>>> If I revert this patch, then ethernet is back to working on the platform.
>>
>> Is the Device Tree source for this platform available somewhere to look at?
>>
> 
> Yes, I'm using the DTS that is in the mainline:
> 
> arch/arm/boot/dts/socfpga.dtsi
> arch/arm/boot/dts/socfpga_cyclone5.dtsi
> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts

There are no PHY devices in any of these DTS files, instead there is the
non-standard "phy-addr" property which is set to 0xffffffff supposedly
to indicate that the MDIO bus should be scanned. This is likely part of
your problem. The stmmac driver seems to be looking for "snps,phy-addr"
and not "phy-addr", so I am not even clear how this is supposed to work,
and the driver mentions this custom property is deprecated anyway.

The core problem is in
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
which manually detects the PHY, that is mostly fine, except that it does
not really seem to work here for a reason that is still unclear to me.

Your Ethernet PHYs need to be declared in Device Tree, see
Documentation/devicetree/bindings/net/phy.txt

but aside from that, what should be the PHY driver used by our platform?
ID 00221611 does not seem to be claimed by any known PHY driver, so that
leave us with the Generic PHY driver I guess?

Do you have specific PHY fixups required for this platform?
-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 20:25     ` Florian Fainelli
@ 2015-10-15 20:35       ` David Daney
  2015-10-15 20:49         ` Dinh Nguyen
  2015-12-03 20:48       ` Pavel Machek
  1 sibling, 1 reply; 30+ messages in thread
From: David Daney @ 2015-10-15 20:35 UTC (permalink / raw)
  To: Florian Fainelli, Dinh Nguyen
  Cc: David S. Miller, david.daney, netdev, linux-kernel

On 10/15/2015 01:25 PM, Florian Fainelli wrote:
> On 15/10/15 12:59, Dinh Nguyen wrote:
>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>> Hi,
>>>>
>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>>>> platform which is using the stmmac ethernet driver.
>>>
>>> It is not clear to me how this relates to what you are seeing yet.
>>>
>>>>
>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>> image. If I use the bootloader to tftp an image then ethernet is working
>>>> fine. So I think the PHY is not getting enabled properly.
>>>>
>>>> If I revert this patch, then ethernet is back to working on the platform.
>>>
>>> Is the Device Tree source for this platform available somewhere to look at?
>>>
>>
>> Yes, I'm using the DTS that is in the mainline:
>>
>> arch/arm/boot/dts/socfpga.dtsi
>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>
> There are no PHY devices in any of these DTS files, instead there is the
> non-standard "phy-addr" property which is set to 0xffffffff supposedly
> to indicate that the MDIO bus should be scanned. This is likely part of
> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
> and not "phy-addr", so I am not even clear how this is supposed to work,
> and the driver mentions this custom property is deprecated anyway.
>

I think it is OK not to expose the PHYs in the device tree if they can 
be accurately probed without knowing information from the device tree.

> The core problem is in
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
> which manually detects the PHY, that is mostly fine, except that it does
> not really seem to work here for a reason that is still unclear to me.
>

I agree with this analysis.  I have also been looking at the code and 
cannot see anything that depends on what the parent device of the PHY 
is.  So it is a bit mystifying.


I noticed in your original message you had in the boot log this:

.
.
.
[    0.804992] libphy: stmmac: probed
[    0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
.
.
.

Does this text change with and without the 8b63ec1837fa patch?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 20:35       ` David Daney
@ 2015-10-15 20:49         ` Dinh Nguyen
  2015-10-15 21:30           ` Florian Fainelli
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-15 20:49 UTC (permalink / raw)
  To: David Daney, Florian Fainelli
  Cc: David S. Miller, david.daney, netdev, linux-kernel

On 10/15/2015 03:35 PM, David Daney wrote:
> On 10/15/2015 01:25 PM, Florian Fainelli wrote:
>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>> Hi,
>>>>>
>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>>>> the bus' parent." seems to have broken ethernet support for the
>>>>> SoCFPGA
>>>>> platform which is using the stmmac ethernet driver.
>>>>
>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>
>>>>>
>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>> image. If I use the bootloader to tftp an image then ethernet is
>>>>> working
>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>
>>>>> If I revert this patch, then ethernet is back to working on the
>>>>> platform.
>>>>
>>>> Is the Device Tree source for this platform available somewhere to
>>>> look at?
>>>>
>>>
>>> Yes, I'm using the DTS that is in the mainline:
>>>
>>> arch/arm/boot/dts/socfpga.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>
>> There are no PHY devices in any of these DTS files, instead there is the
>> non-standard "phy-addr" property which is set to 0xffffffff supposedly
>> to indicate that the MDIO bus should be scanned. This is likely part of
>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>> and not "phy-addr", so I am not even clear how this is supposed to work,
>> and the driver mentions this custom property is deprecated anyway.
>>
> 
> I think it is OK not to expose the PHYs in the device tree if they can
> be accurately probed without knowing information from the device tree.
> 
>> The core problem is in
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>> which manually detects the PHY, that is mostly fine, except that it does
>> not really seem to work here for a reason that is still unclear to me.
>>
> 
> I agree with this analysis.  I have also been looking at the code and
> cannot see anything that depends on what the parent device of the PHY
> is.  So it is a bit mystifying.
> 
> 
> I noticed in your original message you had in the boot log this:
> 
> .
> .
> .
> [    0.804992] libphy: stmmac: probed
> [    0.808410] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
> .
> .
> .
> 
> Does this text change with and without the 8b63ec1837fa patch?

No, this text does not change with/without the 8b63ec1837fa patch.

Dinh


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 20:49         ` Dinh Nguyen
@ 2015-10-15 21:30           ` Florian Fainelli
  2015-10-16  2:32             ` Dinh Nguyen
  2015-10-16  3:04             ` Dinh Nguyen
  0 siblings, 2 replies; 30+ messages in thread
From: Florian Fainelli @ 2015-10-15 21:30 UTC (permalink / raw)
  To: Dinh Nguyen, David Daney
  Cc: David S. Miller, david.daney, netdev, linux-kernel

On 15/10/15 13:49, Dinh Nguyen wrote:
>>
>> Does this text change with and without the 8b63ec1837fa patch?
> 
> No, this text does not change with/without the 8b63ec1837fa patch.

Could you instrument mdiobus_scan(), get_phy_device() and
phy_device_create/register to see if the parent is NULL, non-NULL?

So far, I cannot see what is wrong with David's changes, quite the
contrary, and if there was something wrong with the PHY device creation,
it should not get you that far.

You have not answered my previous question though, do you have PHY
fixups registered for that ID?
-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 21:30           ` Florian Fainelli
@ 2015-10-16  2:32             ` Dinh Nguyen
  2015-10-16  3:31               ` Andrew Lunn
  2015-10-16  3:04             ` Dinh Nguyen
  1 sibling, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-16  2:32 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: David Daney, David S. Miller, david.daney, netdev, linux-kernel

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
>

Yes, I can do that.
 
> So far, I cannot see what is wrong with David's changes, quite the
> contrary, and if there was something wrong with the PHY device creation,
> it should not get you that far.
> 
> You have not answered my previous question though, do you have PHY
> fixups registered for that ID?

By fixups, do you mean the skew values that are in the device tree?
Those are the only fixups that I have the PHY.

Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
the skew values get programmed to the phy.

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 21:30           ` Florian Fainelli
  2015-10-16  2:32             ` Dinh Nguyen
@ 2015-10-16  3:04             ` Dinh Nguyen
  1 sibling, 0 replies; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-16  3:04 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: David Daney, David S. Miller, david.daney, netdev, linux-kernel

On Thu, 15 Oct 2015, Florian Fainelli wrote:

> On 15/10/15 13:49, Dinh Nguyen wrote:
> >>
> >> Does this text change with and without the 8b63ec1837fa patch?
> > 
> > No, this text does not change with/without the 8b63ec1837fa patch.
> 
> Could you instrument mdiobus_scan(), get_phy_device() and
> phy_device_create/register to see if the parent is NULL, non-NULL?
> 

[    0.800479] get_phy_device bus=0xeee26c00 addr=00000004 phy_id=00221611
[    0.806735] phy_device_create dev->dev.parent=0xeee26c50
[    0.811114] phy_device_register completed
[    0.814675] mdiobus_scan phydev=0xeee27000
[    1.979034] ksz9021_config_init of_node=00000000

---- Without patch ----

[    0.801294] get_phy_device bus=0xeeef6c00 addr=00000004 phy_id=00221611
[    0.807551] phy_device_create dev->dev.parent=0xeed6cc10
[    0.811929] phy_device_register completed
[    0.815510] mdiobus_scan phydev=0xeeef7000
[    1.979034] ksz9021_config_init of_node=0xef203538
[    1.988064] ksz9021_load_values_from_of
[    1.992485] ksz9021_load_values_from_of
[    1.996905] ksz9021_load_values_from_of 

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16  2:32             ` Dinh Nguyen
@ 2015-10-16  3:31               ` Andrew Lunn
  2015-10-16 14:38                 ` Dinh Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2015-10-16  3:31 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Florian Fainelli, David Daney, David S. Miller, david.daney,
	netdev, linux-kernel

> Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
> are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
> the skew values get programmed to the phy.

Ah!

You have the phy device tree parameters in the wrong place. These are
phy paramters, so should really be in the phy node. But
socfpga_cyclone5_socdk.dts has them in the MAC node.

There is nothing in Documentation/devicetree/bindings/net/micrel.txt
which says you are allowed to place them in the MAC node. Obviously
the code did allow this, which is what has now broken.

    Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16  3:31               ` Andrew Lunn
@ 2015-10-16 14:38                 ` Dinh Nguyen
  2015-10-16 15:03                   ` Andrew Lunn
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-16 14:38 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, David Daney, David S. Miller, david.daney,
	netdev, linux-kernel

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> > the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
> > are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
> > the skew values get programmed to the phy.
> 
> Ah!
> 
> You have the phy device tree parameters in the wrong place. These are
> phy paramters, so should really be in the phy node. But
> socfpga_cyclone5_socdk.dts has them in the MAC node.
>

Alright, let me see if I can rework the DTS.
 
> There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> which says you are allowed to place them in the MAC node. Obviously
> the code did allow this, which is what has now broken.
>

I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
in this document I was following the autodetected PHY example. Did I mis-interpret
the example?

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 14:38                 ` Dinh Nguyen
@ 2015-10-16 15:03                   ` Andrew Lunn
  2015-10-16 15:31                     ` Dinh Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2015-10-16 15:03 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Florian Fainelli, David Daney, David S. Miller, david.daney,
	netdev, linux-kernel

On Fri, Oct 16, 2015 at 09:38:37AM -0500, Dinh Nguyen wrote:
> On Fri, 16 Oct 2015, Andrew Lunn wrote:
> 
> > > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> > > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> > > the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
> > > are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
> > > the skew values get programmed to the phy.
> > 
> > Ah!
> > 
> > You have the phy device tree parameters in the wrong place. These are
> > phy paramters, so should really be in the phy node. But
> > socfpga_cyclone5_socdk.dts has them in the MAC node.
> >
> 
> Alright, let me see if I can rework the DTS.

Well, we are not supposed to break device tree bindings. So we should
try to make this work again.
  
> > There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> > which says you are allowed to place them in the MAC node. Obviously
> > the code did allow this, which is what has now broken.
> 
> I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
> in this document I was following the autodetected PHY example. Did I mis-interpret
> the example?

I was looking at the wrong binding documentation. So yes, it is
documented you can do this.

But i still think it is wrong. These are phy properties, implemented
by the phy, so should be in the phy node. So moving them would be
good. But as i said, we should fix backwards compatibility if
possible.

	Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 15:03                   ` Andrew Lunn
@ 2015-10-16 15:31                     ` Dinh Nguyen
  2015-10-16 15:56                       ` Andrew Lunn
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-16 15:31 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, David Daney, David S. Miller, david.daney,
	netdev, linux-kernel

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> On Fri, Oct 16, 2015 at 09:38:37AM -0500, Dinh Nguyen wrote:
> > On Fri, 16 Oct 2015, Andrew Lunn wrote:
> > 
> > > > Another debugging point, the SoCFPGA board has a Micrel ksz9021 PHY attached
> > > > to the ethernet port. What I'm seeing is that with 8b63ec1837fa patch, when
> > > > the call to ksz9021_config_init() is made both of_node and dev->parent->of_node
> > > > are NULL, without the patch the dev->parent->of_node is a valid pointer. Thus
> > > > the skew values get programmed to the phy.
> > > 
> > > Ah!
> > > 
> > > You have the phy device tree parameters in the wrong place. These are
> > > phy paramters, so should really be in the phy node. But
> > > socfpga_cyclone5_socdk.dts has them in the MAC node.
> > >
> > 
> > Alright, let me see if I can rework the DTS.
> 
> Well, we are not supposed to break device tree bindings. So we should
> try to make this work again.
>  

Great..thanks for recognizing that point.
 
> > > There is nothing in Documentation/devicetree/bindings/net/micrel.txt
> > > which says you are allowed to place them in the MAC node. Obviously
> > > the code did allow this, which is what has now broken.
> > 
> > I was following Documentation/devicetree/bindings/net/micrel-ksz90x1.txt and
> > in this document I was following the autodetected PHY example. Did I mis-interpret
> > the example?
> 
> I was looking at the wrong binding documentation. So yes, it is
> documented you can do this.
> 
> But i still think it is wrong. These are phy properties, implemented
> by the phy, so should be in the phy node. So moving them would be
> good. But as i said, we should fix backwards compatibility if
> possible.
>

I moved the phy settings to a separate phy node, but it did not seem to make a
difference at all.

So I think I'll move to inspect what Florian had suggested, and that was to look
at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register 

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 15:31                     ` Dinh Nguyen
@ 2015-10-16 15:56                       ` Andrew Lunn
  2015-10-16 16:47                         ` David Daney
  2015-10-16 18:17                         ` Florian Fainelli
  0 siblings, 2 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-10-16 15:56 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: Florian Fainelli, David Daney, David S. Miller, david.daney,
	netdev, linux-kernel

> So I think I'll move to inspect what Florian had suggested, and that was to look
> at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register 

I have a suspicion. If you look at the phy driver it does:

static int ksz9021_config_init(struct phy_device *phydev)
{
        const struct device *dev = &phydev->dev;
        const struct device_node *of_node = dev->of_node;

        if (!of_node && dev->parent->of_node)
                of_node = dev->parent->of_node;


In your case, you don't have a phy node in your device tree, so of_node
is NULL. So it looks in the parent device.

    phylib: Make PHYs children of their MDIO bus, not the bus' parent.

changed what the parent is. It is now the mdio device. Before, i
suspect it was the MAC. Hence it found your properties in the MAC
node.

What i think you might want to do is change this code. Rather than
look a dev->parent->of_node; you might want
phydev->attached_dev->dev->of_node.

This assumes the phy has been attached to the MAC. I've no idea of the
ordering, so maybe it has not been attached yet?

dp83867.c has similar code. However quick grep did not find any
mainline users with properties in the MAC node. If that is true, i
would suggest removing the code looking in the parent for that phy
driver.

	Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 15:56                       ` Andrew Lunn
@ 2015-10-16 16:47                         ` David Daney
  2015-10-16 19:10                           ` Dinh Nguyen
  2015-10-16 18:17                         ` Florian Fainelli
  1 sibling, 1 reply; 30+ messages in thread
From: David Daney @ 2015-10-16 16:47 UTC (permalink / raw)
  To: Andrew Lunn, Dinh Nguyen, Florian Fainelli
  Cc: David S. Miller, david.daney, netdev, linux-kernel

On 10/16/2015 08:56 AM, Andrew Lunn wrote:
>> So I think I'll move to inspect what Florian had suggested, and that was to look
>> at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>
> I have a suspicion. If you look at the phy driver it does:
>
> static int ksz9021_config_init(struct phy_device *phydev)
> {
>          const struct device *dev = &phydev->dev;
>          const struct device_node *of_node = dev->of_node;
>
>          if (!of_node && dev->parent->of_node)
>                  of_node = dev->parent->of_node;
>

Maybe we need to walk up the hierarchy.

Perhaps something like:

const struct device *dev_walker;

dev_walker = &phydev->dev;
do {
    of_node = dev_walker->of_node;
    dev_walker = dev_walker->parent;
} while (!of_node && dev_walker);

An alternative would be to assign the bus the same of_node as the bus 
parent.

If either approach works, you can add:
  Acked-by: David Daney <david.daney@cavium.com>

to the patch that implements it.

>
> In your case, you don't have a phy node in your device tree, so of_node
> is NULL. So it looks in the parent device.
>
>      phylib: Make PHYs children of their MDIO bus, not the bus' parent.
>
> changed what the parent is. It is now the mdio device. Before, i
> suspect it was the MAC. Hence it found your properties in the MAC
> node.
>
> What i think you might want to do is change this code. Rather than
> look a dev->parent->of_node; you might want
> phydev->attached_dev->dev->of_node.
>
> This assumes the phy has been attached to the MAC. I've no idea of the
> ordering, so maybe it has not been attached yet?
>
> dp83867.c has similar code. However quick grep did not find any
> mainline users with properties in the MAC node. If that is true, i
> would suggest removing the code looking in the parent for that phy
> driver.
>
> 	Andrew
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 15:56                       ` Andrew Lunn
  2015-10-16 16:47                         ` David Daney
@ 2015-10-16 18:17                         ` Florian Fainelli
  1 sibling, 0 replies; 30+ messages in thread
From: Florian Fainelli @ 2015-10-16 18:17 UTC (permalink / raw)
  To: Andrew Lunn, Dinh Nguyen
  Cc: David Daney, David S. Miller, david.daney, netdev, linux-kernel

On 16/10/15 08:56, Andrew Lunn wrote:
>> So I think I'll move to inspect what Florian had suggested, and that was to look
>> at: drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register 
> 
> I have a suspicion. If you look at the phy driver it does:
> 
> static int ksz9021_config_init(struct phy_device *phydev)
> {
>         const struct device *dev = &phydev->dev;
>         const struct device_node *of_node = dev->of_node;
> 
>         if (!of_node && dev->parent->of_node)
>                 of_node = dev->parent->of_node;
> 
> 
> In your case, you don't have a phy node in your device tree, so of_node
> is NULL. So it looks in the parent device.
> 
>     phylib: Make PHYs children of their MDIO bus, not the bus' parent.
> 
> changed what the parent is. It is now the mdio device. Before, i
> suspect it was the MAC. Hence it found your properties in the MAC
> node.
> 
> What i think you might want to do is change this code. Rather than
> look a dev->parent->of_node; you might want
> phydev->attached_dev->dev->of_node.

Yes, that would work, by the time config_init() executes, you are
guaranteed to have a valid attached_dev pointer.

> 
> This assumes the phy has been attached to the MAC. I've no idea of the
> ordering, so maybe it has not been attached yet?
> 
> dp83867.c has similar code. However quick grep did not find any
> mainline users with properties in the MAC node. If that is true, i
> would suggest removing the code looking in the parent for that phy
> driver.

I think we should fix these drivers to look for these properties where
they were used to, as a short term plan, and then make sure that their
binding document is very specific about the fact that these properties,
are by definition, properties of the Ethernet PHY, not the Ethernet MAC.
-- 
Florian

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 16:47                         ` David Daney
@ 2015-10-16 19:10                           ` Dinh Nguyen
  2015-10-16 19:38                             ` Andrew Lunn
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-16 19:10 UTC (permalink / raw)
  To: David Daney
  Cc: Andrew Lunn, Florian Fainelli, David S. Miller, david.daney,
	netdev, linux-kernel

On Fri, 16 Oct 2015, David Daney wrote:

> On 10/16/2015 08:56 AM, Andrew Lunn wrote:
> > > So I think I'll move to inspect what Florian had suggested, and that was
> > > to look
> > > at:
> > > drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
> > 
> > I have a suspicion. If you look at the phy driver it does:
> > 
> > static int ksz9021_config_init(struct phy_device *phydev)
> > {
> >          const struct device *dev = &phydev->dev;
> >          const struct device_node *of_node = dev->of_node;
> > 
> >          if (!of_node && dev->parent->of_node)
> >                  of_node = dev->parent->of_node;
> > 
> 
> Maybe we need to walk up the hierarchy.
> 
> Perhaps something like:
> 
> const struct device *dev_walker;
> 
> dev_walker = &phydev->dev;
> do {
>    of_node = dev_walker->of_node;
>    dev_walker = dev_walker->parent;
> } while (!of_node && dev_walker);
> 

The above code seems to have fixed the issue.

> An alternative would be to assign the bus the same of_node as the bus parent.
> 
> If either approach works, you can add:
>  Acked-by: David Daney <david.daney@cavium.com>
> 
> to the patch that implements it.
> 
> > 
> > In your case, you don't have a phy node in your device tree, so of_node
> > is NULL. So it looks in the parent device.
> > 
> >      phylib: Make PHYs children of their MDIO bus, not the bus' parent.
> > 
> > changed what the parent is. It is now the mdio device. Before, i
> > suspect it was the MAC. Hence it found your properties in the MAC
> > node.
> > 
> > What i think you might want to do is change this code. Rather than
> > look a dev->parent->of_node; you might want
> > phydev->attached_dev->dev->of_node.
> > 
> > This assumes the phy has been attached to the MAC. I've no idea of the
> > ordering, so maybe it has not been attached yet?
> > 
> > dp83867.c has similar code. However quick grep did not find any
> > mainline users with properties in the MAC node. If that is true, i
> > would suggest removing the code looking in the parent for that phy
> > driver.
> > 
> > 	Andrew
> > 
> 
> 

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 19:10                           ` Dinh Nguyen
@ 2015-10-16 19:38                             ` Andrew Lunn
  2015-10-16 20:24                               ` David Daney
  2015-10-19 15:14                               ` Dinh Nguyen
  0 siblings, 2 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-10-16 19:38 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: David Daney, Florian Fainelli, David S. Miller, david.daney,
	netdev, linux-kernel

> > Maybe we need to walk up the hierarchy.
> > 
> > Perhaps something like:
> > 
> > const struct device *dev_walker;
> > 
> > dev_walker = &phydev->dev;
> > do {
> >    of_node = dev_walker->of_node;
> >    dev_walker = dev_walker->parent;
> > } while (!of_node && dev_walker);
> > 
> 
> The above code seems to have fixed the issue.

What i don't like about this is that it allows you to put these
properties in the mdio device node. These are phy properties, not mdio
properties....

If phydev->attached_dev->dev->of_node works, that would be my
preference.

       Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 19:38                             ` Andrew Lunn
@ 2015-10-16 20:24                               ` David Daney
  2015-10-16 20:29                                 ` Andrew Lunn
  2015-10-19 15:14                               ` Dinh Nguyen
  1 sibling, 1 reply; 30+ messages in thread
From: David Daney @ 2015-10-16 20:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Dinh Nguyen, Florian Fainelli, David S. Miller, david.daney,
	netdev, linux-kernel

On 10/16/2015 12:38 PM, Andrew Lunn wrote:
>>> Maybe we need to walk up the hierarchy.
>>>
>>> Perhaps something like:
>>>
>>> const struct device *dev_walker;
>>>
>>> dev_walker = &phydev->dev;
>>> do {
>>>     of_node = dev_walker->of_node;
>>>     dev_walker = dev_walker->parent;
>>> } while (!of_node && dev_walker);
>>>
>>
>> The above code seems to have fixed the issue.
>
> What i don't like about this is that it allows you to put these
> properties in the mdio device node. These are phy properties, not mdio
> properties....

Yes, we know that the device tree is not correctly formed.

You have a choice:

A) Fix the device tree and any code that may have to change to work with 
a good device tree.

B) Change the code to work with the screwy existing device tree.  The 
above seems to work, other things may also be possible.

I can't really make any decisions as to what the best way to proceed is, 
as I neither have the hardware in question, nor the time to work on it.

David Daney

>
> If phydev->attached_dev->dev->of_node works, that would be my
> preference.
>
>         Andrew
>


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 20:24                               ` David Daney
@ 2015-10-16 20:29                                 ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-10-16 20:29 UTC (permalink / raw)
  To: David Daney
  Cc: Dinh Nguyen, Florian Fainelli, David S. Miller, david.daney,
	netdev, linux-kernel

On Fri, Oct 16, 2015 at 01:24:16PM -0700, David Daney wrote:
> On 10/16/2015 12:38 PM, Andrew Lunn wrote:
> >>>Maybe we need to walk up the hierarchy.
> >>>
> >>>Perhaps something like:
> >>>
> >>>const struct device *dev_walker;
> >>>
> >>>dev_walker = &phydev->dev;
> >>>do {
> >>>    of_node = dev_walker->of_node;
> >>>    dev_walker = dev_walker->parent;
> >>>} while (!of_node && dev_walker);
> >>>
> >>
> >>The above code seems to have fixed the issue.
> >
> >What i don't like about this is that it allows you to put these
> >properties in the mdio device node. These are phy properties, not mdio
> >properties....
> 
> Yes, we know that the device tree is not correctly formed.
> 
> You have a choice:
> 
> A) Fix the device tree and any code that may have to change to work
> with a good device tree.
> 
> B) Change the code to work with the screwy existing device tree.
> The above seems to work, other things may also be possible.
> 
> I can't really make any decisions as to what the best way to proceed
> is, as I neither have the hardware in question, nor the time to work
> on it.

Hi hope Dinh will test with phydev->attached_dev->dev->of_node.

   Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-19 15:14                               ` Dinh Nguyen
@ 2015-10-19 10:50                                 ` Dinh Nguyen
  0 siblings, 0 replies; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-19 10:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Daney, Florian Fainelli, David S. Miller, david.daney,
	peppe.cavallaro, srinivas.kandagatla, maxime.coquelin,
	patrice.chotard, heiko, Bnetdev, linux-kernel

On Mon, 19 Oct 2015, Dinh Nguyen wrote:

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

> 
> On Fri, 16 Oct 2015, Andrew Lunn wrote:
> 
> > > > Maybe we need to walk up the hierarchy.
> > > > 
> > > > Perhaps something like:
> > > > 
> > > > const struct device *dev_walker;
> > > > 
> > > > dev_walker = &phydev->dev;
> > > > do {
> > > >    of_node = dev_walker->of_node;
> > > >    dev_walker = dev_walker->parent;
> > > > } while (!of_node && dev_walker);
> > > > 
> > > 
> > > The above code seems to have fixed the issue.
> > 
> > What i don't like about this is that it allows you to put these
> > properties in the mdio device node. These are phy properties, not mdio
> > properties....
> >
> 
AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now now placing a hard requirement that the phy must be
in a separate node. For now, this is breaking SoCFPGA, but I think it may also
impact Rockchip and STi?

> > If phydev->attached_dev->dev->of_node works, that would be my
> > preference.
> >

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-16 19:38                             ` Andrew Lunn
  2015-10-16 20:24                               ` David Daney
@ 2015-10-19 15:14                               ` Dinh Nguyen
  2015-10-19 10:50                                 ` Dinh Nguyen
  1 sibling, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-10-19 15:14 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Daney, Florian Fainelli, David S. Miller, david.daney,
	peppe.cavallaro, srinivas.kandagatla, maxime.coquelin,
	patrice.chotard, heiko, netdev, linux-kernel

+CC Giuseppe Cavallaro
+CC STi and Rockchip Maintainers

This is approaching beyond my breadth of knowledge on this subject, so I just
wanted to get some further insight.

On Fri, 16 Oct 2015, Andrew Lunn wrote:

> > > Maybe we need to walk up the hierarchy.
> > > 
> > > Perhaps something like:
> > > 
> > > const struct device *dev_walker;
> > > 
> > > dev_walker = &phydev->dev;
> > > do {
> > >    of_node = dev_walker->of_node;
> > >    dev_walker = dev_walker->parent;
> > > } while (!of_node && dev_walker);
> > > 
> > 
> > The above code seems to have fixed the issue.
> 
> What i don't like about this is that it allows you to put these
> properties in the mdio device node. These are phy properties, not mdio
> properties....
>

AFAICT, the stmmac driver is allowing for the phy node to be part of the mdio.
In the function, stmmac_init_phy(), there is a separate check of a standalone
phy_node, and the case where the phy is part of the mdio.

commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
the bus' parent." is now placing a hard requirement to have a PHY as a
separate node. For now this is only breaking SoCFPGA, but perhaps might
have affected STi41x and RockChip, but maybe haven't been spotted because
perhaps the testing has the bootloader setting up the PHYs?

> If phydev->attached_dev->dev->of_node works, that would be my
> preference.
>

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-10-15 20:25     ` Florian Fainelli
  2015-10-15 20:35       ` David Daney
@ 2015-12-03 20:48       ` Pavel Machek
  2015-12-03 21:23         ` David Daney
  1 sibling, 1 reply; 30+ messages in thread
From: Pavel Machek @ 2015-12-03 20:48 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Dinh Nguyen, David S. Miller, david.daney, netdev, linux-kernel

On Thu 2015-10-15 13:25:59, Florian Fainelli wrote:
> On 15/10/15 12:59, Dinh Nguyen wrote:
> > On 10/15/2015 03:03 PM, Florian Fainelli wrote:
> >> On 15/10/15 12:09, Dinh Nguyen wrote:
> >>> Hi,
> >>>
> >>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
> >>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
> >>> platform which is using the stmmac ethernet driver.
> >>
> >> It is not clear to me how this relates to what you are seeing yet.
> >>
> >>>
> >>> It appears that during DHCP, it cannot get an IP address. This only
> >>> happens if ethernet was not used by the bootloader to tftp an kernel
> >>> image. If I use the bootloader to tftp an image then ethernet is working
> >>> fine. So I think the PHY is not getting enabled properly.
> >>>
> >>> If I revert this patch, then ethernet is back to working on the platform.
> >>
> >> Is the Device Tree source for this platform available somewhere to look at?
> >>
> > 
> > Yes, I'm using the DTS that is in the mainline:
> > 
> > arch/arm/boot/dts/socfpga.dtsi
> > arch/arm/boot/dts/socfpga_cyclone5.dtsi
> > arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> 
> There are no PHY devices in any of these DTS files, instead there is the
> non-standard "phy-addr" property which is set to 0xffffffff supposedly
> to indicate that the MDIO bus should be scanned. This is likely part of
> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
> and not "phy-addr", so I am not even clear how this is supposed to work,
> and the driver mentions this custom property is deprecated anyway.
> 
> The core problem is in
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
> which manually detects the PHY, that is mostly fine, except that it does
> not really seem to work here for a reason that is still unclear to me.
> 
> Your Ethernet PHYs need to be declared in Device Tree, see
> Documentation/devicetree/bindings/net/phy.txt

While updating DTS might be good idea, I don't think you can simply
blame this on DTS. If it worked before the change, it is supposed to
work after the change, otherwise we call that change a "regression"
and revert the change. 

Plus, DTS is supposed to be ABI. Old DTS should still work on new
kernels in ideal world.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-03 20:48       ` Pavel Machek
@ 2015-12-03 21:23         ` David Daney
  2015-12-03 23:17           ` Dinh Nguyen
  2015-12-04  9:38           ` Pavel Machek
  0 siblings, 2 replies; 30+ messages in thread
From: David Daney @ 2015-12-03 21:23 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Florian Fainelli, Dinh Nguyen, David S. Miller, david.daney,
	netdev, linux-kernel

On 12/03/2015 12:48 PM, Pavel Machek wrote:
> On Thu 2015-10-15 13:25:59, Florian Fainelli wrote:
>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>> Hi,
>>>>>
>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus, not
>>>>> the bus' parent." seems to have broken ethernet support for the SoCFPGA
>>>>> platform which is using the stmmac ethernet driver.
>>>>
>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>
>>>>>
>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>> image. If I use the bootloader to tftp an image then ethernet is working
>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>
>>>>> If I revert this patch, then ethernet is back to working on the platform.
>>>>
>>>> Is the Device Tree source for this platform available somewhere to look at?
>>>>
>>>
>>> Yes, I'm using the DTS that is in the mainline:
>>>
>>> arch/arm/boot/dts/socfpga.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>
>> There are no PHY devices in any of these DTS files, instead there is the
>> non-standard "phy-addr" property which is set to 0xffffffff supposedly
>> to indicate that the MDIO bus should be scanned. This is likely part of
>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>> and not "phy-addr", so I am not even clear how this is supposed to work,
>> and the driver mentions this custom property is deprecated anyway.
>>
>> The core problem is in
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>> which manually detects the PHY, that is mostly fine, except that it does
>> not really seem to work here for a reason that is still unclear to me.
>>
>> Your Ethernet PHYs need to be declared in Device Tree, see
>> Documentation/devicetree/bindings/net/phy.txt
>
> While updating DTS might be good idea, I don't think you can simply
> blame this on DTS. If it worked before the change, it is supposed to
> work after the change, otherwise we call that change a "regression"
> and revert the change.

FWIW: My initial patch to address the failure worked with the original DTB.

Also: userspace wasn't broken. So, the commandment about not breaking 
userspace wasn't broken.  Although admittedly, breaking the kernel isn't 
good either.



>
> Plus, DTS is supposed to be ABI. Old DTS should still work on new
> kernels in ideal world.

If you supply the device tree file in the kernel tree, it is not an ABI.

If the device tree is not part of the kernel, and instead comes from the 
boot firmware of the board, then you could make the ABI claim.

David Daney

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-03 21:23         ` David Daney
@ 2015-12-03 23:17           ` Dinh Nguyen
  2015-12-04  1:10             ` Andrew Lunn
  2015-12-04  9:38           ` Pavel Machek
  1 sibling, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-12-03 23:17 UTC (permalink / raw)
  To: David Daney, Pavel Machek
  Cc: Florian Fainelli, David S. Miller, david.daney, netdev, linux-kernel

On 12/03/2015 03:23 PM, David Daney wrote:
> On 12/03/2015 12:48 PM, Pavel Machek wrote:
>> On Thu 2015-10-15 13:25:59, Florian Fainelli wrote:
>>> On 15/10/15 12:59, Dinh Nguyen wrote:
>>>> On 10/15/2015 03:03 PM, Florian Fainelli wrote:
>>>>> On 15/10/15 12:09, Dinh Nguyen wrote:
>>>>>> Hi,
>>>>>>
>>>>>> commit "8b63ec1837fa phylib: Make PHYs children of their MDIO bus,
>>>>>> not
>>>>>> the bus' parent." seems to have broken ethernet support for the
>>>>>> SoCFPGA
>>>>>> platform which is using the stmmac ethernet driver.
>>>>>
>>>>> It is not clear to me how this relates to what you are seeing yet.
>>>>>
>>>>>>
>>>>>> It appears that during DHCP, it cannot get an IP address. This only
>>>>>> happens if ethernet was not used by the bootloader to tftp an kernel
>>>>>> image. If I use the bootloader to tftp an image then ethernet is
>>>>>> working
>>>>>> fine. So I think the PHY is not getting enabled properly.
>>>>>>
>>>>>> If I revert this patch, then ethernet is back to working on the
>>>>>> platform.
>>>>>
>>>>> Is the Device Tree source for this platform available somewhere to
>>>>> look at?
>>>>>
>>>>
>>>> Yes, I'm using the DTS that is in the mainline:
>>>>
>>>> arch/arm/boot/dts/socfpga.dtsi
>>>> arch/arm/boot/dts/socfpga_cyclone5.dtsi
>>>> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>>>
>>> There are no PHY devices in any of these DTS files, instead there is the
>>> non-standard "phy-addr" property which is set to 0xffffffff supposedly
>>> to indicate that the MDIO bus should be scanned. This is likely part of
>>> your problem. The stmmac driver seems to be looking for "snps,phy-addr"
>>> and not "phy-addr", so I am not even clear how this is supposed to work,
>>> and the driver mentions this custom property is deprecated anyway.
>>>
>>> The core problem is in
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c::stmmac_mdio_register
>>> which manually detects the PHY, that is mostly fine, except that it does
>>> not really seem to work here for a reason that is still unclear to me.
>>>
>>> Your Ethernet PHYs need to be declared in Device Tree, see
>>> Documentation/devicetree/bindings/net/phy.txt
>>
>> While updating DTS might be good idea, I don't think you can simply
>> blame this on DTS. If it worked before the change, it is supposed to
>> work after the change, otherwise we call that change a "regression"
>> and revert the change.
> 
> FWIW: My initial patch to address the failure worked with the original DTB.
> 

Can I ask what patch are you referring to? I was sidetracked for a while
on this issue, but I still see it failing as of v4.4-rc3. I'll try to
get back to debugging this.

Dinh


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-03 23:17           ` Dinh Nguyen
@ 2015-12-04  1:10             ` Andrew Lunn
  2015-12-04  1:50               ` Andrew Lunn
  0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2015-12-04  1:10 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: David Daney, Pavel Machek, Florian Fainelli, David S. Miller,
	david.daney, netdev, linux-kernel

> > FWIW: My initial patch to address the failure worked with the original DTB.
> 
> Can I ask what patch are you referring to? I was sidetracked for a while
> on this issue, but I still see it failing as of v4.4-rc3. I'll try to
> get back to debugging this.

Hi Dinh

There are two different patches:

https://lkml.org/lkml/2015/10/16/669

and

https://www.mail-archive.com/netdev@vger.kernel.org/msg83183.html

Although the first one works, it keeps searching up and up and up, so
you could in theory put the phy properties a lot higher than the MAC.

The second patch restricts where it looks for the phy properties to
only the MAC. But it does not work. We would like to understand why it
does not work.

     Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-04  1:10             ` Andrew Lunn
@ 2015-12-04  1:50               ` Andrew Lunn
  2015-12-04 11:27                 ` Dinh Nguyen
  0 siblings, 1 reply; 30+ messages in thread
From: Andrew Lunn @ 2015-12-04  1:50 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: David Daney, Pavel Machek, Florian Fainelli, David S. Miller,
	david.daney, netdev, linux-kernel

On Fri, Dec 04, 2015 at 02:10:50AM +0100, Andrew Lunn wrote:
> > > FWIW: My initial patch to address the failure worked with the original DTB.
> > 
> > Can I ask what patch are you referring to? I was sidetracked for a while
> > on this issue, but I still see it failing as of v4.4-rc3. I'll try to
> > get back to debugging this.
> 
> Hi Dinh
> 
> There are two different patches:
> 
> https://lkml.org/lkml/2015/10/16/669
> 
> and
> 
> https://www.mail-archive.com/netdev@vger.kernel.org/msg83183.html
> 
> Although the first one works, it keeps searching up and up and up, so
> you could in theory put the phy properties a lot higher than the MAC.
> 
> The second patch restricts where it looks for the phy properties to
> only the MAC. But it does not work. We would like to understand why it
> does not work.

Hi Dinh

Please could you run this patch and let us know what it outputs.

Thanks
	Andrew

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index cf6312fafea5..d7ddc0bb0e7f 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/micrel_phy.h>
+#include <linux/netdevice.h>
 #include <linux/of.h>
 #include <linux/clk.h>
 
@@ -339,9 +340,19 @@ static int ksz9021_config_init(struct phy_device *phydev)
 {
 	const struct device *dev = &phydev->dev;
 	const struct device_node *of_node = dev->of_node;
+	const struct device *dev_walker;
 
-	if (!of_node && dev->parent->of_node)
-		of_node = dev->parent->of_node;
+	dev_info(dev, "dev->parent: %s\n", dev_name(dev->parent));
+	dev_info(dev, "phydev->attached_dev->dev: %s\n", dev_name(&phydev->attached_dev->dev));
+
+	dev_walker = &phydev->dev;
+	do {
+		of_node = dev_walker->of_node;
+		dev_info(dev, "walking: %s %p\n",
+			 dev_name(dev_walker), of_node);
+		dev_walker = dev_walker->parent;
+
+	} while (!of_node && dev_walker);
 
 	if (of_node) {
 		ksz9021_load_values_from_of(phydev, of_node,

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-03 21:23         ` David Daney
  2015-12-03 23:17           ` Dinh Nguyen
@ 2015-12-04  9:38           ` Pavel Machek
  1 sibling, 0 replies; 30+ messages in thread
From: Pavel Machek @ 2015-12-04  9:38 UTC (permalink / raw)
  To: David Daney
  Cc: Florian Fainelli, Dinh Nguyen, David S. Miller, david.daney,
	netdev, linux-kernel


> >While updating DTS might be good idea, I don't think you can simply
> >blame this on DTS. If it worked before the change, it is supposed to
> >work after the change, otherwise we call that change a "regression"
> >and revert the change.
> 
> FWIW: My initial patch to address the failure worked with the original DTB.
> 
> Also: userspace wasn't broken. So, the commandment about not breaking
> userspace wasn't broken.  Although admittedly, breaking the kernel isn't
> good either.

You can't break neither kernel nor userspace.

> >Plus, DTS is supposed to be ABI. Old DTS should still work on new
> >kernels in ideal world.
> 
> If you supply the device tree file in the kernel tree, it is not an ABI.
> 
> If the device tree is not part of the kernel, and instead comes from the
> boot firmware of the board, then you could make the ABI claim.

It is an ABI if it was declared so, and it was. Yes, it _can_ come
from kernel tree. That does not mean it has to.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-04  1:50               ` Andrew Lunn
@ 2015-12-04 11:27                 ` Dinh Nguyen
  2015-12-04 14:31                   ` Andrew Lunn
  0 siblings, 1 reply; 30+ messages in thread
From: Dinh Nguyen @ 2015-12-04 11:27 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David Daney, Pavel Machek, Florian Fainelli, David S. Miller,
	david.daney, netdev, linux-kernel

Hi Andrew,

On Fri, 4 Dec 2015, Andrew Lunn wrote:

> On Fri, Dec 04, 2015 at 02:10:50AM +0100, Andrew Lunn wrote:
> > > > FWIW: My initial patch to address the failure worked with the original DTB.
> > > 
> > > Can I ask what patch are you referring to? I was sidetracked for a while
> > > on this issue, but I still see it failing as of v4.4-rc3. I'll try to
> > > get back to debugging this.
> > 
> > Hi Dinh
> > 
> > There are two different patches:
> > 
> > https://lkml.org/lkml/2015/10/16/669
> > 
> > and
> > 
> > https://www.mail-archive.com/netdev@vger.kernel.org/msg83183.html
> > 
> > Although the first one works, it keeps searching up and up and up, so
> > you could in theory put the phy properties a lot higher than the MAC.
> > 
> > The second patch restricts where it looks for the phy properties to
> > only the MAC. But it does not work. We would like to understand why it
> > does not work.
> 
> Hi Dinh
> 
> Please could you run this patch and let us know what it outputs.
> 
> Thanks
> 	Andrew
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index cf6312fafea5..d7ddc0bb0e7f 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -26,6 +26,7 @@
>  #include <linux/module.h>
>  #include <linux/phy.h>
>  #include <linux/micrel_phy.h>
> +#include <linux/netdevice.h>
>  #include <linux/of.h>
>  #include <linux/clk.h>
>  
> @@ -339,9 +340,19 @@ static int ksz9021_config_init(struct phy_device *phydev)
>  {
>  	const struct device *dev = &phydev->dev;
>  	const struct device_node *of_node = dev->of_node;
> +	const struct device *dev_walker;
>  
> -	if (!of_node && dev->parent->of_node)
> -		of_node = dev->parent->of_node;
> +	dev_info(dev, "dev->parent: %s\n", dev_name(dev->parent));
> +	dev_info(dev, "phydev->attached_dev->dev: %s\n", dev_name(&phydev->attached_dev->dev));
> +
> +	dev_walker = &phydev->dev;
> +	do {
> +		of_node = dev_walker->of_node;
> +		dev_info(dev, "walking: %s %p\n",
> +			 dev_name(dev_walker), of_node);
> +		dev_walker = dev_walker->parent;
> +
> +	} while (!of_node && dev_walker);
>  
>  	if (of_node) {
>  		ksz9021_load_values_from_of(phydev, of_node,
> 

Here is the output from the above patch:

[    1.042049] mmc0: new high speed SDHC card at address aaaa
[    1.048017] mmcblk0: mmc0:aaaa SU32G 29.7 GiB
[    1.053506]  mmcblk0: p1 p2 p3 p4
[    1.057708] dwc2 ffb40000.usb: Configuration mismatch. Forcing host mode
[    1.064418] dwc2 ffb40000.usb: no platform data or transceiver defined
[    1.070966] Micrel KSZ9021 Gigabit PHY stmmac-0:04: dev->parent: stmmac-0
[    1.077746] Micrel KSZ9021 Gigabit PHY stmmac-0:04: phydev->attached_dev->dev: eth0
[    1.085389] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: stmmac-0:04   (null)
[    1.092841] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: stmmac-0   (null)
[    1.100042] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: ff702000.ethernet ef9f3538
[    1.133638] Sending DHCP requests ..
[    5.104138] socfpga-dwmac ff702000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx

BR,
Dinh

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: SoCFPGA ethernet broken
  2015-12-04 11:27                 ` Dinh Nguyen
@ 2015-12-04 14:31                   ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-12-04 14:31 UTC (permalink / raw)
  To: Dinh Nguyen
  Cc: David Daney, Pavel Machek, Florian Fainelli, David S. Miller,
	david.daney, netdev, linux-kernel

> > @@ -339,9 +340,19 @@ static int ksz9021_config_init(struct phy_device *phydev)
> >  {
> >  	const struct device *dev = &phydev->dev;
> >  	const struct device_node *of_node = dev->of_node;
> > +	const struct device *dev_walker;
> >  
> > -	if (!of_node && dev->parent->of_node)
> > -		of_node = dev->parent->of_node;
> > +	dev_info(dev, "dev->parent: %s\n", dev_name(dev->parent));
> > +	dev_info(dev, "phydev->attached_dev->dev: %s\n", dev_name(&phydev->attached_dev->dev));
> > +
> > +	dev_walker = &phydev->dev;
> > +	do {
> > +		of_node = dev_walker->of_node;
> > +		dev_info(dev, "walking: %s %p\n",
> > +			 dev_name(dev_walker), of_node);
> > +		dev_walker = dev_walker->parent;
> > +
> > +	} while (!of_node && dev_walker);
> >  
> >  	if (of_node) {
> >  		ksz9021_load_values_from_of(phydev, of_node,
> > 
> 
> Here is the output from the above patch:
> 
> [    1.042049] mmc0: new high speed SDHC card at address aaaa
> [    1.048017] mmcblk0: mmc0:aaaa SU32G 29.7 GiB
> [    1.053506]  mmcblk0: p1 p2 p3 p4
> [    1.057708] dwc2 ffb40000.usb: Configuration mismatch. Forcing host mode
> [    1.064418] dwc2 ffb40000.usb: no platform data or transceiver defined
> [    1.070966] Micrel KSZ9021 Gigabit PHY stmmac-0:04: dev->parent: stmmac-0
> [    1.077746] Micrel KSZ9021 Gigabit PHY stmmac-0:04: phydev->attached_dev->dev: eth0
> [    1.085389] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: stmmac-0:04   (null)
> [    1.092841] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: stmmac-0   (null)
> [    1.100042] Micrel KSZ9021 Gigabit PHY stmmac-0:04: walking: ff702000.ethernet ef9f3538

Ah! So we have an intermediary device in the
chain. phydev->attached_dev->dev points to this intermediate device,
rather than the platform device, which is why my patch failed.

I don't know the network stack well enough. Is this consider broken?
Is this valid? Is there any generic code which might try looking in
netdev->dev.of_node?  Do other MAC drivers have an intermediate
device?

We could modify the stmac driver so that phydev->attached_dev->dev
does point to the platform device, and then my patch works. But if
there are other MAC drivers which are structured like this, they are
also broken when used with the Micrel PHY. So then walking up the
device tree is a better solution.

Comments from more knowledgeable people requested!

       Andrew

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-12-04 14:31 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-15 19:09 SoCFPGA ethernet broken Dinh Nguyen
2015-10-15 20:03 ` Florian Fainelli
2015-10-15 19:59   ` Dinh Nguyen
2015-10-15 20:25     ` Florian Fainelli
2015-10-15 20:35       ` David Daney
2015-10-15 20:49         ` Dinh Nguyen
2015-10-15 21:30           ` Florian Fainelli
2015-10-16  2:32             ` Dinh Nguyen
2015-10-16  3:31               ` Andrew Lunn
2015-10-16 14:38                 ` Dinh Nguyen
2015-10-16 15:03                   ` Andrew Lunn
2015-10-16 15:31                     ` Dinh Nguyen
2015-10-16 15:56                       ` Andrew Lunn
2015-10-16 16:47                         ` David Daney
2015-10-16 19:10                           ` Dinh Nguyen
2015-10-16 19:38                             ` Andrew Lunn
2015-10-16 20:24                               ` David Daney
2015-10-16 20:29                                 ` Andrew Lunn
2015-10-19 15:14                               ` Dinh Nguyen
2015-10-19 10:50                                 ` Dinh Nguyen
2015-10-16 18:17                         ` Florian Fainelli
2015-10-16  3:04             ` Dinh Nguyen
2015-12-03 20:48       ` Pavel Machek
2015-12-03 21:23         ` David Daney
2015-12-03 23:17           ` Dinh Nguyen
2015-12-04  1:10             ` Andrew Lunn
2015-12-04  1:50               ` Andrew Lunn
2015-12-04 11:27                 ` Dinh Nguyen
2015-12-04 14:31                   ` Andrew Lunn
2015-12-04  9:38           ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.