Messages in this thread Patch in this message | | | Date | Tue, 12 Dec 2023 09:29:43 +0000 | From | Simon Horman <> | Subject | Re: [RFC PATCH net-next v3 5/8] net: pcs: add driver for MediaTek USXGMII PCS |
| |
On Tue, Dec 12, 2023 at 03:47:47AM +0000, Daniel Golle wrote: > Add driver for USXGMII PCS found in the MediaTek MT7988 SoC and supporting > USXGMII, 10GBase-R and 5GBase-R interface modes. > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Hi Daniel,
some minor feedback from my side.
...
> diff --git a/drivers/net/pcs/pcs-mtk-usxgmii.c b/drivers/net/pcs/pcs-mtk-usxgmii.c
...
> +static int mtk_usxgmii_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode, > + phy_interface_t interface, > + const unsigned long *advertising, > + bool permit_pause_to_mac) > +{ > + struct mtk_usxgmii_pcs *mpcs = pcs_to_mtk_usxgmii_pcs(pcs); > + unsigned int an_ctrl = 0, link_timer = 0, xfi_mode = 0, adapt_mode = 0; > + bool mode_changed = false;
nit: please consider arranging local variables in networking code in reverse xmas tree order - longest line to shortest.
This may be useful: https://github.com/ecree-solarflare/xmastree
... > diff --git a/include/linux/pcs/pcs-mtk-usxgmii.h b/include/linux/pcs/pcs-mtk-usxgmii.h > new file mode 100644 > index 0000000000000..ef936d9c5f116 > --- /dev/null > +++ b/include/linux/pcs/pcs-mtk-usxgmii.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __LINUX_PCS_MTK_USXGMII_H > +#define __LINUX_PCS_MTK_USXGMII_H > + > +#include <linux/phylink.h> > + > +/** > + * mtk_usxgmii_select_pcs() - Get MediaTek PCS instance > + * @np: Pointer to device node indentifying a MediaTek USXGMII PCS nit: identifying
./scripts/checkpatch.py --codespell is your friend here.
> + * @mode: Ethernet PHY interface mode > + * > + * Return PCS identified by a device node and the PHY interface mode in use > + * > + * Return: Pointer to phylink PCS instance of NULL > + */ > +#if IS_ENABLED(CONFIG_PCS_MTK_USXGMII) > +struct phylink_pcs *mtk_usxgmii_pcs_get(struct device *dev, struct device_node *np);
nit: The kernel doc above does not match the signature of mtk_usxgmii_pcs_get().
./scripts/kernel-doc -none is helpful here.
> +void mtk_usxgmii_pcs_put(struct phylink_pcs *pcs); > +#else > +static inline struct phylink_pcs *mtk_usxgmii_pcs_get(struct device *dev, struct device_node *np) > +{ > + return NULL; > +} > +static inline void mtk_usxgmii_pcs_put(struct phylink_pcs *pcs) { } > +#endif /* IS_ENABLED(CONFIG_PCS_MTK_USXGMII) */ > + > +#endif /* __LINUX_PCS_MTK_USXGMII_H */ > -- > 2.43.0
| |