drivers: eth: phy: tja1103: Remove ISR thread and add auto mode.

Re-use the workqueue instead of having dedicated thread for handling
interrupts. This reduces memory usage and complexity.

Furthermore adds an auto mode for 100BASE-T1 negotiation.

Signed-off-by: Peter van der Perk <[email protected]>

Author: PetervdPerk-NXP

drivers/ethernet/phy/phy_tja1103.c

@@ -18,6 +18,7 @@
 #include <zephyr/net/mdio.h>
 #include <zephyr/drivers/gpio.h>
 #include <zephyr/drivers/mdio.h>
+#include <zephyr/random/random.h>
 
 #include <zephyr/logging/log.h>
 LOG_MODULE_REGISTER(phy_tja1103, CONFIG_PHY_LOG_LEVEL);
@@ -65,15 +66,12 @@ struct phy_tja1103_data {
 	const struct device *dev;
 	struct phy_link_state state;
 	struct k_sem sem;
-	struct k_sem offload_sem;
 	phy_callback_t cb;
 	struct gpio_callback phy_tja1103_int_callback;
 	void *cb_data;
-
-	K_KERNEL_STACK_MEMBER(irq_thread_stack, CONFIG_PHY_TJA1103_IRQ_THREAD_STACK_SIZE);
-	struct k_thread irq_thread;
-
-	struct k_work_delayable monitor_work;
+	struct k_work_delayable phy_work;
+	uint64_t start_time;
+	uint32_t wait_time;
 };
 
 static inline int phy_tja1103_c22_read(const struct device *dev, uint16_t reg, uint16_t *val)
@@ -153,140 +151,118 @@ static int phy_tja1103_id(const struct device *dev, uint32_t *phy_id)
 	return 0;
 }
 
-static int update_link_state(const struct device *dev)
+static int phy_tja1103_get_link_state(const struct device *dev, struct phy_link_state *state)
 {
 	struct phy_tja1103_data *const data = dev->data;
-	bool link_up;
 	uint16_t val;
+	int rc = 0;
+
+	k_sem_take(&data->sem, K_FOREVER);
 
 	if (phy_tja1103_c45_read(dev, MDIO_MMD_VENDOR_SPECIFIC1, TJA1103_PHY_STATUS, &val) < 0) {
 		return -EIO;
 	}
 
-	link_up = (val & TJA1103_PHY_STATUS_LINK_STAT) != 0;
+	k_sem_give(&data->sem);
 
-	/* Let workqueue re-schedule and re-check if the
-	 * link status is unchanged this time
-	 */
-	if (data->state.is_up == link_up) {
-		return -EAGAIN;
-	}
+	/* TJA1103 Only supports 100BASE Full-duplex */
+	state->speed = LINK_FULL_100BASE;
+	state->is_up = (val & TJA1103_PHY_STATUS_LINK_STAT) != 0;
 
-	data->state.is_up = link_up;
+	LOG_DBG("TJA1103 Link state %i", state->is_up);
 
-	return 0;
+	return rc;
 }
 
