From: Krzysztof Kozlowski <krzk@kernel.org>
To: Lakshay Piplani <lakshay.piplani@nxp.com>,
linux-kernel@vger.kernel.org, linux-i3c@lists.infradead.org,
alexandre.belloni@bootlin.com, krzk+dt@kernel.org,
robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org,
broonie@kernel.org, lee@kernel.org, Frank.Li@nxp.com,
lgirdwood@gmail.com
Cc: vikash.bansal@nxp.com, priyanka.jain@nxp.com, aman.kumarpandey@nxp.com
Subject: Re: [PATCH v16 6/8] i3c: hub: Add support for the I3C interface in the I3C hub
Date: Thu, 27 Aug 2026 12:09:16 +0200 [thread overview]
Message-ID: <b2d596af-ec24-406f-bcd7-813db0aebeb5@kernel.org> (raw)
In-Reply-To: <20260826103819.1614843-7-lakshay.piplani@nxp.com>
On 26/08/2026 12:38, Lakshay Piplani wrote:
> +/**
> + * i3c_hub_init() - Initialize hub context
> + * @hub: Hub instance
> + * @ops: Vendor callbacks
> + * @hub_dev: I3C hub device
> + */
> +void i3c_hub_init(struct i3c_hub *hub,
> + const struct i3c_hub_ops *ops,
> + struct i3c_device *hub_dev)
> +{
> + struct i3c_master_controller *parent;
> + unsigned int depth;
> +
> + hub->ops = ops;
> + hub->hub_dev = hub_dev;
> + mutex_init(&hub->lock);
> +
> + if (!IS_ENABLED(CONFIG_LOCKDEP))
So without lockdep you do not initialize? Honestly, kerneldoc is useless
here and explains nothing. Kerneldoc is pretty clear - you initialize
hub context thus why hub context should be uninitialized without lockdep?
> + return;
> +
> + if (WARN_ON_ONCE(!hub_dev || !hub_dev->desc))
> + return;
How is this possible? Why panicking the machine?
> +
> + parent = i3c_dev_get_master(hub_dev->desc);
> + if (WARN_ON_ONCE(!parent))
> + return;
Why panicking here (WARN on panic)?
> +
> + /*
> + * The routing mutex has the same hub nesting depth as the virtual
> + * controllers this hub exposes, so the parent controller is one level
> + * shallower. Keying it once here, rather than per port, avoids
> + * reclassifying the single shared routing mutex from a later port that
> + * may already have used it.
> + */
> + depth = i3c_hub_controller_depth(parent) + 1;
> + if (WARN_ONCE(depth > I3C_HUB_MAX_LOCK_DEPTH,
> + "i3c-hub: routing lock depth %u exceeds lockdep support\n",
> + depth))
> + depth = I3C_HUB_MAX_LOCK_DEPTH;
> +
> + lockdep_set_class(&hub->lock, &i3c_hub_routing_lock_keys[depth - 1]);
> +}
> +EXPORT_SYMBOL_GPL(i3c_hub_init);
> +
> +const struct i3c_master_controller_ops *i3c_hub_master_ops(void)
> +{
> + return &i3c_hub_master_ops_data;
> +}
> +EXPORT_SYMBOL_GPL(i3c_hub_master_ops);
Do you really need a wrapper call over single variable? Can the i3c hub
core code be NOT present when your I3C hub driver is buillin?
Best regards,
Krzysztof
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Lakshay Piplani <lakshay.piplani@nxp.com>,
linux-kernel@vger.kernel.org, linux-i3c@lists.infradead.org,
alexandre.belloni@bootlin.com, krzk+dt@kernel.org,
robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org,
broonie@kernel.org, lee@kernel.org, Frank.Li@nxp.com,
lgirdwood@gmail.com
Cc: vikash.bansal@nxp.com, priyanka.jain@nxp.com, aman.kumarpandey@nxp.com
Subject: Re: [PATCH v16 6/8] i3c: hub: Add support for the I3C interface in the I3C hub
Date: Thu, 27 Aug 2026 12:09:16 +0200 [thread overview]
Message-ID: <b2d596af-ec24-406f-bcd7-813db0aebeb5@kernel.org> (raw)
In-Reply-To: <20260826103819.1614843-7-lakshay.piplani@nxp.com>
On 26/08/2026 12:38, Lakshay Piplani wrote:
> +/**
> + * i3c_hub_init() - Initialize hub context
> + * @hub: Hub instance
> + * @ops: Vendor callbacks
> + * @hub_dev: I3C hub device
> + */
> +void i3c_hub_init(struct i3c_hub *hub,
> + const struct i3c_hub_ops *ops,
> + struct i3c_device *hub_dev)
> +{
> + struct i3c_master_controller *parent;
> + unsigned int depth;
> +
> + hub->ops = ops;
> + hub->hub_dev = hub_dev;
> + mutex_init(&hub->lock);
> +
> + if (!IS_ENABLED(CONFIG_LOCKDEP))
So without lockdep you do not initialize? Honestly, kerneldoc is useless
here and explains nothing. Kerneldoc is pretty clear - you initialize
hub context thus why hub context should be uninitialized without lockdep?
> + return;
> +
> + if (WARN_ON_ONCE(!hub_dev || !hub_dev->desc))
> + return;
How is this possible? Why panicking the machine?
> +
> + parent = i3c_dev_get_master(hub_dev->desc);
> + if (WARN_ON_ONCE(!parent))
> + return;
Why panicking here (WARN on panic)?
> +
> + /*
> + * The routing mutex has the same hub nesting depth as the virtual
> + * controllers this hub exposes, so the parent controller is one level
> + * shallower. Keying it once here, rather than per port, avoids
> + * reclassifying the single shared routing mutex from a later port that
> + * may already have used it.
> + */
> + depth = i3c_hub_controller_depth(parent) + 1;
> + if (WARN_ONCE(depth > I3C_HUB_MAX_LOCK_DEPTH,
> + "i3c-hub: routing lock depth %u exceeds lockdep support\n",
> + depth))
> + depth = I3C_HUB_MAX_LOCK_DEPTH;
> +
> + lockdep_set_class(&hub->lock, &i3c_hub_routing_lock_keys[depth - 1]);
> +}
> +EXPORT_SYMBOL_GPL(i3c_hub_init);
> +
> +const struct i3c_master_controller_ops *i3c_hub_master_ops(void)
> +{
> + return &i3c_hub_master_ops_data;
> +}
> +EXPORT_SYMBOL_GPL(i3c_hub_master_ops);
Do you really need a wrapper call over single variable? Can the i3c hub
core code be NOT present when your I3C hub driver is buillin?
Best regards,
Krzysztof
next prev parent reply other threads:[~2026-08-27 10:09 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-26 10:38 [PATCH v16 0/8] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:38 ` [PATCH v16 1/8] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:52 ` sashiko-bot
2026-08-26 10:52 ` sashiko-bot
2026-08-26 16:36 ` Frank Li
2026-08-26 16:36 ` Frank Li
2026-08-26 10:38 ` [PATCH v16 2/8] i3c: master: Add controller-only device operation helpers Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:50 ` sashiko-bot
2026-08-26 10:50 ` sashiko-bot
2026-08-26 16:42 ` Frank Li
2026-08-26 16:42 ` Frank Li
2026-08-27 7:02 ` Lakshay Piplani
2026-08-27 7:02 ` Lakshay Piplani
2026-08-26 10:38 ` [PATCH v16 3/8] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:38 ` [PATCH v16 4/8] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:56 ` sashiko-bot
2026-08-26 10:56 ` sashiko-bot
2026-08-27 10:03 ` Krzysztof Kozlowski
2026-08-27 10:03 ` Krzysztof Kozlowski
2026-08-28 16:13 ` [EXT] " Lakshay Piplani
2026-08-28 16:13 ` Lakshay Piplani
2026-09-09 6:05 ` Krzysztof Kozlowski
2026-09-09 6:05 ` Krzysztof Kozlowski
2026-08-26 10:38 ` [PATCH v16 5/8] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 10:48 ` sashiko-bot
2026-08-26 10:48 ` sashiko-bot
2026-08-26 10:38 ` [PATCH v16 6/8] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-26 11:04 ` sashiko-bot
2026-08-26 11:04 ` sashiko-bot
2026-08-26 17:02 ` Frank Li
2026-08-26 17:02 ` Frank Li
2026-08-27 7:07 ` Lakshay Piplani
2026-08-27 7:07 ` Lakshay Piplani
2026-08-27 10:09 ` Krzysztof Kozlowski [this message]
2026-08-27 10:09 ` Krzysztof Kozlowski
2026-08-28 20:16 ` [EXT] " Lakshay Piplani
2026-08-28 20:16 ` Lakshay Piplani
2026-08-26 10:38 ` [PATCH v16 7/8] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
2026-08-27 10:16 ` Krzysztof Kozlowski
2026-08-27 10:16 ` Krzysztof Kozlowski
2026-08-28 20:26 ` [EXT] " Lakshay Piplani
2026-08-28 20:26 ` Lakshay Piplani
2026-08-26 10:38 ` [PATCH v16 8/8] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
2026-08-26 10:38 ` Lakshay Piplani
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b2d596af-ec24-406f-bcd7-813db0aebeb5@kernel.org \
--to=krzk@kernel.org \
--cc=Frank.Li@nxp.com \
--cc=alexandre.belloni@bootlin.com \
--cc=aman.kumarpandey@nxp.com \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lakshay.piplani@nxp.com \
--cc=lee@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-i3c@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=priyanka.jain@nxp.com \
--cc=robh@kernel.org \
--cc=vikash.bansal@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.