All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Jacky Chou <jacky_chou@aspeedtech.com>
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [net v2] net: ftgmac100: refactor getting phy device handle
Date: Tue, 22 Oct 2024 07:57:53 +0100	[thread overview]
Message-ID: <20241022065753.GN402847@kernel.org> (raw)
In-Reply-To: <20241021023705.2953048-1-jacky_chou@aspeedtech.com>

On Mon, Oct 21, 2024 at 10:37:05AM +0800, Jacky Chou wrote:
> The ftgmac100 supports NC-SI mode, dedicated PHY and fixed-link
> PHY. The dedicated PHY is using the phy_handle property to get
> phy device handle and the fixed-link phy is using the fixed-link
> property to register a fixed-link phy device.
> 
> In of_phy_get_and_connect function, it help driver to get and register
> these PHYs handle.
> Therefore, here refactors this part by using of_phy_get_and_connect.

Hi Jacky,

I understand the aim of this patch, and I think it is nice that we
can drop about 20 lines of code. But I did have some trouble understanding
the paragraph above. I wonder if the following is clearer:

  Consolidate the handling of dedicated PHY and fixed-link phy by taking
  advantage of logic in of_phy_get_and_connect() which handles both of
  these cases, rather than open coding the same logic in ftgmac100_probe().

> 
> Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com>
> ---
> v2:
>   - enable mac asym pause support for fixed-link PHY
>   - remove fixes information

I agree that this is not a fix. And should not have a Fixes tag and so on.
But as such it should be targeted at net rather than net-next.

  Subject: [net-next vX] ...

The code themselves changes look good to me. But I think the two points
above, in combination, warrant a v3.

-- 
pw-bot: changes-requested

...

  reply	other threads:[~2024-10-22  6:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-21  2:37 [net v2] net: ftgmac100: refactor getting phy device handle Jacky Chou
2024-10-22  6:57 ` Simon Horman [this message]
2024-10-22  7:13   ` 回覆: " Jacky Chou
2024-10-22  7:34     ` Simon Horman

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=20241022065753.GN402847@kernel.org \
    --to=horms@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacky_chou@aspeedtech.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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.