Messages in this thread |  | | Date | Sun, 4 Dec 2022 03:49:12 +0100 | From | Piergiorgio Beruto <> | Subject | Re: [PATCH net-next 1/4] net/ethtool: Add netlink interface for the PLCA RS |
| |
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@gmail.com> > > --- > > > > 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
|  |