Re: [PATCH net-next 1/4] net/ethtool: Add netlink interface for the PLCA RS

[Piergiorgio Beruto]

Hello Randy,
thank you for your feedback. Although I have worked on the kernel for
quite some time now, I'm pretty new to this process.

Please, see my answers interleaved.

On Sat, Dec 03, 2022 at 06:37:13PM -0800, Randy Dunlap wrote:
> Hi--
> 
> On 12/3/22 18:30, Piergiorgio Beruto wrote:
> > Add support for configuring the PLCA Reconciliation Sublayer on
> > multi-drop PHYs that support IEEE802.3cg-2019 Clause 148 (e.g.,
> > 10BASE-T1S). This patch adds the appropriate netlink interface
> > to ethtool.
> > 
> > Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@xxxxxxxxx>
> > ---
> 
> 
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index e5b6cb1a77f9..99e3497b6aa1 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -543,6 +543,40 @@ int phy_ethtool_get_stats(struct phy_device *phydev,
> >  }
> >  EXPORT_SYMBOL(phy_ethtool_get_stats);
> >  
> 
> What is the meaning of all these empty kernel-doc comment blocks?
> Why are they here?
> Please delete them.
These functions are placeholders that I've used to have the kernel
compile. The next patch amends those functions and adds the proper
kernel-doc.

Do you want me to just remove the kernel-doc and leave the functions
TODO? Or would you like me to merge patches 1 and 2?

I did this to split the work into smaller, logically isolated and
compiling commits. Please, let me know if I did that wrong.
 
> > +/**
> > + *
> > + */
> > +int phy_ethtool_get_plca_cfg(struct phy_device *dev,
> > +			     struct phy_plca_cfg *plca_cfg)
> > +{
> > +	// TODO
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_get_plca_cfg);
> > +
> > +/**
> > + *
> > + */
> > +int phy_ethtool_set_plca_cfg(struct phy_device *dev,
> > +			     struct netlink_ext_ack *extack,
> > +			     const struct phy_plca_cfg *plca_cfg)
> > +{
> > +	// TODO
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_set_plca_cfg);
> > +
> > +/**
> > + *
> > + */
> > +int phy_ethtool_get_plca_status(struct phy_device *dev,
> > +				struct phy_plca_status *plca_st)
> > +{
> > +	// TODO
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(phy_ethtool_get_plca_status);
> 
> Thanks.
> -- 
> ~Randy
Thank you and kind regards,
   Piergiorgio