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

[Andrew Lunn]

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@xxxxxxxxxxxxxx>
> > > ---
> > >  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