[PATCH v16 7/8] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
Krzysztof Kozlowski
krzk at kernel.org
Thu Aug 27 03:16:46 PDT 2026
On 26/08/2026 12:38, Lakshay Piplani wrote:
> +
> +static int p3h2x4x_configure_ldo(struct device *dev)
> +{
> + static const char * const supplies[] = {
> + "vcc1",
> + "vcc2",
> + "vcc3",
> + "vcc4"
> + };
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(supplies); i++) {
> + ret = devm_regulator_get_enable_optional(dev, supplies[i]);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(dev, ret, "Failed to enable %s\n",
> + supplies[i]);
> + }
> +
> + /* This delay is required for the regulator to stabilize its output voltage */
> + fsleep(5000);
Instead your regulators miss ramp delays.
> +
> + return 0;
> +}
...
> +
> +static void p3h2x4x_get_target_port_dt_conf(struct device *dev,
> + const struct device_node *node)
> +{
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
> + u64 tp_port;
> +
> + for_each_available_child_of_node_scoped(node, dev_node) {
Why do you need scoped loop?
> + if (of_property_read_reg(dev_node, 0, &tp_port, NULL))
> + continue;
> +
> + if (tp_port < p3h2x4x->num_target_ports) {
> + if (p3h2x4x_i3c_hub->tp_bus[tp_port].of_node) {
> + dev_warn(dev, "Duplicate target port %llu in DT\n", tp_port);
> + continue;
> + }
> +
> + p3h2x4x_i3c_hub->tp_bus[tp_port].of_node = of_node_get(dev_node);
> + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_mask = P3H2X4X_SET_BIT(tp_port);
> + p3h2x4x_i3c_hub->tp_bus[tp_port].p3h2x4x_i3c_hub = p3h2x4x_i3c_hub;
> + p3h2x4x_i3c_hub->tp_bus[tp_port].tp_port = tp_port;
> + }
> + }
> +}
> +
> +static int p3h2x4x_parse_tp_dt_settings(struct device *dev,
> + const struct device_node *node,
> + struct tp_configuration tp_config[])
> +{
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(dev->parent);
> + u64 id;
> + int ret;
> +
> + for_each_available_child_of_node_scoped(node, tp_node) {
> + enum p3h2x4x_tp_mode mode;
> +
> + /*
> + * Only "i3c" and "smbus" children describe target ports. Skip any
> + * other child (for example the MFD "regulators" container), which
> + * has no "reg" property.
> + */
> + if (of_node_name_eq(tp_node, "i3c"))
> + mode = P3H2X4X_TP_MODE_I3C;
> + else if (of_node_name_eq(tp_node, "smbus"))
> + mode = P3H2X4X_TP_MODE_SMBUS;
> + else
> + continue;
> +
> + ret = of_property_read_reg(tp_node, 0, &id, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to read reg for %pOF\n",
> + tp_node);
> +
> + if (id >= p3h2x4x->num_target_ports)
> + return dev_err_probe(dev, -EINVAL,
> + "Invalid target port index %llu\n",
> + id);
> +
> + tp_config[id].mode = mode;
> + tp_config[id].pullup_en =
> + of_property_read_bool(tp_node, "nxp,pullup-enable");
> + }
> +
> + return 0;
> +}
> +
> +static int p3h2x4x_get_hub_dt_conf(struct device *dev,
> + const struct device_node *node)
> +{
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
> +
> + of_property_read_u32(node, "nxp,tp0145-pullup-ohms",
> + &p3h2x4x_i3c_hub->hub_config.tp0145_pullup);
> + of_property_read_u32(node, "nxp,tp2367-pullup-ohms",
> + &p3h2x4x_i3c_hub->hub_config.tp2367_pullup);
> + of_property_read_u32(node, "nxp,cp0-io-strength-ohms",
> + &p3h2x4x_i3c_hub->hub_config.cp0_io_strength);
> + of_property_read_u32(node, "nxp,cp1-io-strength-ohms",
> + &p3h2x4x_i3c_hub->hub_config.cp1_io_strength);
> + of_property_read_u32(node, "nxp,tp0145-io-strength-ohms",
> + &p3h2x4x_i3c_hub->hub_config.tp0145_io_strength);
> + of_property_read_u32(node, "nxp,tp2367-io-strength-ohms",
> + &p3h2x4x_i3c_hub->hub_config.tp2367_io_strength);
> +
> + return p3h2x4x_parse_tp_dt_settings(dev, node,
> + p3h2x4x_i3c_hub->hub_config.tp_config);
> +}
> +
> +static void p3h2x4x_default_configuration(struct device *dev)
> +{
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub = dev_get_drvdata(dev);
> + int tp_count;
> +
> + p3h2x4x_i3c_hub->hub_config.tp0145_pullup = P3H2X4X_DFT_TP_PULLUP_OHMS;
> + p3h2x4x_i3c_hub->hub_config.tp2367_pullup = P3H2X4X_DFT_TP_PULLUP_OHMS;
> + p3h2x4x_i3c_hub->hub_config.cp0_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
> + p3h2x4x_i3c_hub->hub_config.cp1_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
> + p3h2x4x_i3c_hub->hub_config.tp0145_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
> + p3h2x4x_i3c_hub->hub_config.tp2367_io_strength = P3H2X4X_DFT_IO_STRENGTH_OHMS;
> +
> + for (tp_count = 0; tp_count < P3H2X4X_TP_MAX_COUNT; ++tp_count)
> + p3h2x4x_i3c_hub->hub_config.tp_config[tp_count].mode = P3H2X4X_TP_MODE_I3C;
> +}
> +
> +static void p3h2x4x_unregister_smbus_adapters_action(void *data)
> +{
> + p3h2x4x_unregister_smbus_adapters(data);
> +}
> +
> +static void p3h2x4x_put_target_port_of_nodes(void *data)
> +{
> + struct p3h2x4x_i3c_hub_dev *hub = data;
> + int tp;
> +
> + for (tp = 0; tp < P3H2X4X_TP_MAX_COUNT; tp++) {
> + of_node_put(hub->tp_bus[tp].of_node);
> + hub->tp_bus[tp].of_node = NULL;
> + }
> +}
> +
> +static void p3h2x4x_clear_i3c_hub_priv(void *data)
> +{
> + struct p3h2x4x *p3h2x4x = data;
> +
> + /* Drop the IBI handler backpointer; see the ordering note at the registration site. */
> + p3h2x4x->i3c_hub_priv = NULL;
> +}
> +
> +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> +{
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> + struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
> + struct device *dev = &pdev->dev;
> + struct device_node *node;
> + int ret, i;
> +
> + p3h2x4x_i3c_hub = devm_kzalloc(dev, sizeof(*p3h2x4x_i3c_hub), GFP_KERNEL);
> + if (!p3h2x4x_i3c_hub)
> + return -ENOMEM;
> +
> + p3h2x4x_i3c_hub->regmap = p3h2x4x->regmap;
> + p3h2x4x_i3c_hub->dev = dev;
> +
> + platform_set_drvdata(pdev, p3h2x4x_i3c_hub);
> + device_set_of_node_from_dev(dev, dev->parent);
> +
> + p3h2x4x_default_configuration(dev);
> +
> + ret = devm_mutex_init(dev, &p3h2x4x_i3c_hub->etx_mutex);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < P3H2X4X_TP_MAX_COUNT; i++) {
> + ret = devm_mutex_init(dev, &p3h2x4x_i3c_hub->tp_bus[i].port_mutex);
> + if (ret)
> + return ret;
> + }
> +
> + /* get hub node from DT */
> + node = dev_of_node(dev);
> + if (!node)
> + return dev_err_probe(dev, -ENODEV, "No Device Tree entry found\n");
> +
> + ret = p3h2x4x_get_hub_dt_conf(dev, node);
> + if (ret)
> + return ret;
> +
> + p3h2x4x_get_target_port_dt_conf(dev, node);
> +
> + ret = devm_add_action_or_reset(dev,
> + p3h2x4x_put_target_port_of_nodes,
> + p3h2x4x_i3c_hub);
> + if (ret)
> + return ret;
> +
> + ret = p3h2x4x_configure_hw(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to configure the HUB\n");
> +
> + /* Register virtual I3C master controllers for I3C target ports */
> + if (p3h2x4x->i3cdev) {
> + p3h2x4x_i3c_hub->i3cdev = p3h2x4x->i3cdev;
> + /*
> + * Publish the hub context in the MFD parent struct rather than
> + * via i3cdev_set_drvdata(), which would overwrite the parent's
> + * drvdata (struct p3h2x4x) that the IBI handler and other MFD
> + * callbacks rely on. Publish it before p3h2x4x_tp_i3c_algo()
> + * enables IBI, since the IBI handler dereferences it.
> + */
> + p3h2x4x->i3c_hub_priv = p3h2x4x_i3c_hub;
> +
> + /*
> + * Register the clear action before enabling IBI so that, on the
> + * devm LIFO unwind (probe failure or removal), the pointer is
> + * cleared only after IBI has been disabled and freed.
> + */
> + ret = devm_add_action_or_reset(dev, p3h2x4x_clear_i3c_hub_priv,
> + p3h2x4x);
> + if (ret)
> + return ret;
> +
> + ret = p3h2x4x_tp_i3c_algo(p3h2x4x_i3c_hub);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register i3c bus\n");
> + }
> +
> + /* Register virtual I2C adapters for SMBus target ports */
> + ret = p3h2x4x_tp_smbus_algo(p3h2x4x_i3c_hub);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to add i2c adapter\n");
> +
> + ret = devm_add_action_or_reset(dev,
> + p3h2x4x_unregister_smbus_adapters_action,
> + p3h2x4x_i3c_hub);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id p3h2x4x_i3c_hub_id[] = {
> + { "p3h2x4x-i3c-hub" },
Use named initializers. In every patch of yours.
> + { }
> +};
..
> +
> +/**
> + * p3h2x4x_tp_i3c_algo - Register I3C virtual masters for I3C target ports.
> + * @p3h2x4x_hub: p3h2x4x device structure.
> + * Return: 0 in case of success, negative error code on failure.
> + */
> +int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
> +{
> + struct i3c_master_controller *parent = i3c_dev_get_master(p3h2x4x_hub->i3cdev->desc);
> + struct p3h2x4x *p3h2x4x = dev_get_drvdata(p3h2x4x_hub->dev->parent);
> + u8 tp, ntwk_mask = 0;
> + int ret;
> +
> + p3h2x4x_hub->hub = devm_kzalloc(p3h2x4x_hub->dev,
> + sizeof(*p3h2x4x_hub->hub),
> + GFP_KERNEL);
> +
In multiple places you added blank lines between the call and if()
check. Don't.
> + if (!p3h2x4x_hub->hub)
> + return -ENOMEM;
Best regards,
Krzysztof
More information about the linux-i3c
mailing list