Re: [net-next PATCH v4 03/11] net: pcs: Add subsystem

On 5/20/2025 1:43 AM, Sean Anderson wrote:

On 5/14/25 12:22, Lei Wei wrote:

quoted

On 5/13/2025 12:10 AM, Sean Anderson wrote:

quoted

+/**
+ * pcs_register_full() - register a new PCS
+ * @dev: The device requesting the PCS
+ * @fwnode: The PCS's firmware node; typically @dev.fwnode
+ * @pcs: The PCS to register
+ *
+ * Registers a new PCS which can be attached to a phylink.
+ *
+ * Return: 0 on success, or -errno on error
+ */
+int pcs_register_full(struct device *dev, struct fwnode_handle *fwnode,
+              struct phylink_pcs *pcs)
+{
+    struct pcs_wrapper *wrapper;
+
+    if (!dev || !pcs->ops)
+        return -EINVAL;
+
+    if (!pcs->ops->pcs_an_restart || !pcs->ops->pcs_config ||
+        !pcs->ops->pcs_get_state)
+        return -EINVAL;
+
+    wrapper = kzalloc(sizeof(*wrapper), GFP_KERNEL);
+    if (!wrapper)
+        return -ENOMEM;

How about the case where pcs is removed and then comes back again? Should we find the original wrapper and attach it to pcs again instead of creating a new wrapper?

When the PCS is removed the old wrapper is removed from pcs_wrappers, so
it can no longer be looked up any more. I think trying to save/restore
the wrapper would be much more trouble than it's worth.

In the case where Ethernet is not removed but PCS is removed and then
comes back (when the sysfs unbind followed by bind method is used),
it will not work because the Ethernet probe will not be initiated again, 
to call "pcs_get" again to obtain the new wrapper, it would still hold 
the old wrapper.

--Sean