Re: [RFC PATCH net-next v3 5/8] net: pcs: add driver for MediaTek USXGMII PCS
From: Simon Horman <horms@kernel.org>
Date: 2023-12-12 09:29:58
Also in:
linux-arm-kernel, linux-devicetree, linux-mediatek, linux-phy, lkml
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. ...
quoted hunk ↗ jump to hunk
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
...quoted hunk ↗ jump to hunk
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