Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Andrew Lunn]

On Mon, Jan 29, 2024 at 09:19:58PM +0800, Choong Yong Liang wrote:
> > >   static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
> > >   	.mac_select_pcs = stmmac_mac_select_pcs,
> > >   	.mac_config = stmmac_mac_config,
> > >   	.mac_link_down = stmmac_mac_link_down,
> > >   	.mac_link_up = stmmac_mac_link_up,
> > > +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> > > +	.mac_prepare = stmmac_mac_prepare,
> > > +#endif
> > 
> > Please no for the platform-specific ifdef's in the generic code.
> > STMMAC driver is already overwhelmed with clumsy solutions. Let's not
> > add another one.
> > 
> > -Serge(y)
> > 
> > >   };
> > >   /**
> > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > > index c0079a7574ae..aa7d4d96391c 100644
> > > --- a/include/linux/stmmac.h
> > > +++ b/include/linux/stmmac.h
> > > @@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
> > >   	int (*serdes_powerup)(struct net_device *ndev, void *priv);
> > >   	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
> > >   	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> > > +	int (*config_serdes)(struct net_device *ndev, void *priv);
> > >   	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
> > >   	int (*init)(struct platform_device *pdev, void *priv);
> > >   	void (*exit)(struct platform_device *pdev, void *priv);
> > > -- 
> > > 2.25.1
> > > 
> > > 
> Hi Russell and Serge,
> 
> Thank you for pointing that out.
> The ifdef was removed and the config serdes will be implemented in
> mac_link_up in the new patch series.

Hi Choong

Please trim the text when replying. It can be hard to find actually
replies when having to do lots and lots of page downs. Just give the
context needed to understand your reply.

	Andrew