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