Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers
From: Maxime Ripard @ 2026-06-25  8:05 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
	Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
	Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
	dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260602-dw-hdmi-qp-scramb-v7-4-445eb54ee1ed@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 1749 bytes --]

On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
> +static int drm_scdc_reset_crtc(struct drm_connector *connector,
> +			       struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_crtc *crtc;
> +	u8 config;
> +	int ret;
> +
> +	if (!ctx)
> +		return 0;
> +
> +	/*
> +	 * This is normally part of .detect_ctx() call path, which already holds
> +	 * connection_mutex through @ctx.  However, re-acquiring it with the
> +	 * same context is a no-op and makes the helper safe under any caller.
> +	 */
> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex, ctx);
> +	if (ret)
> +		return ret;
> +
> +	if (!connector->state)
> +		return 0;
> +
> +	crtc = connector->state->crtc;
> +	if (!crtc)
> +		return 0;
> +
> +	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> +	if (ret) {
> +		drm_scdc_dbg(connector, "Failed to read TMDS config: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if ((config & SCDC_SCRAMBLING_ENABLE) &&
> +	    (config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40))
> +		return 0;

I don't think we should care about the bit clock ratio here. From a
technical standpoint, and since you ignore low rates for the scrambler
for now, they are indeed equivalents. But semantically, scrambler can be
enabled for any rate, and the bit clock ratio should be changed only for
rates higher than 340MHz.

This function makes sure to trigger a modeset when the scrambler is
enabled or not. From a spec point of view, it's somewhat orthogonal to
the bit clock ratio. From a function name point of view, it's
misleading.

So yeah, I'd just drop the check on the bit clock ratio here. Drivers
will then get to enable either when the commit actually happens.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v7 04/30] drm/display: scdc_helper: Add HDMI 2.0 scrambling management helpers
From: Maxime Ripard @ 2026-06-25  7:53 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
	Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
	Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
	dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <2f5ce8eb-ef47-4238-ad68-ce62981a1845@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 3076 bytes --]

On Sat, Jun 13, 2026 at 04:30:02AM +0300, Cristian Ciocaltea wrote:
> On 6/12/26 4:43 PM, Maxime Ripard wrote:
> > On Tue, Jun 02, 2026 at 01:44:04AM +0300, Cristian Ciocaltea wrote:
> >> +/**
> >> + * drm_scdc_start_scrambling - activate scrambling and monitor SCDC status
> >> + * @connector: connector
> >> + *
> >> + * Enables scrambling and high TMDS clock ratio on both source and sink sides.
> >> + * Additionally, use a delayed work item to monitor the scrambling status on
> >> + * the sink side and retry the operation, as some displays refuse to set the
> >> + * scrambling bit right away.
> >> + *
> >> + * Returns:
> >> + * Zero if scrambling is set successfully, an error code otherwise.
> >> + */
> >> +int drm_scdc_start_scrambling(struct drm_connector *connector)
> >> +{
> >> +	struct drm_display_info *info = &connector->display_info;
> >> +	struct drm_connector_hdmi *hdmi = &connector->hdmi;
> >> +	int ret;
> >> +	u8 ver;
> >> +
> >> +	if (!hdmi->scrambler_supported) {
> >> +		drm_scdc_dbg(connector, "Scrambler not supported, bailing.\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	if (!info->is_hdmi ||
> >> +	    !info->hdmi.scdc.supported ||
> >> +	    !info->hdmi.scdc.scrambling.supported) {
> >> +		drm_scdc_dbg(connector, "Sink doesn't support scrambling.\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	drm_scdc_dbg(connector, "Enabling scrambling\n");
> >> +
> >> +	ret = drm_scdc_readb(connector->ddc, SCDC_SINK_VERSION, &ver);
> >> +	if (ret) {
> >> +		drm_scdc_dbg(connector, "Failed to read SCDC_SINK_VERSION: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	ret = drm_scdc_writeb(connector->ddc, SCDC_SOURCE_VERSION,
> >> +			      min_t(u8, ver, SCDC_MAX_SOURCE_VERSION));
> >> +	if (ret) {
> >> +		drm_scdc_dbg(connector, "Failed to write SCDC_SOURCE_VERSION: %d\n", ret);
> >> +		return ret;
> >> +	}
> >> +
> >> +	hdmi->scdc_cb = drm_scdc_monitor_scrambler;
> >> +	WRITE_ONCE(hdmi->scrambler_enabled, true);
> >> +
> >> +	ret = drm_scdc_try_scrambling_setup(connector);
> >> +	if (!ret)
> >> +		ret = hdmi->funcs->scrambler_enable(connector);
> >> +
> >> +	if (ret) {
> >> +		WRITE_ONCE(hdmi->scrambler_enabled, false);
> >> +		cancel_delayed_work_sync(&hdmi->scdc_work);
> >> +		hdmi->scdc_cb = NULL;
> >> +
> >> +		drm_scdc_set_scrambling(connector, false);
> >> +		drm_scdc_set_high_tmds_clock_ratio(connector, false);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(drm_scdc_start_scrambling);
> > 
> > This is also pretty significantly different than what vc4 implemented. I
> > don't necessarily mind, but claiming that it's functionally equivalent
> > isn't true. I think it would be a much better strategy to turn the vc4
> > code into helpers as is because it's been merged 4 years ago and we know
> > it's solid.
> 
> As explained in patch 25, I think there's been a slight misunderstanding of how
> the CRTC reset logic was actually implemented.

You're absolutely right, let me review it again. sorry.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: usb: Add Rockchip RK3568 compatible for EHCI and OHCI
From: Krzysztof Kozlowski @ 2026-06-25  7:46 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman, Diederik de Haas, devicetree, linux-rockchip,
	linux-usb, linux-arm-kernel, linux-kernel
In-Reply-To: <20260624192726.781864-2-jonas@kwiboo.se>

On Wed, Jun 24, 2026 at 07:27:24PM +0000, Jonas Karlman wrote:
> The Rockchip RK3568 EHCI/OHCI controller depends on clk_usbphy1_480m
> being enabled, or the system may freeze when registers are accessed.
> 
> Add Rockchip RK3568 EHCI and OHCI compatibles with a similar four-clock
> constraint as RK3588, also extend the EHCI constraint to include RK3588
> to match similar requirements of RK3588.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> Existing DTs for RK3568 use the plain generic-ehci/ohci compatible,
> next patch make use of these new compatibles and adds the missing
> clk_usbphy1_480m clock references.
> 
> Existing DTs for RK3588 have contained the required four clocks since
> the initial addition of the EHCI/OHCI nodes.
> 
> Changes in v2:
> - Include rockchip,rk3588-ehci in the EHCI constraint
> - Make clocks prop required for EHCI and OHCI
> ---
>  .../devicetree/bindings/usb/generic-ehci.yaml      | 14 ++++++++++++++
>  .../devicetree/bindings/usb/generic-ohci.yaml      |  7 ++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> index 55a5aa7d7a54..a39f01e740b1 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> @@ -52,6 +52,7 @@ properties:
>                - ibm,476gtr-ehci
>                - nxp,lpc1850-ehci
>                - qca,ar7100-ehci
> +              - rockchip,rk3568-ehci
>                - rockchip,rk3588-ehci
>                - snps,hsdk-v1.0-ehci
>                - socionext,uniphier-ehci
> @@ -186,6 +187,19 @@ allOf:
>        required:
>          - clocks
>          - clock-names
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3568-ehci
> +              - rockchip,rk3588-ehci
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 4
> +      required:
> +        - clocks

This is ABI break for RK3588, so your commit msg should also mention
that RK3588 is not working for example. Otherwise you provided rationale
only for breaking RK3568 ABI.

Same for OHCI part.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v7 25/30] drm/vc4: hdmi: Convert to common HDMI 2.0 SCDC scrambling helpers
From: Maxime Ripard @ 2026-06-25  7:44 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Sandy Huang,
	Heiko Stübner, Andy Yan, Daniel Stone, Dave Stevenson,
	Maíra Canal, Raspberry Pi Kernel Maintenance, kernel,
	dri-devel, linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <8937d785-4daa-4246-9553-24aed7d279b5@collabora.com>


[-- Attachment #1.1: Type: text/plain, Size: 19934 bytes --]

On Sat, Jun 13, 2026 at 03:41:20AM +0300, Cristian Ciocaltea wrote:
> Hi Maxime,
> 
> On 6/12/26 3:04 PM, Maxime Ripard wrote:
> > Hi,
> > 
> > On Tue, Jun 02, 2026 at 01:44:25AM +0300, Cristian Ciocaltea wrote:
> >> Replace the vc4-local scrambling implementation with the newly
> >> introduced DRM common SCDC scrambling infrastructure:
> >>
> >> - Advertise source-side scrambling support by setting
> >>   connector->hdmi.scrambling_supported based on the variant's
> >>   max_pixel_clock before drmm_connector_hdmi_init().
> >>
> >> - Provide minimal .scrambler_{enable|disable} connector callbacks that
> >>   only toggle the VC5 HDMI_SCRAMBLER_CTL register.  Sink-side SCDC
> >>   programming and periodic status monitoring are now delegated to
> >>   drm_scdc_{start|stop}_scrambling().
> >>
> >> - Replace vc4_hdmi_enable_scrambling() with a conditional call to
> >>   drm_scdc_start_scrambling() in post_crtc_enable, gated on
> >>   conn_state->hdmi.scrambler_needed (computed by the HDMI state helper).
> >>
> >> - Replace vc4_hdmi_disable_scrambling() with drm_scdc_stop_scrambling()
> >>   in post_crtc_disable.
> >>
> >> - Drop vc4_hdmi_reset_link() and vc4_hdmi_handle_hotplug(), switching
> >>   the .detect_ctx() path to
> >>   drm_atomic_helper_connector_hdmi_hotplug_ctx() which internally calls
> >>   drm_scdc_sync_status() to trigger a CRTC reset on reconnection.
> >>
> >> - Drop the local scrambling_work delayed workqueue and scdc_enabled
> >>   flag, now tracked by the common drm_connector_hdmi layer.
> >>
> >> - Drop vc4_hdmi_supports_scrambling() and
> >>   vc4_hdmi_mode_needs_scrambling() helpers, inlining the remaining 4KP60
> >>   warning with an explicit drm_hdmi_compute_mode_clock() check.
> >>
> >> - Seed connector->hdmi.scrambler_enabled = true in connector_init() to
> >>   ensure drm_scdc_stop_scrambling() runs at boot and disables any stale
> >>   scrambling state left by the bootloader.
> >>
> >> No functional change is expected for the supported modes.
> >>
> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > 
> > I'd really like it to be broken down into several patches:
> > 
> >> ---
> >>  drivers/gpu/drm/vc4/vc4_hdmi.c | 265 ++++++-----------------------------------
> >>  drivers/gpu/drm/vc4/vc4_hdmi.h |   8 --
> >>  2 files changed, 35 insertions(+), 238 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> index 046ac4f43ba8..02f6ca6ab52b 100644
> >> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> >> @@ -114,31 +114,6 @@
> >>  #define HSM_MIN_CLOCK_FREQ	120000000
> >>  #define CEC_CLOCK_FREQ 40000
> >>  
> >> -static bool vc4_hdmi_supports_scrambling(struct vc4_hdmi *vc4_hdmi)
> >> -{
> >> -	struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> >> -
> >> -	lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> -	if (!display->is_hdmi)
> >> -		return false;
> >> -
> >> -	if (!display->hdmi.scdc.supported ||
> >> -	    !display->hdmi.scdc.scrambling.supported)
> >> -		return false;
> >> -
> >> -	return true;
> >> -}
> >> -
> >> -static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode,
> >> -					   unsigned int bpc,
> >> -					   enum drm_output_color_format fmt)
> >> -{
> >> -	unsigned long long clock = drm_hdmi_compute_mode_clock(mode, bpc, fmt);
> >> -
> >> -	return clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> -}
> >> -
> >>  static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
> >>  {
> >>  	struct drm_debugfs_entry *entry = m->private;
> >> @@ -272,124 +247,6 @@ static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi)
> >>  static void vc4_hdmi_cec_update_clk_div(struct vc4_hdmi *vc4_hdmi) {}
> >>  #endif
> >>  
> >> -static int vc4_hdmi_reset_link(struct drm_connector *connector,
> >> -			       struct drm_modeset_acquire_ctx *ctx)
> >> -{
> >> -	struct drm_device *drm;
> >> -	struct vc4_hdmi *vc4_hdmi;
> >> -	struct drm_connector_state *conn_state;
> >> -	struct drm_crtc_state *crtc_state;
> >> -	struct drm_crtc *crtc;
> >> -	bool scrambling_needed;
> >> -	u8 config;
> >> -	int ret;
> >> -
> >> -	if (!connector)
> >> -		return 0;
> >> -
> >> -	drm = connector->dev;
> >> -	ret = drm_modeset_lock(&drm->mode_config.connection_mutex, ctx);
> >> -	if (ret)
> >> -		return ret;
> >> -
> >> -	conn_state = connector->state;
> >> -	crtc = conn_state->crtc;
> >> -	if (!crtc)
> >> -		return 0;
> >> -
> >> -	ret = drm_modeset_lock(&crtc->mutex, ctx);
> >> -	if (ret)
> >> -		return ret;
> >> -
> >> -	crtc_state = crtc->state;
> >> -	if (!crtc_state->active)
> >> -		return 0;
> >> -
> >> -	vc4_hdmi = connector_to_vc4_hdmi(connector);
> >> -	mutex_lock(&vc4_hdmi->mutex);
> >> -
> >> -	if (!vc4_hdmi_supports_scrambling(vc4_hdmi)) {
> >> -		mutex_unlock(&vc4_hdmi->mutex);
> >> -		return 0;
> >> -	}
> >> -
> >> -	scrambling_needed = vc4_hdmi_mode_needs_scrambling(&vc4_hdmi->saved_adjusted_mode,
> >> -							   vc4_hdmi->output_bpc,
> >> -							   vc4_hdmi->output_format);
> >> -	if (!scrambling_needed) {
> >> -		mutex_unlock(&vc4_hdmi->mutex);
> >> -		return 0;
> >> -	}
> >> -
> >> -	if (conn_state->commit &&
> >> -	    !try_wait_for_completion(&conn_state->commit->hw_done)) {
> >> -		mutex_unlock(&vc4_hdmi->mutex);
> >> -		return 0;
> >> -	}
> >> -
> >> -	ret = drm_scdc_readb(connector->ddc, SCDC_TMDS_CONFIG, &config);
> >> -	if (ret < 0) {
> >> -		drm_err(drm, "Failed to read TMDS config: %d\n", ret);
> >> -		mutex_unlock(&vc4_hdmi->mutex);
> >> -		return 0;
> >> -	}
> >> -
> >> -	if (!!(config & SCDC_SCRAMBLING_ENABLE) == scrambling_needed) {
> >> -		mutex_unlock(&vc4_hdmi->mutex);
> >> -		return 0;
> >> -	}
> >> -
> >> -	mutex_unlock(&vc4_hdmi->mutex);
> >> -
> >> -	/*
> >> -	 * HDMI 2.0 says that one should not send scrambled data
> >> -	 * prior to configuring the sink scrambling, and that
> >> -	 * TMDS clock/data transmission should be suspended when
> >> -	 * changing the TMDS clock rate in the sink. So let's
> >> -	 * just do a full modeset here, even though some sinks
> >> -	 * would be perfectly happy if were to just reconfigure
> >> -	 * the SCDC settings on the fly.
> >> -	 */
> >> -	return drm_atomic_helper_reset_crtc(crtc, ctx);
> >> -}
> > 
> > This one doesn't look functionally equivalent to me to
> > drm_scdc_reset_crtc: this part was, in part, making sure we would only
> > reset the scrambler if it was enabled in the first place.
> > drm_scdc_reset_crtc() doesn't and will always trigger a modeset on
> > hotplug. That's unnecessary and a significant functional different.
> 
> drm_scdc_reset_crtc() alone was not meant to be an equivalent of
> vc4_hdmi_reset_link(), as it only checks the sink side and it serves as an
> internal helper used exclusively by drm_scdc_sync_status().  
> 
> As a matter of fact, the latter is the one responsible for verifying if the
> scrambler was enabled on the controller side before attempting to invoke the
> reset logic, hence we should get the same behavior.  But we don't invoke it
> directly either, it's part of the drm_atomic_helper_connector_hdmi_hotplug_ctx()
> call path.

Oh, right, sorry.

> > I'd argue that it's drm_scdc_reset_crtc() that needs to align to what
> > vc4 was doing, not the opposite.
> 
> The only difference consists in dropping the crtc state check:
> 
>     ret = drm_modeset_lock(&crtc->mutex, ctx);
>     if (ret)
>             return ret;
> 
>     crtc_state = crtc->state;
>     if (!crtc_state->active)
>             return 0;
> 
> The rationale was that when CRTC is inactive, drm_atomic_helper_reset_crtc()
> should result in a no-op commit anyway.

A commit is expensive, so I'd skip it if we can.

> And the one for the in-flight commit:
> 
>     if (conn_state->commit &&
>         !try_wait_for_completion(&conn_state->commit->hw_done)) {
>             mutex_unlock(&vc4_hdmi->mutex);
>             return 0;
>     }

And yeah, we'll need this one too.

> Both checks are also missing in drm_bridge_helper_reset_crtc(), taken as an
> initial reference. Should we still keep any/both and sync the bridge helper
> accordingly?

Yes, but I'd expect the bridge helpers to converge / reuse your helpers
eventually anyway?

> >> -static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> >> -				    struct drm_modeset_acquire_ctx *ctx,
> >> -				    enum drm_connector_status status)
> >> -{
> >> -	struct drm_connector *connector = &vc4_hdmi->connector;
> >> -	int ret;
> >> -
> >> -	/*
> >> -	 * NOTE: This function should really be called with vc4_hdmi->mutex
> >> -	 * held, but doing so results in reentrancy issues since
> >> -	 * cec_s_phys_addr() might call .adap_enable, which leads to that
> >> -	 * funtion being called with our mutex held.
> >> -	 *
> >> -	 * A similar situation occurs with vc4_hdmi_reset_link() that
> >> -	 * will call into our KMS hooks if the scrambling was enabled.
> >> -	 *
> >> -	 * Concurrency isn't an issue at the moment since we don't share
> >> -	 * any state with any of the other frameworks so we can ignore
> >> -	 * the lock for now.
> >> -	 */
> >> -
> >> -	drm_atomic_helper_connector_hdmi_hotplug(connector, status);
> >> -
> >> -	if (status != connector_status_connected)
> >> -		return;
> >> -
> >> -	for (;;) {
> >> -		ret = vc4_hdmi_reset_link(connector, ctx);
> >> -		if (ret == -EDEADLK) {
> >> -			drm_modeset_backoff(ctx);
> >> -			continue;
> >> -		}
> >> -
> >> -		break;
> >> -	}
> >> -}
> >> -
> >>  static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >>  					 struct drm_modeset_acquire_ctx *ctx,
> >>  					 bool force)
> >> @@ -401,8 +258,8 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >>  	/*
> >>  	 * NOTE: This function should really take vc4_hdmi->mutex, but
> >>  	 * doing so results in reentrancy issues since
> >> -	 * vc4_hdmi_handle_hotplug() can call into other functions that
> >> -	 * would take the mutex while it's held here.
> >> +	 * drm_atomic_helper_connector_hdmi_hotplug_ctx() can call into other
> >> +	 * functions that would take the mutex while it's held here.
> >>  	 *
> >>  	 * Concurrency isn't an issue at the moment since we don't share
> >>  	 * any state with any of the other frameworks so we can ignore
> >> @@ -425,10 +282,11 @@ static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
> >>  			status = connector_status_connected;
> >>  	}
> >>  
> >> -	vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
> >> +	ret = drm_atomic_helper_connector_hdmi_hotplug_ctx(connector, status, ctx);
> >> +
> >>  	pm_runtime_put(&vc4_hdmi->pdev->dev);
> >>  
> >> -	return status;
> >> +	return ret == -EDEADLK ? ret : status;
> >>  }
> >>  
> >>  static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
> >> @@ -441,9 +299,12 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
> >>  	if (!vc4->hvs->vc5_hdmi_enable_hdmi_20) {
> >>  		struct drm_device *drm = connector->dev;
> >>  		const struct drm_display_mode *mode;
> >> +		unsigned long long clock;
> >>  
> >>  		list_for_each_entry(mode, &connector->probed_modes, head) {
> >> -			if (vc4_hdmi_mode_needs_scrambling(mode, 8, DRM_OUTPUT_COLOR_FORMAT_RGB444)) {
> >> +			clock = drm_hdmi_compute_mode_clock(mode, 8,
> >> +							    DRM_OUTPUT_COLOR_FORMAT_RGB444);
> >> +			if (clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ) {
> > 
> > This should be a patch of its own, but I think we should turn
> > vc4_hdmi_mode_needs_scrambling() into a helper, instead of checking the
> > clock rate in every driver that might need it. From a logical standpoint
> > it's equivalent, but not from a semantic one.
> 
> Ack.
> 
> > 
> >>  				drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz.");
> >>  				drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60.");
> >>  			}
> >> @@ -540,6 +401,9 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
> >>  	if (vc4_hdmi->variant->supports_hdr)
> >>  		max_bpc = 12;
> >>  
> >> +	connector->hdmi.scrambler_supported =
> >> +		vc4_hdmi->variant->max_pixel_clock > HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ;
> >> +
> >>  	ret = drmm_connector_hdmi_init(dev, connector,
> >>  				       "Broadcom", "Videocore",
> >>  				       &vc4_hdmi_connector_funcs,
> >> @@ -561,6 +425,14 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
> >>  
> >>  	drm_connector_helper_add(connector, &vc4_hdmi_connector_helper_funcs);
> >>  
> >> +	/*
> >> +	 * Since we don't know the state of the controller and its
> >> +	 * display (if any), let's assume it's always enabled.
> >> +	 * drm_scdc_stop_scrambling() will thus run at boot, make
> >> +	 * sure it's disabled, and avoid any inconsistency.
> >> +	 */
> >> +	connector->hdmi.scrambler_enabled = connector->hdmi.scrambler_supported;
> >> +
> >>  	/*
> >>  	 * Some of the properties below require access to state, like bpc.
> >>  	 * Allocate some default initial connector state with our reset helper.
> >> @@ -786,93 +658,30 @@ static int vc4_hdmi_write_spd_infoframe(struct drm_connector *connector,
> >>  					buffer, len);
> >>  }
> >>  
> >> -#define SCRAMBLING_POLLING_DELAY_MS	1000
> >> -
> >> -static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder)
> >> +static int vc4_hdmi_scrambler_enable(struct drm_connector *connector)
> >>  {
> >> -	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> >> -	struct drm_connector *connector = &vc4_hdmi->connector;
> >> -	struct drm_device *drm = connector->dev;
> >> -	const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
> >> +	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> >>  	unsigned long flags;
> >> -	int idx;
> >> -
> >> -	lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> -	if (!vc4_hdmi_supports_scrambling(vc4_hdmi))
> >> -		return;
> >> -
> >> -	if (!vc4_hdmi_mode_needs_scrambling(mode,
> >> -					    vc4_hdmi->output_bpc,
> >> -					    vc4_hdmi->output_format))
> >> -		return;
> >> -
> >> -	if (!drm_dev_enter(drm, &idx))
> >> -		return;
> > 
> > drm_dev_enter should be kept here
> 
> Sorry, somehow I missed to realize when I prepared the patches that I
> accidentally dropped these during my initial driver rework.
> 
> > 
> >> -	drm_scdc_set_high_tmds_clock_ratio(connector, true);
> >> -	drm_scdc_set_scrambling(connector, true);
> >>  
> >>  	spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
> >>  	HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) |
> >>  		   VC5_HDMI_SCRAMBLER_CTL_ENABLE);
> >>  	spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >>  
> >> -	drm_dev_exit(idx);
> > 
> > And exit here.
> > 
> >> -static void vc4_hdmi_disable_scrambling(struct drm_encoder *encoder)
> >> +static int vc4_hdmi_scrambler_disable(struct drm_connector *connector)
> >>  {
> >> -	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
> >> -	struct drm_connector *connector = &vc4_hdmi->connector;
> >> -	struct drm_device *drm = connector->dev;
> >> +	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> >>  	unsigned long flags;
> >> -	int idx;
> >> -
> >> -	lockdep_assert_held(&vc4_hdmi->mutex);
> >> -
> >> -	if (!vc4_hdmi->scdc_enabled)
> >> -		return;
> >> -
> >> -	vc4_hdmi->scdc_enabled = false;
> >> -
> >> -	if (delayed_work_pending(&vc4_hdmi->scrambling_work))
> >> -		cancel_delayed_work_sync(&vc4_hdmi->scrambling_work);
> >> -
> >> -	if (!drm_dev_enter(drm, &idx))
> >> -		return;
> > 
> > Ditto
> > 
> >>  	spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
> >>  	HDMI_WRITE(HDMI_SCRAMBLER_CTL, HDMI_READ(HDMI_SCRAMBLER_CTL) &
> >>  		   ~VC5_HDMI_SCRAMBLER_CTL_ENABLE);
> >>  	spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >>  
> >> -	drm_scdc_set_scrambling(connector, false);
> >> -	drm_scdc_set_high_tmds_clock_ratio(connector, false);
> >> -
> >> -	drm_dev_exit(idx);
> >> -}
> >> -
> >> -static void vc4_hdmi_scrambling_wq(struct work_struct *work)
> >> -{
> >> -	struct vc4_hdmi *vc4_hdmi = container_of(to_delayed_work(work),
> >> -						 struct vc4_hdmi,
> >> -						 scrambling_work);
> >> -	struct drm_connector *connector = &vc4_hdmi->connector;
> >> -
> >> -	if (drm_scdc_get_scrambling_status(connector))
> >> -		return;
> >> -
> >> -	drm_scdc_set_high_tmds_clock_ratio(connector, true);
> >> -	drm_scdc_set_scrambling(connector, true);
> >> -
> >> -	queue_delayed_work(system_percpu_wq, &vc4_hdmi->scrambling_work,
> >> -			   msecs_to_jiffies(SCRAMBLING_POLLING_DELAY_MS));
> >> +	return 0;
> >>  }
> >>  
> >>  static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
> >> @@ -917,7 +726,7 @@ static void vc4_hdmi_encoder_post_crtc_disable(struct drm_encoder *encoder,
> >>  		spin_unlock_irqrestore(&vc4_hdmi->hw_lock, flags);
> >>  	}
> >>  
> >> -	vc4_hdmi_disable_scrambling(encoder);
> >> +	drm_scdc_stop_scrambling(&vc4_hdmi->connector);
> > 
> > I don't think the names here are right. It's not *only* related to scdc
> > but also to the HDMI controller. I'm fine with using a scdc prefix but
> > only for the things related to scdc. This is related (in part) to the
> > HDMI controller, so it should use a drm_connector_hdmi prefix.
> 
> Ack. I guess we should also move these helpers out of drm_scdc_helper.c, but
> unsure where.  FWIW I'm currently working on adding HDMI 2.1 FRL support, and
> implemented the link training in a dedicated drm_hdmi_frl_helper.c.  
> 
> Should we create drm_hdmi_scrambler_helper.c?  Or maybe have a common one to
> hold both - any suggestion for the naming?

display/drm_hdmi_helper.c sounds like a great place for both?

> > 
> >>  	drm_dev_exit(idx);
> >>  
> >> @@ -1625,6 +1434,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
> >>  	struct drm_display_info *display = &vc4_hdmi->connector.display_info;
> >>  	bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
> >>  	bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
> >> +	struct drm_connector_state *conn_state;
> >>  	unsigned long flags;
> >>  	int ret;
> >>  	int idx;
> >> @@ -1693,7 +1503,10 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct drm_encoder *encoder,
> >>  	}
> >>  
> >>  	vc4_hdmi_recenter_fifo(vc4_hdmi);
> >> -	vc4_hdmi_enable_scrambling(encoder);
> >> +
> >> +	conn_state = drm_atomic_get_new_connector_state(state, connector);
> >> +	if (conn_state && conn_state->hdmi.scrambler_needed)
> >> +		drm_scdc_start_scrambling(connector);
> > 
> > And the nice thing with a drm_connector_hdmi_* prefix is that you don't
> > have to shoehorn it into SCDC support anymore, so you can take a state
> > as an argument, and do the check in the helper instead of doing it in
> > every driver and hoping they get it right.
> 
> I was about to consider a similar approach before deciding to let drivers manage
> the logic, i.e. to prevent loosing flexibility later when dealing with HDMI 2.1.
> That was mostly in the context of supporting drivers to define if/when a display
> mode that fits in TMDS should be sent over FRL. 
> 
> Thinking again, that's not really a valid concern right now, e.g. will use TMDS
> by default for all supported modes, and switch to FRL only when absolutely
> required.  Later we might consider extending the infrastructure to support
> dynamic control if required.

Thanks for working on FRL as well :)

I agree, let's focus on getting HDMI 2.0 right, and then we'll try to
make 2.1 the easiest to work with for drivers.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Sakari Ailus @ 2026-06-25  7:24 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Frank Li, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624222042.GN851255@killaraus.ideasonboard.com>

Hi Laurent,

On Thu, Jun 25, 2026 at 01:20:42AM +0300, Laurent Pinchart wrote:
> On Wed, Jun 24, 2026 at 03:46:48PM -0500, Frank Li wrote:
> > On Wed, Jun 24, 2026 at 11:02:37PM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> > > > On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > > > > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > > > > simplify media code.
> > > > > >
> > > > > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > > > > rkisp1-dev.c only silience improvement.
> > > > > >
> > > > > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > > > > >
> > > > > > Build test only.
> > > > > >
> > > > > > Sakari Ailus:
> > > > > > 	when I try to improve the patch
> > > > > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > > > > camss.c pattern, so I create this small improvement firstly.
> > > > >
> > > > > Those are nice cleanups, thank you.
> > > > >
> > > > > After applying this series, the only left users of the
> > > > > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> > > >
> > > > I already checked previously, two place use it.
> > > >
> > > > fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> > > > ep is NULL, which totally equial to scoped() version.
> > > >
> > > > another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> > > > caller to call put().
> > > >
> > > > if use scoped() version, need use no_free_ptr() at return, which make think
> > > > a little bit complex.
> > >
> > > It would introduce a tiny bit of extra complexity there, but the
> > > advantage (in my opinion) is that we'll be able to remove the less safe
> > > fwnode_graph_for_each_endpoint() macro.
> > >
> > > Now one may argue that the risk of
> > > fwnode_graph_for_each_endpoint_scoped() is returning the iterator
> > > without using no_free_ptr(). I wonder if that would be easier to catch
> > > in static analysis tools than the current pattern that leaks a reference
> > > when exiting the loop early.
> > 
> > It's not big deal, if everyone prefer drop fwnode_graph_for_each_endpoint(),
> > I can do it.
> 
> Let's see what others think. If people prefer keeping both versions,
> I'll be OK with that.

I'd prefer to keep both: it depends on the use case which one is better. 

-- 
Kind regards,

Sakari Ailus

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

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

On Fri, Jun 19, 2026 at 11:13:40AM +0200, Gerald Loacker wrote:
> 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

Missing default here. If default is unknown, explain that in commit msg.

Best regards,
Krzysztof


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 1/4] device property: Introduce fwnode_graph_for_each_endpoint_scoped()
From: Andy Shevchenko @ 2026-06-25  6:33 UTC (permalink / raw)
  To: Frank.Li
  Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Mauro Carvalho Chehab,
	Dafna Hirschfeld, Laurent Pinchart, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624-fw_scoped-v2-1-0a8db472af4a@nxp.com>

On Wed, Jun 24, 2026 at 01:00:09PM -0400, Frank.Li@oss.nxp.com wrote:

> Similar to recently propose for_each_child_of_node_scoped() this new
> version of the loop macro instantiates a new local struct fwnode_handle *
> that uses the __free(fwnode_handle) auto cleanup handling so that if a
> reference to a node is held on early exit from the loop the reference will
> be released. If the loop runs to completion, the child pointer will be NULL
> and no action will be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits. If there is a need to retain the
> reference, then return_ptr(child) or no_free_ptr(child) may be used to
> safely disable the auto cleanup.

...

> +#define fwnode_graph_for_each_endpoint_scoped(fwnode, child)			\
> +	for (struct fwnode_handle *child __free(fwnode_handle) =		\
> +		fwnode_graph_get_next_endpoint(fwnode, NULL);		\

Now there is a misindentation of the \, id est an additional tab is missing.

> +	     child; child = fwnode_graph_get_next_endpoint(fwnode, child))

Collect more tags and send a v3 :-)

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Andy Shevchenko @ 2026-06-25  6:31 UTC (permalink / raw)
  To: Frank Li
  Cc: Laurent Pinchart, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <ajxCOE3avXXLlrfT@SMW015318>

On Wed, Jun 24, 2026 at 03:46:48PM -0500, Frank Li wrote:
> On Wed, Jun 24, 2026 at 11:02:37PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> > > On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > > > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > > > simplify media code.
> > > > >
> > > > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > > > rkisp1-dev.c only silience improvement.
> > > > >
> > > > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > > > >
> > > > > Build test only.
> > > > >
> > > > > Sakari Ailus:
> > > > > 	when I try to improve the patch
> > > > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > > > camss.c pattern, so I create this small improvement firstly.
> > > >
> > > > Those are nice cleanups, thank you.
> > > >
> > > > After applying this series, the only left users of the
> > > > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> > >
> > > I already checked previously, two place use it.
> > >
> > > fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> > > ep is NULL, which totally equial to scoped() version.
> > >
> > > another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> > > caller to call put().
> > >
> > > if use scoped() version, need use no_free_ptr() at return, which make think
> > > a little bit complex.
> >
> > It would introduce a tiny bit of extra complexity there, but the
> > advantage (in my opinion) is that we'll be able to remove the less safe
> > fwnode_graph_for_each_endpoint() macro.
> >
> > Now one may argue that the risk of
> > fwnode_graph_for_each_endpoint_scoped() is returning the iterator
> > without using no_free_ptr(). I wonder if that would be easier to catch
> > in static analysis tools than the current pattern that leaks a reference
> > when exiting the loop early.
> 
> It's not big deal, if everyone prefer drop fwnode_graph_for_each_endpoint(),
> I can do it.

I slightly tend to the safest option (see below), but as a compromise I can
suggest to inline the fwnode_graph_for_each_endpoint() into that single user
that doesn't need a put. However, this may uglify the code and rise a question
of the consistency. So, consider that suggestion with grain of salt and apply
only if we have wider agreement with it.

> > > It'd better leave these as it.

TL;DR:
This is the safest option, of course. And as mentioned above I slightly
prefer this way. Another argument is that in some cases we might want to
have it in the future and since we have an existing user, let it live.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Laurent Pinchart @ 2026-06-24 22:20 UTC (permalink / raw)
  To: Frank Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <ajxCOE3avXXLlrfT@SMW015318>

On Wed, Jun 24, 2026 at 03:46:48PM -0500, Frank Li wrote:
> On Wed, Jun 24, 2026 at 11:02:37PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> > > On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > > > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > > > simplify media code.
> > > > >
> > > > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > > > rkisp1-dev.c only silience improvement.
> > > > >
> > > > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > > > >
> > > > > Build test only.
> > > > >
> > > > > Sakari Ailus:
> > > > > 	when I try to improve the patch
> > > > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > > > camss.c pattern, so I create this small improvement firstly.
> > > >
> > > > Those are nice cleanups, thank you.
> > > >
> > > > After applying this series, the only left users of the
> > > > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> > >
> > > I already checked previously, two place use it.
> > >
> > > fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> > > ep is NULL, which totally equial to scoped() version.
> > >
> > > another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> > > caller to call put().
> > >
> > > if use scoped() version, need use no_free_ptr() at return, which make think
> > > a little bit complex.
> >
> > It would introduce a tiny bit of extra complexity there, but the
> > advantage (in my opinion) is that we'll be able to remove the less safe
> > fwnode_graph_for_each_endpoint() macro.
> >
> > Now one may argue that the risk of
> > fwnode_graph_for_each_endpoint_scoped() is returning the iterator
> > without using no_free_ptr(). I wonder if that would be easier to catch
> > in static analysis tools than the current pattern that leaks a reference
> > when exiting the loop early.
> 
> It's not big deal, if everyone prefer drop fwnode_graph_for_each_endpoint(),
> I can do it.

Let's see what others think. If people prefer keeping both versions,
I'll be OK with that.

> > > It'd better leave these as it.

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH V14 0/9] Add Invensense ICM42607
From: Chris Morgan @ 2026-06-24 21:18 UTC (permalink / raw)
  To: Chris Morgan
  Cc: linux-iio, andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
	linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
	andriy.shevchenko
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>

On Wed, Jun 24, 2026 at 01:23:39PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for the ICM42607 IMU. This sensor shares the same
> functionality but a different register layout with the existing
> ICM42600.
> 
> This driver should work with the ICM42607 and ICM42607P over both I2C
> and SPI, however only the ICM42607P over I2C could be tested.
> 
> Changes Since V1:
>  - Instead of creating a new driver, merged with the existing inv_icm42600
>    driver. This necessitated adding some code to the existing driver to
>    permit using a different register layout for the same functionality.
>  - Split changes up a bit more to decrease the size of the individual
>    patches. Note that patch 0004 is still pretty hefty; if I need to split
>    further I may need to create some temporary stub functions.
>  - Used guard() and PM_RUNTIME_ACQUIRE_AUTOSUSPEND() on the new functions
>    per Jonathan's recommendations.
> 
> Changes Since V2:
>  - Went back to using a new driver on advice from Invensense engineer.
>  - Further split changes up into smaller chunks of functionality. Note
>    still that the largest patch is approximately 900 lines, and that while
>    the driver compiles cleanly at each commit it is not able to drive the
>    hardware until the commit that adds the Interrupt (as it also adds the
>    Makefile).
>  - Change the error to a warning when the devicetree binding does not match
>    the hardware ID.
>  - Dropped the ack on the devicetree bindings, as I am creating a new file
>    (for a new driver) instead of modifying the existing one.
> 
> Changes Since V3:
>  - Numerous small fixes (too many to list here). Thank you to everyone who
>    provided feedback.
>  - Split power management additions into an additional commit to break
>    things up further.
>  - Consolidated devicetree documentation in existing
>    invensense,icm42600.yaml file.
>  - Removed most of the FIELD_PREP from header file to c files to make code
>    easier to read.
>  - Changed scale values to 2D arrays for Gyro and Accelerometer.
>  - Removed IIO_CHAN_INFO_CALIBBIAS attribute.
> 
> Changes Since V4:
>  - Additional numerous small fixes, thank you again for all the feedback.
>  - Dropped power control API and instead run device in low noise mode.
>  - Split devicetree bindings into two distinct changes.
>  - Reordered adding of enums and structs to main header file so that they
>    are only brought in when needed.
>  - Stopped using enum for driver data and instead am using pointer to
>    device specific driver data.
> 
> Changes Since V5:
>  - Corrected use of "dev_warn_probe" to just "dev_warn".
>  - Fixed some return scenarios which would unconditionally return 0
>    when an error was present.
>  - Corrected use of max() to min() for bounds checking. max() was
>    incorrect.
>  - Fixed using "st->conf.accel.odr" in the gyroscope function. It
>    should have been "st->conf.gyro.odr" which it now is.
>  - Additional small fixes suggested by "sashiko.dev".
>  - Added a regmap cache. I used the datasheet to try and determine
>    which registers might change without explicit writes.
> 
> Changes Since V6:
>  - Corrected additional errors identified by sashiko.dev, mostly
>    fixing potential deadlocks, missing calls for pm runtime, and
>    potential overflow issues.
> 
> Changes Since V7:
>  - Dropped Wake on Movement patches, since some of the functionality
>    was only available for a device on which I cannot test.
>  - Dropped support for SPI 3-Wire mode, since it complicated the
>    bus setup (and I lack the hardware to test such features anyway).
>  - Fixed a few additional bugs identified by sashiko.dev bot.
> 
> Changes Since V8:
>  - Added back IRQ dropped accidentally when dropping wake on movement
>    patches.
>  - Dropped "Reviewed-By" tag on patch 2 because of substantial changes
>    made to devicetree binding documentation.
>  - Additional small fixes as suggested.
> 
> Changes Since V9:
>  - Removed interrupts (and buffers) from the driver. I previously was
>    unable to detect deadlocks because it turns out my IRQ was not even
>    wired correctly in my device.
>  - Updated devicetree binding commits to make interrupts optional for
>    users of the icm42607 driver.
> 
> Changes Since V10:
>  - Explicitly specified enum values in header file.
>  - Removed additional dead code for buffer handling.
>  - Cleaned up headers.
>  - Added additional locks as requested by sashiko.dev bot.
> 
> Changes Since V11:
>  - Since driver has shrunk in size considerably, moved i2c bits into
>    first code commit. This ensures that the very first commit with code
>    can now be compiled. The commit after that adds SPI support as it
>    was in the previous versions.
>  - Used pahole to optimize inv_icm42607_state. Reordering elements
>    reduced size in memory from 384 bytes to 256 bytes.
>  - Added a map of all readable registers and all writeable registers
>    according to the datasheet.
>  - Added back some missing headers pointed out by the maintainers.
>  - Added FIELD_PREP in a few more places to make the code more
>    obvious on what it's doing.
>  - Added a comment to the power management code to note that
>    temperature sensor being enabled doesn't matter as the clocks
>    are off by default when the gyro and accel channels are off.
>  - Removed iio_device_claim_direct() calls since it was no longer
>    needed.
>  - Fixed shared_by_all attributes for temperature sensor.
>  - Additional miscellanous fixes as requested.
> 
> Changes Since V12:
>  - Removed aligned buffer from inv_icm42607_state struct as we do not
>    currently have the need for it.
>  - Corrected the order of the odr values in the accel and gyro files
>    as the values were out of order (the place in the array corresponds
>    to the register value).
>  - Stopped setting the clock value depending upon the temp config. The
>    datasheet advised to keep using the default value.
>  - Corrected logic when changing between states. We only need to pause
>    when a sensor changes from off to an on state or when the gyro
>    changes from an on state to off.
>  - Added missing includes for several files.
> 
> Changes Since V13:
>  - Refactored inv_icm42607_set_accel_conf() and
>    inv_icm42607_set_gyro_conf() into a single function.
>  - Refactored inv_icm42607_accel_read_sensor() and
>    inv_icm42607_gyro_read_sensor() into a single function.
>  - Merged inv_icm42607_set_temp_conf() into initial init function
>    since it only really needs to be called once.
>  - Saved adding temp sensor for last and updated
>    inv_icm42607_temp_read() to either confirm other sensors are already
>    enabled or enable the accelerometer so it can get a reading.
>  - Updated inv_icm42607_set_pwr_mgmt0() so that it does not update the
>    sensor mode and forcibly keep the sensor enabled.
>  - Added inv_icm42607_temp_filter_bw enums since it appears to use
>    different values than the accel or gyro sensor.
>  - Set the temp startup time from 77ms to 77us, as I previously misread
>    the datasheet.
>  - Additional minor fixes.
> 
> Chris Morgan (9):
>   dt-bindings: iio: imu: icm42600: Add mount-matrix
>   dt-bindings: iio: imu: icm42600: Add icm42607
>   iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
>   iio: imu: inv_icm42607: Add SPI For icm42607
>   iio: imu: inv_icm42607: Add PM support for icm42607
>   iio: imu: inv_icm42607: Add Accelerometer for icm42607
>   iio: imu: inv_icm42607: Add Gyroscope to icm42607
>   iio: imu: inv_icm42607: Add Temp Support in icm42607
>   arm64: dts: rockchip: Add icm42607p IMU for RG-DS
> 
>  .../bindings/iio/imu/invensense,icm42600.yaml |  20 +-
>  .../dts/rockchip/rk3568-anbernic-rg-ds.dts    |   8 +-
>  drivers/iio/imu/Kconfig                       |   1 +
>  drivers/iio/imu/Makefile                      |   1 +
>  drivers/iio/imu/inv_icm42607/Kconfig          |  30 +
>  drivers/iio/imu/inv_icm42607/Makefile         |  13 +
>  drivers/iio/imu/inv_icm42607/inv_icm42607.h   | 423 ++++++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 317 +++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 606 ++++++++++++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_gyro.c  | 313 +++++++++
>  .../iio/imu/inv_icm42607/inv_icm42607_i2c.c   |  98 +++
>  .../iio/imu/inv_icm42607/inv_icm42607_spi.c   | 108 ++++
>  .../iio/imu/inv_icm42607/inv_icm42607_temp.c  |  99 +++
>  .../iio/imu/inv_icm42607/inv_icm42607_temp.h  |  37 ++
>  14 files changed, 2072 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/imu/inv_icm42607/Kconfig
>  create mode 100644 drivers/iio/imu/inv_icm42607/Makefile
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
>  create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
> 
> -- 
> 2.43.0
> 

I got sashiko.dev's review, and agree with everything except 2 points.

1) It asks "Is it appropriate to omit the interrupts property based on
driver behavior rather than hardware wiring?" I would say no, but in
my case the hardware is not wired for an interrupt so it's moot.

2) It asks "Does this code update the software state cache for the
sensor mode?" No, it doesn't. The goal is for the device to stay
powered up for up to 2 seconds until runtime PM turns it off if it
isn't active, but I don't mind something turning around and shutting
the hardware down if it's not in use before then. Anything that needs
to read the specific sensor in question will be holding the mutex so
that there shouldn't be a risk of another thread turning stuff off
while I'm trying to use it, I don't think. What I want to avoid is
calling to turn the hardware on for a one-shot read and then
immediately turning it off once I'm done. I can wait the two seconds
for runtime PM to do it when the alternative is to call the shutdown
routine every single time for one-shot reads and wait for the startup
and in the case of the gyro shutdown times otherwise.

Thoughts?

I can fix the remaining issues it points out and resubmit.

Thank you,
Chris

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Frank Li @ 2026-06-24 20:46 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624200237.GJ851255@killaraus.ideasonboard.com>

On Wed, Jun 24, 2026 at 11:02:37PM +0300, Laurent Pinchart wrote:
> On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> > On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > > simplify media code.
> > > >
> > > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > > rkisp1-dev.c only silience improvement.
> > > >
> > > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > > >
> > > > Build test only.
> > > >
> > > > Sakari Ailus:
> > > > 	when I try to improve the patch
> > > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > > camss.c pattern, so I create this small improvement firstly.
> > >
> > > Those are nice cleanups, thank you.
> > >
> > > After applying this series, the only left users of the
> > > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> >
> > I already checked previously, two place use it.
> >
> > fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> > ep is NULL, which totally equial to scoped() version.
> >
> > another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> > caller to call put().
> >
> > if use scoped() version, need use no_free_ptr() at return, which make think
> > a little bit complex.
>
> It would introduce a tiny bit of extra complexity there, but the
> advantage (in my opinion) is that we'll be able to remove the less safe
> fwnode_graph_for_each_endpoint() macro.
>
> Now one may argue that the risk of
> fwnode_graph_for_each_endpoint_scoped() is returning the iterator
> without using no_free_ptr(). I wonder if that would be easier to catch
> in static analysis tools than the current pattern that leaks a reference
> when exiting the loop early.

It's not big deal, if everyone prefer drop fwnode_graph_for_each_endpoint(),
I can do it.

Frank

>
> > It'd better leave these as it.
>
> --
> Regards,
>
> Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Laurent Pinchart @ 2026-06-24 20:02 UTC (permalink / raw)
  To: Frank Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <ajwxcn2LXS8InAjZ@SMW015318>

On Wed, Jun 24, 2026 at 02:35:14PM -0500, Frank Li wrote:
> On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > > simplify media code.
> > >
> > > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > > rkisp1-dev.c only silience improvement.
> > >
> > > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> > >
> > > Build test only.
> > >
> > > Sakari Ailus:
> > > 	when I try to improve the patch
> > > "Add common helper library for 1-to-1 subdev registration", I found need
> > > camss.c pattern, so I create this small improvement firstly.
> >
> > Those are nice cleanups, thank you.
> >
> > After applying this series, the only left users of the
> > fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
> 
> I already checked previously, two place use it.
> 
> fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
> ep is NULL, which totally equial to scoped() version.
> 
> another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
> caller to call put().
> 
> if use scoped() version, need use no_free_ptr() at return, which make think
> a little bit complex.

It would introduce a tiny bit of extra complexity there, but the
advantage (in my opinion) is that we'll be able to remove the less safe
fwnode_graph_for_each_endpoint() macro.

Now one may argue that the risk of
fwnode_graph_for_each_endpoint_scoped() is returning the iterator
without using no_free_ptr(). I wonder if that would be easier to catch
in static analysis tools than the current pattern that leaks a reference
when exiting the loop early.

> It'd better leave these as it.

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Frank Li @ 2026-06-24 19:35 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624191935.GG851255@killaraus.ideasonboard.com>

On Wed, Jun 24, 2026 at 10:19:35PM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> > Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> > simplify media code.
> >
> > Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> > rkisp1-dev.c only silience improvement.
> >
> > Anyways, *_for_each_*_scoped() already use widely and make code clean.
> >
> > Build test only.
> >
> > Sakari Ailus:
> > 	when I try to improve the patch
> > "Add common helper library for 1-to-1 subdev registration", I found need
> > camss.c pattern, so I create this small improvement firstly.
>
> Those are nice cleanups, thank you.
>
> After applying this series, the only left users of the
> fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.

I already checked previously, two place use it.

fwnode_graph_get_endpoint_count(), it will go though all endpoints, last
ep is NULL, which totally equial to scoped() version.

another one fwnode_graph_get_endpoint_by_id(), which return ep, expect
caller to call put().

if use scoped() version, need use no_free_ptr() at return, which make think
a little bit complex.

It'd better leave these as it.

Frank

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* [PATCH v2 1/2] dt-bindings: usb: Add Rockchip RK3568 compatible for EHCI and OHCI
From: Jonas Karlman @ 2026-06-24 19:27 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Greg Kroah-Hartman
  Cc: Diederik de Haas, devicetree, linux-rockchip, linux-usb,
	linux-arm-kernel, linux-kernel, Jonas Karlman
In-Reply-To: <20260624192726.781864-1-jonas@kwiboo.se>

The Rockchip RK3568 EHCI/OHCI controller depends on clk_usbphy1_480m
being enabled, or the system may freeze when registers are accessed.

Add Rockchip RK3568 EHCI and OHCI compatibles with a similar four-clock
constraint as RK3588, also extend the EHCI constraint to include RK3588
to match similar requirements of RK3588.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
Existing DTs for RK3568 use the plain generic-ehci/ohci compatible,
next patch make use of these new compatibles and adds the missing
clk_usbphy1_480m clock references.

Existing DTs for RK3588 have contained the required four clocks since
the initial addition of the EHCI/OHCI nodes.

Changes in v2:
- Include rockchip,rk3588-ehci in the EHCI constraint
- Make clocks prop required for EHCI and OHCI
---
 .../devicetree/bindings/usb/generic-ehci.yaml      | 14 ++++++++++++++
 .../devicetree/bindings/usb/generic-ohci.yaml      |  7 ++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
index 55a5aa7d7a54..a39f01e740b1 100644
--- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
@@ -52,6 +52,7 @@ properties:
               - ibm,476gtr-ehci
               - nxp,lpc1850-ehci
               - qca,ar7100-ehci
+              - rockchip,rk3568-ehci
               - rockchip,rk3588-ehci
               - snps,hsdk-v1.0-ehci
               - socionext,uniphier-ehci
@@ -186,6 +187,19 @@ allOf:
       required:
         - clocks
         - clock-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - rockchip,rk3568-ehci
+              - rockchip,rk3588-ehci
+    then:
+      properties:
+        clocks:
+          minItems: 4
+      required:
+        - clocks
 
 unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
index d42f448fa204..19449a6b3033 100644
--- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
@@ -47,6 +47,7 @@ properties:
               - hpe,gxp-ohci
               - ibm,476gtr-ohci
               - ingenic,jz4740-ohci
+              - rockchip,rk3568-ohci
               - rockchip,rk3588-ohci
               - snps,hsdk-v1.0-ohci
           - const: generic-ohci
@@ -198,11 +199,15 @@ allOf:
       properties:
         compatible:
           contains:
-            const: rockchip,rk3588-ohci
+            enum:
+              - rockchip,rk3568-ohci
+              - rockchip,rk3588-ohci
     then:
       properties:
         clocks:
           minItems: 4
+      required:
+        - clocks
     else:
       properties:
         clocks:
-- 
2.54.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* [PATCH v2 0/2] rockchip: Fix devices suspend freeze on RK3568/RK3566
From: Jonas Karlman @ 2026-06-24 19:27 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Diederik de Haas, Greg Kroah-Hartman, devicetree, linux-rockchip,
	linux-usb, linux-arm-kernel, linux-kernel, Jonas Karlman

This series fixes a system freeze during suspend in ohci_suspend() due
to clk_usbphy1_480m not being enabled when EHCI/OHCI registers are
accessed on e.g. a Raxa ROCK 3C board.

Following pm_test modes work on my ROCK 3C with the missing usbphy clk
refs added:

  echo N > /sys/module/printk/parameters/console_suspend

  echo devices > /sys/power/pm_test
  echo platform > /sys/power/pm_test
  echo processors > /sys/power/pm_test
  echo core > /sys/power/pm_test

  echo mem > /sys/power/state

Changes in v2:
- Include rockchip,rk3588-ehci in the EHCI constraint
- Make clocks prop required for EHCI and OHCI
- Collect t-b tag

Jonas Karlman (2):
  dt-bindings: usb: Add Rockchip RK3568 compatible for EHCI and OHCI
  arm64: dts: rockchip: Fix devices suspend freeze on RK3568/RK3566

 .../devicetree/bindings/usb/generic-ehci.yaml    | 14 ++++++++++++++
 .../devicetree/bindings/usb/generic-ohci.yaml    |  7 ++++++-
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi    | 16 ++++++++--------
 3 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.54.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* [PATCH v2 2/2] arm64: dts: rockchip: Fix devices suspend freeze on RK3568/RK3566
From: Jonas Karlman @ 2026-06-24 19:27 UTC (permalink / raw)
  To: Heiko Stuebner, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Peter Geis, Michael Riesch
  Cc: Diederik de Haas, Greg Kroah-Hartman, devicetree, linux-rockchip,
	linux-usb, linux-arm-kernel, linux-kernel, Jonas Karlman
In-Reply-To: <20260624192726.781864-1-jonas@kwiboo.se>

The EHCI/OHCI controller depends on clk_usbphy1_480m being enabled, or
the system may freeze when registers are accessed, i.e. during suspend
in ohci_suspend().

Add the missing clk_usbphy1_480m clocks reference to EHCI/OHCI
controllers to ensure the clock is enabled when ECHI/OHCI registers are
accessed to prevent a system freeze.

Fixes suspend pm_test issue with EHCI/OHCI devices due to the missing
clk_usbphy1_480m reference and makes following pm_test modes work:

  echo N > /sys/module/printk/parameters/console_suspend

  echo devices > /sys/power/pm_test
  echo platform > /sys/power/pm_test
  echo processors > /sys/power/pm_test
  echo core > /sys/power/pm_test

  echo mem > /sys/power/state

Fixes: 91c4c3e06a25 ("arm64: dts: rockchip: add usb2 nodes to rk3568 device tree")
Fixes: 78f7186095db ("arm64: dts: rockchip: rename and sort the rk356x usb2 phy handles")
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2: Collect t-b tag
---
 arch/arm64/boot/dts/rockchip/rk356x-base.dtsi | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
index a5832895bd39..c930a6fd6ea0 100644
--- a/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk356x-base.dtsi
@@ -321,44 +321,44 @@ its: msi-controller@fd440000 {
 	};
 
 	usb_host0_ehci: usb@fd800000 {
-		compatible = "generic-ehci";
+		compatible = "rockchip,rk3568-ehci", "generic-ehci";
 		reg = <0x0 0xfd800000 0x0 0x40000>;
 		interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
-			 <&cru PCLK_USB>;
+			 <&cru PCLK_USB>, <&usb2phy1>;
 		phys = <&usb2phy1_otg>;
 		phy-names = "usb";
 		status = "disabled";
 	};
 
 	usb_host0_ohci: usb@fd840000 {
-		compatible = "generic-ohci";
+		compatible = "rockchip,rk3568-ohci", "generic-ohci";
 		reg = <0x0 0xfd840000 0x0 0x40000>;
 		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST0>, <&cru HCLK_USB2HOST0_ARB>,
-			 <&cru PCLK_USB>;
+			 <&cru PCLK_USB>, <&usb2phy1>;
 		phys = <&usb2phy1_otg>;
 		phy-names = "usb";
 		status = "disabled";
 	};
 
 	usb_host1_ehci: usb@fd880000 {
-		compatible = "generic-ehci";
+		compatible = "rockchip,rk3568-ehci", "generic-ehci";
 		reg = <0x0 0xfd880000 0x0 0x40000>;
 		interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
-			 <&cru PCLK_USB>;
+			 <&cru PCLK_USB>, <&usb2phy1>;
 		phys = <&usb2phy1_host>;
 		phy-names = "usb";
 		status = "disabled";
 	};
 
 	usb_host1_ohci: usb@fd8c0000 {
-		compatible = "generic-ohci";
+		compatible = "rockchip,rk3568-ohci", "generic-ohci";
 		reg = <0x0 0xfd8c0000 0x0 0x40000>;
 		interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cru HCLK_USB2HOST1>, <&cru HCLK_USB2HOST1_ARB>,
-			 <&cru PCLK_USB>;
+			 <&cru PCLK_USB>, <&usb2phy1>;
 		phys = <&usb2phy1_host>;
 		phy-names = "usb";
 		status = "disabled";
-- 
2.54.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* Re: [PATCH v2 0/4] media: add and use fwnode_graph_for_each_endpoint_scoped()
From: Laurent Pinchart @ 2026-06-24 19:19 UTC (permalink / raw)
  To: Frank.Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624-fw_scoped-v2-0-0a8db472af4a@nxp.com>

Hi Frank,

On Wed, Jun 24, 2026 at 01:00:08PM -0400, Frank.Li@oss.nxp.com wrote:
> Add new helper macro fwnode_graph_for_each_endpoint_scoped() and use it
> simplify media code.
> 
> Typical example should qualcomm's driver (camss.c), the v4l2_mc.c and
> rkisp1-dev.c only silience improvement.
> 
> Anyways, *_for_each_*_scoped() already use widely and make code clean.
> 
> Build test only.
> 
> Sakari Ailus:
> 	when I try to improve the patch
> "Add common helper library for 1-to-1 subdev registration", I found need
> camss.c pattern, so I create this small improvement firstly.

Those are nice cleanups, thank you.

After applying this series, the only left users of the
fwnode_graph_for_each_endpoint() macro are in drivers/base/property.c.
They can all be trivially replaced with the scoped variant. Should we
add a patch to use fwnode_graph_for_each_endpoint_scoped() everywhere,
and drop fwnode_graph_for_each_endpoint() ?

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v2:
> - colllect review by tags
> - fix typo and indent.
> - see each patch's change log.
> - Link to v1: https://patch.msgid.link/20260622-fw_scoped-v1-0-a37d0aac0a68@nxp.com
> 
> ---
> Frank Li (4):
>       device property: Introduce fwnode_graph_for_each_endpoint_scoped()
>       media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code
>       media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code
>       media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code
> 
>  drivers/media/platform/qcom/camss/camss.c           | 17 +++++------------
>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c |  4 +---
>  drivers/media/v4l2-core/v4l2-mc.c                   |  5 +----
>  include/linux/property.h                            |  5 +++++
>  4 files changed, 12 insertions(+), 19 deletions(-)
> ---
> base-commit: 3ce97bd3c4f18608335e709c24d6a40e7036cab8
> change-id: 20260620-fw_scoped-5dab644510a1

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 4/4] media: qcom: camss: use fwnode_graph_for_each_endpoint_scoped() to simplify code
From: Laurent Pinchart @ 2026-06-24 19:17 UTC (permalink / raw)
  To: Frank.Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624-fw_scoped-v2-4-0a8db472af4a@nxp.com>

On Wed, Jun 24, 2026 at 01:00:12PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use fwnode_graph_for_each_endpoint_scoped() to simplify code.
> 
> No functional changes.
> 
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> change in v2
> - fix typo simplify
> - collect andy, gouniou and loic's review tags
> ---
>  drivers/media/platform/qcom/camss/camss.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 2123f6388e3d7..23f3cc30a15a5 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -4793,30 +4793,23 @@ static int camss_parse_endpoint_node(struct device *dev,
>  static int camss_parse_ports(struct camss *camss)
>  {
>  	struct device *dev = camss->dev;
> -	struct fwnode_handle *fwnode = dev_fwnode(dev), *ep;
> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
>  	int ret;
>  
> -	fwnode_graph_for_each_endpoint(fwnode, ep) {
> +	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
>  		struct camss_async_subdev *csd;
>  
>  		csd = v4l2_async_nf_add_fwnode_remote(&camss->notifier, ep,
>  						      typeof(*csd));
> -		if (IS_ERR(csd)) {
> -			ret = PTR_ERR(csd);
> -			goto err_cleanup;
> -		}
> +		if (IS_ERR(csd))
> +			return PTR_ERR(csd);
>  
>  		ret = camss_parse_endpoint_node(dev, ep, csd);
>  		if (ret < 0)
> -			goto err_cleanup;
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -err_cleanup:
> -	fwnode_handle_put(ep);
> -
> -	return ret;
>  }
>  
>  /*

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 3/4] media: rkisp1: use fwnode_graph_for_each_endpoint_scoped() to simplify code
From: Laurent Pinchart @ 2026-06-24 19:16 UTC (permalink / raw)
  To: Frank.Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li
In-Reply-To: <20260624-fw_scoped-v2-3-0a8db472af4a@nxp.com>

On Wed, Jun 24, 2026 at 01:00:11PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use fwnode_graph_for_each_endpoint_scoped() to simplify code.
> 
> No functional changes.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> Andy: Keep original code because too much break. and I am working on more
> generally solution, so the whole function can be replaced.
> ---
>  drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index 1791c02a40ae1..f90e01301943c 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -187,7 +187,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  {
>  	struct v4l2_async_notifier *ntf = &rkisp1->notifier;
>  	struct fwnode_handle *fwnode = dev_fwnode(rkisp1->dev);
> -	struct fwnode_handle *ep;
>  	unsigned int index = 0;
>  	int ret = 0;
>  
> @@ -195,7 +194,7 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  
>  	ntf->ops = &rkisp1_subdev_notifier_ops;
>  
> -	fwnode_graph_for_each_endpoint(fwnode, ep) {
> +	fwnode_graph_for_each_endpoint_scoped(fwnode, ep) {
>  		struct fwnode_handle *port;
>  		struct v4l2_fwnode_endpoint vep = { };
>  		struct rkisp1_sensor_async *rk_asd;
> @@ -286,7 +285,6 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
>  	}
>  
>  	if (ret) {
> -		fwnode_handle_put(ep);
>  		v4l2_async_nf_cleanup(ntf);
>  		return ret;
>  	}

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 2/4] media: mc: use fwnode_graph_for_each_endpoint_scoped() to simpilfy code
From: Laurent Pinchart @ 2026-06-24 19:14 UTC (permalink / raw)
  To: Frank.Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624-fw_scoped-v2-2-0a8db472af4a@nxp.com>

On Wed, Jun 24, 2026 at 01:00:10PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Use cleanup helper fwnode_graph_for_each_endpoint_scoped() to simpilfy
> code.
> 
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-mc.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
> index 937d358697e19..5d7fcd67dc42e 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -324,12 +324,10 @@ EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
>  int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
>  				    struct media_pad *sink, u32 flags)
>  {
> -	struct fwnode_handle *endpoint;
> -
>  	if (!(sink->flags & MEDIA_PAD_FL_SINK))
>  		return -EINVAL;
>  
> -	fwnode_graph_for_each_endpoint(src_sd->fwnode, endpoint) {
> +	fwnode_graph_for_each_endpoint_scoped(src_sd->fwnode, endpoint) {
>  		struct fwnode_handle *remote_ep;
>  		int src_idx, sink_idx, ret;
>  		struct media_pad *src;
> @@ -397,7 +395,6 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd,
>  				src_sd->entity.name, src_idx,
>  				sink->entity->name, sink_idx, ret);
>  
> -			fwnode_handle_put(endpoint);
>  			return ret;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* Re: [PATCH v2 1/4] device property: Introduce fwnode_graph_for_each_endpoint_scoped()
From: Laurent Pinchart @ 2026-06-24 19:13 UTC (permalink / raw)
  To: Frank.Li
  Cc: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
	Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
	Mauro Carvalho Chehab, Dafna Hirschfeld, Heiko Stuebner,
	Bryan O'Donoghue, Vladimir Zapolskiy, Loic Poulain,
	driver-core, linux-acpi, linux-kernel, linux-media,
	linux-rockchip, linux-arm-kernel, linux-arm-msm, imx, Guoniu Zhou,
	Frank Li, Guoniu Zhou
In-Reply-To: <20260624-fw_scoped-v2-1-0a8db472af4a@nxp.com>

Hi Frank,

Thank you for the patch.

On Wed, Jun 24, 2026 at 01:00:09PM -0400, Frank.Li@oss.nxp.com wrote:
> From: Frank Li <Frank.Li@nxp.com>
> 
> Similar to recently propose for_each_child_of_node_scoped() this new
> version of the loop macro instantiates a new local struct fwnode_handle *
> that uses the __free(fwnode_handle) auto cleanup handling so that if a
> reference to a node is held on early exit from the loop the reference will
> be released. If the loop runs to completion, the child pointer will be NULL
> and no action will be taken.
> 
> The reason this is useful is that it removes the need for
> fwnode_handle_put() on early loop exits. If there is a need to retain the
> reference, then return_ptr(child) or no_free_ptr(child) may be used to
> safely disable the auto cleanup.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Nice idea.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> change in v2
> - collect Andy and Guoniu's reviewed-by tags
> - fix indention
> - remove extra space in commit message
> ---
>  include/linux/property.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 14c304db46648..d51824c13d2cc 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -545,6 +545,11 @@ unsigned int fwnode_graph_get_endpoint_count(const struct fwnode_handle *fwnode,
>  	for (child = fwnode_graph_get_next_endpoint(fwnode, NULL); child;	\
>  	     child = fwnode_graph_get_next_endpoint(fwnode, child))
>  
> +#define fwnode_graph_for_each_endpoint_scoped(fwnode, child)			\
> +	for (struct fwnode_handle *child __free(fwnode_handle) =		\
> +		fwnode_graph_get_next_endpoint(fwnode, NULL);		\
> +	     child; child = fwnode_graph_get_next_endpoint(fwnode, child))
> +
>  int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
>  				struct fwnode_endpoint *endpoint);
>  

-- 
Regards,

Laurent Pinchart

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply

* [PATCH V14 8/9] iio: imu: inv_icm42607: Add Temp Support in icm42607
From: Chris Morgan @ 2026-06-24 18:23 UTC (permalink / raw)
  To: linux-iio
  Cc: andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
	linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
	andriy.shevchenko, Chris Morgan
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>

From: Chris Morgan <macromorgan@hotmail.com>

Add functions for reading temperature sensor data.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/iio/imu/inv_icm42607/Makefile         |  1 +
 .../iio/imu/inv_icm42607/inv_icm42607_accel.c |  8 ++
 .../iio/imu/inv_icm42607/inv_icm42607_gyro.c  |  8 ++
 .../iio/imu/inv_icm42607/inv_icm42607_temp.c  | 99 +++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_temp.h  | 37 +++++++
 5 files changed, 153 insertions(+)
 create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
 create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h

diff --git a/drivers/iio/imu/inv_icm42607/Makefile b/drivers/iio/imu/inv_icm42607/Makefile
index 8e73385c8f4b..7b907e019601 100644
--- a/drivers/iio/imu/inv_icm42607/Makefile
+++ b/drivers/iio/imu/inv_icm42607/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o
 inv-icm42607-y += inv_icm42607_core.o
 inv-icm42607-y += inv_icm42607_gyro.o
 inv-icm42607-y += inv_icm42607_accel.o
+inv-icm42607-y += inv_icm42607_temp.o
 
 obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o
 inv-icm42607-i2c-y += inv_icm42607_i2c.o
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
index 8ef9fdae1bc8..5ff6756b9515 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
@@ -15,6 +15,7 @@
 #include <linux/types.h>
 
 #include "inv_icm42607.h"
+#include "inv_icm42607_temp.h"
 
 #define INV_ICM42607_ACCEL_CHAN(_modifier, _index, _ext_info)			\
 {										\
@@ -40,6 +41,7 @@ enum inv_icm42607_accel_scan {
 	INV_ICM42607_ACCEL_SCAN_X,
 	INV_ICM42607_ACCEL_SCAN_Y,
 	INV_ICM42607_ACCEL_SCAN_Z,
+	INV_ICM42607_ACCEL_SCAN_TEMP,
 };
 
 static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = {
@@ -54,6 +56,7 @@ static const struct iio_chan_spec inv_icm42607_accel_channels[] = {
 				inv_icm42607_accel_ext_infos),
 	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z,
 				inv_icm42607_accel_ext_infos),
+	INV_ICM42607_TEMP_CHAN(INV_ICM42607_ACCEL_SCAN_TEMP),
 };
 
 static const int inv_icm42607_accel_scale_nano[][2] = {
@@ -186,6 +189,11 @@ static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
 	switch (chan->type) {
 	case IIO_ACCEL:
 		break;
+	case IIO_TEMP:
+		if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+			return inv_icm42607_temp_read_raw(indio_dev, chan,
+							  val, val2, mask);
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
index c7215b3826ad..4e5db5e19e9f 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
@@ -15,6 +15,7 @@
 #include <linux/types.h>
 
 #include "inv_icm42607.h"
+#include "inv_icm42607_temp.h"
 
 #define INV_ICM42607_GYRO_CHAN(_modifier, _index, _ext_info)			\
 {										\
@@ -40,6 +41,7 @@ enum inv_icm42607_gyro_scan {
 	INV_ICM42607_GYRO_SCAN_X,
 	INV_ICM42607_GYRO_SCAN_Y,
 	INV_ICM42607_GYRO_SCAN_Z,
+	INV_ICM42607_GYRO_SCAN_TEMP,
 };
 
 static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
@@ -54,6 +56,7 @@ static const struct iio_chan_spec inv_icm42607_gyro_channels[] = {
 			       inv_icm42607_gyro_ext_infos),
 	INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z,
 			       inv_icm42607_gyro_ext_infos),
+	INV_ICM42607_TEMP_CHAN(INV_ICM42607_GYRO_SCAN_TEMP),
 };
 
 static const int inv_icm42607_gyro_scale_nano[][2] = {
@@ -182,6 +185,11 @@ static int inv_icm42607_gyro_read_raw(struct iio_dev *indio_dev,
 	switch (chan->type) {
 	case IIO_ANGL_VEL:
 		break;
+	case IIO_TEMP:
+		if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+			return inv_icm42607_temp_read_raw(indio_dev, chan,
+							  val, val2, mask);
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
new file mode 100644
index 000000000000..f7e286ecb5cd
--- /dev/null
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2026 InvenSense, Inc.
+ */
+
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "inv_icm42607.h"
+#include "inv_icm42607_temp.h"
+
+static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct device *dev = regmap_get_device(st->map);
+	int ret, gyro_mode, accel_mode;
+	unsigned int val;
+	u8 raw[2];
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	/*
+	 * Check if both the gyro and accel are off and if so, enable one
+	 * of them. The temp sensor cannot be read if both the gyro and
+	 * accel sensor are off. Prefer to enable the accel over the gyro
+	 * as the datasheet says the gyro uses 5x more power and it has
+	 * a minimum run time of 45ms.
+	 */
+	ret = regmap_read(st->map, INV_ICM42607_REG_PWR_MGMT0, &val);
+	if (ret)
+		return ret;
+
+	accel_mode = FIELD_GET(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, val);
+	gyro_mode = FIELD_GET(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, val);
+	if (!gyro_mode && !accel_mode) {
+		/* enable accel sensor */
+		conf.mode = INV_ICM42607_SENSOR_MODE_LOW_NOISE;
+//		ret = inv_icm42607_set_accel_conf(st, &conf);
+		ret = inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
+		if (ret)
+			return ret;
+	}
+
+	ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1,
+			       raw, sizeof(raw));
+	if (ret)
+		return ret;
+
+	*temp = get_unaligned_be16(raw);
+	if (*temp == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}
+
+int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	s16 temp;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = inv_icm42607_temp_read(st, &temp);
+		if (ret)
+			return ret;
+		*val = temp;
+		return IIO_VAL_INT;
+	/*
+	 * T°C = (temp / 128) + 25
+	 * Tm°C = 1000 * ((temp * 100 / 12800) + 25)
+	 * scale: 100000 / 12800 ~= 7.8125
+	 * offset: 3200
+	 */
+	case IIO_CHAN_INFO_SCALE:
+		*val = 7;
+		*val2 = 812500000;
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = 3200;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
new file mode 100644
index 000000000000..cb7b460ffb44
--- /dev/null
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2026 InvenSense, Inc.
+ */
+
+#ifndef INV_ICM42607_TEMP_H_
+#define INV_ICM42607_TEMP_H_
+
+#include <linux/bitops.h>
+
+struct iio_dev;
+struct iio_chan_spec;
+
+#define INV_ICM42607_TEMP_CHAN(_index)				\
+{								\
+	.type = IIO_TEMP,					\
+	.info_mask_separate =					\
+		BIT(IIO_CHAN_INFO_RAW) |			\
+		BIT(IIO_CHAN_INFO_OFFSET) |			\
+		BIT(IIO_CHAN_INFO_SCALE),			\
+	.info_mask_shared_by_all =				\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_all_available =			\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.scan_index = _index,					\
+	.scan_type = {						\
+		.sign = 's',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+	},							\
+}
+
+int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int *val, int *val2, long mask);
+
+#endif
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* [PATCH V14 6/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
From: Chris Morgan @ 2026-06-24 18:23 UTC (permalink / raw)
  To: linux-iio
  Cc: andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
	linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
	andriy.shevchenko, Chris Morgan
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>

From: Chris Morgan <macromorgan@hotmail.com>

Add icm42607 accelerometer sensor for icm42607.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/iio/imu/inv_icm42607/Makefile         |   1 +
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |  38 +++
 .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 309 ++++++++++++++++++
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 169 ++++++++++
 4 files changed, 517 insertions(+)
 create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c

diff --git a/drivers/iio/imu/inv_icm42607/Makefile b/drivers/iio/imu/inv_icm42607/Makefile
index be109102e203..372c6d6bdcec 100644
--- a/drivers/iio/imu/inv_icm42607/Makefile
+++ b/drivers/iio/imu/inv_icm42607/Makefile
@@ -2,6 +2,7 @@
 
 obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o
 inv-icm42607-y += inv_icm42607_core.o
+inv-icm42607-y += inv_icm42607_accel.o
 
 obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o
 inv-icm42607-i2c-y += inv_icm42607_i2c.o
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
index 219cfcf7ac99..cd78b43f36fa 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
@@ -87,6 +87,17 @@ enum inv_icm42607_filter_bw {
 	INV_ICM42607_FILTER_BW_NB
 };
 
+/* Low-Power mode sensor data filter (averaging) */
+enum inv_icm42607_filter_avg {
+	INV_ICM42607_FILTER_AVG_2X = 0,
+	INV_ICM42607_FILTER_AVG_4X = 1,
+	INV_ICM42607_FILTER_AVG_8X = 2,
+	INV_ICM42607_FILTER_AVG_16X = 3,
+	INV_ICM42607_FILTER_AVG_32X = 4,
+	INV_ICM42607_FILTER_AVG_64X = 5,
+	/* values 6 and 7 also correspond to 64x. */
+};
+
 /* Temperature sensor data filter (bandwidth) */
 enum inv_icm42607_temp_filter_bw {
 	INV_ICM42607_TEMP_FILTER_BYPASS = 0,
@@ -106,6 +117,7 @@ struct inv_icm42607_sensor_conf {
 	int odr;
 	int filter;
 };
+#define INV_ICM42607_SENSOR_CONF_INIT		{ -1, -1, -1, -1 }
 
 struct inv_icm42607_conf {
 	struct inv_icm42607_sensor_conf gyro;
@@ -129,6 +141,7 @@ struct inv_icm42607_suspended {
  *  @hw:		Hardware specific data.
  *  @lock:		lock for serializing multiple registers access.
  *  @map:		regmap pointer.
+ *  @indio_accel:	accelerometer IIO device.
  *  @vddio_supply:	I/O voltage regulator for the chip.
  *  @vddio_en:		I/O voltage status for runtime PM.
  *  @suspended:		suspended sensors configuration.
@@ -139,6 +152,7 @@ struct inv_icm42607_state {
 	const struct inv_icm42607_hw *hw;
 	struct mutex lock;
 	struct regmap *map;
+	struct iio_dev *indio_accel;
 	struct regulator *vddio_supply;
 	bool vddio_en;
 	struct inv_icm42607_suspended suspended;
@@ -146,6 +160,16 @@ struct inv_icm42607_state {
 	struct iio_mount_matrix orientation;
 };
 
+/**
+ * struct inv_icm42607_sensor_state - sensor state variables
+ * @power_mode:		sensor requested power mode (for common frequencies)
+ * @filter:		sensor filter.
+ */
+struct inv_icm42607_sensor_state {
+	enum inv_icm42607_sensor_mode power_mode;
+	int filter;
+};
+
 /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
 
 /* Register Map for User Bank 0 */
@@ -374,8 +398,22 @@ extern const struct inv_icm42607_hw inv_icm42607_hw_data;
 extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
 extern const struct dev_pm_ops inv_icm42607_pm_ops;
 
+const struct iio_mount_matrix *
+inv_icm42607_get_mount_matrix(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan);
+
+int inv_icm42607_set_sensor_conf(struct inv_icm42607_state *st,
+				 struct inv_icm42607_sensor_conf *conf,
+				 enum iio_chan_type chan_type);
+
+int inv_icm42607_read_sensor(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     s16 *val);
+
 int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,
 			    inv_icm42607_bus_setup bus_setup);
 
+struct iio_dev *inv_icm42607_accel_init(struct inv_icm42607_state *st);
+
 #endif
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
new file mode 100644
index 000000000000..8ef9fdae1bc8
--- /dev/null
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2026 InvenSense, Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include "inv_icm42607.h"
+
+#define INV_ICM42607_ACCEL_CHAN(_modifier, _index, _ext_info)			\
+{										\
+	.type = IIO_ACCEL,							\
+	.modified = 1,								\
+	.channel2 = _modifier,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.scan_index = _index,							\
+	.scan_type = {								\
+		.sign = 's',							\
+		.realbits = 16,							\
+		.storagebits = 16,						\
+		.endianness = IIO_BE,						\
+	},									\
+	.ext_info = _ext_info,							\
+}
+
+enum inv_icm42607_accel_scan {
+	INV_ICM42607_ACCEL_SCAN_X,
+	INV_ICM42607_ACCEL_SCAN_Y,
+	INV_ICM42607_ACCEL_SCAN_Z,
+};
+
+static const struct iio_chan_spec_ext_info inv_icm42607_accel_ext_infos[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
+	{ }
+};
+
+static const struct iio_chan_spec inv_icm42607_accel_channels[] = {
+	INV_ICM42607_ACCEL_CHAN(IIO_MOD_X, INV_ICM42607_ACCEL_SCAN_X,
+				inv_icm42607_accel_ext_infos),
+	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Y, INV_ICM42607_ACCEL_SCAN_Y,
+				inv_icm42607_accel_ext_infos),
+	INV_ICM42607_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42607_ACCEL_SCAN_Z,
+				inv_icm42607_accel_ext_infos),
+};
+
+static const int inv_icm42607_accel_scale_nano[][2] = {
+	[INV_ICM42607_ACCEL_FS_16G] = { 0, 4788403 },
+	[INV_ICM42607_ACCEL_FS_8G] = { 0, 2394202 },
+	[INV_ICM42607_ACCEL_FS_4G] = { 0, 1197101 },
+	[INV_ICM42607_ACCEL_FS_2G] = { 0, 598550 },
+};
+
+static int inv_icm42607_accel_read_scale(struct iio_dev *indio_dev,
+					 int *val, int *val2)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	unsigned int idx;
+
+	guard(mutex)(&st->lock);
+
+	idx = st->conf.accel.fs;
+
+	*val = inv_icm42607_accel_scale_nano[idx][0];
+	*val2 = inv_icm42607_accel_scale_nano[idx][1];
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int inv_icm42607_accel_write_scale(struct iio_dev *indio_dev,
+					  int val, int val2)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	size_t scales_len = ARRAY_SIZE(inv_icm42607_accel_scale_nano);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int idx;
+	int ret;
+
+	for (idx = 0; idx < scales_len; idx++) {
+		if (val == inv_icm42607_accel_scale_nano[idx][0] &&
+		    val2 == inv_icm42607_accel_scale_nano[idx][1])
+			break;
+	}
+	if (idx >= scales_len)
+		return -EINVAL;
+
+	conf.fs = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
+}
+
+/* IIO format int + micro , values 0-5 reserved. */
+static const int inv_icm42607_accel_odr[][2] = {
+	[INV_ICM42607_ODR_1600HZ] = { 1600, 0 },
+	[INV_ICM42607_ODR_800HZ] = { 800, 0 },
+	[INV_ICM42607_ODR_400HZ] = { 400, 0 },
+	[INV_ICM42607_ODR_200HZ] = { 200, 0 },
+	[INV_ICM42607_ODR_100HZ] = { 100, 0 },
+	[INV_ICM42607_ODR_50HZ] = { 50, 0 },
+	[INV_ICM42607_ODR_25HZ] = { 25, 0 },
+	[INV_ICM42607_ODR_12_5HZ] = { 12, 500000 },
+	[INV_ICM42607_ODR_6_25HZ_LP] = { 6, 250000 },
+	[INV_ICM42607_ODR_3_125HZ_LP] = { 3, 125000 },
+	[INV_ICM42607_ODR_1_5625HZ_LP] = { 1, 562500 },
+};
+
+static int inv_icm42607_accel_read_odr(struct inv_icm42607_state *st,
+				       int *val, int *val2)
+{
+	unsigned int odr;
+	unsigned int i;
+
+	guard(mutex)(&st->lock);
+
+	odr = st->conf.accel.odr;
+
+	for (i = 5; i < ARRAY_SIZE(inv_icm42607_accel_odr); ++i) {
+		if (i == odr)
+			break;
+	}
+	if (i >= ARRAY_SIZE(inv_icm42607_accel_odr))
+		return -EINVAL;
+
+	*val = inv_icm42607_accel_odr[i][0];
+	*val2 = inv_icm42607_accel_odr[i][1];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev,
+					int val, int val2)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int idx;
+	int ret;
+
+	for (idx = 5; idx < ARRAY_SIZE(inv_icm42607_accel_odr); ++idx) {
+		if (val == inv_icm42607_accel_odr[idx][0] &&
+		    val2 == inv_icm42607_accel_odr[idx][1])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42607_accel_odr))
+		return -EINVAL;
+
+	conf.odr = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
+}
+
+static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
+				       struct iio_chan_spec const *chan,
+				       int *val, int *val2, long mask)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	s16 data;
+	int ret;
+
+	switch (chan->type) {
+	case IIO_ACCEL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = inv_icm42607_read_sensor(indio_dev, chan, &data);
+		if (ret)
+			return ret;
+		*val = data;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		return inv_icm42607_accel_read_scale(indio_dev, val, val2);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return inv_icm42607_accel_read_odr(st, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_accel_read_avail(struct iio_dev *indio_dev,
+					 struct iio_chan_spec const *chan,
+					 const int **vals,
+					 int *type, int *length, long mask)
+{
+	if (chan->type != IIO_ACCEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)inv_icm42607_accel_scale_nano;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(inv_icm42607_accel_scale_nano) * 2;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (const int *)inv_icm42607_accel_odr[5];
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = (ARRAY_SIZE(inv_icm42607_accel_odr) - 5) * 2;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_accel_write_raw(struct iio_dev *indio_dev,
+					struct iio_chan_spec const *chan,
+					int val, int val2, long mask)
+{
+	int ret;
+
+	if (chan->type != IIO_ACCEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		ret = inv_icm42607_accel_write_scale(indio_dev, val, val2);
+		return ret;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return inv_icm42607_accel_write_odr(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_accel_write_raw_get_fmt(struct iio_dev *indio_dev,
+						struct iio_chan_spec const *chan,
+						long mask)
+{
+	if (chan->type != IIO_ACCEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info inv_icm42607_accel_info = {
+	.read_raw = inv_icm42607_accel_read_raw,
+	.read_avail = inv_icm42607_accel_read_avail,
+	.write_raw = inv_icm42607_accel_write_raw,
+	.write_raw_get_fmt = inv_icm42607_accel_write_raw_get_fmt,
+};
+
+struct iio_dev *inv_icm42607_accel_init(struct inv_icm42607_state *st)
+{
+	struct device *dev = regmap_get_device(st->map);
+	struct inv_icm42607_sensor_state *accel_st;
+	struct iio_dev *indio_dev;
+	const char *name;
+	int ret;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s-accel", st->hw->name);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*accel_st));
+	if (!indio_dev)
+		return ERR_PTR(-ENOMEM);
+	accel_st = iio_priv(indio_dev);
+
+	accel_st->power_mode = INV_ICM42607_SENSOR_MODE_LOW_NOISE;
+	accel_st->filter = INV_ICM42607_FILTER_BW_73HZ;
+
+	iio_device_set_drvdata(indio_dev, st);
+	indio_dev->name = name;
+	indio_dev->info = &inv_icm42607_accel_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = inv_icm42607_accel_channels;
+	indio_dev->num_channels = ARRAY_SIZE(inv_icm42607_accel_channels);
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return indio_dev;
+}
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index ad23386a37b7..26cedec0c97c 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -19,6 +19,7 @@
 #include <linux/time.h>
 #include <linux/timekeeping.h>
 #include <linux/types.h>
+#include <linux/unaligned.h>
 
 #include "inv_icm42607.h"
 
@@ -106,6 +107,15 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
 };
 EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
 
+const struct iio_mount_matrix *
+inv_icm42607_get_mount_matrix(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan)
+{
+	const struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+
+	return &st->orientation;
+}
+
 static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
 				      enum inv_icm42607_sensor_mode gyro,
 				      enum inv_icm42607_sensor_mode accel)
@@ -171,6 +181,160 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
 	return 0;
 }
 
+/*
+ * Sanity test between old and new config values, and note if we need
+ * to update register config0 or config1.
+ */
+static void inv_icm42607_update_config(struct inv_icm42607_sensor_conf *conf,
+				       struct inv_icm42607_sensor_conf *oldconf,
+				       bool *config0, bool *config1)
+{
+	*config0 = false;
+	*config1 = false;
+
+	if (conf->mode < 0)
+		conf->mode = oldconf->mode;
+	if (conf->fs < 0)
+		conf->fs = oldconf->fs;
+	if (conf->odr < 0)
+		conf->odr = oldconf->odr;
+	if (conf->filter < 0)
+		conf->filter = oldconf->filter;
+
+	if (conf->fs != oldconf->fs || conf->odr != oldconf->odr)
+		*config0 = true;
+
+	if (conf->filter != oldconf->filter)
+		*config1 = true;
+}
+
+int inv_icm42607_set_sensor_conf(struct inv_icm42607_state *st,
+				 struct inv_icm42607_sensor_conf *conf,
+				 enum iio_chan_type chan_type)
+{
+	struct inv_icm42607_sensor_conf *oldconf;
+	bool config0, config1;
+	unsigned int val;
+	int ret;
+
+	switch (chan_type) {
+	case IIO_ACCEL:
+		oldconf = &st->conf.accel;
+		break;
+	case IIO_ANGL_VEL:
+		oldconf = &st->conf.gyro;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	inv_icm42607_update_config(conf, oldconf, &config0, &config1);
+
+	if (config0) {
+		if (chan_type == IIO_ANGL_VEL) {
+			val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK, conf->fs);
+			val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK, conf->odr);
+			ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
+		} else {
+			val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG0_FS_SEL_MASK, conf->fs);
+			val |= FIELD_PREP(INV_ICM42607_ACCEL_CONFIG0_ODR_MASK, conf->odr);
+			ret = regmap_write(st->map, INV_ICM42607_REG_ACCEL_CONFIG0, val);
+		}
+		if (ret)
+			return ret;
+
+		oldconf->fs = conf->fs;
+		oldconf->odr = conf->odr;
+	}
+
+	if (config1) {
+		if (chan_type == IIO_ANGL_VEL) {
+			val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
+					 conf->filter);
+			ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
+						 INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
+		} else {
+			val = FIELD_PREP(INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK,
+					 conf->filter);
+			ret = regmap_update_bits(st->map, INV_ICM42607_REG_ACCEL_CONFIG1,
+						 INV_ICM42607_ACCEL_CONFIG1_FILTER_MASK, val);
+		}
+		if (ret)
+			return ret;
+
+		oldconf->filter = conf->filter;
+	}
+
+	if (chan_type == IIO_ANGL_VEL)
+		return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode);
+
+	return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode);
+}
+
+int inv_icm42607_read_sensor(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     s16 *val)
+{
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct inv_icm42607_sensor_state *sensor_st = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int reg;
+	u8 data[2];
+	int ret;
+
+	if ((chan->type != IIO_ANGL_VEL) && (chan->type != IIO_ACCEL))
+		return -EINVAL;
+
+	switch (chan->channel2) {
+	case IIO_MOD_X:
+		if (chan->type == IIO_ANGL_VEL)
+			reg = INV_ICM42607_REG_GYRO_DATA_X1;
+		else
+			reg = INV_ICM42607_REG_ACCEL_DATA_X1;
+		break;
+	case IIO_MOD_Y:
+		if (chan->type == IIO_ANGL_VEL)
+			reg = INV_ICM42607_REG_GYRO_DATA_Y1;
+		else
+			reg = INV_ICM42607_REG_ACCEL_DATA_Y1;
+		break;
+	case IIO_MOD_Z:
+		if (chan->type == IIO_ANGL_VEL)
+			reg = INV_ICM42607_REG_GYRO_DATA_Z1;
+		else
+			reg = INV_ICM42607_REG_ACCEL_DATA_Z1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	/* enable sensor */
+	conf.mode = sensor_st->power_mode;
+	conf.filter = sensor_st->filter;
+	ret = inv_icm42607_set_sensor_conf(st, &conf, chan->type);
+	if (ret)
+		return ret;
+
+	/* read sensor register data */
+	ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
+	if (ret)
+		return ret;
+
+	*val = get_unaligned_be16(data);
+	if (*val == INV_ICM42607_DATA_INVALID)
+		return -EINVAL;
+
+	return 0;
+}
+
 static int inv_icm42607_set_init_conf(struct inv_icm42607_state *st,
 				      const struct inv_icm42607_conf *conf)
 {
@@ -365,6 +529,11 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(dev);
 
+	/* Initialize IIO device for Accel */
+	st->indio_accel = inv_icm42607_accel_init(st);
+	if (IS_ERR(st->indio_accel))
+		return PTR_ERR(st->indio_accel);
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* [PATCH V14 7/9] iio: imu: inv_icm42607: Add Gyroscope to icm42607
From: Chris Morgan @ 2026-06-24 18:23 UTC (permalink / raw)
  To: linux-iio
  Cc: andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
	linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
	andriy.shevchenko, Chris Morgan
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>

From: Chris Morgan <macromorgan@hotmail.com>

Add gyroscope functions to the icm42607 driver.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/iio/imu/inv_icm42607/Makefile         |   1 +
 drivers/iio/imu/inv_icm42607/inv_icm42607.h   |   4 +
 .../iio/imu/inv_icm42607/inv_icm42607_core.c  |   5 +
 .../iio/imu/inv_icm42607/inv_icm42607_gyro.c  | 305 ++++++++++++++++++
 4 files changed, 315 insertions(+)
 create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c

diff --git a/drivers/iio/imu/inv_icm42607/Makefile b/drivers/iio/imu/inv_icm42607/Makefile
index 372c6d6bdcec..8e73385c8f4b 100644
--- a/drivers/iio/imu/inv_icm42607/Makefile
+++ b/drivers/iio/imu/inv_icm42607/Makefile
@@ -2,6 +2,7 @@
 
 obj-$(CONFIG_INV_ICM42607) += inv-icm42607.o
 inv-icm42607-y += inv_icm42607_core.o
+inv-icm42607-y += inv_icm42607_gyro.o
 inv-icm42607-y += inv_icm42607_accel.o
 
 obj-$(CONFIG_INV_ICM42607_I2C) += inv-icm42607-i2c.o
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
index cd78b43f36fa..81d56920f356 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
@@ -142,6 +142,7 @@ struct inv_icm42607_suspended {
  *  @lock:		lock for serializing multiple registers access.
  *  @map:		regmap pointer.
  *  @indio_accel:	accelerometer IIO device.
+ *  @indio_gyro:	gyroscope IIO device.
  *  @vddio_supply:	I/O voltage regulator for the chip.
  *  @vddio_en:		I/O voltage status for runtime PM.
  *  @suspended:		suspended sensors configuration.
@@ -153,6 +154,7 @@ struct inv_icm42607_state {
 	struct mutex lock;
 	struct regmap *map;
 	struct iio_dev *indio_accel;
+	struct iio_dev *indio_gyro;
 	struct regulator *vddio_supply;
 	bool vddio_en;
 	struct inv_icm42607_suspended suspended;
@@ -414,6 +416,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 			    const struct inv_icm42607_hw *hw,
 			    inv_icm42607_bus_setup bus_setup);
 
+struct iio_dev *inv_icm42607_gyro_init(struct inv_icm42607_state *st);
+
 struct iio_dev *inv_icm42607_accel_init(struct inv_icm42607_state *st);
 
 #endif
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 26cedec0c97c..a849c68227ae 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -534,6 +534,11 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	if (IS_ERR(st->indio_accel))
 		return PTR_ERR(st->indio_accel);
 
+	/* Initialize IIO device for Gyro */
+	st->indio_gyro = inv_icm42607_gyro_init(st);
+	if (IS_ERR(st->indio_gyro))
+		return PTR_ERR(st->indio_gyro);
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
new file mode 100644
index 000000000000..c7215b3826ad
--- /dev/null
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2026 InvenSense, Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/device/devres.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include "inv_icm42607.h"
+
+#define INV_ICM42607_GYRO_CHAN(_modifier, _index, _ext_info)			\
+{										\
+	.type = IIO_ANGL_VEL,							\
+	.modified = 1,								\
+	.channel2 = _modifier,							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),		\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.scan_index = _index,							\
+	.scan_type = {								\
+		.sign = 's',							\
+		.realbits = 16,							\
+		.storagebits = 16,						\
+		.endianness = IIO_BE,						\
+	},									\
+	.ext_info = _ext_info,							\
+}
+
+enum inv_icm42607_gyro_scan {
+	INV_ICM42607_GYRO_SCAN_X,
+	INV_ICM42607_GYRO_SCAN_Y,
+	INV_ICM42607_GYRO_SCAN_Z,
+};
+
+static const struct iio_chan_spec_ext_info inv_icm42607_gyro_ext_infos[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, inv_icm42607_get_mount_matrix),
+	{ }
+};
+
+static const struct iio_chan_spec inv_icm42607_gyro_channels[] = {
+	INV_ICM42607_GYRO_CHAN(IIO_MOD_X, INV_ICM42607_GYRO_SCAN_X,
+			       inv_icm42607_gyro_ext_infos),
+	INV_ICM42607_GYRO_CHAN(IIO_MOD_Y, INV_ICM42607_GYRO_SCAN_Y,
+			       inv_icm42607_gyro_ext_infos),
+	INV_ICM42607_GYRO_CHAN(IIO_MOD_Z, INV_ICM42607_GYRO_SCAN_Z,
+			       inv_icm42607_gyro_ext_infos),
+};
+
+static const int inv_icm42607_gyro_scale_nano[][2] = {
+	[INV_ICM42607_GYRO_FS_2000DPS] = { 0, 1065264 },
+	[INV_ICM42607_GYRO_FS_1000DPS] = { 0, 532632 },
+	[INV_ICM42607_GYRO_FS_500DPS] = { 0, 266316 },
+	[INV_ICM42607_GYRO_FS_250DPS] = { 0, 133158 },
+};
+
+static int inv_icm42607_gyro_read_scale(struct iio_dev *indio_dev,
+					int *val, int *val2)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	unsigned int idx;
+
+	guard(mutex)(&st->lock);
+
+	idx = st->conf.gyro.fs;
+
+	*val = inv_icm42607_gyro_scale_nano[idx][0];
+	*val2 = inv_icm42607_gyro_scale_nano[idx][1];
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int inv_icm42607_gyro_write_scale(struct iio_dev *indio_dev,
+					 int val, int val2)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int idx;
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	size_t scales_len = ARRAY_SIZE(inv_icm42607_gyro_scale_nano);
+	int ret;
+
+	for (idx = 0; idx < scales_len; idx++) {
+		if (val == inv_icm42607_gyro_scale_nano[idx][0] &&
+		    val2 == inv_icm42607_gyro_scale_nano[idx][1])
+			break;
+	}
+	if (idx >= scales_len)
+		return -EINVAL;
+
+	conf.fs = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_sensor_conf(st, &conf, IIO_ANGL_VEL);
+}
+
+static const int inv_icm42607_gyro_odr[][2] = {
+	[INV_ICM42607_ODR_1600HZ] = { 1600, 0 },
+	[INV_ICM42607_ODR_800HZ] = { 800, 0 },
+	[INV_ICM42607_ODR_400HZ] = { 400, 0 },
+	[INV_ICM42607_ODR_200HZ] = { 200, 0 },
+	[INV_ICM42607_ODR_100HZ] = { 100, 0 },
+	[INV_ICM42607_ODR_50HZ] = { 50, 0 },
+	[INV_ICM42607_ODR_25HZ] = { 25, 0 },
+	[INV_ICM42607_ODR_12_5HZ] = { 12, 500000 },
+};
+
+static int inv_icm42607_gyro_read_odr(struct inv_icm42607_state *st,
+				      int *val, int *val2)
+{
+	unsigned int odr;
+	unsigned int i;
+
+	guard(mutex)(&st->lock);
+
+	odr = st->conf.gyro.odr;
+
+	for (i = 5; i < ARRAY_SIZE(inv_icm42607_gyro_odr); ++i) {
+		if (i == odr)
+			break;
+	}
+	if (i >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	*val = inv_icm42607_gyro_odr[i][0];
+	*val2 = inv_icm42607_gyro_odr[i][1];
+
+	return IIO_VAL_INT_PLUS_MICRO;
+}
+
+static int inv_icm42607_gyro_write_odr(struct iio_dev *indio_dev,
+				       int val, int val2)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	struct device *dev = regmap_get_device(st->map);
+	unsigned int idx;
+	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
+	int ret;
+
+	for (idx = 5; idx < ARRAY_SIZE(inv_icm42607_gyro_odr); ++idx) {
+		if (val == inv_icm42607_gyro_odr[idx][0] &&
+		    val2 == inv_icm42607_gyro_odr[idx][1])
+			break;
+	}
+	if (idx >= ARRAY_SIZE(inv_icm42607_gyro_odr))
+		return -EINVAL;
+
+	conf.odr = idx;
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&st->lock);
+
+	return inv_icm42607_set_sensor_conf(st, &conf, IIO_ANGL_VEL);
+}
+
+static int inv_icm42607_gyro_read_raw(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val, int *val2, long mask)
+{
+	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
+	s16 data;
+	int ret;
+
+	switch (chan->type) {
+	case IIO_ANGL_VEL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = inv_icm42607_read_sensor(indio_dev, chan, &data);
+		if (ret)
+			return ret;
+		*val = data;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		return inv_icm42607_gyro_read_scale(indio_dev, val, val2);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return inv_icm42607_gyro_read_odr(st, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_gyro_read_avail(struct iio_dev *indio_dev,
+					struct iio_chan_spec const *chan,
+					const int **vals,
+					int *type, int *length, long mask)
+{
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)inv_icm42607_gyro_scale_nano;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(inv_icm42607_gyro_scale_nano) * 2;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (const int *)inv_icm42607_gyro_odr[5];
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = (ARRAY_SIZE(inv_icm42607_gyro_odr) - 5) * 2;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_gyro_write_raw(struct iio_dev *indio_dev,
+				       struct iio_chan_spec const *chan,
+				       int val, int val2, long mask)
+{
+	int ret;
+
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		ret = inv_icm42607_gyro_write_scale(indio_dev, val, val2);
+		return ret;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return inv_icm42607_gyro_write_odr(indio_dev, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int inv_icm42607_gyro_write_raw_get_fmt(struct iio_dev *indio_dev,
+					       struct iio_chan_spec const *chan,
+					       long mask)
+{
+	if (chan->type != IIO_ANGL_VEL)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info inv_icm42607_gyro_info = {
+	.read_raw = inv_icm42607_gyro_read_raw,
+	.read_avail = inv_icm42607_gyro_read_avail,
+	.write_raw = inv_icm42607_gyro_write_raw,
+	.write_raw_get_fmt = inv_icm42607_gyro_write_raw_get_fmt,
+};
+
+struct iio_dev *inv_icm42607_gyro_init(struct inv_icm42607_state *st)
+{
+	struct device *dev = regmap_get_device(st->map);
+	const char *name;
+	struct inv_icm42607_sensor_state *gyro_st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s-gyro", st->hw->name);
+	if (!name)
+		return ERR_PTR(-ENOMEM);
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*gyro_st));
+	if (!indio_dev)
+		return ERR_PTR(-ENOMEM);
+	gyro_st = iio_priv(indio_dev);
+
+	gyro_st->power_mode = INV_ICM42607_SENSOR_MODE_LOW_NOISE;
+	gyro_st->filter = INV_ICM42607_FILTER_BW_73HZ;
+
+	iio_device_set_drvdata(indio_dev, st);
+	indio_dev->name = name;
+	indio_dev->info = &inv_icm42607_gyro_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = inv_icm42607_gyro_channels;
+	indio_dev->num_channels = ARRAY_SIZE(inv_icm42607_gyro_channels);
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return indio_dev;
+}
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related

* [PATCH V14 9/9] arm64: dts: rockchip: Add icm42607p IMU for RG-DS
From: Chris Morgan @ 2026-06-24 18:23 UTC (permalink / raw)
  To: linux-iio
  Cc: andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
	linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
	andriy.shevchenko, Chris Morgan
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>

From: Chris Morgan <macromorgan@hotmail.com>

Add the Invensense ICM42607P IMU for the Anbernic RG-DS. Mount-matrix
was tested with iio-sensor-proxy and reports correct orientation.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts b/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
index 8d906ab02c5f..b770bfd5268d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
@@ -871,7 +871,13 @@ aw87391_pa_r: audio-codec@5b {
 		sound-name-prefix = "Right Amp";
 	};
 
-	/* invensense,icm42607p at 0x68 */
+	icm42607p: imu@68 {
+		compatible = "invensense,icm42607p";
+		reg = <0x68>;
+		mount-matrix = "-1", "0", "0",
+			       "0", "1", "0",
+			       "0", "0", "-1";
+	};
 };
 
 &i2c3 {
-- 
2.43.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ 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