Thread (13 messages) 13 messages, 7 authors, 2025-02-19

Re: [PATCH 2/2] drivers: i3c: Add driver for NXP P3H2x4x i3c-hub device

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-02-12 16:53:41
Also in: linux-i3c, lkml

On 12/02/2025 14:22, Aman Kumar Pandey wrote:
+
+static void p3h2x4x_of_get_tp_dt_conf(struct device *dev,
+					const struct device_node *node)
+{
+	struct p3h2x4x *priv = dev_get_drvdata(dev);
+	struct device_node *dev_node;
+	int tp_port;
+
+	for_each_available_child_of_node(node, dev_node) {
+		if (!dev_node->full_name ||
+			(sscanf(dev_node->full_name, "target-port@%d", &tp_port) != 1))
NAK, undocumented ABI. Also weird code, why are you parsing DT structure
manually? Use proper functions to get the reg.


+			continue;
+
+		if (tp_port < P3H2x4x_TP_MAX_COUNT) {
+			priv->tp_bus[tp_port].dt_available = true;
+			priv->tp_bus[tp_port].of_node = dev_node;
+			priv->tp_bus[tp_port].tp_mask = BIT(tp_port);
+			priv->tp_bus[tp_port].priv = priv;
+			priv->tp_bus[tp_port].tp_port = tp_port;
+		}
+	}
+}
+
+/* return true when backend node exist */
+static bool p3h2x4x_is_backend_node_exist(int port, struct p3h2x4x *priv, u32 addr)
+{
+	struct smbus_device *backend = NULL;
+
+	list_for_each_entry(backend,
+			&priv->tp_bus[port].tp_device_entry, list) {
+		if (backend->addr == addr)
+			return true;
+	}
+	return false;
+}
+
+static int p3h2x4x_read_backend_from_p3h2x4x_dts(struct device_node *i3c_node_target,
+					struct p3h2x4x *priv)
+{
+	struct device_node *i3c_node_tp;
+	const char *compatible;
+	int tp_port, ret;
+	u32 addr_dts;
+	struct smbus_device *backend;
+
+	if (sscanf(i3c_node_target->full_name, "target-port@%d", &tp_port) == 0)
+		return -EINVAL;
+
+	if (tp_port > P3H2x4x_TP_MAX_COUNT)
+		return -ERANGE;
+
+	if (tp_port < 0)
+		return -EINVAL;
+
+	INIT_LIST_HEAD(&priv->tp_bus[tp_port].tp_device_entry);
+
+	if (priv->settings.tp[tp_port].mode == P3H2x4x_TP_MODE_I3C)
+		return 0;
+
+	for_each_available_child_of_node(i3c_node_target, i3c_node_tp) {
+
+		ret = of_property_read_u32(i3c_node_tp, "reg", &addr_dts);
+		if (ret)
+			return ret;
+
+		if (p3h2x4x_is_backend_node_exist(tp_port, priv, addr_dts))
+			continue;
+
+		ret = of_property_read_string(i3c_node_tp, "compatible", &compatible);
+		if (ret)
+			return ret;
+
+		backend = kzalloc(sizeof(*backend), GFP_KERNEL);
+		if (!backend)
+			return -ENOMEM;
+
+		backend->addr = addr_dts;
+		backend->compatible = compatible;
+		backend->tp_device_dt_node = i3c_node_tp;
+		backend->client = NULL;
+
+		list_add(&backend->list,
+			&priv->tp_bus[tp_port].tp_device_entry);
+	}
+
+	return 0;
+}
+
+static void p3h2x4x_parse_dt_tp(struct device *dev,
+				const struct device_node *i3c_node_hub,
+				struct p3h2x4x *priv)
+{
+	struct device_node *i3c_node_target;
+	int ret;
+
+	for_each_available_child_of_node(i3c_node_hub, i3c_node_target) {
+		if (!strcmp(i3c_node_target->name, "target-port")) {
+			ret = p3h2x4x_read_backend_from_p3h2x4x_dts(i3c_node_target, priv);
+			if (ret)
+				dev_err(dev, "DTS entry invalid - error %d", ret);
+		}
+	}
+}
+
+static int p3h2x4x_get_tp_local_device_dt_setting(struct device *dev,
+					const struct device_node *node, u32 id)
+{
+	u8 i;
+	u32 local_dev_count, local_dev;
+	struct p3h2x4x *priv = dev_get_drvdata(dev);
+
+	if (!of_get_property(node, "local_dev", &local_dev_count))
Oh no, read DTS conding style before upstreaming such code.


Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help