All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Saravana Kannan <saravanak@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	netdev@vger.kernel.org, kernel-team@android.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Neil Armstrong <narmstrong@baylibre.com>,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH v2] of: property: fw_devlink: Add support for "phy-handle" property
Date: Wed, 1 Sep 2021 09:22:11 +0200	[thread overview]
Message-ID: <2023d07e-18bb-e129-760a-18b17ff772cd@samsung.com> (raw)
In-Reply-To: <CAGETcx-SqTeGdKF=CD9=Ujo2xrWMw3NnimE7zj+d-4HckmaJVQ@mail.gmail.com>

Hi Saravana,

On 01.09.2021 04:37, Saravana Kannan wrote:
> On Tue, Aug 24, 2021 at 12:31 AM Saravana Kannan <saravanak@google.com> wrote:
>> On Tue, Aug 24, 2021 at 12:03 AM Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>> On 23.08.2021 20:22, Saravana Kannan wrote:
>>>> On Mon, Aug 23, 2021 at 5:08 AM Marek Szyprowski
>>>> <m.szyprowski@samsung.com> wrote:
>>>>> On 18.08.2021 04:17, Saravana Kannan wrote:
>>>>>> Allows tracking dependencies between Ethernet PHYs and their consumers.
>>>>>>
>>>>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>>>>> Cc: netdev@vger.kernel.org
>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>>> This patch landed recently in linux-next as commit cf4b94c8530d ("of:
>>>>> property: fw_devlink: Add support for "phy-handle" property"). It breaks
>>>>> ethernet operation on my Amlogic-based ARM64 boards: Odroid C4
>>>>> (arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts) and N2
>>>>> (meson-g12b-odroid-n2.dts) as well as Khadas VIM3/VIM3l
>>>>> (meson-g12b-a311d-khadas-vim3.dts and meson-sm1-khadas-vim3l.dts).
>>>>>
>>>>> In case of OdroidC4 I see the following entries in the
>>>>> /sys/kernel/debug/devices_deferred:
>>>>>
>>>>> ff64c000.mdio-multiplexer
>>>>> ff3f0000.ethernet
>>>>>
>>>>> Let me know if there is anything I can check to help debugging this issue.
>>>> I'm fairly certain you are hitting this issue because the PHY device
>>>> doesn't have a compatible property. And so the device link dependency
>>>> is propagated up to the mdio bus. But busses as suppliers aren't good
>>>> because busses never "probe".
>>>>
>>>> PHY seems to be one of those cases where it's okay to have the
>>>> compatible property but also okay to not have it. You can confirm my
>>>> theory by checking for the list of suppliers under
>>>> ff64c000.mdio-multiplexer. You'd see mdio@0 (ext_mdio) and if you look
>>>> at the "status" file under the folder it should be "dormant". If you
>>>> add a compatible property that fits the formats a PHY node can have,
>>>> that should also fix your issue (not the solution though).
>>> Where should I look for the mentioned device links 'status' file?
>>>
>>> # find /sys -name ff64c000.mdio-multiplexer
>>> /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer
>>> /sys/bus/platform/devices/ff64c000.mdio-multiplexer
>>>
>>> # ls -l /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer
>>> total 0
>> This is the folder I wanted you to check.
>>
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04
>>> consumer:platform:ff3f0000.ethernet ->
>>> ../../../../virtual/devlink/platform:ff64c000.mdio-multiplexer--platform:ff3f0000.ethernet
>> But I should have asked to look for the consumer list and not the
>> supplier list. In any case, we can see that the ethernet is marked as
>> the consumer of the mdio-multiplexer instead of the PHY device. So my
>> hunch seems to be right.
>>
>>> -rw-r--r-- 1 root root 4096 Jan  1 00:04 driver_override
>>> -r--r--r-- 1 root root 4096 Jan  1 00:04 modalias
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04 of_node ->
>>> ../../../../../firmware/devicetree/base/soc/bus@ff600000/mdio-multiplexer@4c000
>>> drwxr-xr-x 2 root root    0 Jan  1 00:02 power
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04 subsystem ->
>>> ../../../../../bus/platform
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04
>>> supplier:platform:ff63c000.system-controller:clock-controller ->
>>> ../../../../virtual/devlink/platform:ff63c000.system-controller:clock-controller--platform:ff64c000.mdio-multiplexer
>>> -rw-r--r-- 1 root root 4096 Jan  1 00:04 uevent
>>> -r--r--r-- 1 root root 4096 Jan  1 00:04 waiting_for_supplier
>>>
>>> # cat
>>> /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer/waiting_for_supplier
>>> 0
>>>
>>> I'm also not sure what compatible string should I add there.
>> It should have been added to external_phy: ethernet-phy@0. But don't
>> worry about it (because you need to use a specific format for the
>> compatible string).
>>
> Marek,
>
> Can you give this a shot?
> https://lore.kernel.org/lkml/20210831224510.703253-1-saravanak@google.com/
>
> This is not the main fix for the case you brought up, but it should
> fix your issue as a side effect of fixing another issue.

I've just checked it and it doesn't help in my case. 
ff64c000.mdio-multiplexer and ff3f0000.ethernet are still not probed 
after applying this patch.

> The main fix for your issue would be to teach fw_devlink that
> phy-handle always points to the actual DT node that'll become a device
> even if it doesn't have a compatible property. I'll send that out
> later.

I'm waiting for the proper fix then.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Saravana Kannan <saravanak@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	netdev@vger.kernel.org, kernel-team@android.com,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Neil Armstrong <narmstrong@baylibre.com>,
	linux-amlogic@lists.infradead.org
Subject: Re: [PATCH v2] of: property: fw_devlink: Add support for "phy-handle" property
Date: Wed, 1 Sep 2021 09:22:11 +0200	[thread overview]
Message-ID: <2023d07e-18bb-e129-760a-18b17ff772cd@samsung.com> (raw)
In-Reply-To: <CAGETcx-SqTeGdKF=CD9=Ujo2xrWMw3NnimE7zj+d-4HckmaJVQ@mail.gmail.com>

Hi Saravana,

On 01.09.2021 04:37, Saravana Kannan wrote:
> On Tue, Aug 24, 2021 at 12:31 AM Saravana Kannan <saravanak@google.com> wrote:
>> On Tue, Aug 24, 2021 at 12:03 AM Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>> On 23.08.2021 20:22, Saravana Kannan wrote:
>>>> On Mon, Aug 23, 2021 at 5:08 AM Marek Szyprowski
>>>> <m.szyprowski@samsung.com> wrote:
>>>>> On 18.08.2021 04:17, Saravana Kannan wrote:
>>>>>> Allows tracking dependencies between Ethernet PHYs and their consumers.
>>>>>>
>>>>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>>>>> Cc: netdev@vger.kernel.org
>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com>
>>>>> This patch landed recently in linux-next as commit cf4b94c8530d ("of:
>>>>> property: fw_devlink: Add support for "phy-handle" property"). It breaks
>>>>> ethernet operation on my Amlogic-based ARM64 boards: Odroid C4
>>>>> (arm64/boot/dts/amlogic/meson-sm1-odroid-c4.dts) and N2
>>>>> (meson-g12b-odroid-n2.dts) as well as Khadas VIM3/VIM3l
>>>>> (meson-g12b-a311d-khadas-vim3.dts and meson-sm1-khadas-vim3l.dts).
>>>>>
>>>>> In case of OdroidC4 I see the following entries in the
>>>>> /sys/kernel/debug/devices_deferred:
>>>>>
>>>>> ff64c000.mdio-multiplexer
>>>>> ff3f0000.ethernet
>>>>>
>>>>> Let me know if there is anything I can check to help debugging this issue.
>>>> I'm fairly certain you are hitting this issue because the PHY device
>>>> doesn't have a compatible property. And so the device link dependency
>>>> is propagated up to the mdio bus. But busses as suppliers aren't good
>>>> because busses never "probe".
>>>>
>>>> PHY seems to be one of those cases where it's okay to have the
>>>> compatible property but also okay to not have it. You can confirm my
>>>> theory by checking for the list of suppliers under
>>>> ff64c000.mdio-multiplexer. You'd see mdio@0 (ext_mdio) and if you look
>>>> at the "status" file under the folder it should be "dormant". If you
>>>> add a compatible property that fits the formats a PHY node can have,
>>>> that should also fix your issue (not the solution though).
>>> Where should I look for the mentioned device links 'status' file?
>>>
>>> # find /sys -name ff64c000.mdio-multiplexer
>>> /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer
>>> /sys/bus/platform/devices/ff64c000.mdio-multiplexer
>>>
>>> # ls -l /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer
>>> total 0
>> This is the folder I wanted you to check.
>>
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04
>>> consumer:platform:ff3f0000.ethernet ->
>>> ../../../../virtual/devlink/platform:ff64c000.mdio-multiplexer--platform:ff3f0000.ethernet
>> But I should have asked to look for the consumer list and not the
>> supplier list. In any case, we can see that the ethernet is marked as
>> the consumer of the mdio-multiplexer instead of the PHY device. So my
>> hunch seems to be right.
>>
>>> -rw-r--r-- 1 root root 4096 Jan  1 00:04 driver_override
>>> -r--r--r-- 1 root root 4096 Jan  1 00:04 modalias
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04 of_node ->
>>> ../../../../../firmware/devicetree/base/soc/bus@ff600000/mdio-multiplexer@4c000
>>> drwxr-xr-x 2 root root    0 Jan  1 00:02 power
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04 subsystem ->
>>> ../../../../../bus/platform
>>> lrwxrwxrwx 1 root root    0 Jan  1 00:04
>>> supplier:platform:ff63c000.system-controller:clock-controller ->
>>> ../../../../virtual/devlink/platform:ff63c000.system-controller:clock-controller--platform:ff64c000.mdio-multiplexer
>>> -rw-r--r-- 1 root root 4096 Jan  1 00:04 uevent
>>> -r--r--r-- 1 root root 4096 Jan  1 00:04 waiting_for_supplier
>>>
>>> # cat
>>> /sys/devices/platform/soc/ff600000.bus/ff64c000.mdio-multiplexer/waiting_for_supplier
>>> 0
>>>
>>> I'm also not sure what compatible string should I add there.
>> It should have been added to external_phy: ethernet-phy@0. But don't
>> worry about it (because you need to use a specific format for the
>> compatible string).
>>
> Marek,
>
> Can you give this a shot?
> https://lore.kernel.org/lkml/20210831224510.703253-1-saravanak@google.com/
>
> This is not the main fix for the case you brought up, but it should
> fix your issue as a side effect of fixing another issue.

I've just checked it and it doesn't help in my case. 
ff64c000.mdio-multiplexer and ff3f0000.ethernet are still not probed 
after applying this patch.

> The main fix for your issue would be to teach fw_devlink that
> phy-handle always points to the actual DT node that'll become a device
> even if it doesn't have a compatible property. I'll send that out
> later.

I'm waiting for the proper fix then.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2021-09-01  7:22 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18  2:17 [PATCH v2] of: property: fw_devlink: Add support for "phy-handle" property Saravana Kannan
2021-08-18 17:00 ` Rob Herring
     [not found] ` <CGME20210823120849eucas1p11d3919886444358472be3edd1c662755@eucas1p1.samsung.com>
2021-08-23 12:08   ` Marek Szyprowski
2021-08-23 12:08     ` Marek Szyprowski
2021-08-23 12:42     ` Rob Herring
2021-08-23 12:42       ` Rob Herring
2021-08-23 13:16     ` Andrew Lunn
2021-08-23 13:16       ` Andrew Lunn
2021-08-23 18:13       ` Saravana Kannan
2021-08-23 18:13         ` Saravana Kannan
2021-08-23 19:50         ` Andrew Lunn
2021-08-23 19:50           ` Andrew Lunn
2021-08-24  6:52       ` Marek Szyprowski
2021-08-24  6:52         ` Marek Szyprowski
2021-08-23 18:22     ` Saravana Kannan
2021-08-23 18:22       ` Saravana Kannan
2021-08-23 19:58       ` Andrew Lunn
2021-08-23 19:58         ` Andrew Lunn
2021-08-23 20:48         ` Saravana Kannan
2021-08-23 20:48           ` Saravana Kannan
2021-08-23 22:01           ` Andrew Lunn
2021-08-23 22:01             ` Andrew Lunn
2021-08-23 22:08           ` Rob Herring
2021-08-23 22:08             ` Rob Herring
2021-08-24  7:03       ` Marek Szyprowski
2021-08-24  7:03         ` Marek Szyprowski
2021-08-24  7:31         ` Saravana Kannan
2021-08-24  7:31           ` Saravana Kannan
2021-09-01  2:37           ` Saravana Kannan
2021-09-01  2:37             ` Saravana Kannan
2021-09-01  7:22             ` Marek Szyprowski [this message]
2021-09-01  7:22               ` Marek Szyprowski
2021-09-08 21:58               ` [PATCH v1] RFC: of: property: fix phy-hanlde issue Saravana Kannan
2021-09-09  8:03                 ` Marek Szyprowski
2021-09-14  0:54                   ` Saravana Kannan
2021-09-14  4:44                     ` Saravana Kannan
2021-09-14  6:15                       ` Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2023d07e-18bb-e129-760a-18b17ff772cd@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=andrew@lunn.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=kernel-team@android.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=narmstrong@baylibre.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=saravanak@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.