-static int phy_tja1103_get_link_state(const struct device *dev, struct phy_link_state *state)
+#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios)
+static void phy_tja1103_ack_irq(const struct device *dev)
 {
-	struct phy_tja1103_data *const data = dev->data;
-	const struct phy_tja1103_config *const cfg = dev->config;
-	int rc = 0;
-
-	k_sem_take(&data->sem, K_FOREVER);
+	uint16_t irq;
 
-	/* If Interrupt is configured then the workqueue will not
-	 * update the link state periodically so do it explicitly
-	 */
-	if (cfg->gpio_interrupt.port != NULL) {
-		rc = update_link_state(dev);
+	if (phy_tja1103_c45_read(dev, MDIO_MMD_VENDOR_SPECIFIC1, TJA1103_PHY_FUNC_IRQ_MSTATUS,
+				 &irq) < 0) {
+		return;
 	}
 
-	memcpy(state, &data->state, sizeof(struct phy_link_state));
-
-	k_sem_give(&data->sem);
-
-	return rc;
+	/* Handling Link related Functional IRQs */
+	if (irq & (TJA1103_PHY_FUNC_IRQ_LINK_EVENT | TJA1103_PHY_FUNC_IRQ_LINK_AVAIL)) {
+		/* Ack the assered link related interrupts */
+		phy_tja1103_c45_write(dev, MDIO_MMD_VENDOR_SPECIFIC1, TJA1103_PHY_FUNC_IRQ_ACK,
+				      irq);
+	}
 }
 
-static void invoke_link_cb(const struct device *dev)
+static void phy_tja1103_handle_irq(const struct device *port, struct gpio_callback *cb,
+				   uint32_t pins)
 {
-	struct phy_tja1103_data *const data = dev->data;
-	struct phy_link_state state;
-
-	if (data->cb == NULL) {
-		return;
-	}
+	ARG_UNUSED(pins);
+	ARG_UNUSED(port);
 
-	/* Send callback only on link state change */
-	if (phy_tja1103_get_link_state(dev, &state) != 0) {
-		return;
-	}
+	struct phy_tja1103_data *const data =
+		CONTAINER_OF(cb, struct phy_tja1103_data, phy_tja1103_int_callback);
 
-	data->cb(dev, &state, data->cb_data);
+	/* Trigger workqueue before leaving the ISR */
+	k_work_reschedule(&data->phy_work, K_NO_WAIT);
 }
+#endif
 
-static void monitor_work_handler(struct k_work *work)
+static void phy_work_handler(struct k_work *work)
 {
 	struct k_work_delayable *dwork = k_work_delayable_from_work(work);
 	struct phy_tja1103_data *const data =
-		CONTAINER_OF(dwork, struct phy_tja1103_data, monitor_work);
+		CONTAINER_OF(dwork, struct phy_tja1103_data, phy_work);
 	const struct device *dev = data->dev;
+	struct phy_link_state state = {};
 	int rc;
+	const struct phy_tja1103_config *const cfg = dev->config;
 
-	k_sem_take(&data->sem, K_FOREVER);
-
-	rc = update_link_state(dev);
-
-	k_sem_give(&data->sem);
+	rc = phy_tja1103_get_link_state(dev, &state);
 
-	/* If link state has changed and a callback is set, invoke callback */
-	if (rc == 0) {
-		invoke_link_cb(dev);
+	/* Update link state and trigger callback if changed */
+	if (rc == 0 && (state.speed != data->state.speed || state.is_up != data->state.is_up)) {
+		memcpy(&data->state, &state, sizeof(struct phy_link_state));
+		if (data->cb) {
+			data->cb(dev, &data->state, data->cb_data);
+		}
 	}
 
-	/* Submit delayed work */
-	k_work_reschedule(&data->monitor_work, K_MSEC(CONFIG_PHY_MONITOR_PERIOD));
-}
+	if (cfg->master_slave == 3 && !data->state.is_up &&
+	    k_uptime_delta(&data->start_time) > data->wait_time) {
+		uint16_t val;
 
-static void phy_tja1103_irq_offload_thread(void *p1, void *p2, void *p3)
-{
-	ARG_UNUSED(p2);
-	ARG_UNUSED(p3);
+		data->start_time = k_uptime_get();
+		data->wait_time = 1000 + (sys_rand32_get() % 2001);
 
-	const struct device *dev = p1;
-	struct phy_tja1103_data *const data = dev->data;
-	uint16_t irq;
+		phy_tja1103_c45_read(dev, MDIO_MMD_PMAPMD, MDIO_PMA_PMD_BT1_CTRL, &val);
 
-	for (;;) {
-		/* await trigger from ISR */
-		k_sem_take(&data->offload_sem, K_FOREVER);
+		val ^= MDIO_PMA_PMD_BT1_CTRL_CFG_MST;
 
-		if (phy_tja1103_c45_read(dev, MDIO_MMD_VENDOR_SPECIFIC1,
-					 TJA1103_PHY_FUNC_IRQ_MSTATUS, &irq) < 0) {
-			return;
-		}
+		phy_tja1103_c45_write(dev, MDIO_MMD_PMAPMD, MDIO_PMA_PMD_BT1_CTRL, val);
+	}
 
-		/* Handling Link related Functional IRQs */
-		if (irq & (TJA1103_PHY_FUNC_IRQ_LINK_EVENT | TJA1103_PHY_FUNC_IRQ_LINK_AVAIL)) {
-			/* Send callback to MAC on link status changed */
-			invoke_link_cb(dev);
+#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios)
+	if (cfg->gpio_interrupt.port) {
+		phy_tja1103_ack_irq(dev);
 
-			/* Ack the assered link related interrupts */
-			phy_tja1103_c45_write(dev, MDIO_MMD_VENDOR_SPECIFIC1,
-					      TJA1103_PHY_FUNC_IRQ_ACK, irq);
+		if (cfg->master_slave == 3 && !data->state.is_up) {
+			k_work_reschedule(&data->phy_work, K_MSEC(data->wait_time + 10));
 		}
-	}
-}
-
-static void phy_tja1103_handle_irq(const struct device *port, struct gpio_callback *cb,
-				   uint32_t pins)
-{
-	ARG_UNUSED(pins);
-	ARG_UNUSED(port);
 
-	struct phy_tja1103_data *const data =
-		CONTAINER_OF(cb, struct phy_tja1103_data, phy_tja1103_int_callback);
+		return;
+	}
+#endif
 
-	/* Trigger BH before leaving the ISR */
-	k_sem_give(&data->offload_sem);
+	/* Submit delayed work */
+	k_work_reschedule(&data->phy_work, K_MSEC(CONFIG_PHY_MONITOR_PERIOD));
 }
 
 static void phy_tja1103_cfg_irq_poll(const struct device *dev)
 {
 	struct phy_tja1103_data *const data = dev->data;
-	const struct phy_tja1103_config *const cfg = dev->config;
+
+#if DT_ANY_INST_HAS_PROP_STATUS_OKAY(int_gpios)
 	int ret;
+	const struct phy_tja1103_config *const cfg = dev->config;
 
 	if (cfg->gpio_interrupt.port != NULL) {
 		if (!gpio_is_ready_dt(&cfg->gpio_interrupt)) {
@@ -311,9 +287,8 @@ static void phy_tja1103_cfg_irq_poll(const struct device *dev)
 			return;
 		}
 
-		ret = phy_tja1103_c45_write(
-			dev, MDIO_MMD_VENDOR_SPECIFIC1, TJA1103_PHY_FUNC_IRQ_EN,
-			(TJA1103_PHY_FUNC_IRQ_LINK_EVENT_EN | TJA1103_PHY_FUNC_IRQ_LINK_AVAIL_EN));
+		ret = phy_tja1103_c45_write(dev, MDIO_MMD_VENDOR_SPECIFIC1, TJA1103_PHY_FUNC_IRQ_EN,
+					    (TJA1103_PHY_FUNC_IRQ_LINK_EVENT_EN));
 		if (ret < 0) {
 			return;
 		}
@@ -323,19 +298,15 @@ static void phy_tja1103_cfg_irq_poll(const struct device *dev)
 			LOG_ERR("Failed to enable INT, %d", ret);
 			return;
 		}
+	}
+#endif
 
-		/* PHY initialized, IRQ configured, now initialize the BH handler */
-		k_thread_create(&data->irq_thread, data->irq_thread_stack,
-				CONFIG_PHY_TJA1103_IRQ_THREAD_STACK_SIZE,
-				phy_tja1103_irq_offload_thread, (void *)dev, NULL, NULL,
-				CONFIG_PHY_TJA1103_IRQ_THREAD_PRIO, K_ESSENTIAL, K_NO_WAIT);
-		k_thread_name_set(&data->irq_thread, "phy_tja1103_irq_offload");
+	data->start_time = k_uptime_get();
+	data->wait_time = 1000 + (sys_rand32_get() % 4001);
 
-	} else {
-		k_work_init_delayable(&data->monitor_work, monitor_work_handler);
+	k_work_init_delayable(&data->phy_work, phy_work_handler);
 
-		monitor_work_handler(&data->monitor_work.work);
-	}
+	phy_work_handler(&data->phy_work.work);
 }
 
 static int phy_tja1103_init(const struct device *dev)
@@ -354,7 +325,7 @@ static int phy_tja1103_init(const struct device *dev)
 	ret = WAIT_FOR(!phy_tja1103_id(dev, &phy_id) && phy_id == TJA1103_ID,
 		       TJA1103_AWAIT_RETRY_COUNT * TJA1103_AWAIT_DELAY_POLL_US,
 		       k_sleep(K_USEC(TJA1103_AWAIT_DELAY_POLL_US)));
-	if (ret < 0) {
+	if (ret == 0) {
 		LOG_ERR("Unable to obtain PHY ID for device 0x%x", cfg->phy_addr);
 		return -ENODEV;
 	}
@@ -411,17 +382,19 @@ static int phy_tja1103_init(const struct device *dev)
 
 static int phy_tja1103_link_cb_set(const struct device *dev, phy_callback_t cb, void *user_data)
 {
+	int rc = 0;
 	struct phy_tja1103_data *const data = dev->data;
 
 	data->cb = cb;
 	data->cb_data = user_data;
 
-	/* Invoke the callback to notify the caller of the current
-	 * link status.
-	 */
-	invoke_link_cb(dev);
+	rc = phy_tja1103_get_link_state(dev, &data->state);
 
-	return 0;
+	if (rc == 0) {
+		data->cb(dev, &data->state, data->cb_data);
+	}
+
+	return rc;
 }
 
 static DEVICE_API(ethphy, phy_tja1103_api) = {
@@ -440,7 +413,6 @@ static DEVICE_API(ethphy, phy_tja1103_api) = {
 	};                                                                                         \
 	static struct phy_tja1103_data phy_tja1103_data_##n = {                                    \
 		.sem = Z_SEM_INITIALIZER(phy_tja1103_data_##n.sem, 1, 1),                          \
-		.offload_sem = Z_SEM_INITIALIZER(phy_tja1103_data_##n.offload_sem, 0, 1),          \
 	};                                                                                         \
 	DEVICE_DT_INST_DEFINE(n, &phy_tja1103_init, NULL, &phy_tja1103_data_##n,                   \
 			      &phy_tja1103_config_##n, POST_KERNEL, CONFIG_PHY_INIT_PRIORITY,      \

dts/bindings/ethernet/phy/nxp,tja1103.yaml

@@ -27,3 +27,4 @@ properties:
       - "default"
       - "master"
       - "slave"
+      - "auto"