Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1248128 > unrolled thread

Re: SoCFPGA ethernet broken

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: SoCFPGA ethernet broken Florian Fainelli <f.fainelli@gmail.com> - 2015-10-15 22:10 +0200
    Re: SoCFPGA ethernet broken Florian Fainelli <f.fainelli@gmail.com> - 2015-10-15 22:30 +0200
      Re: SoCFPGA ethernet broken David Daney <ddaney@caviumnetworks.com> - 2015-10-15 22:40 +0200
        Re: SoCFPGA ethernet broken Florian Fainelli <f.fainelli@gmail.com> - 2015-10-15 23:40 +0200
          Re: SoCFPGA ethernet broken Andrew Lunn <andrew@lunn.ch> - 2015-10-16 05:40 +0200
            Re: SoCFPGA ethernet broken Andrew Lunn <andrew@lunn.ch> - 2015-10-16 17:10 +0200
              Re: SoCFPGA ethernet broken Andrew Lunn <andrew@lunn.ch> - 2015-10-16 18:00 +0200
                Re: SoCFPGA ethernet broken Florian Fainelli <f.fainelli@gmail.com> - 2015-10-16 20:20 +0200
                Re: SoCFPGA ethernet broken Andrew Lunn <andrew@lunn.ch> - 2015-10-16 21:40 +0200
                  Re: SoCFPGA ethernet broken Andrew Lunn <andrew@lunn.ch> - 2015-10-16 22:30 +0200
                  Re: SoCFPGA ethernet broken David Daney <ddaney@caviumnetworks.com> - 2015-10-16 22:30 +0200

#1248128 — Re: SoCFPGA ethernet broken

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1248146

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248151

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248201

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248318

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248832

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248927

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249044

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249108

> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249139

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249142

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
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web