All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Louis-Alexis Eyraud" <louisalexis.eyraud@collabora.com>
Cc: vkoul@kernel.org, olteanv@gmail.com, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, neil.armstrong@linaro.org,
	linux-scsi@vger.kernel.org, linux-phy@lists.infradead.org
Subject: Re: [PATCH v12 24/24] scsi: ufs: mediatek: Add MT8196 compatible, update copyright
Date: Mon, 14 Sep 2026 13:41:42 +0000	[thread overview]
Message-ID: <20260914134142.6DBEF1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260914-mt8196-ufs-v12-24-9279d7ef814d@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Kernel panic in ufs_mtk_09v_on() and ufs_mtk_09v_off() error paths due to missing NULL checks on host->reg_avdd09.
--

commit 3e1ab912b68bb7adad59190a0d213096bb41991c
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

scsi: ufs: mediatek: Add MT8196 compatible, update copyright

The MT8196's UFS controller has a new compatible. Add the necessary
struct definitions to support it.

> THe MT8196's UFS controller has a new compatible. Add the necessary
  ^^^
[Severity: Low]
This isn't a bug, but there's a typo (THe) in the commit message.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index ef8924704c902..ceb52c40d266c 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c

[ ... ]

> @@ -2264,10 +2270,18 @@ static const struct ufs_mtk_soc_data mt8192_8195_data = {
>  	.num_reg_names = ARRAY_SIZE(ufs_mtk_regs_avdd12_ckbuf_avdd18),
>  };
>  
> +static const struct ufs_mtk_soc_data mt8196_data = {
> +	.has_avdd09 = true,
> +	.has_avdd09_1 = true,

[Severity: High]
Does enabling has_avdd09_1 introduce a NULL pointer dereference in the error
handling paths for ufs_mtk_09v_on() and ufs_mtk_09v_off()?

If a device tree omits avdd09 but provides avdd09-1, host->reg_avdd09 will
be NULL. In ufs_mtk_09v_on(), the code properly checks if host->reg_avdd09
is valid before enabling it. However, if enabling host->reg_avdd09_1
subsequently fails, the error handler blindly executes:

    regulator_disable(host->reg_avdd09);

without checking if host->reg_avdd09 is NULL.

The core regulator subsystem unconditionally dereferences the struct regulator
pointer when CONFIG_REGULATOR is enabled, which would cause a panic.

The exact same issue appears to exist in ufs_mtk_09v_off(), where it calls:

    regulator_enable(host->reg_avdd09);

unconditionally if disabling avdd09-1 fails.

Since the MT8196 compatible is the first to set has_avdd09_1 = true, does
this make these error paths reachable?

> +	.reg_names = ufs_mtk_regs_avdd12_ckbuf,
> +	.num_reg_names = ARRAY_SIZE(ufs_mtk_regs_avdd12_ckbuf),
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-mt8196-ufs-v12-0-9279d7ef814d@collabora.com?part=24

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Louis-Alexis Eyraud" <louisalexis.eyraud@collabora.com>
Cc: vkoul@kernel.org, olteanv@gmail.com, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, neil.armstrong@linaro.org,
	linux-scsi@vger.kernel.org, linux-phy@lists.infradead.org
Subject: Re: [PATCH v12 24/24] scsi: ufs: mediatek: Add MT8196 compatible, update copyright
Date: Mon, 14 Sep 2026 13:41:42 +0000	[thread overview]
Message-ID: <20260914134142.6DBEF1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260914-mt8196-ufs-v12-24-9279d7ef814d@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Kernel panic in ufs_mtk_09v_on() and ufs_mtk_09v_off() error paths due to missing NULL checks on host->reg_avdd09.
--

commit 3e1ab912b68bb7adad59190a0d213096bb41991c
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

scsi: ufs: mediatek: Add MT8196 compatible, update copyright

The MT8196's UFS controller has a new compatible. Add the necessary
struct definitions to support it.

> THe MT8196's UFS controller has a new compatible. Add the necessary
  ^^^
[Severity: Low]
This isn't a bug, but there's a typo (THe) in the commit message.

> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index ef8924704c902..ceb52c40d266c 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c

[ ... ]

> @@ -2264,10 +2270,18 @@ static const struct ufs_mtk_soc_data mt8192_8195_data = {
>  	.num_reg_names = ARRAY_SIZE(ufs_mtk_regs_avdd12_ckbuf_avdd18),
>  };
>  
> +static const struct ufs_mtk_soc_data mt8196_data = {
> +	.has_avdd09 = true,
> +	.has_avdd09_1 = true,

[Severity: High]
Does enabling has_avdd09_1 introduce a NULL pointer dereference in the error
handling paths for ufs_mtk_09v_on() and ufs_mtk_09v_off()?

If a device tree omits avdd09 but provides avdd09-1, host->reg_avdd09 will
be NULL. In ufs_mtk_09v_on(), the code properly checks if host->reg_avdd09
is valid before enabling it. However, if enabling host->reg_avdd09_1
subsequently fails, the error handler blindly executes:

    regulator_disable(host->reg_avdd09);

without checking if host->reg_avdd09 is NULL.

The core regulator subsystem unconditionally dereferences the struct regulator
pointer when CONFIG_REGULATOR is enabled, which would cause a panic.

The exact same issue appears to exist in ufs_mtk_09v_off(), where it calls:

    regulator_enable(host->reg_avdd09);

unconditionally if disabling avdd09-1 fails.

Since the MT8196 compatible is the first to set has_avdd09_1 = true, does
this make these error paths reachable?

> +	.reg_names = ufs_mtk_regs_avdd12_ckbuf,
> +	.num_reg_names = ARRAY_SIZE(ufs_mtk_regs_avdd12_ckbuf),
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-mt8196-ufs-v12-0-9279d7ef814d@collabora.com?part=24

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2026-09-14 13:41 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-14 11:38 [PATCH v12 00/24] MediaTek UFS Cleanup and MT8196 Enablement Louis-Alexis Eyraud
2026-09-14 11:38 ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 01/24] dt-bindings: phy: Add mediatek,mt8196-ufsphy variant Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 02/24] dt-bindings: ufs: mediatek,ufs: Complete the binding Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 12:04   ` AngeloGioacchino Del Regno
2026-09-14 12:04     ` AngeloGioacchino Del Regno
2026-09-24 15:26   ` Rob Herring (Arm)
2026-09-24 15:26     ` Rob Herring (Arm)
2026-09-14 11:38 ` [PATCH v12 03/24] dt-bindings: ufs: mediatek,ufs: Add mt8196 variant Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 12:04   ` AngeloGioacchino Del Regno
2026-09-14 12:04     ` AngeloGioacchino Del Regno
2026-09-24 15:27   ` Rob Herring (Arm)
2026-09-24 15:27     ` Rob Herring (Arm)
2026-09-14 11:38 ` [PATCH v12 04/24] scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 05/24] phy: mediatek: ufs: Add support for resets Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 06/24] scsi: ufs: mediatek: Rework resets Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 07/24] scsi: ufs: mediatek: Rework 0.9V regulator Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 12:19   ` sashiko-bot
2026-09-14 12:19     ` sashiko-bot
2026-09-14 11:38 ` [PATCH v12 08/24] scsi: ufs: mediatek: Add dual 0.9V supply support Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 12:02   ` AngeloGioacchino Del Regno
2026-09-14 12:02     ` AngeloGioacchino Del Regno
2026-09-14 12:21   ` sashiko-bot
2026-09-14 12:21     ` sashiko-bot
2026-09-14 11:38 ` [PATCH v12 09/24] scsi: ufs: mediatek: Rework init function Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 10/24] scsi: ufs: mediatek: Rework the crypt-boost stuff Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 11/24] scsi: ufs: mediatek: Handle misc host voltage regulators Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 12/24] scsi: ufs: mediatek: Remove undocumented downstream reset cruft Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 11:38 ` [PATCH v12 13/24] scsi: ufs: mediatek: Remove vendor kernel quirks cruft Louis-Alexis Eyraud
2026-09-14 11:38   ` Louis-Alexis Eyraud
2026-09-14 12:45   ` sashiko-bot
2026-09-14 12:45     ` sashiko-bot
2026-09-14 11:39 ` [PATCH v12 14/24] scsi: ufs: mediatek: Use the common PHY framework Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 12:58   ` sashiko-bot
2026-09-14 12:58     ` sashiko-bot
2026-09-14 11:39 ` [PATCH v12 15/24] scsi: ufs: mediatek: Remove mediatek,ufs-broken-rtc property Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 12:52   ` sashiko-bot
2026-09-14 12:52     ` sashiko-bot
2026-09-14 11:39 ` [PATCH v12 16/24] scsi: ufs: mediatek: Rework _ufs_mtk_clk_scale error paths Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 13:03   ` sashiko-bot
2026-09-14 13:03     ` sashiko-bot
2026-09-14 11:39 ` [PATCH v12 17/24] scsi: ufs: mediatek: Clean up logging prints Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 18/24] scsi: ufs: mediatek: Rework ufs_mtk_wait_idle_state Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 19/24] scsi: ufs: mediatek: Don't acquire dvfsrc-vcore twice Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 20/24] scsi: ufs: mediatek: Rework hardware version reading Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 21/24] scsi: ufs: mediatek: Back up idle timer in per-instance struct Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 22/24] scsi: ufs: mediatek: Remove ret local from link_startup_notify Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 11:39 ` [PATCH v12 23/24] scsi: ufs: mediatek: Remove undocumented "clk-scale-up-vcore-min" Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 13:25   ` sashiko-bot
2026-09-14 13:25     ` sashiko-bot
2026-09-14 11:39 ` [PATCH v12 24/24] scsi: ufs: mediatek: Add MT8196 compatible, update copyright Louis-Alexis Eyraud
2026-09-14 11:39   ` Louis-Alexis Eyraud
2026-09-14 13:41   ` sashiko-bot [this message]
2026-09-14 13:41     ` sashiko-bot

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=20260914134142.6DBEF1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=louisalexis.eyraud@collabora.com \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /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.