Devicetree
 help / color / mirror / Atom feed
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

  parent reply	other threads:[~2026-08-27 10:09 UTC|newest]

Thread overview: 22+ 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 ` [PATCH v16 1/8] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-08-26 10:52   ` sashiko-bot
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:50   ` sashiko-bot
2026-08-26 16:42   ` Frank Li
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 ` [PATCH v16 4/8] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
2026-08-26 10:56   ` sashiko-bot
2026-08-27 10:03   ` 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: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 11:04   ` sashiko-bot
2026-08-26 17:02     ` Frank Li
2026-08-27  7:07       ` Lakshay Piplani
2026-08-27 10:09   ` Krzysztof Kozlowski [this message]
2026-08-26 10:38 ` [PATCH v16 7/8] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-08-27 10:16   ` Krzysztof Kozlowski
2026-08-26 10:38 ` [PATCH v16 8/8] i3c: hub: p3h2x4x: Add SMBus slave mode support 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox