Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
From: Gerald Loacker @ 2026-06-19  9:13 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
	devicetree, Gerald Loacker
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-0-323356c2cc2e@wolfvision.net>

The rk1808 hsfreq table capped at 2499 Mbps, preventing a data rate of
exactly 2500 Mbps. Extend the final entry to 2500 Mbps to support this
rate.

This is essential for RK3588 reusing this array and fully supporting
rates up to 2500 Mbps.

Fixes: bd1f775d6027 ("phy/rockchip: add Innosilicon-based CSI dphy")
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
 drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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[] = {

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/3] dt-bindings: phy: rockchip-inno-csi-dphy: add rockchip,clk-lane-phase property
From: Gerald Loacker @ 2026-06-19  9:13 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
	devicetree, Gerald Loacker
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-0-323356c2cc2e@wolfvision.net>

Add support for the optional rockchip,clk-lane-phase device tree property
to allow board-specific tuning of the clock lane sampling phase for
improved signal integrity across supported data rates.

Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
 .../devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml          | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
index 03950b3cad08c..010950a8a8856 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/rockchip-inno-csi-dphy.yaml
@@ -56,6 +56,15 @@ properties:
     description:
       Some additional phy settings are access through GRF regs.
 
+  rockchip,clk-lane-phase:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 7
+    description:
+      Clock lane sampling phase selection (hardware tap index 0–7). Each step
+      corresponds to an approximately 40 ps delay as described in the hardware
+      specification.
+
 required:
   - compatible
   - reg

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
From: Gerald Loacker @ 2026-06-19  9:13 UTC (permalink / raw)
  To: Vinod Koul, Neil Armstrong, Heiko Stuebner, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-phy, linux-arm-kernel, linux-rockchip, linux-kernel,
	devicetree, Gerald Loacker
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-0-323356c2cc2e@wolfvision.net>

At high data rates like 4K60 (2500 Mbps), such as when using an
LT6911GXD bridge chip on an RK3588 board, fixed default timing parameters
can cause signal integrity issues and clock-data recovery failures.
The driver currently lacks a mechanism to adjust the clock lane sampling
phase to compensate for board-specific trace variations.

Resolve this by parsing and applying the optional 'rockchip,clk-lane-phase'
device tree property. This enables board-specific tuning of the clock
lane sampling phase in ~40 ps steps (range 0-7) to optimize link
stability. If the property is absent, the driver falls back to the
hardware default.

Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
---
 drivers/phy/rockchip/phy-rockchip-inno-csidphy.c | 25 ++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
index 5281f8dea0ad3..3a15840e86cad 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
@@ -69,6 +69,10 @@
 #define RK1808_CSIDPHY_CLK_CALIB_EN		0x168
 #define RK3568_CSIDPHY_CLK_CALIB_EN		0x168
 
+#define CSIDPHY_LANE_CLK_3_PHASE		0x38
+#define CSIDPHY_CLK_PHASE_MASK			GENMASK(6, 4)
+#define CSIDPHY_CLK_PHASE_DEFAULT		3
+
 #define RESETS_MAX				2
 
 /*
@@ -151,6 +155,7 @@ struct rockchip_inno_csidphy {
 	const struct dphy_drv_data *drv_data;
 	struct phy_configure_opts_mipi_dphy config;
 	u8 hsfreq;
+	int clk_phase;
 };
 
 static inline void write_grf_reg(struct rockchip_inno_csidphy *priv,
@@ -304,6 +309,13 @@ static int rockchip_inno_csidphy_power_on(struct phy *phy)
 		rockchip_inno_csidphy_ths_settle(priv, priv->hsfreq,
 						 CSIDPHY_LANE_THS_SETTLE(i));
 
+	if (priv->clk_phase >= 0) {
+		val = readl(priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
+		val &= ~CSIDPHY_CLK_PHASE_MASK;
+		val |= FIELD_PREP(CSIDPHY_CLK_PHASE_MASK, priv->clk_phase);
+		writel(val, priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
+	}
+
 	write_grf_reg(priv, GRF_DPHY_CSIPHY_CLKLANE_EN, 0x1);
 	write_grf_reg(priv, GRF_DPHY_CSIPHY_DATALANE_EN,
 		      GENMASK(priv->config.lanes - 1, 0));
@@ -449,6 +461,7 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct phy_provider *phy_provider;
 	struct phy *phy;
+	u32 phase;
 	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -464,6 +477,18 @@ static int rockchip_inno_csidphy_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	priv->clk_phase = -1;
+	if (device_property_read_u32(dev, "rockchip,clk-lane-phase",
+				     &phase) == 0) {
+		if (phase >= BIT(3)) {
+			dev_err(dev,
+				"rockchip,clk-lane-phase %u out of range [0,7]\n",
+				phase);
+			return -EINVAL;
+		}
+		priv->clk_phase = phase;
+	}
+
 	priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
 						    "rockchip,grf");
 	if (IS_ERR(priv->grf)) {

-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse
From: Nuno Sá @ 2026-06-19  9:16 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: rodrigo.alencar, linux-iio, devicetree, linux-kernel, linux-doc,
	linux-hardening, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, David Lechner, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
	Shuah Khan, Kees Cook, Gustavo A. R. Silva
In-Reply-To: <x3aijvc4buo7aqbchikuoyyrgiq3afidtkla37h2rg4tvfdbc3@h42qp3estg2s>

On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote:
> On 18/06/26 16:06, Nuno Sá wrote:
> > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:
> > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > 
> > > Move logic to create a channel prefix for naming attribute files into a
> > > separate __iio_chan_prefix_emit() function for reuse.
> 
> ...
> 
> > > +static int __iio_chan_prefix_emit(const struct iio_chan_spec *chan,
> > > +				  enum iio_shared_by shared_by,
> > > +				  char *buf, size_t len)
> > > +{
> > > +	const char *dir = iio_direction[chan->output];
> > > +	const char *type = iio_chan_type_name_spec[chan->type];
> > > +	int n = 0;
> > > +
> > > +	switch (shared_by) {
> > > +	case IIO_SHARED_BY_ALL:
> > > +		buf[0] = '\0'; /* empty channel prefix */
> > > +		break;
> > > +	case IIO_SHARED_BY_DIR:
> > > +		n = scnprintf(buf, len, "%s", dir);
> > > +		break;
> > > +	case IIO_SHARED_BY_TYPE:
> > > +		n = scnprintf(buf, len, "%s_%s", dir, type);
> > > +		if (chan->differential)
> > > +			n += scnprintf(buf + n, len - n, "-%s", type);
> > > +		break;
> > > +	case IIO_SEPARATE:
> > > +		if (chan->indexed) {
> > > +			n = scnprintf(buf, len, "%s_%s%d", dir, type,
> > > +				      chan->channel);
> > > +			if (chan->differential)
> > > +				n += scnprintf(buf + n, len - n, "-%s%d", type,
> > > +					       chan->channel2);
> > > +		} else {
> > > +			if (chan->differential) {
> > > +				WARN(1, "Differential channels must be indexed\n");
> > > +				return -EINVAL;
> > > +			}
> > > +			n = scnprintf(buf, len, "%s_%s", dir, type);
> > > +		}
> > > +
> > > +		if (chan->modified) {
> > > +			if (chan->differential) {
> > > +				WARN(1, "Differential channels can not have modifier\n");
> > > +				return -EINVAL;
> > 
> > WARN() looks too much to me. dev_error() as we're treating it as such. I
> > guess you don't want to pass struct device but not really an issue IMHO.
> 
> __iio_device_attr_init() also used WARN(), probably because it didnt have
> access to a dev pointer. It would not be a problem to add an extra param.

Hmm, fair enough. Maybe a chance to change it. Not sure how others feel
about it.

>  
> > 
> > > +			}
> > > +			n += scnprintf(buf + n, len - n, "_%s",
> > > +				       iio_modifier_names[chan->channel2]);
> > > +		}
> > > +
> > > +		if (chan->extend_name)
> > > +			n += scnprintf(buf + n, len - n, "_%s", chan->extend_name);
> > > +		break;
> > > +	}
> > > +
> > > +	if (n > 0 && n < len - 1) { /* prefix termination if not empty */
> > > +		buf[n++] = '_';
> > > +		buf[n] = '\0';
> > > +	}
> > > +
> > 
> > Can't we handle the above in the caller on kasprintf()? Then we could
> > simplify and return in place.
> 
> I felt like doing this here would get a cleaner logic in the caller, which
> would have to add the '_' conditionally.
> 

I think it makes things more clear in the caller given we return n
anyways but I don't feel strong about it.

- Nuno Sá

> > 
> > > +	return n;
> > > +}
> > > +
> > >  /**
> > >   * iio_device_id() - query the unique ID for the device
> > >   * @indio_dev:		Device structure whose ID is being queried
> > > @@ -1100,106 +1159,19 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
> > >  						size_t len),
> > >  			   enum iio_shared_by shared_by)
> > >  {
> > > -	int ret = 0;
> > > -	char *name = NULL;
> > > -	char *full_postfix;
> > > +	char prefix[NAME_MAX + 1];
> > > +	int ret;
> > >  
> > >  	sysfs_attr_init(&dev_attr->attr);
> > >  
> > > -	/* Build up postfix of <extend_name>_<modifier>_postfix */
> > > -	if (chan->modified && (shared_by == IIO_SEPARATE)) {
> > > -		if (chan->extend_name)
> > > -			full_postfix = kasprintf(GFP_KERNEL, "%s_%s_%s",
> > > -						 iio_modifier_names[chan->channel2],
> > > -						 chan->extend_name,
> > > -						 postfix);
> > > -		else
> > > -			full_postfix = kasprintf(GFP_KERNEL, "%s_%s",
> > > -						 iio_modifier_names[chan->channel2],
> > > -						 postfix);
> > > -	} else {
> > > -		if (chan->extend_name == NULL || shared_by != IIO_SEPARATE)
> > > -			full_postfix = kstrdup(postfix, GFP_KERNEL);
> > > -		else
> > > -			full_postfix = kasprintf(GFP_KERNEL,
> > > -						 "%s_%s",
> > > -						 chan->extend_name,
> > > -						 postfix);
> > > -	}
> > > -	if (full_postfix == NULL)
> > > +	ret = __iio_chan_prefix_emit(chan, shared_by, prefix, sizeof(prefix));
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix);
> > > +	if (!dev_attr->attr.name)
> > >  		return -ENOMEM;
> > 
> > I don't oppose the change. Looks like a nice cleanup. But bear in mind
> > this very sensible as any subtle mistake means ABI breakage.
> 
> Yes! I tried to be careful... this is dangerous stuff!
> 
> -- 
> Kind regards,
> 
> Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse
From: Nuno Sá @ 2026-06-19  9:19 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rodrigo Alencar, rodrigo.alencar, linux-iio, devicetree,
	linux-kernel, linux-doc, linux-hardening, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, David Lechner,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
	Gustavo A. R. Silva
In-Reply-To: <ajQ1bZSNHQ96pyJx@ashevche-desk.local>

On Thu, Jun 18, 2026 at 09:14:05PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote:
> > On 18/06/26 16:06, Nuno Sá wrote:
> > > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:
> 
> ...
> 
> > > > +	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix);
> > > > +	if (!dev_attr->attr.name)
> > > >  		return -ENOMEM;
> > > 
> > > I don't oppose the change. Looks like a nice cleanup.
> 
> May I oppose it? I found use scnprintf() is harder to follow in comparison to
> nice kasprintf() that takes care for the dynamically allocated buffer.

Tend to agree a bit given I was used to the older code. So matching the
old logic with the new one is an exercise, yes.

> 
> Also there is a chance to get a name silently cut due to insufficient space.
> Besides that this function can't be used (again due to 'c') in kasprintf()-like
> wrapper. I do not consider this as a good approach. Have you looked at seq_buf
> instead?

Not so sure the above bothers me that much.

> 
> > > But bear in mind this very sensible as any subtle mistake means ABI breakage.
> 
> Which immediately raises a question of test coverage. Do we have one? If not,
> this code must be accompanied with one.

The above is the more concerning part to me.

- Nuno Sá

> 
> > Yes! I tried to be careful... this is dangerous stuff!
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse
From: Nuno Sá @ 2026-06-19  9:20 UTC (permalink / raw)
  To: Rodrigo Alencar
  Cc: Andy Shevchenko, rodrigo.alencar, linux-iio, devicetree,
	linux-kernel, linux-doc, linux-hardening, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, David Lechner,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
	Gustavo A. R. Silva
In-Reply-To: <dlisetsssjoyodmv5ubl4rzhxtla3g46mrrzv2f65nqecel5fu@dqiqsayr4aip>

On Fri, Jun 19, 2026 at 08:43:24AM +0100, Rodrigo Alencar wrote:
> On 18/06/26 21:14, Andy Shevchenko wrote:
> > On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote:
> > > On 18/06/26 16:06, Nuno Sá wrote:
> > > > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:
> > 
> > ...
> > 
> > > > > +	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix);
> > > > > +	if (!dev_attr->attr.name)
> > > > >  		return -ENOMEM;
> > > > 
> > > > I don't oppose the change. Looks like a nice cleanup.
> > 
> > May I oppose it? I found use scnprintf() is harder to follow in comparison to
> > nice kasprintf() that takes care for the dynamically allocated buffer.
> 
> In the next patch the function is reused in a sysfs attribute read handler,
> a context wich would not be nice to have dynamic allocation. vscnprintf() is
> the main building block of sysfs_emit() which limits the buffer length to
> a page size, so I used scnprintf() trying not to deviate much from that. 
> 
> kasprintf() it is still used in the caller, where the logic was a bit confusing
> as it tried to avoid multiple allocations.
>  
> > Also there is a chance to get a name silently cut due to insufficient space.
> > Besides that this function can't be used (again due to 'c') in kasprintf()-like
> > wrapper. I do not consider this as a good approach. Have you looked at seq_buf
> > instead?
> 
> NAME_MAX is not the maximum length a filename can have? I suppose there should be
> enough space for the channel-prefix. Indeed, seq_buf can be used and it cleans up
> things a bit as it tracks the the position in the buffer.
> 
> > 
> > > > But bear in mind this very sensible as any subtle mistake means ABI breakage.
> > 
> > Which immediately raises a question of test coverage. Do we have one? If not,
> > this code must be accompanied with one.
> 
> Agreed. Will see to have tests for v7.

libiio now has an emulator backend. Maybe something that can be used to
test this. But ideally we can have some kunit for validation.

- Nuno Sá

> 
> > > Yes! I tried to be careful... this is dangerous stuff!
> 
> -- 
> Kind regards,
> 
> Rodrigo Alencar

^ permalink raw reply

* Re: [PATCH v2 1/3] phy: rockchip: phy-rockchip-inno-csidphy: fix rk1808 hsfreq table
From: sashiko-bot @ 2026-06-19  9:23 UTC (permalink / raw)
  To: Gerald Loacker
  Cc: olteanv, vkoul, conor+dt, linux-phy, robh, devicetree,
	neil.armstrong
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

^ permalink raw reply

* Re: [PATCH v2 3/3] phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning
From: sashiko-bot @ 2026-06-19  9:26 UTC (permalink / raw)
  To: Gerald Loacker
  Cc: neil.armstrong, vkoul, robh, devicetree, olteanv, linux-phy,
	conor+dt
In-Reply-To: <20260619-feature-mipi-csi-dphy-4k60-v2-3-323356c2cc2e@wolfvision.net>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The macro `CSIDPHY_CLK_PHASE_DEFAULT` is defined but never used.
--

commit 47571c2082c0165537db96312865f3a9805e768d
Author: Gerald Loacker <gerald.loacker@wolfvision.net>

phy: rockchip: phy-rockchip-inno-csidphy: add clock lane phase tuning

This commit adds support for parsing the 'rockchip,clk-lane-phase' device
tree property to adjust the clock lane sampling phase. It enables
board-specific tuning to optimize link stability at high data rates, falling
back to hardware defaults if the property is absent.

> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> index 5281f8dea0ad3..3a15840e86cad 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-csidphy.c
> @@ -69,6 +69,10 @@
>  #define RK1808_CSIDPHY_CLK_CALIB_EN		0x168
>  #define RK3568_CSIDPHY_CLK_CALIB_EN		0x168
>  
> +#define CSIDPHY_LANE_CLK_3_PHASE		0x38
> +#define CSIDPHY_CLK_PHASE_MASK			GENMASK(6, 4)
> +#define CSIDPHY_CLK_PHASE_DEFAULT		3

[Severity: Low]
Is this macro intended to be used somewhere? 

It appears to be defined but never used, since the driver falls back to the
hardware default by initializing priv->clk_phase to -1 in
rockchip_inno_csidphy_probe() and conditionally skipping the register write in
rockchip_inno_csidphy_power_on():

	if (priv->clk_phase >= 0) {
		val = readl(priv->phy_base + CSIDPHY_LANE_CLK_3_PHASE);
		...
	}

Can CSIDPHY_CLK_PHASE_DEFAULT be safely removed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feature-mipi-csi-dphy-4k60-v2-0-323356c2cc2e@wolfvision.net?part=3

^ permalink raw reply

* [PATCH 0/5] hwmon: add Altera Stratix 10 SoC FPGA hardware  monitor support
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc

From: Tze Yee Ng <tze.yee.ng@altera.com>

This series adds hardware monitor support for Altera Stratix 10 SoC FPGA
devices. Temperature and voltage sensors are accessed through the 
Stratix 10 service layer and Secure Device Manager.

Patch 1 adds the device tree binding for the hwmon node and sensor layout.
Patch 2 extends the Stratix 10 service layer binding with an optional
hwmon child node. Patch 3 adds async HWMON read commands to the service
firmware driver. Patch 4 adds the hwmon driver, using the async service
interface when available and falling back to synchronous reads otherwise.
Patch 5 enables the hwmon node on the Stratix 10 SoCDK.

Tze Yee Ng (5):
  dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding
  dt-bindings: firmware: svc: add hwmon property
  firmware: stratix10-svc: add async HWMON read commands
  hwmon: add Stratix 10 SoC FPGA hardware monitor driver
  arm64: dts: socfpga: stratix10: add hwmon node

 .../firmware/intel,stratix10-svc.yaml         |   4 +
 .../bindings/hwmon/altr,stratix10-hwmon.yaml  | 164 +++++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/stratix10-hwmon.rst       |  31 +
 MAINTAINERS                                   |   9 +
 .../boot/dts/altera/socfpga_stratix10.dtsi    |   5 +
 .../dts/altera/socfpga_stratix10_socdk.dts    |  33 +
 drivers/firmware/stratix10-svc.c              |  12 +
 drivers/hwmon/Kconfig                         |  10 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/stratix10-hwmon.c               | 575 ++++++++++++++++++
 include/linux/firmware/intel/stratix10-smc.h  |  38 ++
 12 files changed, 883 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
 create mode 100644 Documentation/hwmon/stratix10-hwmon.rst
 create mode 100644 drivers/hwmon/stratix10-hwmon.c

-- 
2.43.7


^ permalink raw reply

* [PATCH 1/5] dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <cover.1781861409.git.tze.yee.ng@altera.com>

From: Tze Yee Ng <tze.yee.ng@altera.com>

