From: sashiko-bot@kernel.org
To: "Gerald Loacker" <gerald.loacker@wolfvision.net>
Cc: olteanv@gmail.com, vkoul@kernel.org, conor+dt@kernel.org,
linux-phy@lists.infradead.org, robh@kernel.org,
devicetree@vger.kernel.org, neil.armstrong@linaro.org
Subject: Re: [PATCH v2 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
Date: Fri, 19 Jun 2026 09:23:00 +0000 [thread overview]
Message-ID: <20260619092301.D68CF1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-1-323356c2cc2e@wolfvision.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Valid lowest frequency ranges are incorrectly rejected because `hsfreq == 0x00` is treated as an error by the `if (!hsfreq)` check.
--
commit d7a2518a95d2ab7b8f2baea0da9f35646ffb6136
Author: Gerald Loacker <gerald.loacker@wolfvision.net>
phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
The rk1808 hsfreq table previously capped at 2499 Mbps, preventing a data rate
of exactly 2500 Mbps. This commit extends the final entry to 2500 Mbps to
fully support this rate, which is also essential for RK3588 as it reuses
this array.
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index c79fb53d8ee5c..5281f8dea0ad3 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -170,7 +170,7 @@ static const struct hsfreq_range rk1808_mipidphy_hsfreq_ranges[] = {
> { 299, 0x06}, { 399, 0x08}, { 499, 0x0b}, { 599, 0x0e},
> { 699, 0x10}, { 799, 0x12}, { 999, 0x16}, {1199, 0x1e},
> {1399, 0x23}, {1599, 0x2d}, {1799, 0x32}, {1999, 0x37},
> - {2199, 0x3c}, {2399, 0x41}, {2499, 0x46}
> + {2199, 0x3c}, {2399, 0x41}, {2500, 0x46}
> };
>
> static const struct hsfreq_range rk3326_mipidphy_hsfreq_ranges[] = {
[Severity: High]
This isn't a bug introduced by this patch, but does the configuration logic
reject valid low frequency ranges?
Looking at rk3326_mipidphy_hsfreq_ranges[], the first entry configures low
data rates using 0x00:
{ 109, 0x00 }, ...
In rockchip_inno_csidphy_configure(), hsfreq is initialized to 0 and validated
with:
for (i = 0; i < drv_data->num_hsfreq_ranges; i++) {
if (drv_data->hsfreq_ranges[i].range_h >= data_rate_mbps) {
hsfreq = drv_data->hsfreq_ranges[i].cfg_bit;
break;
}
}
if (!hsfreq)
return -EINVAL;
When a low data rate is requested (e.g., <= 109 Mbps for rk3326), the loop
correctly assigns hsfreq = 0x00. However, the subsequent if (!hsfreq) check
treats this valid configuration as an error state and returns -EINVAL.
This same pattern appears in drivers/phy/rockchip/phy-rockchip-dphy-rx0.c
during rk_dphy_configure() where low-speed configurations are similarly
rejected.
Could the validation check be updated to differentiate between an uninitialized
value and a valid 0x00 configuration?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feature-mipi-csi-dphy-4k60-v2-0-323356c2cc2e@wolfvision.net?part=1
next prev parent reply other threads:[~2026-06-19 9:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-19 9:13 [PATCH v2 0/3] phy: rockchip: inno-csidphy: fix 2500 Mbps support and add clock lane phase tuning Gerald Loacker
2026-06-19 9:13 ` [PATCH v2 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table Gerald Loacker
2026-06-19 9:23 ` sashiko-bot [this message]
2026-06-19 9:13 ` [PATCH v2 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property Gerald Loacker
2026-06-19 9:13 ` [PATCH v2 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning Gerald Loacker
2026-06-19 9:26 ` 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=20260619092301.D68CF1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gerald.loacker@wolfvision.net \
--cc=linux-phy@lists.infradead.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox