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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 08/05/2025 06:57, 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;
> +	u64 tp_port;
> +
> +	for_each_available_child_of_node(node, dev_node) {
> +		if (!dev_node->name || of_node_cmp(dev_node->name, "target-port"))


Same NAK as before. You ignored the comment and nothing improved.

> +			continue;
> +
> +		if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
> +			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_dts(struct device_node *i3c_node_target,
> +					 struct p3h2x4x *priv)
> +{
> +	struct device_node *tp_node;
> +	const char *compatible;
> +	u64 tp_port, addr_dts;
> +	int ret;
> +
> +	struct smbus_device *backend;
> +
> +	if (of_property_read_reg(i3c_node_target, 0, &tp_port, NULL))
> +		return -EINVAL;
> +
> +	if (tp_port >= P3H2x4x_TP_MAX_COUNT || tp_port < 0)
> +		return -ERANGE;
> +
> +	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, tp_node) {
> +		ret = of_property_read_reg(tp_node, 0, &addr_dts, NULL);
> +		if (ret)
> +			return ret;
> +
> +		if (p3h2x4x_is_backend_node_exist(tp_port, priv, addr_dts))
> +			continue;
> +
> +		ret = of_property_read_string(tp_node, "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 = tp_node;
> +		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 (of_node_cmp(i3c_node_target->name, "target-port") == 0) {
> +			ret = p3h2x4x_read_backend_from_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)
> +{
> +	struct p3h2x4x *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = of_property_read_variable_u8_array(node, "local_dev", priv->tp_bus[id].local_dev_list,

Nothing improved.

Just like for first patch you send the same buggy code ignoring review.


Best regards,
Krzysztof

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c