Document the device tree binding for the Altera Stratix 10 SoC FPGA
hardware monitor, including temperature and voltage sensor nodes.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 .../bindings/hwmon/altr,stratix10-hwmon.yaml  | 164 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
new file mode 100644
index 000000000000..5bd98660ee7b
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/altr,stratix10-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Altera Hardware Monitor for Stratix 10 SoC FPGA
+
+maintainers:
+  - Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
+  - Tze Yee Ng <tze.yee.ng@altera.com>
+
+description: |
+  The Altera Stratix 10 SoC FPGA hardware monitor unit provides on-chip
+  voltage and temperature sensors. These sensors can be used to monitor
+  external voltages and on-chip operating conditions such as internal
+  power rails and on-chip junction temperatures.
+
+  The specific sensor configuration varies by device. Check the device
+  documentation to verify which sensors are available.
+
+  Stratix 10 voltage sensors:
+
+    page 0, channel 2 = 0.8V VCC
+    page 0, channel 3 = 1.8V VCCIO_SDM
+    page 0, channel 6 = 0.9V VCCERAM
+
+  Stratix 10 temperature sensors:
+
+    page 0, channel 0 = main die
+    page 0, channel 1 = tile bottom left
+    page 0, channel 2 = tile middle left
+    page 0, channel 3 = tile top left
+    page 0, channel 4 = tile bottom right
+    page 0, channel 5 = tile middle right
+    page 0, channel 6 = tile top right
+    page 0, channel 7 = hbm2 bottom
+    page 0, channel 8 = hbm2 top
+
+properties:
+  compatible:
+    const: altr,stratix10-hwmon
+
+  temperature:
+    description:
+      The temperature node specifies mappings of temperature sensor diodes on
+      the Stratix 10 SoC FPGA main die and tile die.
+    type: object
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+    patternProperties:
+      "^input(@[0-9a-f]+)?$":
+        description:
+          The input node specifies each individual temperature sensor.
+        type: object
+        properties:
+          reg:
+            description:
+              Sensor channel index in the lower 16-bits (0-15). For temperature
+              sensors, the page number is encoded in the upper 16-bits.
+              The driver encodes the SMC request argument as a channel
+              bitmask (1 << channel) in bits 0..15, with the page number
+              placed in bits 16..31. Channel values >= 16 are rejected to
+              avoid overlap with the page field. For example, reg = <2>
+              selects channel 2 and the driver passes 0x4 to the service layer.
+          label:
+            description:
+              A descriptive name for this channel (e.g. "Main Die" or
+              "Tile Bottom Left").
+        required:
+          - reg
+        additionalProperties: false
+    required:
+      - '#address-cells'
+      - '#size-cells'
+    additionalProperties: false
+
+  voltage:
+    description:
+      The voltage node specifies mappings of voltage sensors on the Stratix 10
+      SoC FPGA analog to digital converter of the Secure Device Manager (SDM).
+    type: object
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+    patternProperties:
+      "^input(@[0-9a-f]+)?$":
+        description:
+          The input node specifies each individual voltage sensor.
+        type: object
+        properties:
+          reg:
+            description:
+              Sensor channel index in the lower 16-bits (0-15). The driver
+              encodes the SMC request argument as a channel bitmask
+              (1 << channel). For example, reg = <2> selects channel 2 and
+              the driver passes 0x4 to the service layer.
+          label:
+            description:
+              A descriptive name for this channel (e.g. "0.8V VCC" or
+              "1.8V VCCIO_SDM").
+        required:
+          - reg
+        additionalProperties: false
+    required:
+      - '#address-cells'
+      - '#size-cells'
+    additionalProperties: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    hwmon {
+      compatible = "altr,stratix10-hwmon";
+
+      voltage {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        input@2 {
+          label = "0.8V VCC";
+          reg = <2>;
+        };
+
+        input@3 {
+          label = "1.8V VCCIO_SDM";
+          reg = <3>;
+        };
+
+        input@6 {
+          label = "0.9V VCCERAM";
+          reg = <6>;
+        };
+      };
+
+      temperature {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        input@0 {
+          label = "Main Die";
+          reg = <0>;
+        };
+
+        input@1 {
+          label = "Tile Bottom Left";
+          reg = <1>;
+        };
+
+        input@2 {
+          label = "Tile Middle Left";
+          reg = <2>;
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 6aa3fe2ee1bb..678f6c429627 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -937,6 +937,13 @@ ALPS PS/2 TOUCHPAD DRIVER
 R:	Pali Rohár <pali@kernel.org>
 F:	drivers/input/mouse/alps.*
 
+ALTERA STRATIX 10 SoC FPGA HWMON DRIVER
+M:	Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
+M:	Tze Yee Ng <tze.yee.ng@altera.com>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
+
 ALTERA MAILBOX DRIVER
 M:	Tien Sung Ang <tiensung.ang@altera.com>
 S:	Maintained
-- 
2.43.7


^ permalink raw reply related

* [PATCH 2/5] dt-bindings: firmware: svc: add hwmon property
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <cover.1781861409.git.tze.yee.ng@altera.com>

From: Tze Yee Ng <tze.yee.ng@altera.com>

Altera Stratix 10 SoCFPGA supports hardware monitor access through the
service layer mailbox. Add an optional hwmon child node to the service
layer binding so device trees can describe the hardware monitor.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 .../devicetree/bindings/firmware/intel,stratix10-svc.yaml     | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
index b42cfa78b28b..86ffdb10132f 100644
--- a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
+++ b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.yaml
@@ -62,6 +62,10 @@ properties:
     $ref: /schemas/fpga/intel,stratix10-soc-fpga-mgr.yaml
     description: Optional child node for fpga manager to perform fabric configuration.
 
+  hwmon:
+    $ref: /schemas/hwmon/altr,stratix10-hwmon.yaml
+    description: Optional child node for Stratix 10 hardware monitor.
+
 required:
   - compatible
   - method
-- 
2.43.7


^ permalink raw reply related

* [PATCH 3/5] firmware: stratix10-svc: add async HWMON read commands
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <cover.1781861409.git.tze.yee.ng@altera.com>

From: Tze Yee Ng <tze.yee.ng@altera.com>

Add asynchronous Stratix 10 service layer support for hardware monitor
temperature and voltage read commands in stratix10_svc_async_send() and
stratix10_svc_async_prepare_response().

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/firmware/stratix10-svc.c             | 12 +++++++
 include/linux/firmware/intel/stratix10-smc.h | 38 ++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index e9e35d67ef96..2cfdac31402c 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -1311,6 +1311,14 @@ int stratix10_svc_async_send(struct stratix10_svc_chan *chan, void *msg,
 		args.a0 = INTEL_SIP_SMC_ASYNC_RSU_NOTIFY;
 		args.a2 = p_msg->arg[0];
 		break;
+	case COMMAND_HWMON_READTEMP:
+		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READTEMP;
+		args.a2 = p_msg->arg[0];
+		break;
+	case COMMAND_HWMON_READVOLT:
+		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READVOLT;
+		args.a2 = p_msg->arg[0];
+		break;
 	default:
 		dev_err(ctrl->dev, "Invalid command ,%d\n", p_msg->command);
 		ret = -EINVAL;
@@ -1404,6 +1412,10 @@ static int stratix10_svc_async_prepare_response(struct stratix10_svc_chan *chan,
 		 */
 		data->kaddr1 = (void *)&handle->res;
 		break;
+	case COMMAND_HWMON_READTEMP:
+	case COMMAND_HWMON_READVOLT:
+		data->kaddr1 = (void *)&handle->res.a2;
+		break;
 
 	default:
 		dev_alert(ctrl->dev, "Invalid command\n ,%d", p_msg->command);
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index 935dba3633b5..4eb3a6e9659d 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -680,6 +680,44 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
 #define INTEL_SIP_SMC_ASYNC_POLL \
 	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_POLL)
 
+/**
+ * Request INTEL_SIP_SMC_ASYNC_HWMON_READTEMP
+ * Async call to request temperature
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_HWMON_READTEMP
+ * a1 transaction job id
+ * a2 Temperature Channel
+ * a3-a17 not used
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_REJECTED
+ * or INTEL_SIP_SMC_STATUS_BUSY
+ * a1-a17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READTEMP	0xE8
+#define INTEL_SIP_SMC_ASYNC_HWMON_READTEMP \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READTEMP)
+
+/**
+ * Request INTEL_SIP_SMC_ASYNC_HWMON_READVOLT
+ * Async call to request voltage
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_ASYNC_HWMON_READVOLT
+ * a1 transaction job id
+ * a2 Voltage Channel
+ * a3-a17 not used
+ *
+ * Return status
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_REJECTED
+ * or INTEL_SIP_SMC_STATUS_BUSY
+ * a1-a17 not used
+ */
+#define INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READVOLT	0xE9
+#define INTEL_SIP_SMC_ASYNC_HWMON_READVOLT \
+	INTEL_SIP_SMC_ASYNC_VAL(INTEL_SIP_SMC_ASYNC_FUNC_ID_HWMON_READVOLT)
+
 /**
  * Request INTEL_SIP_SMC_ASYNC_RSU_GET_SPT
  * Async call to get RSU SPT from SDM.
-- 
2.43.7


^ permalink raw reply related

* [PATCH 4/5] hwmon: add Stratix 10 SoC FPGA hardware monitor driver
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <cover.1781861409.git.tze.yee.ng@altera.com>

From: Tze Yee Ng <tze.yee.ng@altera.com>

Add a hardware monitoring driver for Altera Stratix 10 SoC FPGA devices
that reads temperature and voltage sensors through the Stratix 10 service
layer. Use the asynchronous service layer interface when available, with
a synchronous fallback.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 Documentation/hwmon/index.rst           |   1 +
 Documentation/hwmon/stratix10-hwmon.rst |  31 ++
 MAINTAINERS                             |   2 +
 drivers/hwmon/Kconfig                   |  10 +
 drivers/hwmon/Makefile                  |   1 +
 drivers/hwmon/stratix10-hwmon.c         | 575 ++++++++++++++++++++++++
 6 files changed, 620 insertions(+)
 create mode 100644 Documentation/hwmon/stratix10-hwmon.rst
 create mode 100644 drivers/hwmon/stratix10-hwmon.c

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 8b655e5d6b68..30f533301903 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -244,6 +244,7 @@ Hardware Monitoring Kernel Drivers
    sparx5-temp
    spd5118
    stpddc60
+   stratix10-hwmon
    surface_fan
    sy7636a-hwmon
    tc654
diff --git a/Documentation/hwmon/stratix10-hwmon.rst b/Documentation/hwmon/stratix10-hwmon.rst
new file mode 100644
index 000000000000..61b682fe177a
--- /dev/null
+++ b/Documentation/hwmon/stratix10-hwmon.rst
@@ -0,0 +1,31 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver stratix10-hwmon
+=============================
+
+Supported chips:
+
+ * Altera Stratix 10 SoC FPGA
+
+Authors:
+      - Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
+      - Tze Yee Ng <tze.yee.ng@altera.com>
+
+Description
+-----------
+
+This driver supports hardware monitoring for Altera Stratix 10 SoC FPGA
+devices through the Secure Device Manager and Stratix 10 service layer.
+
+The following sensor types are supported:
+
+  * temperature
+  * voltage
+
+Usage Notes
+-----------
+
+The driver relies on a device tree node to enumerate sensors present on the
+specific device. See
+Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml for details
+of the device-tree node.
diff --git a/MAINTAINERS b/MAINTAINERS
index 678f6c429627..5afdf286f8f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -943,6 +943,8 @@ M:	Tze Yee Ng <tze.yee.ng@altera.com>
 L:	linux-hwmon@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
+F:	Documentation/hwmon/stratix10-hwmon.rst
+F:	drivers/hwmon/stratix10-hwmon.c
 
 ALTERA MAILBOX DRIVER
 M:	Tien Sung Ang <tiensung.ang@altera.com>
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 14e4cea48acc..8eff1c71a226 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2112,6 +2112,16 @@ config SENSORS_SMSC47M192
 	  This driver can also be built as a module. If so, the module
 	  will be called smsc47m192.
 
+config SENSORS_ALTERA_SOCFPGA_STRATIX10
+	tristate "Altera SoC FPGA Stratix 10 hardware monitoring features"
+	depends on INTEL_STRATIX10_SERVICE
+	help
+	  If you say yes here you get support for the temperature and
+	  voltage sensors of Altera SoC FPGA Stratix 10 devices.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called stratix10-hwmon.
+
 config SENSORS_SMSC47B397
 	tristate "SMSC LPC47B397-NC"
 	depends on HAS_IOPORT
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 982ee2c6f9de..7e643de0e7d4 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -217,6 +217,7 @@ obj-$(CONFIG_SENSORS_SMPRO)	+= smpro-hwmon.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
 obj-$(CONFIG_SENSORS_SMSC47M1)	+= smsc47m1.o
 obj-$(CONFIG_SENSORS_SMSC47M192)+= smsc47m192.o
+obj-$(CONFIG_SENSORS_ALTERA_SOCFPGA_STRATIX10)	+= stratix10-hwmon.o
 obj-$(CONFIG_SENSORS_SPARX5)	+= sparx5-temp.o
 obj-$(CONFIG_SENSORS_SPD5118)	+= spd5118.o
 obj-$(CONFIG_SENSORS_STTS751)	+= stts751.o
diff --git a/drivers/hwmon/stratix10-hwmon.c b/drivers/hwmon/stratix10-hwmon.c
new file mode 100644
index 000000000000..7ed1116e57b8
--- /dev/null
+++ b/drivers/hwmon/stratix10-hwmon.c
@@ -0,0 +1,575 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Altera Stratix 10 SoC FPGA hardware monitoring driver
+ *
+ * Copyright (c) 2026 Altera Corporation
+ *
+ * Authors:
+ *	Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
+ *	Tze Yee Ng <tze.yee.ng@altera.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/firmware/intel/stratix10-svc-client.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define HWMON_TIMEOUT			msecs_to_jiffies(SVC_HWMON_REQUEST_TIMEOUT_MS)
+#define HWMON_RETRY_SLEEP_MS		1U
+#define HWMON_ASYNC_MSG_RETRY		3U
+#define STRATIX10_HWMON_MAXSENSORS	16
+#define STRATIX10_HWMON_TEMPERATURE	"temperature"
+#define STRATIX10_HWMON_VOLTAGE		"voltage"
+#define STRATIX10_HWMON_CHANNEL_MASK	GENMASK(15, 0)
+#define STRATIX10_HWMON_PAGE_SHIFT	16
+#define STRATIX10_HWMON_ATTR_VISIBLE	0444
+/* Temperature from SDM is signed Q8.8 millidegrees Celsius (8 fractional bits). */
+#define STRATIX10_HWMON_TEMP_FRAC_BITS	8
+#define STRATIX10_HWMON_TEMP_FRAC_DIV	BIT(STRATIX10_HWMON_TEMP_FRAC_BITS)
+/* Voltage from SDM is unsigned Q16 (millivolts, 16 fractional bits). */
+#define STRATIX10_HWMON_VOLT_FRAC_BITS	16
+#define STRATIX10_HWMON_VOLT_FRAC_DIV	BIT(STRATIX10_HWMON_VOLT_FRAC_BITS)
+
+#define ETEMP_INACTIVE			0x80000000U
+#define ETEMP_TOO_OLD			0x80000001U
+#define ETEMP_NOT_PRESENT		0x80000002U
+#define ETEMP_TIMEOUT			0x80000003U
+#define ETEMP_CORRUPT			0x80000004U
+#define ETEMP_BUSY			0x80000005U
+#define ETEMP_NOT_INITIALIZED		0x800000FFU
+
+struct stratix10_hwmon_priv {
+	struct stratix10_svc_chan *chan;
+	struct stratix10_svc_client client;
+	struct completion completion;
+	struct mutex lock;	/* protect SVC calls */
+	bool async;
+	u32 temperature;
+	u32 voltage;
+	int temperature_channels;
+	int voltage_channels;
+	const char *temp_chan_names[STRATIX10_HWMON_MAXSENSORS];
+	const char *volt_chan_names[STRATIX10_HWMON_MAXSENSORS];
+	u32 temp_chan[STRATIX10_HWMON_MAXSENSORS];
+	u32 volt_chan[STRATIX10_HWMON_MAXSENSORS];
+};
+
+static umode_t stratix10_hwmon_is_visible(const void *dev,
+					  enum hwmon_sensor_types type,
+					 u32 attr, int chan)
+{
+	const struct stratix10_hwmon_priv *priv = dev;
+
+	switch (type) {
+	case hwmon_temp:
+		if (chan < priv->temperature_channels)
+			return STRATIX10_HWMON_ATTR_VISIBLE;
+		return 0;
+	case hwmon_in:
+		if (chan < priv->voltage_channels)
+			return STRATIX10_HWMON_ATTR_VISIBLE;
+		return 0;
+	default:
+		return 0;
+	}
+}
+
+static void stratix10_hwmon_readtemp_cb(struct stratix10_svc_client *client,
+					struct stratix10_svc_cb_data *data)
+{
+	struct stratix10_hwmon_priv *priv = client->priv;
+
+	if (data->status == BIT(SVC_STATUS_OK)) {
+		priv->temperature = (u32)*(unsigned long *)data->kaddr1;
+	} else if (data->kaddr1) {
+		dev_err(client->dev, "%s failed with status 0x%x, value 0x%lx\n",
+			__func__, data->status,
+			*(unsigned long *)data->kaddr1);
+	} else {
+		dev_err(client->dev, "%s failed with status 0x%x\n",
+			__func__, data->status);
+	}
+
+	complete(&priv->completion);
+}
+
+static void stratix10_hwmon_readvolt_cb(struct stratix10_svc_client *client,
+					struct stratix10_svc_cb_data *data)
+{
+	struct stratix10_hwmon_priv *priv = client->priv;
+
+	if (data->status == BIT(SVC_STATUS_OK)) {
+		priv->voltage = (u32)*(unsigned long *)data->kaddr1;
+	} else if (data->kaddr1) {
+		dev_err(client->dev, "%s failed with status 0x%x, value 0x%lx\n",
+			__func__, data->status,
+			*(unsigned long *)data->kaddr1);
+	} else {
+		dev_err(client->dev, "%s failed with status 0x%x\n",
+			__func__, data->status);
+	}
+
+	complete(&priv->completion);
+}
+
+static void stratix10_hwmon_async_callback(void *ptr)
+{
+	if (ptr)
+		complete(ptr);
+}
+
+static int stratix10_hwmon_parse_temp(long *val, u32 temperature)
+{
+	switch (temperature) {
+	case ETEMP_INACTIVE:
+	case ETEMP_NOT_PRESENT:
+	case ETEMP_CORRUPT:
+	case ETEMP_NOT_INITIALIZED:
+		return -EOPNOTSUPP;
+	case ETEMP_TIMEOUT:
+	case ETEMP_BUSY:
+	case ETEMP_TOO_OLD:
+		return -EAGAIN;
+	default:
+		/* Convert Q8.8 millidegrees Celsius to millidegrees for hwmon. */
+		*val = (long)(s32)temperature / STRATIX10_HWMON_TEMP_FRAC_DIV;
+		return 0;
+	}
+}
+
+static int stratix10_hwmon_encode_temp_arg(u32 reg, u64 *arg)
+{
+	u32 page = (reg >> STRATIX10_HWMON_PAGE_SHIFT) & STRATIX10_HWMON_CHANNEL_MASK;
+	u32 channel = reg & STRATIX10_HWMON_CHANNEL_MASK;
+
+	if (channel >= STRATIX10_HWMON_MAXSENSORS)
+		return -EINVAL;
+
+	*arg = (1ULL << channel) | ((u64)page << STRATIX10_HWMON_PAGE_SHIFT);
+	return 0;
+}
+
+static int stratix10_hwmon_encode_volt_arg(u32 reg, u64 *arg)
+{
+	u32 channel = reg & STRATIX10_HWMON_CHANNEL_MASK;
+
+	if (channel >= STRATIX10_HWMON_MAXSENSORS)
+		return -EINVAL;
+
+	*arg = 1ULL << channel;
+	return 0;
+}
+
+static int stratix10_hwmon_async_read(struct device *dev,
+				      enum hwmon_sensor_types type,
+				     struct stratix10_svc_client_msg *msg)
+{
+	struct stratix10_hwmon_priv *priv = dev_get_drvdata(dev);
+	struct stratix10_svc_cb_data data = {};
+	struct completion completion;
+	unsigned long wait_ret;
+	void *handle = NULL;
+	int status, index, ret;
+
+	init_completion(&completion);
+
+	for (index = 0; index < HWMON_ASYNC_MSG_RETRY; index++) {
+		status = stratix10_svc_async_send(priv->chan, msg, &handle,
+						  stratix10_hwmon_async_callback,
+						  &completion);
+		if (status == 0)
+			break;
+		dev_warn(dev, "Failed to send async message\n");
+		msleep(HWMON_RETRY_SLEEP_MS);
+	}
+
+	if (status && !handle)
+		return status;
+
+	wait_ret = wait_for_completion_io_timeout(&completion, HWMON_TIMEOUT);
+	if (wait_ret > 0)
+		dev_dbg(dev, "Received async interrupt\n");
+	else if (wait_ret == 0)
+		dev_dbg(dev, "Timeout occurred, trying to poll the response\n");
+
+	ret = -ETIMEDOUT;
+	for (index = 0; index < HWMON_ASYNC_MSG_RETRY; index++) {
+		status = stratix10_svc_async_poll(priv->chan, handle, &data);
+		if (status == -EAGAIN) {
+			dev_dbg(dev, "Async message is still in progress\n");
+		} else if (status < 0) {
+			dev_alert(dev, "Failed to poll async message: %d\n", status);
+			ret = status;
+			break;
+		} else if (status == 0) {
+			ret = 0;
+			break;
+		}
+		msleep(HWMON_RETRY_SLEEP_MS);
+	}
+
+	if (ret) {
+		dev_err(dev, "Failed to get async response\n");
+		goto done;
+	}
+
+	if (data.status) {
+		dev_err(dev, "%s returned 0x%x from SDM\n", __func__,
+			data.status);
+		ret = -EFAULT;
+		goto done;
+	}
+
+	if (type == hwmon_temp)
+		priv->temperature = (u32)*(unsigned long *)data.kaddr1;
+	else
+		priv->voltage = (u32)*(unsigned long *)data.kaddr1;
+
+	ret = 0;
+
+done:
+	stratix10_svc_async_done(priv->chan, handle);
+	return ret;
+}
+
+static int stratix10_hwmon_sync_read(struct device *dev,
+				     enum hwmon_sensor_types type,
+				    struct stratix10_svc_client_msg *msg)
+{
+	struct stratix10_hwmon_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	reinit_completion(&priv->completion);
+
+	if (type == hwmon_temp)
+		priv->client.receive_cb = stratix10_hwmon_readtemp_cb;
+	else
+		priv->client.receive_cb = stratix10_hwmon_readvolt_cb;
+
+	ret = stratix10_svc_send(priv->chan, msg);
+	if (ret < 0)
+		goto status_done;
+
+	ret = wait_for_completion_interruptible_timeout(&priv->completion,
+							HWMON_TIMEOUT);
+	if (!ret) {
+		dev_err(priv->client.dev, "timeout waiting for SMC call\n");
+		ret = -ETIMEDOUT;
+		goto status_done;
+	}
+	if (ret < 0) {
+		dev_err(priv->client.dev, "error %d waiting for SMC call\n", ret);
+		goto status_done;
+	}
+
+	ret = 0;
+
+status_done:
+	stratix10_svc_done(priv->chan);
+	return ret;
+}
+
+static int stratix10_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+				u32 attr, int chan, long *val)
+{
+	struct stratix10_hwmon_priv *priv = dev_get_drvdata(dev);
+	struct stratix10_svc_client_msg msg = {0};
+	int ret;
+
+	if (chan >= STRATIX10_HWMON_MAXSENSORS)
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case hwmon_temp:
+		ret = stratix10_hwmon_encode_temp_arg(priv->temp_chan[chan],
+						      &msg.arg[0]);
+		if (ret)
+			return ret;
+		msg.command = COMMAND_HWMON_READTEMP;
+		break;
+	case hwmon_in:
+		ret = stratix10_hwmon_encode_volt_arg(priv->volt_chan[chan],
+						      &msg.arg[0]);
+		if (ret)
+			return ret;
+		msg.command = COMMAND_HWMON_READVOLT;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	guard(mutex)(&priv->lock);
+	if (priv->async)
+		ret = stratix10_hwmon_async_read(dev, type, &msg);
+	else
+		ret = stratix10_hwmon_sync_read(dev, type, &msg);
+	if (ret)
+		return ret;
+
+	if (type == hwmon_temp)
+		ret = stratix10_hwmon_parse_temp(val, priv->temperature);
+	else
+		/* Convert Q16 millivolts to millivolts for hwmon. */
+		*val = (long)priv->voltage / STRATIX10_HWMON_VOLT_FRAC_DIV;
+	return ret;
+}
+
+static int stratix10_hwmon_read_string(struct device *dev,
+				       enum hwmon_sensor_types type, u32 attr,
+				      int chan, const char **str)
+{
+	struct stratix10_hwmon_priv *priv = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_in:
+		*str = priv->volt_chan_names[chan];
+		return 0;
+	case hwmon_temp:
+		*str = priv->temp_chan_names[chan];
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static const struct hwmon_ops stratix10_hwmon_ops = {
+	.is_visible = stratix10_hwmon_is_visible,
+	.read = stratix10_hwmon_read,
+	.read_string = stratix10_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *stratix10_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
+	NULL
+};
+
+static const struct hwmon_chip_info stratix10_hwmon_chip_info = {
+	.ops = &stratix10_hwmon_ops,
+	.info = stratix10_hwmon_info,
+};
+
+static int stratix10_hwmon_add_channel(struct device *dev, const char *type,
+				       u32 val, const char *label,
+				      struct stratix10_hwmon_priv *priv)
+{
+	if (!strcmp(type, STRATIX10_HWMON_TEMPERATURE)) {
+		if (priv->temperature_channels >= STRATIX10_HWMON_MAXSENSORS) {
+			dev_warn(dev, "Can't add temp node %s, too many channels\n",
+				 label);
+			return 0;
+		}
+
+		priv->temp_chan_names[priv->temperature_channels] = label;
+		priv->temp_chan[priv->temperature_channels] = val;
+		priv->temperature_channels++;
+		return 0;
+	}
+
+	if (!strcmp(type, STRATIX10_HWMON_VOLTAGE)) {
+		if (priv->voltage_channels >= STRATIX10_HWMON_MAXSENSORS) {
+			dev_warn(dev, "Can't add voltage node %s, too many channels\n",
+				 label);
+			return 0;
+		}
+
+		priv->volt_chan_names[priv->voltage_channels] = label;
+		priv->volt_chan[priv->voltage_channels] = val;
+		priv->voltage_channels++;
+		return 0;
+	}
+
+	dev_warn(dev, "unsupported sensor type %s\n", type);
+	return 0;
+}
+
+static int stratix10_hwmon_probe_child_from_dt(struct device *dev,
+					       struct device_node *child,
+					      struct stratix10_hwmon_priv *priv)
+{
+	struct device_node *grandchild;
+	const char *label;
+	u32 val;
+	int ret;
+
+	for_each_child_of_node(child, grandchild) {
+		ret = of_property_read_u32(grandchild, "reg", &val);
+		if (ret) {
+			dev_err(dev, "missing reg property of %pOFn\n",
+				grandchild);
+			of_node_put(grandchild);
+			return ret;
+		}
+
+		ret = of_property_read_string(grandchild, "label", &label);
+		if (ret)
+			label = grandchild->name;
+
+		stratix10_hwmon_add_channel(dev, child->name, val, label, priv);
+	}
+
+	return 0;
+}
+
+static int stratix10_hwmon_probe_from_dt(struct device *dev,
+					 struct stratix10_hwmon_priv *priv)
+{
+	struct device_node *child;
+	int ret;
+
+	if (!dev->of_node)
+		return 0;
+
+	for_each_child_of_node(dev->of_node, child) {
+		ret = stratix10_hwmon_probe_child_from_dt(dev, child, priv);
+		if (ret) {
+			of_node_put(child);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int stratix10_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct stratix10_hwmon_priv *priv;
+	struct device *hwmon_dev;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->client.dev = dev;
+	priv->client.priv = priv;
+	init_completion(&priv->completion);
+	mutex_init(&priv->lock);
+
+	ret = stratix10_hwmon_probe_from_dt(dev, priv);
+	if (ret) {
+		dev_err(dev, "Unable to probe from device tree\n");
+		return ret;
+	}
+
+	if (!priv->temperature_channels && !priv->voltage_channels) {
+		dev_err(dev, "no temperature or voltage channels in device tree\n");
+		return -ENODEV;
+	}
+
+	priv->chan = stratix10_svc_request_channel_byname(&priv->client,
+							  SVC_CLIENT_HWMON);
+	if (IS_ERR(priv->chan)) {
+		ret = PTR_ERR(priv->chan);
+		if (ret == -EPROBE_DEFER)
+			dev_dbg(dev, "service channel %s not ready, deferring probe\n",
+				SVC_CLIENT_HWMON);
+		else
+			dev_err(dev, "couldn't get service channel %s: %d\n",
+				SVC_CLIENT_HWMON, ret);
+		return ret;
+	}
+
+	ret = stratix10_svc_add_async_client(priv->chan, false);
+	switch (ret) {
+	case 0:
+		priv->async = true;
+		break;
+	case -EINVAL:
+		dev_dbg(dev, "async operations not supported, using sync mode\n");
+		priv->async = false;
+		break;
+	default:
+		dev_err(dev, "failed to add async client: %d\n", ret);
+		stratix10_svc_free_channel(priv->chan);
+		return ret;
+	}
+
+	dev_info(dev, "Initialized %d temperature and %d voltage channels\n",
+		 priv->temperature_channels, priv->voltage_channels);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "stratix10_hwmon",
+							 priv,
+							 &stratix10_hwmon_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev)) {
+		if (priv->async)
+			stratix10_svc_remove_async_client(priv->chan);
+		stratix10_svc_free_channel(priv->chan);
+		return PTR_ERR(hwmon_dev);
+	}
+
+	platform_set_drvdata(pdev, priv);
+	return 0;
+}
+
+static void stratix10_hwmon_remove(struct platform_device *pdev)
+{
+	struct stratix10_hwmon_priv *priv = platform_get_drvdata(pdev);
+
+	if (priv->async)
+		stratix10_svc_remove_async_client(priv->chan);
+	stratix10_svc_free_channel(priv->chan);
+}
+
+static const struct of_device_id stratix10_hwmon_of_match[] = {
+	{ .compatible = "altr,stratix10-hwmon" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, stratix10_hwmon_of_match);
+
+static struct platform_driver stratix10_hwmon_driver = {
+	.driver = {
+		.name = "stratix10-hwmon",
+		.of_match_table = stratix10_hwmon_of_match,
+	},
+	.probe = stratix10_hwmon_probe,
+	.remove = stratix10_hwmon_remove,
+};
+module_platform_driver(stratix10_hwmon_driver);
+
+MODULE_AUTHOR("Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>");
+MODULE_AUTHOR("Tze Yee Ng <tze.yee.ng@altera.com>");
+MODULE_DESCRIPTION("Altera Stratix 10 SoC FPGA hardware monitoring driver");
+MODULE_LICENSE("GPL");
-- 
2.43.7


^ permalink raw reply related

* [PATCH 5/5] arm64: dts: socfpga: stratix10: add hwmon node
From: tze.yee.ng @ 2026-06-19  9:38 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-hwmon, devicetree, linux-kernel, Dinh Nguyen, Mahesh Rao,
	Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <cover.1781861409.git.tze.yee.ng@altera.com>

From: Tze Yee Ng <tze.yee.ng@altera.com>

Add an hwmon child node under the Stratix 10 service layer and describe
the SoCDK voltage and temperature sensors using the altr,stratix10-hwmon
compatible.

Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 .../boot/dts/altera/socfpga_stratix10.dtsi    |  5 +++
 .../dts/altera/socfpga_stratix10_socdk.dts    | 33 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index 0d9cad0c0351..afb11e6f6813 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -78,6 +78,11 @@ svc {
 			fpga_mgr: fpga-mgr {
 				compatible = "intel,stratix10-soc-fpga-mgr";
 			};
+
+			temp_volt: hwmon {
+				compatible = "altr,stratix10-hwmon";
+				status = "disabled";
+			};
 		};
 	};
 
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
index e2a1cea7f3da..01a8ffe430ed 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
@@ -134,3 +134,36 @@ root: partition@4200000 {
 		};
 	};
 };
+
+&temp_volt {
+	status = "okay";
+
+	voltage {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		input@2 {
+			label = "0.8V VCC";
+			reg = <2>;
+		};
+
+		input@3 {
+			label = "1.8V VCCIO_SDM";
+			reg = <3>;
+		};
+
+		input@6 {
+			label = "0.9V VCCERAM";
+			reg = <6>;
+		};
+	};
+
+	temperature {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		input@0 {
+			label = "Main Die SDM";
+			reg = <0x0>;
+		};
+	};
+};
-- 
2.43.7


^ permalink raw reply related

* Re: [PATCH 2/5] dt-bindings: firmware: svc: add hwmon property
From: sashiko-bot @ 2026-06-19  9:44 UTC (permalink / raw)
  To: tze.yee.ng; +Cc: robh, linux-hwmon, devicetree, conor+dt
In-Reply-To: <be798fdfb7ec76e1f7d04c1fd00126c88c8a2e31.1781861409.git.tze.yee.ng@altera.com>

> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> Altera Stratix 10 SoCFPGA supports hardware monitor access through the
> service layer mailbox. Add an optional hwmon child node to the service
> layer binding so device trees can describe the hardware monitor.
> 
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781861409.git.tze.yee.ng@altera.com?part=2


^ permalink raw reply

* Re: [PATCH 1/5] dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding
From: sashiko-bot @ 2026-06-19  9:46 UTC (permalink / raw)
  To: tze.yee.ng; +Cc: linux-hwmon, conor+dt, devicetree, robh
In-Reply-To: <ac5a118394e96f707823463fedd32b6a484c1ceb.1781861409.git.tze.yee.ng@altera.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The pattern property `^input(@[0-9a-f]+)?$` allows child nodes with a `reg` property to omit the unit-address, violating Devicetree specifications.
- [Medium] The `reg` property in the child input nodes is missing `maxItems: 1`, allowing multiple register entries.
- [Low] The MAINTAINERS file entries are not sorted alphabetically.
- [Low] The Signed-off-by chain does not match the patch authorship.
--

commit d11da1e6aa7b26667b6a632670fdfdcc9fd27dee
Author: Tze Yee Ng <tze.yee.ng@altera.com>

dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding

This commit documents the device tree binding for the Altera Stratix 10
SoC FPGA hardware monitor. It adds bindings for temperature and voltage
sensor nodes.

> Author: Tze Yee Ng <tze.yee.ng@altera.com>
> Date:   Fri Jun 19 09:38:52 2026 +0000
> 
>     dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding
>     
>     Document the device tree binding for the Altera Stratix 10 SoC FPGA
>     hardware monitor, including temperature and voltage sensor nodes.
>     
>     Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
>     Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>

[Severity: Low]
The commit author is Tze Yee Ng, but the first Signed-off-by tag belongs
to Nazim Amirul.

According to the patch submission guidelines, the primary author must be
the first person to sign off on the patch.

Should the authorship be corrected with a From: line in the commit message
body, or should the Signed-off-by order be fixed?

> diff --git a/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> new file mode 100644
> index 0000000000000..5bd98660ee7b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml

[ ... ]

> +  temperature:
> +    description:
> +      The temperature node specifies mappings of temperature sensor diodes on
> +      the Stratix 10 SoC FPGA main die and tile die.
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +    patternProperties:
> +      "^input(@[0-9a-f]+)?$":

[Severity: Medium]
The Devicetree specification requires that if a node has a reg property,
its name must include a unit-address matching the first address in reg.

The regex ^input(@[0-9a-f]+)?$ makes the unit-address optional, which
allows dt-validate to pass syntactically invalid device trees.

Could this be updated to require the unit-address when a reg property
is present?

> +        description:
> +          The input node specifies each individual temperature sensor.
> +        type: object
> +        properties:
> +          reg:
> +            description:

[Severity: Medium]
The reg property in the child input nodes is missing maxItems: 1.

Because dt-schema treats reg as an array of arrays, omitting this allows
a device tree to specify an arbitrary number of register pairs, which
would erroneously pass schema validation.

Should maxItems be added here to restrict it to a single entry?

[ ... ]

> +  voltage:
> +    description:
> +      The voltage node specifies mappings of voltage sensors on the Stratix 10
> +      SoC FPGA analog to digital converter of the Secure Device Manager (SDM).
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +    patternProperties:
> +      "^input(@[0-9a-f]+)?$":

[Severity: Medium]
Similar to the temperature node, this regex makes the unit-address optional.

Does this pattern allow child nodes with a reg property to omit the
unit-address, violating Devicetree specifications?

> +        description:
> +          The input node specifies each individual voltage sensor.
> +        type: object
> +        properties:
> +          reg:
> +            description:

[Severity: Medium]
The reg property here is also missing maxItems: 1.

Can we add a maximum item constraint to ensure multiple register entries
are not allowed for voltage sensors?

[ ... ]

> diff --git a/MAINTAINERS b/MAINTAINERS
> index bcd09e02337b8..5a07cefe3aeee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -938,6 +938,13 @@ ALPS PS/2 TOUCHPAD DRIVER
>  R:	Pali Rohár <pali@kernel.org>
>  F:	drivers/input/mouse/alps.*
>  
> +ALTERA STRATIX 10 SoC FPGA HWMON DRIVER
> +M:	Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> +M:	Tze Yee Ng <tze.yee.ng@altera.com>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> +
>  ALTERA MAILBOX DRIVER

[Severity: Low]
The MAINTAINERS file entries are typically sorted alphabetically to
maintain organization and avoid merge conflicts.

Here, ALTERA STRATIX 10 SoC FPGA HWMON DRIVER is placed before
ALTERA MAILBOX DRIVER.

Should this block be moved further down to maintain alphabetical order?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781861409.git.tze.yee.ng@altera.com?part=1

^ permalink raw reply

* Re: [PATCH 5/5] arm64: dts: socfpga: stratix10: add hwmon node
From: sashiko-bot @ 2026-06-19  9:49 UTC (permalink / raw)
  To: tze.yee.ng; +Cc: devicetree, linux-hwmon, conor+dt, robh
In-Reply-To: <d62c4188cd936883bc14c95dcd27555ec94df7cf.1781861409.git.tze.yee.ng@altera.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware monitoring sysfs attributes remain active after the service channel is freed during driver removal, leading to a kernel panic.
- [High] Incorrect temperature scaling calculation reports temperatures 1000 times too small to the hwmon subsystem.
- [High] Asynchronous sysfs reads are effectively broken and will always block for the full 2-second timeout before falling back to manual polling.
--

commit a3baf92605bf6c042e451ccdb74f299304cebf79
Author: Tze Yee Ng <tze.yee.ng@altera.com>

arm64: dts: socfpga: stratix10: add hwmon node
    
Add an hwmon child node under the Stratix 10 service layer and describe
the SoCDK voltage and temperature sensors using the altr,stratix10-hwmon
compatible.

> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> index e2a1cea7f3da9..01a8ffe430ed0 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dts
[ ... ]
> @@ -134,3 +134,36 @@ root: partition@4200000 {
>  		};
>  	};
>  };
> +
> +&temp_volt {
> +	status = "okay";

[Severity: High]
This isn't a bug introduced by this patch, but enabling this hwmon node exposes
a pre-existing lifecycle issue during driver removal that can lead to a kernel
panic.

The stratix10-hwmon driver uses devm_hwmon_device_register_with_info() for
registration, meaning the sysfs attributes are only unregistered during devres
cleanup after stratix10_hwmon_remove() returns. However, the remove function
synchronously frees the service channel (stratix10_svc_free_channel), which
sets chan->scl to NULL.

If a userspace process reads a sysfs attribute concurrently, the service thread
might unconditionally dereference chan->scl in svc_thread_recv_status_ok():

drivers/firmware/stratix10-svc.c:svc_thread_recv_status_ok() {
    ...
    p_data->chan->scl->receive_cb(p_data->chan->scl, cb_data);
    ...
}

Should the driver manage the service channel lifecycle via
devm_add_action_or_reset() so the channel strictly outlives the sysfs
attributes?

[Severity: High]
This is a pre-existing issue, but enabling this hwmon node exposes an incorrect
temperature scaling calculation.

The hwmon subsystem strictly requires temperature attributes to be reported in
millidegrees Celsius. The hardware reports the temperature in Q8.8 format.

In stratix10_hwmon_parse_temp(), the driver shifts the value right by 8 bits,
effectively returning raw degrees Celsius:

drivers/hwmon/stratix10-hwmon.c:stratix10_hwmon_parse_temp() {
    ...
    *val = (long)(s32)temperature / STRATIX10_HWMON_TEMP_FRAC_DIV;
    ...
}

Because it fails to multiply the result by 1000, a temperature of 50 degrees
Celsius is reported as 50 instead of 50000. Monitoring tools will mistakenly
display this as 0.050 degrees Celsius.

Could the conversion logic be updated to multiply the result by 1000 to
satisfy the hwmon core requirements?

[Severity: High]
This is a pre-existing issue, but enabling this hwmon node exposes a broken
asynchronous read path that induces a 2-second latency spike per read.

The stratix10_hwmon_async_read() function waits on a completion:

drivers/hwmon/stratix10-hwmon.c:stratix10_hwmon_async_read() {
    ...
    wait_ret = wait_for_completion_io_timeout(&completion, HWMON_TIMEOUT);
    ...
}

It expects the callback stored in handle->cb by stratix10_svc_async_send()
to be executed. However, the stratix10-svc driver never actually invokes this
callback anywhere in the service layer.

As a result, the completion is never signaled. Every sysfs read incurs a
mandatory 2-second timeout delay before it falls back to manual polling via
stratix10_svc_async_poll().

Is it possible to either implement the callback invocation in the service layer,
or adjust the async read approach to avoid this latency?

> +
> +	voltage {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		input@2 {
> +			label = "0.8V VCC";
> +			reg = <2>;
> +		};
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781861409.git.tze.yee.ng@altera.com?part=5

^ permalink raw reply

* [PATCH] arm64: dts: qcom: sm8250: Add memory-region for audio PD
From: Vinayak Katoch @ 2026-06-19  9:50 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Bharath Kumar, Chenna Kesava Raju, Ekansh Gupta, linux-arm-msm,
	devicetree, linux-kernel, Vinayak Katoch

Reserve memory region for audio PD dynamic loading and remote heap
requirements. Add the required VMID list for memory ownership
transfers.

Signed-off-by: Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 7076720413ab..3f2f1b644db9 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -10,6 +10,7 @@
 #include <dt-bindings/clock/qcom,gpucc-sm8250.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/dma/qcom-gpi.h>
+#include <dt-bindings/firmware/qcom,scm.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interconnect/qcom,osm-l3.h>
 #include <dt-bindings/interconnect/qcom,sm8250.h>
@@ -863,6 +864,14 @@ cdsp_secure_heap: memory@8bf00000 {
 			reg = <0x0 0x8bf00000 0x0 0x4600000>;
 			no-map;
 		};
+
+		adsp_rpc_remote_heap_mem: adsp-rpc-remote-heap {
+			compatible = "shared-dma-pool";
+			alloc-ranges = <0x0 0x80000000 0x0 0x80000000>;
+			reusable;
+			alignment = <0x0 0x400000>;
+			size = <0x0 0x800000>;
+		};
 	};
 
 	smem {
@@ -6077,6 +6086,9 @@ fastrpc {
 					compatible = "qcom,fastrpc";
 					qcom,glink-channels = "fastrpcglink-apps-dsp";
 					label = "adsp";
+					memory-region = <&adsp_rpc_remote_heap_mem>;
+					qcom,vmids = <QCOM_SCM_VMID_LPASS
+						      QCOM_SCM_VMID_ADSP_HEAP>;
 					qcom,non-secure-domain;
 					#address-cells = <1>;
 					#size-cells = <0>;

---
base-commit: 4fa3f5fabb30bf00d7475d5a33459ea83d639bf9
change-id: 20260617-sm8250-audio-380d342c48cb

Best regards,
-- 
Vinayak Katoch <vinayak.katoch@oss.qualcomm.com>


^ permalink raw reply related

* Re: [PATCH 3/5] firmware: stratix10-svc: add async HWMON read commands
From: sashiko-bot @ 2026-06-19  9:51 UTC (permalink / raw)
  To: tze.yee.ng; +Cc: devicetree, conor+dt, robh, linux-hwmon
In-Reply-To: <69a0f1b93e1dc5b5b8993f8649704c6be60dc1a3.1781861409.git.tze.yee.ng@altera.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cleanup of async client on driver removal causes memory leak and breaks subsequent probes.
- [High] Abandoning async transactions via `stratix10_svc_async_done()` without notifying EL3 firmware causes transaction ID desynchronization.
--

commit 6e40e51555b4f357bf75d356699813e67712ecca
Author: Tze Yee Ng <tze.yee.ng@altera.com>

firmware: stratix10-svc: add async HWMON read commands

This commit adds asynchronous Stratix 10 service layer support for hardware
monitor temperature and voltage read commands. It updates the service
layer functions stratix10_svc_async_send() and
stratix10_svc_async_prepare_response() to process the new messages.

> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
> index 39eb78f5905b3..ca2e354fc20fe 100644
> --- a/drivers/firmware/stratix10-svc.c
> +++ b/drivers/firmware/stratix10-svc.c
> @@ -1317,6 +1317,14 @@ int stratix10_svc_async_send(struct stratix10_svc_chan *chan, void *msg,
>  		args.a0 = INTEL_SIP_SMC_ASYNC_RSU_NOTIFY;
>  		args.a2 = p_msg->arg[0];
>  		break;
> +	case COMMAND_HWMON_READTEMP:
> +		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READTEMP;
> +		args.a2 = p_msg->arg[0];
> +		break;
> +	case COMMAND_HWMON_READVOLT:
> +		args.a0 = INTEL_SIP_SMC_ASYNC_HWMON_READVOLT;
> +		args.a2 = p_msg->arg[0];
> +		break;
>  	default:

[Severity: High]
This is a pre-existing issue, but does abandoning asynchronous transactions
via stratix10_svc_async_done() without notifying the EL3 firmware cause a
transaction ID desynchronization?

When an async transaction times out in stratix10_hwmon_async_read() or
rsu_send_async_msg(), the driver abandons the transaction and frees the ID:

drivers/firmware/stratix10-svc.c:stratix10_svc_async_done() {
    ...
	ida_free(&achan->job_id_pool,
		 STRATIX10_GET_JOBID(handle->transaction_id));
	kfree(handle);
}

Since there is no SMC cancellation command sent to the firmware, the firmware
might still process the timed-out request. If a new command allocates the
same recycled transaction ID, polling the firmware could return the delayed
completion of the original stale transaction.

Will this result in desynchronization between the driver and secure firmware,
potentially returning stale hwmon sensor readings or incorrect statuses?


[Severity: High]
This is a pre-existing issue, but does the missing cleanup of the async
client on driver removal cause a memory leak and break subsequent probes?

Looking at stratix10_rsu_remove() in drivers/firmware/stratix10-rsu.c:

drivers/firmware/stratix10-rsu.c:stratix10_rsu_remove() {
	struct stratix10_rsu_priv *priv = platform_get_drvdata(pdev);

	stratix10_svc_free_channel(priv->chan);
}

It calls stratix10_svc_free_channel() but completely omits calling
stratix10_svc_remove_async_client(). This leaves chan->async_chan
populated on the shared channel array and leaks the asynchronous channel
object and its associated IDA pool.

If the module is unloaded and reloaded, will the next call to
stratix10_svc_add_async_client() inside stratix10_rsu_probe() encounter
a non-NULL async_chan and immediately return -EALREADY?

While the new hwmon driver introduced in this patchset appears to handle
this correctly, it seems the rsu driver remains permanently unable to
probe after a single removal until the system is rebooted.

>  		dev_err(ctrl->dev, "Invalid command ,%d\n", p_msg->command);
>  		ret = -EINVAL;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781861409.git.tze.yee.ng@altera.com?part=3

^ permalink raw reply

* Re: [PATCH v2 3/6] arm64: dts: qcom: kodiak: Add GEM_NOC interconnect for adreno SMMU
From: Bibek Kumar Patro @ 2026-06-19  9:51 UTC (permalink / raw)
  To: Konrad Dybcio, Dmitry Baryshkov
  Cc: Will Deacon, Robin Murphy, Joerg Roedel, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm
In-Reply-To: <3384ecdf-599f-4862-a3c4-9f54b4ddfe63@oss.qualcomm.com>



On 6/18/2026 2:56 PM, Konrad Dybcio wrote:
> On 6/8/26 4:37 PM, Bibek Kumar Patro wrote:
>>
>>
>> On 6/8/2026 7:27 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 26, 2026 at 08:12:04PM +0530, Bibek Kumar Patro wrote:
>>>> On Kodiak platforms, the Adreno SMMU requires a bandwidth vote on
>>>> the GEM_NOC path (MASTER_GPU_TCU -> SLAVE_EBI1) before its registers
>>>> are accessible. Without this vote, the SMMU may become unreachable,
>>>> leading to intermittent probe failures and runtime issues.
>>>>
>>>> Add the required interconnect to ensure reliable register access.
>>>
>>> Does it only concern the GPU SMMU? What about the APPS SMMU? Should it
>>> be voting on other interconnects too? I guess so, because currently I
>>> see that TBUs vote for various interconnects. BTW: should apps_smmu also
>>> vote on the power domains?
>>>
>>
>> This race mainly occurs in GPU SMMU, where the GDSC can have an
> 
> Mainly or exclusively?
> 

It is exclusively in GPU SMMU as of now. We haven't seen any instance on 
other SMMU, e.g pcie AMMU, APPS SMMU.
Also as per my understanding of the architecture which i mentioned 
earlier [1], this issue will only arise in GPU SMMU.

Thanks & regards,
Bibek



> Konrad
> 
>> independent vote on the Adreno SMMU. However, the GEM_NOC vote may
>> already have been removed by the GPU (or any consumer of adreno_smmu,
>> e.g gmu), unless it is explicitly voted by the GPU SMMU (which acts as a
>> supplier for the GPU). This mismatch can lead to SHUB timeouts or NoC
>> errors.
>>
>> Mostly this race reported in suspend/resume cycle (when gpu/gmu devices moves to slumber/suspend state before adreno_smmu powers down
>> and the later doesn't have explicit interconnect voting).
>>
>> In the case of APPS SMMU, such a race is not expected for any known
>> use case. APPS SMMU is part of a shared infrastructure block, and its
>> power is typically kept enabled as long as attached master devices are
>> active. Therefore, explicit power-domain voting from APPS SMMU may not
>> be required.
>>

[1]

>> Thanks,
>> Bibek
>>
>>
>>>>
>>>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/kodiak.dtsi | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>> index fa540d8c2615dc02d941eb16bc7253204c2750bd..eefa4b836a81374ff437ab4bbcbc3fecc1590ab6 100644
>>>> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
>>>> @@ -3386,6 +3386,8 @@ adreno_smmu: iommu@3da0000 {
>>>>                  power-domains = <&gpucc GPU_CC_CX_GDSC>;
>>>>                dma-coherent;
>>>> +            interconnects = <&gem_noc MASTER_GPU_TCU QCOM_ICC_TAG_ALWAYS
>>>> +                     &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>>>>            };
>>>>              gfx_0_tbu: tbu@3dd9000 {
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>>
>>


^ permalink raw reply

* [PATCH v4 0/2] imx8mq-evk and ov5640: Add overlay-based camera support with shared reset handling
From: robby.cai @ 2026-06-19 10:05 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam,
	sebastian.krzyszkowiak, slongerbeam, sakari.ailus, mchehab,
	p.zabel, kieran.bingham
  Cc: kernel, devicetree, imx, linux-arm-kernel, linux-kernel

From: Robby Cai <robby.cai@nxp.com>

This series enables OV5640 camera support on the i.MX8MQ EVK using
device tree overlays, including single- and dual-camera configurations.

The DT overlays describe two OV5640 sensors connected to different
MIPI CSI-2 interfaces:

  - OV5640 (I2C2) -> MIPI CSI1 -> CSI1 bridge
  - OV5640 (I2C1) -> MIPI CSI2 -> CSI2 bridge

On this platform, both sensors share a common reset GPIO line, while
each has an independent powerdown (PWDN) GPIO. Due to this hardware
constraint, proper handling of the shared reset line is required when
both cameras are present.

To address this, the OV5640 driver is updated to use the reset controller
framework, allowing it to correctly support shared reset lines. Legacy
reset-gpios support is retained as a fallback when no reset controller
is defined, ensuring compatibility with existing device tree
descriptions without requiring changes.

Note:
1) With commit 8f040b5c5e3a ("leds: class: Use firmware nodes for device lookup"),
the OV5640 driver reports the following errors:

  [   11.373844] ov5640 0-003c: error -EINVAL: getting privacy LED
  [   11.376442] ov5640 0-003c: probe with driver ov5640 failed with error -22
  [   11.906977] ov5640 1-003c: error -EINVAL: getting privacy LED
  [   11.909793] ov5640 1-003c: probe with driver ov5640 failed with error -22

This issue has been reported and discussed in [1] and related threads.
As a temporary workaround for testing OV5640, the patch can be reverted.

Link [1]: https://lore.kernel.org/all/aignTNlK5kCLmQ2A@tom-desktop/

2) The patch at:
     https://lore.kernel.org/imx/20260619073115.3778313-1-robby.cai@oss.nxp.com/
   is also required for OV5640 to function properly on the i.MX8MQ EVK.




Changes in v4:
- Switch EVK camera support to DT overlays for CSI1/CSI2/dual configurations (Kieran Bingham)
- Convert OV5640 driver to use reset controller framework with GPIO fallback (sashiko)
- Ensure correct handling of reset line (sashiko)

Link to v3: https://lore.kernel.org/imx/20260529132334.3333294-1-robby.cai@nxp.com/

Changes in v3:
- Add OV5640 driver changes to use reset control framework for shared reset
- Drop GPIO hog for reset in DTS

Link to v2: https://lore.kernel.org/imx/20260515111143.2980956-1-robby.cai@nxp.com/

Changes in v2:
- Address comments on MIPI clock configuration (Frank, Sebastian):
  drop the first patch and consolidate the correct clock configuration
  into the second patch
- Address comments from sashiko:
  * Use MEDIA_BUS_TYPE_CSI2_DPHY instead of a literal value
  * Fix a probe-order dependency related to reset handling. Switch to
    software reset, as the shared hardware reset line prevents
    independent reset when both cameras are enabled due to a board
    design limitation
  * Fix incorrect voltage value in the reg_2v8 node

Link to v1: https://lore.kernel.org/imx/20260417110200.753678-1-robby.cai@nxp.com/


Signed-off-by: Robby Cai <robby.cai@nxp.com>

Robby Cai (2):
  arm64: dts: imx8mq-evk: Add OV5640 camera support via overlays
  media: i2c: ov5640: Add reset controller support with GPIO fallback

 arch/arm64/boot/dts/freescale/Makefile        |  7 ++
 .../dts/freescale/imx8mq-evk-ov5640-csi1.dtso | 69 ++++++++++++++++
 .../dts/freescale/imx8mq-evk-ov5640-csi2.dtso | 65 +++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 50 ++++++++++++
 drivers/media/i2c/ov5640.c                    | 80 ++++++++++++++++---
 5 files changed, 261 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso

-- 
2.50.1


^ permalink raw reply

* [PATCH v4 1/2] arm64: dts: imx8mq-evk: Add OV5640 camera support via overlays
From: robby.cai @ 2026-06-19 10:05 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam,
	sebastian.krzyszkowiak, slongerbeam, sakari.ailus, mchehab,
	p.zabel, kieran.bingham
  Cc: kernel, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20260619100532.3779934-1-robby.cai@oss.nxp.com>

From: Robby Cai <robby.cai@nxp.com>

Add overlays for single and dual camera setups on CSI1 and CSI2, enabling
the following media pipelines:

  - OV5640 (I2C2) -> MIPI CSI1 -> CSI1 bridge
  - OV5640 (I2C1) -> MIPI CSI2 -> CSI2 bridge

On the i.MX8MQ EVK, both sensors share a common reset GPIO, while each
sensor has an independent powerdown (PWDN) GPIO.

Both sensors also share the same MCLK source (CLKO2), configured
identically as required by the hardware design.

Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 arch/arm64/boot/dts/freescale/Makefile        |  7 ++
 .../dts/freescale/imx8mq-evk-ov5640-csi1.dtso | 69 +++++++++++++++++++
 .../dts/freescale/imx8mq-evk-ov5640-csi2.dtso | 65 +++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 50 ++++++++++++++
 4 files changed, 191 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 8ddaab127ab9..8507cbdb5556 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -501,6 +501,13 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
 imx8mq-evk-pcie1-ep-dtbs += imx8mq-evk.dtb imx-pcie1-ep.dtbo
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk-pcie1-ep.dtb
 
+imx8mq-evk-ov5640-csi1-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi1.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-csi1.dtb
+imx8mq-evk-ov5640-csi2-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi2.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-csi2.dtb
+imx8mq-evk-ov5640-dual-dtbs := imx8mq-evk.dtb imx8mq-evk-ov5640-csi1.dtbo imx8mq-evk-ov5640-csi2.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8mq-evk-ov5640-dual.dtb
+
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-kontron-pitx-imx8m.dtb
 dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
new file mode 100644
index 000000000000..1e9931802cdc
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi1.dtso
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8mq-clock.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&csi1 {
+	status = "okay";
+};
+
+&i2c2 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+
+	camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_camera1_pwdn>;
+		clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		clock-names = "xclk";
+		assigned-clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		assigned-clock-parents = <&clk IMX8MQ_SYS2_PLL_200M>;
+		assigned-clock-rates = <20000000>;
+		powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+		DOVDD-supply = <&sw4_reg>;
+		AVDD-supply = <&reg_2v8>;
+		DVDD-supply = <&reg_1v5>;
+
+		port {
+			camera1_ep: endpoint {
+				remote-endpoint = <&mipi_csi1_in_ep>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
+&mipi_csi1 {
+	assigned-clock-rates = <266000000>, <200000000>, <66000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			mipi_csi1_in_ep: endpoint {
+				remote-endpoint = <&camera1_ep>;
+				data-lanes = <1 2>;
+				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso
new file mode 100644
index 000000000000..fd247b3b5982
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk-ov5640-csi2.dtso
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8mq-clock.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+
+&csi2 {
+	status = "okay";
+};
+
+&i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_camera2_pwdn>;
+		clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		clock-names = "xclk";
+		assigned-clocks = <&clk IMX8MQ_CLK_CLKO2>;
+		assigned-clock-parents = <&clk IMX8MQ_SYS2_PLL_200M>;
+		assigned-clock-rates = <20000000>;
+		powerdown-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+		DOVDD-supply = <&sw4_reg>;
+		AVDD-supply = <&reg_2v8>;
+		DVDD-supply = <&reg_1v5>;
+
+		port {
+			camera2_ep: endpoint {
+				remote-endpoint = <&mipi_csi2_in_ep>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
+&mipi_csi2 {
+	assigned-clock-rates = <266000000>, <200000000>, <66000000>;
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			mipi_csi2_in_ep: endpoint {
+				remote-endpoint = <&camera2_ep>;
+				data-lanes = <1 2>;
+				bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index e7d87ea81b69..d8c139c9128d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -50,6 +50,20 @@ reg_usdhc2_vmmc: regulator-vsd-3v3 {
 		enable-active-high;
 	};
 
+	reg_1v5: regulator-1v5 {
+		compatible = "regulator-fixed";
+		regulator-name = "DVDD_1V5";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <1500000>;
+	};
+
+	reg_2v8: regulator-2v8 {
+		compatible = "regulator-fixed";
+		regulator-name = "AVDD_2V8";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+	};
+
 	buck2_reg: regulator-buck2 {
 		pinctrl-names = "default";
 		pinctrl-0 = <&pinctrl_buck2>;
@@ -542,12 +556,34 @@ &wdog1 {
 };
 
 &iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_mclk>, <&pinctrl_camera_reset>;
+
 	pinctrl_buck2: vddarmgrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_GPIO1_IO13_GPIO1_IO13		0x19
 		>;
 	};
 
+	pinctrl_camera1_pwdn: camera1pwdngrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x19
+		>;
+	};
+
+	pinctrl_camera2_pwdn: camera2pwdngrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5		0x19
+		>;
+	};
+
+	/* Shared reset line for cameras on CSI1 and CSI2. */
+	pinctrl_camera_reset: cameraresetgrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x19
+		>;
+	};
+
 	pinctrl_fec1: fec1grp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_ENET_MDC_ENET1_MDC			0x3
@@ -575,12 +611,26 @@ MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA			0x4000007f
 		>;
 	};
 
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL			0x4000007f
+			MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA			0x4000007f
+		>;
+	};
+
 	pinctrl_ir: irgrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_GPIO1_IO12_GPIO1_IO12		0x4f
 		>;
 	};
 
+	/* Shared MCLK for cameras on CSI1 and CSI2. */
+	pinctrl_mclk: mclkgrp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_GPIO1_IO15_CCMSRCGPCMIX_CLKO2	0x59
+		>;
+	};
+
 	pinctrl_mipi_dsi: mipidsigrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_ECSPI1_SCLK_GPIO5_IO6		0x16
-- 
2.50.1


^ permalink raw reply related

* [PATCH v4 2/2] media: i2c: ov5640: Add reset controller support with GPIO fallback
From: robby.cai @ 2026-06-19 10:05 UTC (permalink / raw)
  To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, festevam,
	sebastian.krzyszkowiak, slongerbeam, sakari.ailus, mchehab,
	p.zabel, kieran.bingham
  Cc: kernel, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20260619100532.3779934-1-robby.cai@oss.nxp.com>

From: Robby Cai <robby.cai@nxp.com>

Add support for the reset controller framework by acquiring the reset
line using devm_reset_control_get_optional_shared_deasserted(). This
allows the driver to handle reset lines provided by a reset controller,
including shared ones, while avoiding unbalanced deassert counts.

Retain support for legacy reset-gpios as a fallback when no reset
controller is defined. In that case, request the GPIO and keep it in the
deasserted state as the initial configuration.

This enables the driver to support both reset-controller-backed reset
lines and older GPIO-based descriptions while preserving the existing
power-up sequencing behavior.

Signed-off-by: Robby Cai <robby.cai@nxp.com>
---
 drivers/media/i2c/ov5640.c | 80 +++++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 85ecc23b3587..5e6db8aacb11 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <media/v4l2-async.h>
@@ -442,6 +443,7 @@ struct ov5640_dev {
 	u32 xclk_freq;
 
 	struct regulator_bulk_data supplies[OV5640_NUM_SUPPLIES];
+	struct reset_control *reset;
 	struct gpio_desc *reset_gpio;
 	struct gpio_desc *pwdn_gpio;
 	bool   upside_down;
@@ -2431,6 +2433,48 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 	return ov5640_set_framefmt(sensor, &sensor->fmt);
 }
 
+static int ov5640_get_reset(struct device *dev, struct ov5640_dev *sensor)
+{
+	/* use deasserted version to avoid unbalanced deassert counts */
+	sensor->reset =
+	    devm_reset_control_get_optional_shared_deasserted(dev, NULL);
+	if (IS_ERR(sensor->reset))
+		return dev_err_probe(dev, PTR_ERR(sensor->reset),
+				     "Failed to get reset\n");
+	else if (sensor->reset)
+		return 0;
+
+	/*
+	 * fallback to legacy reset-gpios
+	 * GPIOD_OUT_HIGH ensures deasserted state for ACTIVE_LOW reset
+	 */
+	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(sensor->reset_gpio),
+				     "Failed to get reset gpio");
+
+	return 0;
+}
+
+static int ov5640_reset_assert(struct ov5640_dev *sensor)
+{
+	if (sensor->reset)
+		return reset_control_assert(sensor->reset);
+
+	gpiod_set_value_cansleep(sensor->reset_gpio, 1);
+	return 0;
+}
+
+static int ov5640_reset_deassert(struct ov5640_dev *sensor)
+{
+	if (sensor->reset)
+		return reset_control_deassert(sensor->reset);
+
+	gpiod_set_value_cansleep(sensor->reset_gpio, 0);
+	return 0;
+}
+
 static void ov5640_power(struct ov5640_dev *sensor, bool enable)
 {
 	gpiod_set_value_cansleep(sensor->pwdn_gpio, enable ? 0 : 1);
@@ -2448,12 +2492,19 @@ static void ov5640_power(struct ov5640_dev *sensor, bool enable)
  *
  * In such cases, this gpio should be mapped to pwdn_gpio in the driver, and we
  * should still toggle the pwdn_gpio below with the appropriate delays, while
- * the calls to reset_gpio will be ignored.
+ * reset handling (via reset controller or GPIO) will be ignored.
  */
-static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
+static int ov5640_powerup_sequence(struct ov5640_dev *sensor)
 {
+	int ret;
+
 	if (sensor->pwdn_gpio) {
-		gpiod_set_value_cansleep(sensor->reset_gpio, 1);
+		ret = ov5640_reset_assert(sensor);
+		if (ret) {
+			dev_err(&sensor->i2c_client->dev,
+				"Failed to assert reset: %d\n", ret);
+			return ret;
+		}
 
 		/* camera power cycle */
 		ov5640_power(sensor, false);
@@ -2461,7 +2512,13 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
 		ov5640_power(sensor, true);
 		usleep_range(1000, 2000);	/* t3 */
 
-		gpiod_set_value_cansleep(sensor->reset_gpio, 0);
+		ret = ov5640_reset_deassert(sensor);
+		if (ret) {
+			dev_err(&sensor->i2c_client->dev,
+				"Failed to deassert reset: %d\n", ret);
+			ov5640_power(sensor, false);
+			return ret;
+		}
 	} else {
 		/* software reset */
 		ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
@@ -2475,6 +2532,8 @@ static void ov5640_powerup_sequence(struct ov5640_dev *sensor)
 	 */
 	ov5640_write_reg(sensor, OV5640_REG_SYS_CTRL0,
 			 OV5640_REG_SYS_CTRL0_SW_PWDN);
+
+	return 0;
 }
 
 static int ov5640_set_power_on(struct ov5640_dev *sensor)
@@ -2497,7 +2556,9 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
 		goto xclk_off;
 	}
 
-	ov5640_powerup_sequence(sensor);
+	ret = ov5640_powerup_sequence(sensor);
+	if (ret)
+		goto regulator_off;
 
 	ret = ov5640_init_slave_id(sensor);
 	if (ret)
@@ -2507,6 +2568,7 @@ static int ov5640_set_power_on(struct ov5640_dev *sensor)
 
 power_off:
 	ov5640_power(sensor, false);
+regulator_off:
 	regulator_bulk_disable(OV5640_NUM_SUPPLIES, sensor->supplies);
 xclk_off:
 	clk_disable_unprepare(sensor->xclk);
@@ -3914,11 +3976,9 @@ static int ov5640_probe(struct i2c_client *client)
 	if (IS_ERR(sensor->pwdn_gpio))
 		return PTR_ERR(sensor->pwdn_gpio);
 
-	/* request optional reset pin */
-	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
-						     GPIOD_OUT_HIGH);
-	if (IS_ERR(sensor->reset_gpio))
-		return PTR_ERR(sensor->reset_gpio);
+	ret = ov5640_get_reset(dev, sensor);
+	if (ret)
+		return ret;
 
 	v4l2_i2c_subdev_init(&sensor->sd, client, &ov5640_subdev_ops);
 	sensor->sd.internal_ops = &ov5640_internal_ops;
-- 
2.50.1


^ permalink raw reply related

* [PATCH v4 0/2] Add support for Renesas RZ/G3L LVDS encoder
From: Biju @ 2026-06-19 10:10 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, David Airlie,
	Simona Vetter, Philipp Zabel, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm
  Cc: Biju Das, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Luca Ceresoli, Tommaso Merciai, dri-devel, devicetree,
	linux-kernel, linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das

From: Biju Das <biju.das.jz@bp.renesas.com>

Add support for the RZ/G3L LVDS encoder driver. It operates in single-link
mode with 4 lanes (Data) + 1 lane (Clock) and supports pixel clock rates
from 25 to 87 MHz. The LVDS module cannot be used at the same time as
MIPI-DSI. However, LVDS and the DSI interface share a peripheral clock and
the MIPI_DSI_PRESET_N reset signal. Also, the MIPI_DSI_CMN_RSTB and
MIPI_DSI_ARESET_N reset signals must be asserted before using the LVDS
module.

This patch series depend upon [1]

[1]
 https://lore.kernel.org/all/20260608-drm-no-more-bridge-reset-v2-0-0a91018bf886@kernel.org/

v3->v4:
 * Reworked bindings, dropping parent node containing simple-mfd and
   syson.
 * Dropped the tags
 * Dropped the header files clk.h and syscon.h
 * Dropped next_bridge check in attach().
 * Dropped syscon for getting regmap.
 * Replaced the below macros to match the hardware manual:
	LVDS_0_CTL_FMT_SEL_MSK->LVDS_0_CTL_FMT_SEL0_MSK
	LVDS_0_PHY_CH_IO_EN_MSK->LVDS_0_PHY_CH_IO_EN0_MSK
	Replaced LVDS_0_PHY_CH_IO_EN->LVDS_0_PHY_CH_IO_EN0
 * Replaced atomic_reset()->atomic_create_state().
v2->v3:
 * Collected tags.
v2->v2[1]:
 * Dropped patch#1 as it is accepted.
 * Replace drm_atomic_state with drm_atomic_commit in
   rzg3l_lvds_atomic_{en,dis}able().
 * Drop local variable ret and dev_err() messages in
   rzg3l_lvds_atomic_enable(); use WARN_ON() instead to
   capture unexpected failures since atomic_enable should not fail.
 * Drop local variable next_bridge from rzg3l_lvds_probe().
[1] https://lore.kernel.org/all/20260524194457.479681-1-biju.das.jz@bp.renesas.com/
v1->v2:
 * Collected the tags for binding patches.
 * Dropped unused function rzg3l_lvds_is_connected() and removed the 
   corresponding header file rzg3l_lvds.h
 * Dropped next_bridge from struct rzg3l_lvds instead using bridge's
   next_bridge.
 * Replaced pm_runtime_resume_and_get()->pm_runtime_get_sync() as
   atomic_enable doesn't fail and for each enable there always will be an
   atomic_disable() call.
 * Started using DEFINE_RUNTIME_DEV_PM_OPS for PM callback.
 * Replaced rzg3l_lvds_parse_dt() with devm_drm_of_get_bridge() in probe()
 * Started using reset_control_bulk_*() in rzg3l_lvds_pm_runtime_{suspend,
   resume}().

Biju Das (2):
  dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
  drm: renesas: rz-du: Add support for RZ/G3L LVDS encoder

 .../bridge/renesas,r9a08g046-lvds.yaml        | 120 ++++++++
 drivers/gpu/drm/renesas/rz-du/Kconfig         |  13 +
 drivers/gpu/drm/renesas/rz-du/Makefile        |   1 +
 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c    | 285 ++++++++++++++++++
 .../gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h   |  26 ++
 5 files changed, 445 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
 create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds.c
 create mode 100644 drivers/gpu/drm/renesas/rz-du/rzg3l_lvds_regs.h

-- 
2.43.0


^ permalink raw reply

* [PATCH v4 1/2] dt-bindings: display: bridge: Document Renesas RZ/G3L LVDS encoder
From: Biju @ 2026-06-19 10:10 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm
  Cc: Biju Das, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Luca Ceresoli, Tommaso Merciai, dri-devel, devicetree,
	linux-kernel, linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260619101026.323633-1-biju.das.jz@bp.renesas.com>

From: Biju Das <biju.das.jz@bp.renesas.com>

Document the LVDS encoder IP found on the RZ/G3L SoC. It supports
single-link mode. LVDS and the DSI interface share a peripheral clock and
the MIPI_DSI_PRESET_N reset signal. However, the LVDS module cannot be
used at the same time as MIPI-DSI.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Dropped the tags as it is a rework dropping parent node that contains
   simple-mfd and syscon.
v2->v3:
 * Collected tag.
v2->v2[1]:
 * No change.
[1] https://lore.kernel.org/all/20260524195829.960401F000E9@smtp.kernel.org/
v1->v2:
 * Collected tag.
---
 .../bridge/renesas,r9a08g046-lvds.yaml        | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
new file mode 100644
index 000000000000..4cd7b688fbf7
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,r9a08g046-lvds.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/renesas,r9a08g046-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G3L LVDS Encoder
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+  - Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
+
+description: |
+  This binding describes the LVDS encoder embedded in the Renesas RZ/G3L
+  SoC. The encoder can operate in LVDS Single-link mode with 4 lanes
+  (Data) + 1 lane (Clock).
+
+properties:
+  compatible:
+    const: renesas,r9a08g046-lvds
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Peripheral clock
+      - description: PHY clock
+      - description: Dot clock
+
+  clock-names:
+    items:
+      - const: pclk
+      - const: phyclk
+      - const: dotclk
+
+  resets:
+    items:
+      - description: LVDS_RESET_N
+      - description: MIPI_DSI_PRESET_N
+      - description: MIPI_DSI_CMN_RSTB
+      - description: MIPI_DSI_ARESET_N
+
+  reset-names:
+    items:
+      - const: lvdrst
+      - const: prst
+      - const: rst
+      - const: arst
+
+  power-domains:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Input channel, directly connected to the Display Unit.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: |
+          Output channel, directly connected to the LVDS panel or bridge.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - power-domains
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/renesas,r9a08g046-cpg.h>
+
+    lvds@108a0000 {
+        compatible = "renesas,r9a08g046-lvds";
+        reg = <0x108a0000 0x10000>;
+        clocks = <&cpg CPG_MOD R9A08G046_MIPI_DSI_PCLK>,
+                 <&cpg CPG_MOD R9A08G046_LVDS_PLLCLK>,
+                 <&cpg CPG_MOD R9A08G046_LVDS_CLK_DOT0>;
+        clock-names = "pclk", "phyclk", "dotclk";
+        resets = <&cpg R9A08G046_LVDS_RESET_N>,
+                 <&cpg R9A08G046_MIPI_DSI_PRESET_N>,
+                 <&cpg R9A08G046_MIPI_DSI_CMN_RSTB>,
+                 <&cpg R9A08G046_MIPI_DSI_ARESET_N>;
+        reset-names = "lvdrst", "prst", "rst", "arst";
+        power-domains = <&cpg>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                lvds0_in: endpoint {
+                    remote-endpoint = <&du_out_lvds0>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                lvds0_out: endpoint {
+                    remote-endpoint = <&panel_in>;
+                };
+            };
+        };
+    };
+...
-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox