linux-kernel - Re: 回覆: [net] net: ftgmac100: corrcet the phy interface of NC-SI mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <44575610-70e1-48cf-ac19-3edef0b7a58f@lunn.ch>
Date: Mon, 14 Oct 2024 15:22:15 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jacky Chou <jacky_chou@...eedtech.com>
Cc: "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>,
	"jacob.e.keller@...el.com" <jacob.e.keller@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: 回覆: [net] net: ftgmac100:
 corrcet the phy interface of NC-SI mode

On Mon, Oct 14, 2024 at 02:23:40AM +0000, Jacky Chou wrote:
> Hi Andrew,
> 
> Thanks for the review.
> 
> > > In NC-SI specification, NC-SI is using RMII, not MII.
> > >
> > > Fixes: e24a6c874601 ("net: ftgmac100: Get link speed and duplex for
> > > NC-SI")
> > > Signed-off-by: Jacky Chou <jacky_chou@...eedtech.com>
> > > ---
> > >  drivers/net/ethernet/faraday/ftgmac100.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
> > > b/drivers/net/ethernet/faraday/ftgmac100.c
> > > index ae0235a7a74e..85fea13b2879 100644
> > > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > > @@ -1913,7 +1913,7 @@ static int ftgmac100_probe(struct platform_device
> > *pdev)
> > >  			goto err_phy_connect;
> > >  		}
> > >  		err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
> > > -					 PHY_INTERFACE_MODE_MII);
> > > +					 PHY_INTERFACE_MODE_RMII);
> > >  		if (err) {
> > >  			dev_err(&pdev->dev, "Connecting PHY failed\n");
> > >  			goto err_phy_connect;
> > 
> > I'm a but confused here. Please could you expand the commit message. When i
> > look at the code:
> > 
> > 		phydev = fixed_phy_register(PHY_POLL, &ncsi_phy_status, NULL);
> > 		err = phy_connect_direct(netdev, phydev, ftgmac100_adjust_link,
> > 					 PHY_INTERFACE_MODE_MII);
> > 		if (err) {
> > 			dev_err(&pdev->dev, "Connecting PHY failed\n");
> > 			goto err_phy_connect;
> > 		}
> > 
> > The phy being connected to is a fixed PHY. So the interface mode should not
> > matter, at least to the PHY, since there is no physical PHY. Does the MAC driver
> > get this value returned to it, e.g. as part of ftgmac100_adjust_link, and the
> > MAC then configures itself into the wrong interface mode?
> > 
> > For a patch with a Fixes: it is good to describe the problem the user sees.
> 
> Although it is connected to a fixed PHY and do not care what interface mode is, 
> the driver still configures the correct interface.
> 
> In the ftgmac100 driver, the MAC driver does not actually need to know the interface mode 
> connecting the MAC and PHY.
> The driver just needs to get some information from the PHY, like speed, duplex and so on, to 
> configure the MAC.
> 
> Perhaps it is not matter on PHY interface to MAC, it should correct to the correct interface mode 
> for code.

So nothing is actually broken which a user would notice. So please
drop the Fixes: tag, and submit this for net-next.

     Andrew