Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 0/2] iio: temperature: Add support for the STS30 temperature sensor
From: David Lechner @ 2026-06-22 15:45 UTC (permalink / raw)
  To: Maxwell Doose, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	open list:IIO SUBSYSTEM AND DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20260621004626.66629-1-m32285159@gmail.com>

On 6/20/26 7:46 PM, Maxwell Doose wrote:
> Hi all,
> 
> This patch series adds support for the Sensirion STS30 temperature
> sensor family. This driver currently supports non clock stretched single
> shot measurements.
> 
> Given there were very little issues found with the v1 submission, I've
> decided to make this a regular patch series rather than an RFC patch.

You should wait at least one week for feedback on a new driver before
submitting the next revision.

Given that you said in v1 that don't actually have the hardware, I am not
going to review this. We are getting more patches than I can keep up with
already.

> 
> Changes since v1:
> * whole series:
> - Squashed MAINTAINERS updates into both the dt-bindings commit and the
>   driver commit.
> 

^ permalink raw reply

* Re: [PATCH v2 0/2] iio: temperature: Add support for the STS30 temperature sensor
From: Maxwell Doose @ 2026-06-22 15:51 UTC (permalink / raw)
  To: David Lechner
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley,
	open list:IIO SUBSYSTEM AND DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <614988b7-c77f-4f0e-b220-c0acf44bef27@baylibre.com>

On Mon, Jun 22, 2026 at 10:45 AM David Lechner <dlechner@baylibre.com> wrote:
>
> On 6/20/26 7:46 PM, Maxwell Doose wrote:
> > Hi all,
> >
> > This patch series adds support for the Sensirion STS30 temperature
> > sensor family. This driver currently supports non clock stretched single
> > shot measurements.
> >
> > Given there were very little issues found with the v1 submission, I've
> > decided to make this a regular patch series rather than an RFC patch.
>
> You should wait at least one week for feedback on a new driver before
> submitting the next revision.
>
> Given that you said in v1 that don't actually have the hardware, I am not
> going to review this. We are getting more patches than I can keep up with
> already.
>

Fair enough, I'll be away until 4 July anyways.

^ permalink raw reply

* [PATCH v18 00/12] Add Renesas RZ/G3L SD/eMMC support
From: Biju @ 2026-06-22 15:55 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Philipp Zabel, Magnus Damm
  Cc: Biju Das, Wolfram Sang, linux-mmc, devicetree, linux-kernel,
	linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das

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

RZ/G3L SoC has:

Channel 0 supports SD and eMMC (including HS400/HS400ES).
Channel 1 supports SD and eMMC (except for HS400).
Channel 2 supports SD.

The SoC supports a maximum frequency of 150 MHz. The SD0 interface does
not support IOVS and PWEN in the SDHI register (no internal regulator),
unlike SD1 and SD2. It has an internal divider for all modes except HS400.
It also has a 2048-bit divider compared to 512 on others. Moreover
RZ/G3L supports HS400 enhanced strobe mode.

v17->v18:
 * Collected tag
 * Merged patch #4 and #5 and updated commit description
 * Annotated the empty sentinel entries in the OF match tables with a
   "Sentinel." comment for clarity.
 * Retained the tag as it is a trivial cleanup.
 * New patches drop struct renesas_sdhi_hw_info, instead using
   renesas_sdhi_of_data and tmio_mmc_data.
 * Dropped clk, pinctrl, SoC, and board dtsi from this patch series;
   will send later.
v1->v17:
 * Collected tag for binding patch.
 * Resending the series as there is an issue with patch threading from
   patch #14.

Biju Das (12):
  dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
  mmc: renesas_sdhi: Fix whitespace alignment in struct
    renesas_sdhi_of_data
  mmc: renesas_sdhi: Add clk_mask field to support SoC-specific clock
    divider widths
  mmc: renesas_sdhi: Add max_divider field to support SoC-specific clock
    divider ranges
  mmc: renesas_sdhi: Add tuning delay support for RZ/G2L
  mmc: renesas_sdhi: Add TMIO_MMC_INTERNAL_DIVIDER flag
  mmc: renesas_sdhi: Add optional axis/axim reset controls
  mmc: renesas_sdhi: Add RZ/G3L SDHI support
  mmc: renesas_sdhi: Save and restore IOVS across suspend/resume
  mmc: renesas_sdhi: Make HS400 OSEL bit configurable per SoC
  mmc: renesas_sdhi: Add RZ/G3L HS400 support
  mmc: renesas_sdhi: Add HS400 enhanced strobe support for RZ/G3L

 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 101 ++++++--
 drivers/mmc/host/renesas_sdhi.h               |  12 +-
 drivers/mmc/host/renesas_sdhi_core.c          | 239 ++++++++++++++----
 drivers/mmc/host/renesas_sdhi_internal_dmac.c |  73 +++++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c      |  12 +-
 include/linux/platform_data/tmio.h            |  18 ++
 6 files changed, 370 insertions(+), 85 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH v18 01/12] dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
From: Biju @ 2026-06-22 15:55 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm
  Cc: Biju Das, Wolfram Sang, linux-mmc, devicetree, linux-kernel,
	linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das, Conor Dooley
In-Reply-To: <20260622155610.184271-1-biju.das.jz@bp.renesas.com>

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

Document the RZ/G3L (r9a08g046) SDHI controller. The RZ/G3L SDHI
controller is similar to RZ/G2L but has five clocks (core, clkh,
cd, aclk, aclkm) and three resets (rst, axim, axis), so update the
clocks/clock-names maximum to 5 and resets/reset-names maximum to 3.
It has an internal divider for all modes except HS400, and a 2048-bit
divider compared to 512 on others.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
 * No change.
v1->v17:
 * Collected tag.
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 101 +++++++++++++-----
 1 file changed, 75 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 4d66966ce290..16cb395403f6 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -18,6 +18,7 @@ properties:
           - renesas,sdhi-r7s9210 # SH-Mobile AG5
           - renesas,sdhi-r8a73a4 # R-Mobile APE6
           - renesas,sdhi-r8a7740 # R-Mobile A1
+          - renesas,sdhi-r9a08g046 # RZ/G3L
           - renesas,sdhi-r9a09g057 # RZ/V2H(P)
           - renesas,sdhi-sh73a0  # R-Mobile APE6
       - items:
@@ -86,11 +87,11 @@ properties:
 
   clocks:
     minItems: 1
-    maxItems: 4
+    maxItems: 5
 
   clock-names:
     minItems: 1
-    maxItems: 4
+    maxItems: 5
 
   dmas:
     minItems: 4
@@ -116,7 +117,12 @@ properties:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 3
+
+  reset-names:
+    minItems: 1
+    maxItems: 3
 
   pinctrl-0:
     minItems: 1
@@ -155,60 +161,101 @@ allOf:
         properties:
           compatible:
             contains:
-              enum:
-                - renesas,sdhi-r9a09g057
-                - renesas,rzg2l-sdhi
+              const: renesas,sdhi-r9a08g046
       then:
         properties:
           clocks:
             items:
               - description: IMCLK, SDHI channel main clock1.
               - description: CLK_HS, SDHI channel High speed clock which operates
-                             4 times that of SDHI channel main clock1.
+                             2 times that of SDHI channel main clock1.
               - description: IMCLK2, SDHI channel main clock2. When this clock is
                              turned off, external SD card detection cannot be
                              detected.
-              - description: ACLK, SDHI channel bus clock.
+              - description: ACLK/IACLKS, SDHI channel bus clock.
+              - description: IACLKM, SDHI channel bus clock m.
           clock-names:
             items:
               - const: core
               - const: clkh
               - const: cd
               - const: aclk
+              - const: aclkm
+          resets:
+            items:
+              - description: rst, Core reset.
+              - description: axim, SDHI axi bus reset m.
+              - description: axis, SDHI axi bus reset s.
+          reset-names:
+            items:
+              - const: rst
+              - const: axim
+              - const: axis
         required:
           - clock-names
           - resets
+          - reset-names
       else:
         if:
           properties:
             compatible:
               contains:
                 enum:
-                  - renesas,rcar-gen2-sdhi
-                  - renesas,rcar-gen3-sdhi
-                  - renesas,rcar-gen4-sdhi
+                  - renesas,sdhi-r9a09g057
+                  - renesas,rzg2l-sdhi
         then:
           properties:
             clocks:
-              minItems: 1
-              maxItems: 3
-            clock-names:
-              minItems: 1
-              uniqueItems: true
               items:
-                - const: core
-                - enum: [ clkh, cd ]
-                - const: cd
-        else:
-          properties:
-            clocks:
-              minItems: 1
-              maxItems: 2
+                - description: IMCLK, SDHI channel main clock1.
+                - description: CLK_HS, SDHI channel High speed clock which operates
+                               4 times that of SDHI channel main clock1.
+                - description: IMCLK2, SDHI channel main clock2. When this clock is
+                               turned off, external SD card detection cannot be
+                               detected.
+                - description: ACLK, SDHI channel bus clock.
             clock-names:
-              minItems: 1
               items:
                 - const: core
+                - const: clkh
                 - const: cd
+                - const: aclk
+            resets:
+              maxItems: 1
+          required:
+            - clock-names
+            - resets
+        else:
+          if:
+            properties:
+              compatible:
+                contains:
+                  enum:
+                    - renesas,rcar-gen2-sdhi
+                    - renesas,rcar-gen3-sdhi
+                    - renesas,rcar-gen4-sdhi
+          then:
+            properties:
+              clocks:
+                minItems: 1
+                maxItems: 3
+              clock-names:
+                minItems: 1
+                uniqueItems: true
+                items:
+                  - const: core
+                  - enum: [ clkh, cd ]
+                  - const: cd
+          else:
+            properties:
+              clocks:
+                minItems: 1
+                maxItems: 2
+              clock-names:
+                minItems: 1
+                items:
+                  - const: core
+                  - const: cd
 
   - if:
       properties:
@@ -247,7 +294,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,sdhi-r9a09g057
+            enum:
+              - renesas,sdhi-r9a08g046
+              - renesas,sdhi-r9a09g057
     then:
       properties:
         vqmmc-regulator:
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v18 01/12] dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
From: sashiko-bot @ 2026-06-22 16:05 UTC (permalink / raw)
  To: Biju; +Cc: robh, conor+dt, wsa+renesas, devicetree
In-Reply-To: <20260622155610.184271-2-biju.das.jz@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Relaxing the global `maxItems` for `resets` and `reset-names` from 1 to 3 removes the strict constraint for legacy controllers, allowing them to incorrectly define up to 3 resets.
--

commit 5bd955e3c9833c72a7847cd9926c293d95e5c927
Author: Biju Das <biju.das.jz@bp.renesas.com>

dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC

Document the RZ/G3L (r9a08g046) SDHI controller. The RZ/G3L SDHI
controller is similar to RZ/G2L but has five clocks (core, clkh,
cd, aclk, aclkm) and three resets (rst, axim, axis), so update the
clocks/clock-names maximum to 5 and resets/reset-names maximum to 3.
It has an internal divider for all modes except HS400, and a 2048-bit
divider compared to 512 on others.

> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -116,7 +117,12 @@ properties:
>      maxItems: 1
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 3
> +
> +  reset-names:
> +    minItems: 1
> +    maxItems: 3

[ ... ]

> @@ -155,60 +161,101 @@ allOf:
[ ... ]
> +        else:
> +          if:
> +            properties:
> +              compatible:
> +                contains:
> +                  enum:
> +                    - renesas,rcar-gen2-sdhi
> +                    - renesas,rcar-gen3-sdhi
> +                    - renesas,rcar-gen4-sdhi
> +          then:
> +            properties:
> +              clocks:
> +                minItems: 1
> +                maxItems: 3
> +              clock-names:
> +                minItems: 1
> +                uniqueItems: true
> +                items:
> +                  - const: core
> +                  - enum: [ clkh, cd ]
> +                  - const: cd
> +          else:
> +            properties:
> +              clocks:
> +                minItems: 1
> +                maxItems: 2
> +              clock-names:
> +                minItems: 1
> +                items:
> +                  - const: core
> +                  - const: cd

[Severity: Medium]
Since the top-level resets and reset-names maxItems were increased to 3, do
the conditional branches for R-Car Gen2/3/4 and the subsequent fallback branch
need to explicitly constrain resets and reset-names back to maxItems: 1?

Without overriding the top-level relaxation here, it appears legacy controllers
are now permitted to incorrectly define up to 3 resets.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622155610.184271-1-biju.das.jz@bp.renesas.com?part=1

^ permalink raw reply

* Re: [PATCH RFC v5 2/6] Documentation: iio: add Open Sensor Fusion driver overview
From: Jonathan Cameron @ 2026-06-22 16:21 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
	Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
	linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260616072242.3942-3-kimjinseob88@gmail.com>

On Tue, 16 Jun 2026 16:22:38 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:

> Document the Linux IIO mapping for Open Sensor Fusion devices, including
> capability-driven IIO device registration and the initially supported
> receive path.
> 
> Call out that OSF0 is a wire magic value, while protocol_major and
> protocol_minor carry protocol compatibility inside frames. The Linux
> compatible remains the generic Open Sensor Fusion host interface.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
Just one small thing inline.
Otherwise this look good to me.

Thanks,

Jonathan

> ---
>  Documentation/iio/index.rst              |  1 +
>  Documentation/iio/open-sensor-fusion.rst | 71 ++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
>  create mode 100644 Documentation/iio/open-sensor-fusion.rst
> 
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index ba3e609c6..2713ec5e0 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -38,4 +38,5 @@ Industrial I/O Kernel Drivers
>     adxl345
>     bno055
>     ep93xx_adc
> +   open-sensor-fusion
>     opt4060
> diff --git a/Documentation/iio/open-sensor-fusion.rst b/Documentation/iio/open-sensor-fusion.rst
> new file mode 100644
> index 000000000..cf3bbd761
> --- /dev/null
> +++ b/Documentation/iio/open-sensor-fusion.rst
> @@ -0,0 +1,71 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Open Sensor Fusion
> +==================
> +
> +Open Sensor Fusion is a sensor aggregation hub interface. The Linux IIO driver
> +receives OSF protocol frames from an attached device, discovers supported sensor
> +streams through capability reports, and registers matching IIO devices for the
> +sensor classes supported by the driver.
> +
> +This document is a driver-facing overview for the Linux IIO mapping. The full
> +wire protocol, firmware behavior, and hardware model details belong in the Open
> +Sensor Fusion project documentation.
> +
> +Device Model
> +------------
> +
> +An OSF device sends binary frames from the device to the host. The host driver
> +uses ``CAPABILITY_REPORT`` messages to discover which sensor streams are
> +available. Device Tree describes the attached OSF sensor aggregation hub; it does
> +not enumerate the individual sensors discovered at runtime.
> +
> +The currently supported Linux subset exposes:
> +
> +* accelerometer samples as ``IIO_ACCEL`` X/Y/Z channels,
> +* gyroscope samples as ``IIO_ANGL_VEL`` X/Y/Z channels,
> +* magnetometer samples as ``IIO_MAGN`` X/Y/Z channels, and
> +* temperature samples as ``IIO_TEMP``.
> +
> +Protocol Scope
> +---------------
> +
> +The driver supports OSF protocol major version 0 for the initial IIO receive
> +path. The current wire magic is ``OSF0``; that string is a wire-format detail and
> +is not the Linux driver identity. Device Tree keeps the generic
> +``opensensorfusion,osf`` compatible rather than naming a product such as OSF
> +GREEN or a wire magic value.

I'd remove anything that is documented elsewhere (i.e. the dt-binding)
and instead use a cross reference to it.  That will reduce the chance of this getting
out of sync.

> +
> +Protocol versioning is carried by the ``protocol_major`` and ``protocol_minor``
> +fields at fixed offsets in the OSF frame header. The driver currently
> +supports ``protocol_major`` 0. ``protocol_minor`` changes within major version
> +0 are intended to remain backward-compatible within the fixed header layout.
> +Incompatible wire-format changes require a new ``protocol_major``. A future
> +device that cannot expose compatible version discovery through that fixed
> +header layout would need a different Device Tree compatible.
> +
> +The initial Linux driver handles device-to-host frames for:
> +
> +* ``SENSOR_SAMPLE`` buffered and direct-mode sample data,
> +* ``CAPABILITY_REPORT`` based IIO device registration, and
> +* ``DEVICE_STATUS`` cache updates.
> +
> +Vendor-private message types are ignored. Command transport, calibration
> +control ABI, fusion output ABI, and runtime capability removal are outside the
> +initial Linux IIO receive path.
> +
> +Timestamps
> +----------
> +
> +OSF frames include a device-side ``timestamp_us`` field. Buffered IIO samples use
> +an IIO timestamp captured on the host when samples are pushed to IIO buffers.
> +The initial driver does not correlate the device timestamp with the host IIO
> +clock.
> +
> +Compatibility Notes
> +-------------------
> +
> +The project protocol documentation should define the compatibility rules for
> +reserved fields, optional flags, and trailing extension data. Until those rules
> +are finalized, the Linux decoder keeps conservative bounds checks around the
> +currently supported message layouts.


^ permalink raw reply

* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-22 16:24 UTC (permalink / raw)
  To: Nuno Sá
  Cc: Conor Dooley, Janani Sunil, Rodrigo Alencar, Janani Sunil,
	Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
	linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <ajkILRPq_g24g4dH@nsa>

On Mon, 22 Jun 2026 11:17:56 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, Jun 22, 2026 at 10:27:22AM +0100, Jonathan Cameron wrote:
> > On Mon, 22 Jun 2026 10:07:01 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> >   
> > > On Sun, Jun 21, 2026 at 07:35:42PM +0100, Conor Dooley wrote:  
> > > > On Sun, Jun 21, 2026 at 03:33:40PM +0100, Jonathan Cameron wrote:    
> > > > > On Fri, 19 Jun 2026 16:54:11 +0100
> > > > > Nuno Sá <noname.nuno@gmail.com> wrote:
> > > > >     
> > > > > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:    
> > > > > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:      
> > > > > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:      
> > > > > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:      
> > > > > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:      
> > > > > > > > > > > 
> > > > > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:      
> > > > > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > > > > >       
> > > > > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:      
> > > > > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:      
> > > > > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > > > > integrated precision reference.      
> > > > > > > > > > > > > > ...
> > > > > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > That way I suppose that an example would look like...      
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > > > > +  "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > > > > +    type: object
> > > > > > > > > > > > > > > +    description: Child nodes for individual channel configuration
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +    properties:
> > > > > > > > > > > > > > > +      reg:
> > > > > > > > > > > > > > > +        description: Channel number.
> > > > > > > > > > > > > > > +        minimum: 0
> > > > > > > > > > > > > > > +        maximum: 15
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +      adi,output-range-microvolt:
> > > > > > > > > > > > > > > +        description: |
> > > > > > > > > > > > > > > +          Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > > > > +          If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > > > > +        oneOf:
> > > > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > > > +              - const: 0
> > > > > > > > > > > > > > > +              - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > > > +              - const: -5000000
> > > > > > > > > > > > > > > +              - const: 5000000
> > > > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > > > +              - const: -10000000
> > > > > > > > > > > > > > > +              - const: 10000000
> > > > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > > > +              - const: -15000000
> > > > > > > > > > > > > > > +              - const: 15000000
> > > > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > > > +              - const: -20000000
> > > > > > > > > > > > > > > +              - const: 20000000
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +    required:
> > > > > > > > > > > > > > > +      - reg
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +    additionalProperties: false
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +required:
> > > > > > > > > > > > > > > +  - compatible
> > > > > > > > > > > > > > > +  - reg
> > > > > > > > > > > > > > > +  - vdd-supply
> > > > > > > > > > > > > > > +  - avdd-supply
> > > > > > > > > > > > > > > +  - hvdd-supply
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > > > > +  spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > > > > +  spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +allOf:
> > > > > > > > > > > > > > > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +examples:
> > > > > > > > > > > > > > > +  - |
> > > > > > > > > > > > > > > +    #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +    spi {
> > > > > > > > > > > > > > > +        #address-cells = <1>;
> > > > > > > > > > > > > > > +        #size-cells = <0>;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +        dac@0 {
> > > > > > > > > > > > > > > +            compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > > > +            reg = <0>;
> > > > > > > > > > > > > > > +            spi-max-frequency = <25000000>;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > > > +            avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > > > +            hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > > > +            hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            #address-cells = <1>;
> > > > > > > > > > > > > > > +            #size-cells = <0>;
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            channel@0 {
> > > > > > > > > > > > > > > +                reg = <0>;
> > > > > > > > > > > > > > > +                adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > > > +            };
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            channel@1 {
> > > > > > > > > > > > > > > +                reg = <1>;
> > > > > > > > > > > > > > > +                adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > > > +            };
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +            channel@2 {
> > > > > > > > > > > > > > > +                reg = <2>;
> > > > > > > > > > > > > > > +                adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > > > +            };
> > > > > > > > > > > > > > > +        };
> > > > > > > > > > > > > > > +    };      
> > > > > > > > > > > > > > ...
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 	spi {
> > > > > > > > > > > > > > 		#address-cells = <1>;
> > > > > > > > > > > > > > 		#size-cells = <0>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 		multi-dac@0 {
> > > > > > > > > > > > > > 			compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > > 			reg = <0>;
> > > > > > > > > > > > > > 			spi-max-frequency = <25000000>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 			#address-cells = <1>;
> > > > > > > > > > > > > > 			#size-cells = <0>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 			dac@0 {
> > > > > > > > > > > > > > 				reg = <0>;
> > > > > > > > > > > > > > 				vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > > 				avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > > 				hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > > 				hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				#address-cells = <1>;
> > > > > > > > > > > > > > 				#size-cells = <0>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				channel@0 {
> > > > > > > > > > > > > > 					reg = <0>;
> > > > > > > > > > > > > > 					adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > > 				};
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				channel@1 {
> > > > > > > > > > > > > > 					reg = <1>;
> > > > > > > > > > > > > > 					adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > > 				};
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				channel@2 {
> > > > > > > > > > > > > > 					reg = <2>;
> > > > > > > > > > > > > > 					adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > > 				};
> > > > > > > > > > > > > > 			}
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 			dac@1 {
> > > > > > > > > > > > > > 				reg = <1>;
> > > > > > > > > > > > > > 				vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > > 				avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > > 				hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > > 				hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				#address-cells = <1>;
> > > > > > > > > > > > > > 				#size-cells = <0>;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				channel@0 {
> > > > > > > > > > > > > > 					reg = <0>;
> > > > > > > > > > > > > > 					adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > > 				};
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 				channel@1 {
> > > > > > > > > > > > > > 					reg = <1>;
> > > > > > > > > > > > > > 					adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > > 				};
> > > > > > > > > > > > > > 			}
> > > > > > > > > > > > > > 		};
> > > > > > > > > > > > > > 	};
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > then you might need something like:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 	patternProperties:
> > > > > > > > > > > > > > 		"^dac@[0-3]$":
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).      
> > > > > > > > > > > > > Hi Rodrigo,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thank you for looking at this.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > > > > speculatively without a validating use case.      
> > > > > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > > > > > 
> > > > > > > > > > > > Challenge of a binding is we need to anticipate the future.  So I think we do need something
> > > > > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > > > > is just one device with a lot of channels etc.  The snag is that here things are more loosely
> > > > > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > > > > > 
> > > > > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > > > > - Each of these device has 2 ID pins.  The SPI transfers have to contain the 2 bit
> > > > > > > > > > > > value that matches that or they are ignored.  Thus a single bus + 1 chip select can
> > > > > > > > > > > > be used to talk to 4 devices.  Question is what that looks like in device tree + I guess
> > > > > > > > > > > > longer term how to support it cleanly in SPI.      
> > > > > > > > > > 
> > > > > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > > > > see if I can find what I am thinking of...      
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > > > > slightly different properties.
> > > > > > > > > 
> > > > > > > > >   microchip,device-addr:
> > > > > > > > >     description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > >     enum: [0, 1, 2, 3]
> > > > > > > > >     default: 0
> > > > > > > > > 
> > > > > > > > > and
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > >   microchip,hw-device-address:
> > > > > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > >     minimum: 0
> > > > > > > > >     maximum: 3
> > > > > > > > >     description:
> > > > > > > > >       The address is set on a per-device basis by fuses in the factory,
> > > > > > > > >       configured on request. If not requested, the fuses are set for 0x1.
> > > > > > > > >       The device address is part of the device markings to avoid
> > > > > > > > >       potential confusion. This address is coded on two bits, so four possible
> > > > > > > > >       addresses are available when multiple devices are present on the same
> > > > > > > > >       SPI bus with only one Chip Select line for all devices.
> > > > > > > > >       Each device communication starts by a CS falling edge, followed by the
> > > > > > > > >       clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > > > >       which is first one on the wire).
> > > > > > > > > 
> > > > > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > > > > here?
> > > > > > > > >       
> > > > > > > > 
> > > > > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > > > > about solving this at the spi level.
> > > > > > > > 
> > > > > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > > > > together in the same bus.      
> > > > > > > 
> > > > > > > I'm definitely missing something, because that property for the
> > > > > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > > > > are completely different devices with different drivers. They have
> > > > > > > individual device nodes and their own supplies etc etc. These aren't
> > > > > > > per-channel properties on an adc or dac, they're per child device on a
> > > > > > > spi bus.      
> > > > > > 
> > > > > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > > > > devices on the same CS right? Because for this chip we would need
> > > > > > something like:
> > > > > > 
> > > > > > spi {
> > > > > > 	dac@0 {
> > > > > > 		reg = <0>;
> > > > > > 		adi,pin-id = <0>;
> > > > > > 	};
> > > > > > 
> > > > > > 	dac@1 {
> > > > > > 		reg = <0>; // which seems already problematic?
> > > > > > 		adi,pin-id <1>;
> > > > > > 	};
> > > > > > 
> > > > > > 	...
> > > > > > 
> > > > > > 	//up to 4
> > > > > > };    
> > > > > Yeah. It's not clear to me how that works for the microchip devices
> > > > > (I suspect it doesn't!)
> > > > > 
> > > > > Just thinking as I type, but could we do something a bit nasty with
> > > > > a gpio mux that doesn't actually switch but represents the GPIO being
> > > > > shared?  Given this is all tied to the spi bus that should all happen
> > > > > under serializing locks. 
> > > > > 
> > > > > Agreed though that this would be nicer as an SPI thing that let
> > > > > us specify that a single CS is share by multiple devices and their
> > > > > is some other signal acting to select which one we are talking to.    
> > > > 
> > > > Whether it works or not, I think it is the more correct approach. Messing
> > > > with gpio muxes seems completely wrong, given the chip select may not be
> > > > a gpio at all.
> > > > 
> > > > Why do you think the microchip devices won't work? Does the spi core
> > > > reject multiple devices with the same chip select being registered or
> > > > something like that?    
> > > 
> > > Not sure how things work atm. But I'm fairly sure it used to be like
> > > that. SPI would reject devices on the same controller and CS. Now that
> > > we support more than one CS per controller, not sure how things work.   
> > We always supported more than one per CS per controller. I guess you mean
> > per device.  
> 
> Obviously :)
> >   
> > > 
> > > Janani, maybe you can give it a try?  
> > 
> > I think we'd need to get it to work with shared gpio proxy which maybe
> > will just get set up under the hood.  This used to be opt in, but seems
> > that changed fairly recently so maybe some of us are working with out
> > of date knowledge!  I haven't played with it yet, so might not be
> > that simple.
> >   
> 
> What I meant for Janani was basically testing two devices on the same CS
> as in my pseudo DT. For the GPIO, you mean having a way to select
> between devices on the same CS?

Nope. It is what you suggest - the implementation in the gpio layer
is to detect the reuse of the same GPIO and insert a proxy layer that
allows multiple consumers.  I think that will provide different gpio
numbers (well descs really) to each of them but I haven't checked the details
that closely.

> 
> For these devices the pin id numbers get's setted up as part of the spi message
> so my assumption is that all of them will receive the message but only one acks it.

Yup. As much as we have an ack on SPI.  So with a write only message you'd never
know if anyone got it.

Jonathan


> 
> - Nuno Sá
> 
> > Jonathan
> >   
> > > 
> > > - Nuno Sá
> > >   
> >   
> 


^ permalink raw reply

* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-22 16:29 UTC (permalink / raw)
  To: Nuno Sá
  Cc: Rodrigo Alencar, Conor Dooley, Janani Sunil, Janani Sunil,
	Lars-Peter Clausen, Michael Hennerich, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
	linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <ajkMBh-R_7pYaoAn@nsa>

On Mon, 22 Jun 2026 11:29:38 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, Jun 22, 2026 at 10:24:05AM +0100, Rodrigo Alencar wrote:
> > On 21/06/26 15:33, Jonathan Cameron wrote:  
> > > On Fri, 19 Jun 2026 16:54:11 +0100
> > > Nuno Sá <noname.nuno@gmail.com> wrote:
> > >   
> > > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:  
> > > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:    
> > > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:    
> > > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:    
> > > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:    
> > > > > > > > > 
> > > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:    
> > > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > > >     
> > > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:    
> > > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:    
> > > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > > integrated precision reference.    
> > > > > > > > > > > > ...
> > > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > > > 
> > > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > > > 
> > > > > > > > > > > > That way I suppose that an example would look like...    
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > > +  "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > > +    type: object
> > > > > > > > > > > > > +    description: Child nodes for individual channel configuration
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +    properties:
> > > > > > > > > > > > > +      reg:
> > > > > > > > > > > > > +        description: Channel number.
> > > > > > > > > > > > > +        minimum: 0
> > > > > > > > > > > > > +        maximum: 15
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +      adi,output-range-microvolt:
> > > > > > > > > > > > > +        description: |
> > > > > > > > > > > > > +          Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > > +          If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > > +        oneOf:
> > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > +              - const: 0
> > > > > > > > > > > > > +              - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > +              - const: -5000000
> > > > > > > > > > > > > +              - const: 5000000
> > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > +              - const: -10000000
> > > > > > > > > > > > > +              - const: 10000000
> > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > +              - const: -15000000
> > > > > > > > > > > > > +              - const: 15000000
> > > > > > > > > > > > > +          - items:
> > > > > > > > > > > > > +              - const: -20000000
> > > > > > > > > > > > > +              - const: 20000000
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +    required:
> > > > > > > > > > > > > +      - reg
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +    additionalProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +required:
> > > > > > > > > > > > > +  - compatible
> > > > > > > > > > > > > +  - reg
> > > > > > > > > > > > > +  - vdd-supply
> > > > > > > > > > > > > +  - avdd-supply
> > > > > > > > > > > > > +  - hvdd-supply
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > > +  spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > > +  spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +allOf:
> > > > > > > > > > > > > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +examples:
> > > > > > > > > > > > > +  - |
> > > > > > > > > > > > > +    #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +    spi {
> > > > > > > > > > > > > +        #address-cells = <1>;
> > > > > > > > > > > > > +        #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +        dac@0 {
> > > > > > > > > > > > > +            compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > +            reg = <0>;
> > > > > > > > > > > > > +            spi-max-frequency = <25000000>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > +            avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > +            hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > +            hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            #address-cells = <1>;
> > > > > > > > > > > > > +            #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            channel@0 {
> > > > > > > > > > > > > +                reg = <0>;
> > > > > > > > > > > > > +                adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > +            };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            channel@1 {
> > > > > > > > > > > > > +                reg = <1>;
> > > > > > > > > > > > > +                adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > +            };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +            channel@2 {
> > > > > > > > > > > > > +                reg = <2>;
> > > > > > > > > > > > > +                adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > +            };
> > > > > > > > > > > > > +        };
> > > > > > > > > > > > > +    };    
> > > > > > > > > > > > ...
> > > > > > > > > > > > 
> > > > > > > > > > > > 	spi {
> > > > > > > > > > > > 		#address-cells = <1>;
> > > > > > > > > > > > 		#size-cells = <0>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 		multi-dac@0 {
> > > > > > > > > > > > 			compatible = "adi,ad5529r-16";
> > > > > > > > > > > > 			reg = <0>;
> > > > > > > > > > > > 			spi-max-frequency = <25000000>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 			#address-cells = <1>;
> > > > > > > > > > > > 			#size-cells = <0>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 			dac@0 {
> > > > > > > > > > > > 				reg = <0>;
> > > > > > > > > > > > 				vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > 				avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > 				hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > 				hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				#address-cells = <1>;
> > > > > > > > > > > > 				#size-cells = <0>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				channel@0 {
> > > > > > > > > > > > 					reg = <0>;
> > > > > > > > > > > > 					adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > 				};
> > > > > > > > > > > > 
> > > > > > > > > > > > 				channel@1 {
> > > > > > > > > > > > 					reg = <1>;
> > > > > > > > > > > > 					adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > 				};
> > > > > > > > > > > > 
> > > > > > > > > > > > 				channel@2 {
> > > > > > > > > > > > 					reg = <2>;
> > > > > > > > > > > > 					adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > 				};
> > > > > > > > > > > > 			}
> > > > > > > > > > > > 
> > > > > > > > > > > > 			dac@1 {
> > > > > > > > > > > > 				reg = <1>;
> > > > > > > > > > > > 				vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > 				avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > 				hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > 				hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				#address-cells = <1>;
> > > > > > > > > > > > 				#size-cells = <0>;
> > > > > > > > > > > > 
> > > > > > > > > > > > 				channel@0 {
> > > > > > > > > > > > 					reg = <0>;
> > > > > > > > > > > > 					adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > 				};
> > > > > > > > > > > > 
> > > > > > > > > > > > 				channel@1 {
> > > > > > > > > > > > 					reg = <1>;
> > > > > > > > > > > > 					adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > 				};
> > > > > > > > > > > > 			}
> > > > > > > > > > > > 		};
> > > > > > > > > > > > 	};
> > > > > > > > > > > > 
> > > > > > > > > > > > then you might need something like:
> > > > > > > > > > > > 
> > > > > > > > > > > > 	patternProperties:
> > > > > > > > > > > > 		"^dac@[0-3]$":
> > > > > > > > > > > > 
> > > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > > > 
> > > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > > > 
> > > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).    
> > > > > > > > > > > Hi Rodrigo,
> > > > > > > > > > > 
> > > > > > > > > > > Thank you for looking at this.
> > > > > > > > > > > 
> > > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > > speculatively without a validating use case.    
> > > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > > > 
> > > > > > > > > > Challenge of a binding is we need to anticipate the future.  So I think we do need something
> > > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > > is just one device with a lot of channels etc.  The snag is that here things are more loosely
> > > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > > > 
> > > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > > - Each of these device has 2 ID pins.  The SPI transfers have to contain the 2 bit
> > > > > > > > > > value that matches that or they are ignored.  Thus a single bus + 1 chip select can
> > > > > > > > > > be used to talk to 4 devices.  Question is what that looks like in device tree + I guess
> > > > > > > > > > longer term how to support it cleanly in SPI.    
> > > > > > > > 
> > > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > > see if I can find what I am thinking of...    
> > > > > > > 
> > > > > > > 
> > > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > > slightly different properties.
> > > > > > > 
> > > > > > >   microchip,device-addr:
> > > > > > >     description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > >     enum: [0, 1, 2, 3]
> > > > > > >     default: 0
> > > > > > > 
> > > > > > > and
> > > > > > > 
> > > > > > > 
> > > > > > >   microchip,hw-device-address:
> > > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > >     minimum: 0
> > > > > > >     maximum: 3
> > > > > > >     description:
> > > > > > >       The address is set on a per-device basis by fuses in the factory,
> > > > > > >       configured on request. If not requested, the fuses are set for 0x1.
> > > > > > >       The device address is part of the device markings to avoid
> > > > > > >       potential confusion. This address is coded on two bits, so four possible
> > > > > > >       addresses are available when multiple devices are present on the same
> > > > > > >       SPI bus with only one Chip Select line for all devices.
> > > > > > >       Each device communication starts by a CS falling edge, followed by the
> > > > > > >       clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > >       which is first one on the wire).
> > > > > > > 
> > > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > > here?
> > > > > > >     
> > > > > > 
> > > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > > about solving this at the spi level.
> > > > > > 
> > > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > > together in the same bus.    
> > > > > 
> > > > > I'm definitely missing something, because that property for the
> > > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > > are completely different devices with different drivers. They have
> > > > > individual device nodes and their own supplies etc etc. These aren't
> > > > > per-channel properties on an adc or dac, they're per child device on a
> > > > > spi bus.    
> > > > 
> > > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > > devices on the same CS right? Because for this chip we would need
> > > > something like:
> > > > 
> > > > spi {
> > > > 	dac@0 {
> > > > 		reg = <0>;
> > > > 		adi,pin-id = <0>;
> > > > 	};
> > > > 
> > > > 	dac@1 {
> > > > 		reg = <0>; // which seems already problematic?
> > > > 		adi,pin-id <1>;
> > > > 	};
> > > > 
> > > > 	...
> > > > 
> > > > 	//up to 4
> > > > };  
> > > Yeah. It's not clear to me how that works for the microchip devices
> > > (I suspect it doesn't!)
> > > 
> > > Just thinking as I type, but could we do something a bit nasty with
> > > a gpio mux that doesn't actually switch but represents the GPIO being
> > > shared?  Given this is all tied to the spi bus that should all happen
> > > under serializing locks. 
> > > 
> > > Agreed though that this would be nicer as an SPI thing that let
> > > us specify that a single CS is share by multiple devices and their
> > > is some other signal acting to select which one we are talking to.
> > >   
> > 
> > If the device-addressing on the same chip-select is to be handled
> > by the spi framework, wouldn't we lose device-specific features?
> > 
> > I understand that this multi-device feature is there mostly to extend the
> > channel count from 16 to 32, 48 or 64. I suppose the command:
> > 
> > 	"MULTI DEVICE SW LDAC MODE"
> > 
> > exists so that software can update channel values accross multiple devices.  
> 
> Right! You do have a point! I agree the main driver for a feature like
> this is likely to extend the channel count and effectively "aggregate"
> devices.
> 
> But I would say that even with the spi solution the MULTI DEVICE stuff
> should be doable (as we still need a sort of adi,pin-id property). 
> 
> But yes, I do feel that the whole feature is for aggregation so seeing
> one device with 32 channels is the expectation here? Rather than seeing
> two devices with 16 channels.

Agreed - if we have messages that address both devices at once that needs
to be a unified driver and given they are about triggering simultaneous
update of all channels it needs to look like one big device.
This ends up similar to how we handle daisy chain devices.

The question of what to do on devices that don't have this feature
is rather different. Good thing you read the datasheet :)

Jonathan

> 
> - Nuno Sá
> 
> > 
> > -- 
> > Kind regards,
> > 
> > Rodrigo Alencar  


^ permalink raw reply

* Re: [PATCH 2/2] pwm: add Axiado AX3000 PWM driver
From: Uwe Kleine-König @ 2026-06-22 16:29 UTC (permalink / raw)
  To: Petar Stepanovic
  Cc: Akhila Kavi, Prasad Bolisetty, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Harshit Shah, linux-pwm, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-2-c9797a909414@axiado.com>

[-- Attachment #1: Type: text/plain, Size: 9646 bytes --]

Hello Petar,

Just a very high-level review:

On Thu, Jun 18, 2026 at 05:26:57AM -0700, Petar Stepanovic wrote:
> The Axiado AX3000 and AX3005 SoCs include PWM controllers that can be
> used to generate configurable PWM output signals.
> 
> Add a PWM driver with support for configuring period, duty cycle, and
> enable state through the Linux PWM framework.
> 
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> ---
>  MAINTAINERS              |   1 +
>  drivers/pwm/Kconfig      |  11 +++
>  drivers/pwm/Makefile     |   1 +
>  drivers/pwm/pwm-axiado.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 206 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 394c4a3527e8..db93fc235c32 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4319,6 +4319,7 @@ M:	Prasad Bolisetty <pbolisetty@axiado.com>
>  L:	linux-pwm@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
> +F:	drivers/pwm/pwm-axiado.c
>  
>  AXIS ARTPEC ARM64 SoC SUPPORT
>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 6f3147518376..76f6c04b0e23 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -129,6 +129,17 @@ config PWM_ATMEL_TCB
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-atmel-tcb.
>  
> +config PWM_AXIADO
> +	tristate "Axiado PWM support"
> +	depends on ARCH_AXIADO || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  PWM framework driver for the PWM controller found on Axiado
> +	  AX3000 and AX3005 SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-axiado.
> +
>  config PWM_AXI_PWMGEN
>  	tristate "Analog Devices AXI PWM generator"
>  	depends on MICROBLAZE || NIOS2 || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 0dc0d2b69025..4466a29e780a 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_ARGON_FAN_HAT)	+= pwm-argon-fan-hat.o
>  obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
>  obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
>  obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
> +obj-$(CONFIG_PWM_AXIADO)	+= pwm-axiado.o
>  obj-$(CONFIG_PWM_AXI_PWMGEN)	+= pwm-axi-pwmgen.o
>  obj-$(CONFIG_PWM_BCM2835)	+= pwm-bcm2835.o
>  obj-$(CONFIG_PWM_BCM_IPROC)	+= pwm-bcm-iproc.o
> diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
> new file mode 100644
> index 000000000000..db197886c5c4
> --- /dev/null
> +++ b/drivers/pwm/pwm-axiado.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021-2026 Axiado Corporation.

Please add a Limitations paragraph here like the ones found in the newer
driver files. It should answer:

 - Is a period completed on configuration change?
 - Is a period completed on disable?
 - How does the output behave when disabled? (Low? Inactive? Freeze? High-Z?)

Also mention special properties, like being unable to set 0% or 100%
relative duty.

> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +/* Register offsets */
> +#define AX_PWM_CNTRL_REG     0x0000
> +#define AX_PWM_PERIOD_REG    0x0004
> +#define AX_PWM_HIGH_REG      0x0008
> +
> +/* PWM channels */
> +#define AX_PWM_NUM 1

This is only used once, and having

	chip = devm_pwmchip_alloc(dev, 1, sizeof(*axpwm));

below simplifies grepping for channel numbers.

> +
> +/* Period and duty cycle limits */
> +#define AX_PWM_PERIOD_MIN       2
> +#define AX_PWM_PERIOD_MAX       0xfffffffeU
> +#define AX_PWM_DUTY_MIN         1
> +#define AX_PWM_DUTY_MAX         0xfffffffdU

The U suffix is not needed for hex constants (AFAIK).

> +
> +/* Control register bits */
> +#define AX_PWM_CTRL_ENABLE BIT(0)
> +#define AX_PWM_CTRL_DISABLE 0x0
> +
> +struct axiado_pwm_chip {
> +	struct clk *clk;
> +	void __iomem *base;
> +};

If you use axiado_pwm_ as prefix for structs and functions, please use
AXIADO_PWM_ as prefix for #defines.

> +
> +static int axiado_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +			     u64 duty_ns, u64 period_ns)
> +{
> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> +	unsigned long rate;
> +	u64 period_cycles, duty_cycles;
> +
> +	/*
> +	 * The hardware does not support a zero period, 0% duty cycle, or
> +	 * 100% duty cycle. The caller should handle 0% duty cycle by
> +	 * disabling the PWM.
> +	 */
> +	if (!period_ns || !duty_ns || duty_ns >= period_ns)
> +		return -EINVAL;
> +
> +	rate = clk_get_rate(axpwm->clk);
> +	if (!rate)
> +		return -EINVAL;
> +
> +	period_cycles = mul_u64_u64_div_u64(period_ns, rate, NSEC_PER_SEC);
> +	if (period_cycles < AX_PWM_PERIOD_MIN ||
> +	    period_cycles > AX_PWM_PERIOD_MAX)
> +		return -EINVAL;
> +
> +	duty_cycles = mul_u64_u64_div_u64(duty_ns, rate, NSEC_PER_SEC);
> +	if (duty_cycles < AX_PWM_DUTY_MIN ||
> +	    duty_cycles > AX_PWM_DUTY_MAX)
> +		return -EINVAL;
> +
> +	if (duty_cycles >= period_cycles)
> +		return -EINVAL;
> +
> +	writel((u32)period_cycles, axpwm->base + AX_PWM_PERIOD_REG);
> +	writel((u32)duty_cycles, axpwm->base + AX_PWM_HIGH_REG);
> +
> +	return 0;
> +}
> +
> +static int axiado_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +			    const struct pwm_state *state)
> +{
> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> +	int ret;
> +
> +	if (state->polarity != PWM_POLARITY_NORMAL)
> +		return -EINVAL;
> +
> +	if (!state->enabled || !state->duty_cycle) {
> +		if (pwm->state.enabled)
> +			writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +
> +		return 0;
> +	}
> +
> +	ret = axiado_pwm_config(chip, pwm, state->duty_cycle, state->period);
> +	if (ret)
> +		return ret;
> +
> +	if (!pwm->state.enabled)

Ideally check hardware state and not PWM internal variables.

> +		writel(AX_PWM_CTRL_ENABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +
> +	return 0;
> +}
> +
> +static int axiado_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> +	unsigned long rate;
> +	u32 period_cycles;
> +	u32 duty_cycles;
> +	u32 ctrl;
> +
> +	rate = clk_get_rate(axpwm->clk);
> +	if (!rate)
> +		return -EINVAL;
> +
> +	ctrl = readl(axpwm->base + AX_PWM_CNTRL_REG);
> +	period_cycles = readl(axpwm->base + AX_PWM_PERIOD_REG);
> +	duty_cycles = readl(axpwm->base + AX_PWM_HIGH_REG);
> +
> +	state->enabled = !!(ctrl & AX_PWM_CTRL_ENABLE);
> +	state->period = mul_u64_u64_div_u64(period_cycles, NSEC_PER_SEC, rate);
> +	state->duty_cycle = mul_u64_u64_div_u64(duty_cycles, NSEC_PER_SEC, rate);
> +	state->polarity = PWM_POLARITY_NORMAL;

Please test your driver with PWM_DEBUG enabled, the rounding is wrong
here.

> +
> +	return 0;
> +}
> +
> +static const struct pwm_ops axiado_pwm_ops = {
> +	.get_state = axiado_pwm_get_state,
> +	.apply = axiado_pwm_apply,

Please implement the waveform callbacke instead of .get_state() and
.apply()

> +};
> +
> +static void axiado_pwm_disable(void *data)
> +{
> +	struct axiado_pwm_chip *axpwm = data;
> +
> +	writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +}
> +
> +static int axiado_pwm_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct axiado_pwm_chip *axpwm;
> +	struct pwm_chip *chip;
> +	int ret;
> +
> +	chip = devm_pwmchip_alloc(dev, AX_PWM_NUM, sizeof(*axpwm));
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +
> +	axpwm = pwmchip_get_drvdata(chip);
> +
> +	axpwm->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(axpwm->base))
> +		return dev_err_probe(dev, PTR_ERR(axpwm->base),
> +				     "failed to map registers\n");

Start error messages with a capital letter please.

> +
> +	ret = devm_add_action_or_reset(dev, axiado_pwm_disable, axpwm);
> +	if (ret)
> +		return ret;

This isn't supposed to happen. It's the responsibility of the consumer
to disable the PWM before it's freed.

> +
> +

Single empty line only.

> +	axpwm->clk = devm_clk_get_enabled(dev, "pwm");
> +	if (IS_ERR(axpwm->clk))
> +		return dev_err_probe(dev, PTR_ERR(axpwm->clk),
> +				     "failed to get/enable clock\n");

Please ensure that the clk rate doesn't change while the PWM is enabled.
Then you can cache the clk rate and set chip->atomic.

> +
> +	chip->ops = &axiado_pwm_ops;
> +
> +	ret = devm_pwmchip_add(dev, chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add PWM chip\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id axiado_pwm_match[] = {
> +	{ .compatible = "axiado,ax3000-pwm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, axiado_pwm_match);
> +
> +static struct platform_driver axiado_pwm_driver = {
> +	.driver = {
> +		.name =  "axiado-pwm",
> +		.of_match_table = axiado_pwm_match,
> +	},
> +	.probe = axiado_pwm_probe,
> +};
> +
> +module_platform_driver(axiado_pwm_driver);

No empty line between the driver struct and the module_platform helper
please.

> +
> +MODULE_AUTHOR("Axiado Corporation");
> +MODULE_DESCRIPTION("Axiado PWM driver");
> +MODULE_LICENSE("GPL");

Best regards
Uwe

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

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Javier Carrasco @ 2026-06-22 16:29 UTC (permalink / raw)
  To: Flaviu Nistor, Guenter Roeck, Javier Carrasco, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan
  Cc: linux-hwmon, linux-kernel, devicetree, linux-doc
In-Reply-To: <20260622122200.14245-1-flaviu.nistor@gmail.com>

On Mon Jun 22, 2026 at 2:21 PM CEST, Flaviu Nistor wrote:
> Add support for an optional label property similar to other hwmon devices.
> This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
> distinct names to each instance.
>
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> ---
>  .../devicetree/bindings/hwmon/amphenol,chipcap2.yaml         | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> index 17351fdbefce..f00b5a4b14dd 100644
> --- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> @@ -33,6 +33,10 @@ properties:
>    reg:
>      maxItems: 1
>
> +  label:
> +    description:
> +      A descriptive name for this channel, like "ambient" or "psu".
> +
>    interrupts:
>      items:
>        - description: measurement ready indicator
> @@ -72,6 +76,7 @@ examples:
>                           <5 IRQ_TYPE_EDGE_RISING>,
>                           <6 IRQ_TYPE_EDGE_RISING>;
>              interrupt-names = "ready", "low", "high";
> +            label = "somelabel";
>              vdd-supply = <&reg_vdd>;
>          };
>      };

Hello Falviu, thank you for your patch.

Should we not add a reference to hwmon-common.yaml (with
unevelautedProperties instead of additionalProperties), as label is
defined there? I believe that Krzysztof Kozlowski did something similar
for the shunt-resistor-micro-ohms property. Could we follow suit here?

I am also not a big fan of a name like "somelabel", and a more
meaningful name from a "real" example would look better. I know that
some examples have already used "somelabel" as an example, but others
have used more meaningful names too.

Best regards,
Javier Carrasco

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: iommu: Fix interrupt type in example
From: Conor Dooley @ 2026-06-22 16:38 UTC (permalink / raw)
  To: Ashish Mhetre
  Cc: joro, will, robin.murphy, robh, krzk+dt, conor+dt, jonathanh,
	thierry.reding, nicolinc, iommu, devicetree, linux-tegra,
	linux-kernel
In-Reply-To: <20260622065410.2780215-1-amhetre@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

^ permalink raw reply

* Re: [PATCH v2 4/5] dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
From: Conor Dooley @ 2026-06-22 16:40 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Yuanshen Cao, linux-sunxi, devicetree, Frank.Li, vkoul, robh,
	dmaengine, conor+dt
In-Reply-To: <20260621215235.13FF01F000E9@smtp.kernel.org>

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

On Sun, Jun 21, 2026 at 09:52:34PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The IRQ handler loop skips trailing DMA channels if the total number of physical channels is not an exact multiple of the channels per register.
> --
> 
> commit 1a8550cc9da2ef939f555f682f0d5ea9606e1907
> Author: Yuanshen Cao <alex.caoys@gmail.com>
> 
> dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
> 
> Add `allwinner,sun60i-a733-dma` to the list of compatible strings for the
> `sun50i-a64-dma` dtbinding documentation.
> 
> >     While the A733 DMA controller shares many similarities with the sun50i-a64
> >     DMA controller, it requires a specific configuration due to differences in:
> >     - Interrupt register layout and mapping.
> >     - Number of channels per interrupt register.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could this truncating
> integer division skip trailing DMA channels if the total number of physical
> channels is not an exact multiple of the channels per register?

What has this got to do with the binding?

> 
> In drivers/dma/sun6i-dma.c:sun6i_dma_interrupt(), the loop condition
> determines which interrupt registers to poll:
> 
>     for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
> 
> For a controller with 12 physical channels (e.g., sun8i_h3_dma_cfg sets
> nr_max_channels = 12) and 8 channels per register, 12 / 8 evaluates to 1.
> 
> Consequently, the loop only iterates once (for i=0), checking interrupts
> for the first 8 channels and completely ignoring the remaining 4 channels.
> Any DMA transfers on channels 8-11 will never have their completion
> interrupts handled, leading to system hangs or driver timeouts.
> 
> Could this be updated to use DIV_ROUND_UP() to ensure all applicable
> interrupt registers are processed?
> 
> >     - Support for higher (32G) address widths in LLI parameters.
> >     
> >     Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com?part=4

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

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: qcom: shikra: Add MDSP carveout memory and update APM DAIs memory regions
From: Mark Brown @ 2026-06-22 16:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Ajay Kumar Nandam, Srinivas Kandagatla, Liam Girdwood,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
	Konrad Dybcio, linux-arm-msm, linux-sound, devicetree,
	linux-kernel
In-Reply-To: <20260622-defiant-warthog-of-authority-d9d1ba@quoll>

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

On Mon, Jun 22, 2026 at 03:42:15PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 18, 2026 at 05:05:09PM +0530, Ajay Kumar Nandam wrote:

> >  						q6apmdai: dais {
> >  							compatible = "qcom,q6apm-dais";
> > -							qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>;

> There is no such line in next-20260619, which means this is some wrong
> base.

There's a pile of in flight stuff listed in the cover letter as
dependencies, likely it's one of those.

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

^ permalink raw reply

* Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver
From: David Lechner @ 2026-06-22 16:42 UTC (permalink / raw)
  To: Jonathan Cameron, Kurt Borja
  Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Linus Walleij, Bartosz Golaszewski, Nuno Sá,
	Andy Shevchenko, linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20260622104728.039a5ea2@jic23-huawei>

On 6/22/26 4:47 AM, Jonathan Cameron wrote:
> On Sun, 21 Jun 2026 19:18:33 -0500
> "Kurt Borja" <kuurtb@gmail.com> wrote:
> 
>> On Sun Jun 21, 2026 at 9:33 AM -05, Jonathan Cameron wrote:
>>> On Mon, 15 Jun 2026 06:30:28 +0200
>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>  
>>>> On 14/06/2026 22:56, Kurt Borja wrote:  
>>>>> On Sat Jun 13, 2026 at 1:59 PM -05, Krzysztof Kozlowski wrote:
>>>>>
>>>>> [...]
>>>>>     
>>>>>> Functions used by probe() should be before probe(), not somewhere in the
>>>>>> middle of the code. IOW, entire probe is together.    
>>>>>
>>>>> I they all are, it's just that regmap stuff takes a huge chunk. I'll
>>>>> check how to reorganize.
>>>>>
>>>>> [...]
>>>>>     
>>>>>>> +static const struct of_device_id ads1262_of_match[] = {
>>>>>>> +	{ .compatible = "ti,ads1262" },
>>>>>>> +	{ .compatible = "ti,ads1263" },    
>>>>>>
>>>>>> So devices are fully compatible? Then it should be expressed in the
>>>>>> binding and drop one entry here.    
>>>>>
>>>>> Not fully compatible as Jonathan said. One is a subset of the other.    
>>>>
>>>> This is THE meaning of compatible!  
>>>
>>> This one I'm in agreement with. It is a strict subset, so should be
>>> using a fallback.  If the fallback is used, you just get support of the
>>> stuff in the simpler chip (or if you can override it with a chip ID
>>> you might still 'upgrade' to the more complex driver support).
>>> If you do end up with properties that only apply to 'new' parts of
>>> the more complex chip then they should be verified as part of the
>>> binding (assuming you can do that without the verifier complaining
>>> - I haven't checked!)  
>>
>> In v1 I had the "adc" subnode which was specific to ADS1263. Then I
>> agreed to drop the subnode but I'm having second thoughts...
>>
>> If we dropped it, then we would still have some specific stuff.
>> #io-channel-cells would be "const: 2" in ADS1263 chips. Also ADS1263's
>> channels would have an extra ti,vref-adc2 prop, for ADC2 voltage
>> reference selection. I should maybe also add a vref-adc2-supply.
>>
>> Maybe it's better to keep the subnode or, again, go for something like:
>>
>>     spi {
>>         multi-adc@0 {
>>             adc@0 {
>>                 ...
>>                 vref-suppy = <&adc1-vref>;
>>
>>                 channel@0 {
>>                     ...
>>                     reference-source = <ADS1262_VREF_AIN0_AIN1>;
>>                 };
>>             };
>>             adc@1 {
>>                 ...
>>                 vref-suppy = <&adc2-vref>;
>>
>>                 channel@0 {
>>                     ...
>>                     reference-source = <ADS1262_VREF_AIN2_AIN3>;
>>                 };
>>             };
>>         };
>>     };
>>
>> In this case we would have to kinda duplicate channel description, but I
>> don't think it's that bad.
>>
>> Jonathan, Krzysztof, David, thoughts?
>>
>> IMO the ADC2 specific voltage reference stuff is a strong argument for a
>> subnode or the above solution.
> 
> Given you end up with channel specific stuff that differs I think it probably
> makes sense - though I do wonder a bit if that is real.  What's the use case
> for using a different reference for the monitoring / debug than the main one?
> I could imagine some dynamic use where you want to sanity check against
> a wider reference range, but maybe that needs userspace control rather than
> in here?  


I think is is going to mostly be the same, so could be simpler to just
add extra channel properties on an as-needed basis if things do actually
differ between ADC1 and ADC2 rather than having to define all channels
twice.

This seems pretty similar to the discussion of how to handle e.g. measuring
the same inputs with and without the burn-out current enabled in the
ti,ads112c14 series and I think you have convinced me that we should not
be having a separate channel in the devicetree for that either.

> 
> Jonathan
> 
> 
>>
>>>
>>> The SLF3F discussion is about (to me) less obvious case of not a strict
>>> subset, but rather being detectable parts with different channel related
>>> properties.  In that case the ID match is necessary for anything to work.
>>> Anyhow, that discussion is in a different thread and not really relevant
>>> here.
>>>
>>> Jonathan
>>>  
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof  
>>
> 


^ permalink raw reply

* Re: [PATCH v2 4/5] dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
From: Conor Dooley @ 2026-06-22 16:42 UTC (permalink / raw)
  To: Frank Li
  Cc: Yuanshen Cao, Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
	Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Maxime Ripard, dmaengine, linux-arm-kernel, linux-sunxi,
	linux-kernel, devicetree
In-Reply-To: <ajhjj7FLn136qMmt@SMW015318>

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

On Sun, Jun 21, 2026 at 05:19:59PM -0500, Frank Li wrote:
> On Sun, Jun 21, 2026 at 09:40:57PM +0000, Yuanshen Cao wrote:
> 
> subject dt-bindings: dmaengine: ....
> 
> > Add `allwinner,sun60i-a733-dma` to the list of compatible strings for the
> > `sun50i-a64-dma` dtbinding documentation.
> >
> > While the A733 DMA controller shares many similarities with the sun50i-a64
> > DMA controller, it requires a specific configuration due to differences in:
> > - Interrupt register layout and mapping.
> > - Number of channels per interrupt register.
> > - Support for higher (32G) address widths in LLI parameters.
> >
> > Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> > ---
> 
> After fix subject tags,

Do not change this unless you're respinning for another reason. dma v
dmaengine is not worth resubmission, especially since dma is far more
commonly used and is the directory name.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 
> >  Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> > index c3e14eb6cfff..1cc3304b7414 100644
> > --- a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> > +++ b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> > @@ -25,6 +25,7 @@ properties:
> >            - allwinner,sun50i-a64-dma
> >            - allwinner,sun50i-a100-dma
> >            - allwinner,sun50i-h6-dma
> > +          - allwinner,sun60i-a733-dma
> >        - items:
> >            - const: allwinner,sun8i-r40-dma
> >            - const: allwinner,sun50i-a64-dma
> > @@ -70,6 +71,7 @@ if:
> >            - allwinner,sun20i-d1-dma
> >            - allwinner,sun50i-a100-dma
> >            - allwinner,sun50i-h6-dma
> > +          - allwinner,sun60i-a733-dma
> >
> >  then:
> >    properties:
> >
> > --
> > 2.54.0
> >

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

^ permalink raw reply

* Re: [PATCH RFC v5 3/6] iio: osf: add protocol decoding
From: Jonathan Cameron @ 2026-06-22 16:43 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
	Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
	linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260616072242.3942-4-kimjinseob88@gmail.com>

On Tue, 16 Jun 2026 16:22:39 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:

> Add helpers for validating and decoding Open Sensor Fusion frames and the
> message payloads used by the initial receive path.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
A few things inline.

> ---
>  drivers/iio/opensensorfusion/osf_protocol.c | 249 ++++++++++++++++++++
>  drivers/iio/opensensorfusion/osf_protocol.h |  97 ++++++++
>  2 files changed, 346 insertions(+)
>  create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c
>  create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h
> 
> diff --git a/drivers/iio/opensensorfusion/osf_protocol.c b/drivers/iio/opensensorfusion/osf_protocol.c
> new file mode 100644
> index 000000000..5bee545f3
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_protocol.c

> +int osf_protocol_decode_frame(const u8 *buf, size_t len,
> +			      struct osf_frame *frame, size_t *frame_len)
> +{
> +	u32 expected_crc;
> +	u32 actual_crc;
> +	u32 payload_len;
> +	size_t total_len;
> +	u8 major;
> +
> +	if (!buf || !frame || !frame_len)
> +		return -EINVAL;
> +
> +	if (len < OSF_FRAME_MIN_LEN)
> +		return -EMSGSIZE;
> +
> +	if (get_unaligned_le32(buf) != OSF_FRAME_MAGIC)
> +		return -EPROTO;
> +
> +	major = buf[4];
> +	if (major != OSF_PROTOCOL_MAJOR)
> +		return -EPROTO;
> +
> +	if (get_unaligned_le16(buf + 6) != OSF_FRAME_HEADER_LEN)
> +		return -EPROTO;
> +
> +	payload_len = get_unaligned_le32(buf + 10);
> +	if (payload_len > len - OSF_FRAME_MIN_LEN)
> +		return -EMSGSIZE;
> +
> +	if (get_unaligned_le32(buf + 34))
> +		return -EPROTO;
> +
> +	total_len = OSF_FRAME_HEADER_LEN + payload_len + OSF_FRAME_CRC_LEN;
> +	expected_crc = osf_crc32_ieee(buf, OSF_FRAME_HEADER_LEN + payload_len);
> +	actual_crc = get_unaligned_le32(buf + OSF_FRAME_HEADER_LEN + payload_len);
> +
> +	if (actual_crc != expected_crc)
> +		return -EBADMSG;
> +
> +	frame->protocol_minor = buf[5];
> +	frame->message_type = get_unaligned_le16(buf + 8);
> +	frame->payload_len = payload_len;
> +	frame->sequence = get_unaligned_le64(buf + 14);
> +	frame->timestamp_us = get_unaligned_le64(buf + 22);
> +	frame->flags = get_unaligned_le32(buf + 30);
> +	frame->payload = buf + OSF_FRAME_HEADER_LEN;
> +	frame->crc = actual_crc;

Same as below wrt to to using designated initializers for these
structure fills

> +	*frame_len = total_len;
> +
> +	return 0;
> +}
> +
> +int osf_protocol_decode_sensor_sample(const struct osf_frame *frame,
> +				      struct osf_sensor_sample *sample)
> +{
> +	u16 channel_count;
> +	u16 sample_format;
> +	u16 sensor_type;
> +	size_t expected_len;
> +	const u8 *payload;
> +
> +	if (!frame || !sample || !frame->payload)
> +		return -EINVAL;
> +
> +	if (frame->message_type != OSF_MSG_SENSOR_SAMPLE)
> +		return -EPROTO;
> +
> +	if (frame->payload_len < OSF_SENSOR_SAMPLE_BASE_LEN)
> +		return -EMSGSIZE;
> +
> +	payload = frame->payload;
> +	sensor_type = get_unaligned_le16(payload);
> +	channel_count = get_unaligned_le16(payload + 4);
> +	sample_format = get_unaligned_le16(payload + 6);
> +
> +	if (!osf_sensor_type_valid(sensor_type))
> +		return -EPROTO;
> +
> +	if (!channel_count)
> +		return -EPROTO;
> +
> +	if (sample_format != OSF_SAMPLE_FORMAT_S32)
> +		return -EPROTO;
> +
> +	if (get_unaligned_le32(payload + 12))
> +		return -EPROTO;
> +
> +	if (channel_count > (SIZE_MAX - OSF_SENSOR_SAMPLE_BASE_LEN) / sizeof(s32))
> +		return -EOVERFLOW;
> +
> +	expected_len = OSF_SENSOR_SAMPLE_BASE_LEN + channel_count * sizeof(s32);
> +	if (frame->payload_len != expected_len)
> +		return -EMSGSIZE;
> +
> +	sample->sensor_type = sensor_type;
> +	sample->sensor_index = get_unaligned_le16(payload + 2);
> +	sample->channel_count = channel_count;
> +	sample->sample_format = sample_format;
> +	sample->scale_nano = get_unaligned_le32(payload + 8);
> +	sample->samples = payload + OSF_SENSOR_SAMPLE_BASE_LEN;
See below. Designated initializer would help readability a little here.

> +
> +	return 0;
> +}
> +
> +int osf_protocol_sensor_sample_value(const struct osf_sensor_sample *sample,
> +				     unsigned int index, s32 *value)

Given channel count is a u16 and we can't be equal or bigger than it, perhaps
use a u16 for index as well.

> +{
> +	if (!sample || !sample->samples || !value)
> +		return -EINVAL;
> +
> +	if (index >= sample->channel_count)
> +		return -ERANGE;
> +
> +	/* Samples are little-endian two's-complement signed values. */
> +	*value = (s32)get_unaligned_le32(sample->samples + index * sizeof(s32));

sizeof(__le32) slightly more appropriate given that is what you are treating it
as.

> +
> +	return 0;
> +}
> +
> +int osf_protocol_decode_device_status(const struct osf_frame *frame,
> +				      struct osf_device_status *status)
> +{
> +	const u8 *payload;
> +
> +	if (!frame || !status || !frame->payload)
> +		return -EINVAL;
> +
> +	if (frame->message_type != OSF_MSG_DEVICE_STATUS)
> +		return -EPROTO;
> +
> +	if (frame->payload_len != OSF_DEVICE_STATUS_LEN)
> +		return -EMSGSIZE;
> +
> +	payload = frame->payload;
> +	if (get_unaligned_le32(payload + 16))
> +		return -EPROTO;
> +
> +	status->uptime_s = get_unaligned_le32(payload);
> +	status->status_flags = get_unaligned_le32(payload + 4);
> +	status->error_flags = get_unaligned_le32(payload + 8);
> +	status->dropped_frames = get_unaligned_le32(payload + 12);
Similar to below. I'd use a designated initializer for status as it
is all written in one place.

> +
> +	return 0;
> +}

> +
> +int osf_protocol_decode_capability_entry(const struct osf_capability_report *report,
> +					 unsigned int index,
> +					 struct osf_capability_entry *entry)
> +{
> +	u16 sample_format;
> +	u16 sensor_type;
> +	u32 flags;
> +	const u8 *payload;
> +
> +	if (!report || !report->entries || !entry)
> +		return -EINVAL;
> +
> +	if (index >= report->capability_count)

Neater to size index to match capability_count.  Not that
important though.

> +		return -ERANGE;
> +
> +	payload = report->entries + index * OSF_CAP_SENSOR_ENTRY_LEN;
> +	sensor_type = get_unaligned_le16(payload);
> +	sample_format = get_unaligned_le16(payload + 6);
> +	flags = get_unaligned_le32(payload + 12);
> +
> +	if (!osf_sensor_type_valid(sensor_type))
> +		return -EPROTO;
> +
> +	if (sample_format != OSF_SAMPLE_FORMAT_S32)
> +		return -EPROTO;
> +
> +	if (flags & ~OSF_CAPABILITY_FLAGS_MASK)
> +		return -EPROTO;
> +
> +	if (get_unaligned_le32(payload + 16))
> +		return -EPROTO;
> +
> +	entry->sensor_type = sensor_type;
> +	entry->sensor_index = get_unaligned_le16(payload + 2);
> +	entry->channel_count = get_unaligned_le16(payload + 4);
> +	entry->sample_format = sample_format;
> +	entry->scale_nano = get_unaligned_le32(payload + 8);
> +	entry->flags = flags;
neater as designated initializer I think.

	*entry = (struct osf_capability_entry) {
		.sensor_type = sensor_type,
		.sensor_index = get_unaligned_le16(payload + 2),
etc
	};
> +
> +	return 0;
> +}

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: thermal: amlogic: Fix missing header in the example
From: Conor Dooley @ 2026-06-22 16:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Ronald Claveau, linux-pm, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260622100231.438435-3-krzysztof.kozlowski@oss.qualcomm.com>

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

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

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: thermal: amlogic: Correct 'reg' in the example
From: Conor Dooley @ 2026-06-22 16:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Ronald Claveau, linux-pm, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260622100231.438435-4-krzysztof.kozlowski@oss.qualcomm.com>

[-- Attachment #1: Type: text/plain, Size: 53 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>


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

^ permalink raw reply

* [PATCH v18 0/4] Add SDHI support for RZ/G3L SoC
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
  To: Geert Uytterhoeven, Linus Walleij, Magnus Damm, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Biju Das, linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
	Prabhakar Mahadev Lad, Biju Das

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

This series adds pin control and device tree support for the three
SDHI (SD/MMC host interface) controllers on the Renesas RZ/G3L SoC
(r9a08g046), and enables them on the RZ/G3L SMARC EVK platform.

This patch series depend on [1]
[1] https://lore.kernel.org/all/20260622155610.184271-1-biju.das.jz@bp.renesas.com/

v18:
 * Split from patch series [2]
 * Moved sd_ch2 variable near to sd_ch[].
 
[2] https://lore.kernel.org/all/20260603065731.93243-1-biju.das.jz@bp.renesas.com/

Biju Das (4):
  pinctrl: renesas: rzg2l: Add SD channel POC support for RZ/G3L
  arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and
    SDHI1 pincontrol on SMARC EVK
  arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0
  arm64: dts: renesas: rzg3l-smarc-som: Enable SDHI2

 arch/arm64/boot/dts/renesas/r9a08g046.dtsi    |  73 ++++++-
 .../boot/dts/renesas/r9a08g046l48-smarc.dts   |  89 ++++++++
 .../boot/dts/renesas/rzg3l-smarc-som.dtsi     | 199 ++++++++++++++++++
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       |  74 ++++---
 4 files changed, 410 insertions(+), 25 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH v18 2/4] arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
  To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
	Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260622164819.184674-1-biju.das.jz@bp.renesas.com>

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

Add device tree nodes for the three SDHI controllers (SDHI{0,1,2})
on the RZ/G3L SoC (r9a08g046) and enable SDHI1 on the RZ/G3L SMARC
EVK platform with pincontrol and GPIO-based voltage switching
regulator support.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
 * No change.
 * This patch depend on [1]
[1] https://lore.kernel.org/all/20260622155610.184271-2-biju.das.jz@bp.renesas.com/
v1->v17:
 * No change.
---
 arch/arm64/boot/dts/renesas/r9a08g046.dtsi    | 73 ++++++++++++++-
 .../boot/dts/renesas/r9a08g046l48-smarc.dts   | 88 +++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
index c63a857f0e5b..ff2de3f192b5 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
@@ -762,9 +762,80 @@ dmac: dma-controller@11820000 {
 			dma-channels = <16>;
 		};
 
+		sdhi0: mmc@11c00000 {
+			compatible = "renesas,sdhi-r9a08g046";
+			reg = <0x0 0x11c00000 0 0x10000>;
+			interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A08G046_SDHI0_IMCLK>,
+				 <&cpg CPG_MOD R9A08G046_SDHI0_CLK_HS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI0_IMCLK2>,
+				 <&cpg CPG_MOD R9A08G046_SDHI0_IACLKS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI0_IACLKM>;
+			clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+			max-frequency = <150000000>;
+			resets = <&cpg R9A08G046_SDHI0_IXRST>,
+				 <&cpg R9A08G046_SDHI0_IXRSTAXIM>,
+				 <&cpg R9A08G046_SDHI0_IXRSTAXIS>;
+			reset-names = "rst", "axim", "axis";
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
+
 		sdhi1: mmc@11c10000 {
+			compatible = "renesas,sdhi-r9a08g046";
 			reg = <0x0 0x11c10000 0 0x10000>;
-			/* placeholder */
+			interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A08G046_SDHI1_IMCLK>,
+				 <&cpg CPG_MOD R9A08G046_SDHI1_CLK_HS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI1_IMCLK2>,
+				 <&cpg CPG_MOD R9A08G046_SDHI1_IACLKS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI1_IACLKM>;
+			clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+			max-frequency = <150000000>;
+			resets = <&cpg R9A08G046_SDHI1_IXRST>,
+				 <&cpg R9A08G046_SDHI1_IXRSTAXIM>,
+				 <&cpg R9A08G046_SDHI1_IXRSTAXIS>;
+			reset-names = "rst", "axim", "axis";
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			sdhi1_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI1-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-ramp-delay = <1200>;
+				status = "disabled";
+			};
+		};
+
+		sdhi2: mmc@11c20000 {
+			compatible = "renesas,sdhi-r9a08g046";
+			reg = <0x0 0x11c20000 0 0x10000>;
+			interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A08G046_SDHI2_IMCLK>,
+				 <&cpg CPG_MOD R9A08G046_SDHI2_CLK_HS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI2_IMCLK2>,
+				 <&cpg CPG_MOD R9A08G046_SDHI2_IACLKS>,
+				 <&cpg CPG_MOD R9A08G046_SDHI2_IACLKM>;
+			clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+			max-frequency = <150000000>;
+			resets = <&cpg R9A08G046_SDHI2_IXRST>,
+				 <&cpg R9A08G046_SDHI2_IXRSTAXIM>,
+				 <&cpg R9A08G046_SDHI2_IXRSTAXIS>;
+			reset-names = "rst", "axim", "axis";
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			sdhi2_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI2-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-ramp-delay = <1200>;
+				status = "disabled";
+			};
 		};
 
 		eth0: ethernet@11c30000 {
diff --git a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
index 5289efd1a430..0b6b7e109200 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
@@ -14,6 +14,7 @@
 #define SW_GPIO4		1
 #define SW_I3C_EN		0
 #define SW_SER0_PMOD		1
+#define SW_SDIO_M2E		0
 
 #define PMOD_GPIO4		0
 #define PMOD_GPIO6		0
@@ -38,6 +39,7 @@ / {
 	aliases {
 		i2c2 = &i2c2;
 		i2c3 = &i2c3;
+		mmc1 = &sdhi1;
 		serial0 = &rsci2;
 		serial1 = &rsci3;
 		serial2 = &rsci1;
@@ -69,6 +71,19 @@ codec_dai: codec {
 		};
 	};
 #endif
+
+#if RZ_BOOT_MODE3
+	vqmmc_sd1_pvdd: regulator-vqmmc-sd1-pvdd {
+		compatible = "regulator-gpio";
+		regulator-name = "SD1_PVDD";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&pinctrl RZG3L_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
+		gpios-states = <0>;
+		states = <3300000 0>, <1800000 1>;
+		regulator-ramp-delay = <1200>;
+	};
+#endif
 };
 
 &i2c2 {
@@ -175,6 +190,68 @@ scif0_pins: scif0 {
 		power-source = <1800>;
 	};
 
+#if RZ_BOOT_MODE3
+	sd1-pwr-en-hog {
+		gpio-hog;
+		gpios = <RZG3L_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd1_pwr_en";
+	};
+#endif
+
+	sdhi1_pins: sd1 {
+		sd1-cd {
+			pinmux = <RZG3L_PORT_PINMUX(J, 0, 8)>; /* SD1_CD */
+		};
+
+		sd1-clk {
+			pinmux = <RZG3L_PORT_PINMUX(G, 0, 1)>; /* SD1_CLK */
+			power-source = <3300>;
+		};
+
+		sd1-cmd {
+			pinmux = <RZG3L_PORT_PINMUX(G, 1, 1)>; /* SD1_CMD */
+			input-enable;
+			power-source = <3300>;
+			bias-pull-up;
+		};
+
+		sd1-data {
+			pinmux = <RZG3L_PORT_PINMUX(G, 2, 1)>, /* SD1_DAT0 */
+				 <RZG3L_PORT_PINMUX(G, 3, 1)>, /* SD1_DAT1 */
+				 <RZG3L_PORT_PINMUX(G, 4, 1)>, /* SD1_DAT2 */
+				 <RZG3L_PORT_PINMUX(G, 5, 1)>; /* SD1_DAT3 */
+			input-enable;
+			power-source = <3300>;
+		};
+	};
+
+	sdhi1_uhs_pins: sd1-uhs {
+		sd1-cd {
+			pinmux = <RZG3L_PORT_PINMUX(J, 0, 8)>; /* SD1_CD */
+		};
+
+		sd1-clk {
+			pinmux = <RZG3L_PORT_PINMUX(G, 0, 1)>; /* SD1_CLK */
+			power-source = <1800>;
+		};
+
+		sd1-cmd {
+			pinmux = <RZG3L_PORT_PINMUX(G, 1, 1)>; /* SD1_CMD */
+			input-enable;
+			power-source = <1800>;
+		};
+
+		sd1-data {
+			pinmux = <RZG3L_PORT_PINMUX(G, 2, 1)>, /* SD1_DAT0 */
+				 <RZG3L_PORT_PINMUX(G, 3, 1)>, /* SD1_DAT1 */
+				 <RZG3L_PORT_PINMUX(G, 4, 1)>, /* SD1_DAT2 */
+				 <RZG3L_PORT_PINMUX(G, 5, 1)>; /* SD1_DAT3 */
+			input-enable;
+			power-source = <1800>;
+		};
+	};
+
 	ssi0_pins: ssi0 {
 		pinmux = <RZG3L_PORT_PINMUX(H, 0, 9)>, /* SSIF0_RXD */
 			 <RZG3L_PORT_PINMUX(H, 1, 9)>, /* SSIF0_BCK */
@@ -230,6 +307,17 @@ &scif0 {
 	pinctrl-names = "default";
 };
 
+#if RZ_BOOT_MODE3
+&sdhi1 {
+	pinctrl-0 = <&sdhi1_pins>;
+	pinctrl-1 = <&sdhi1_uhs_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vqmmc_sd1_pvdd>;
+};
+#endif
+
 #if !SW_SD2_EN
 &ssi0 {
 	clocks = <&cpg CPG_MOD R9A08G046_SSI0_PCLK2>,
-- 
2.43.0


^ permalink raw reply related

* [PATCH v18 3/4] arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
  To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
	Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260622164819.184674-1-biju.das.jz@bp.renesas.com>

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

Add support for enabling SD card or eMMC on SDHI0 on the RZ/G3L SMARC
SoM. The selection between SD and eMMC is controlled by the
SW_SD0_DEV_SEL macro in the board DTS, which must match the position
of switch SYS.1 on the SoM. By default, eMMC is enabled.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
 * No change.
v1->v17:
 * No change.
---
 .../boot/dts/renesas/r9a08g046l48-smarc.dts   |   1 +
 .../boot/dts/renesas/rzg3l-smarc-som.dtsi     | 111 ++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
index 0b6b7e109200..96cc7ee46a6a 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
@@ -9,6 +9,7 @@
 
 /* Switch selection settings */
 #define RZ_BOOT_MODE3		1
+#define SW_SD0_DEV_SEL		0
 #define SW_SD2_EN		0
 #define SW_DPI_EN		0
 #define SW_GPIO4		1
diff --git a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
index 091a227233cb..446c7780cb30 100644
--- a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
@@ -9,6 +9,10 @@
  * Please set the below switch position on the SoM and the corresponding macro
  * on the board DTS:
  *
+ * Switch position SYS.1, Macro SW_SD0_DEV_SEL:
+ *      0 - SD0 is connected to eMMC (default)
+ *      1 - SD0 is connected to uSD0 card
+ *
  * Switch position SYS.2, Macro SW_I3C_EN:
  *      0 - SMARC_I2C_GP is enabled
  *      1 - I3C is enabled
@@ -37,6 +41,7 @@ aliases {
 		ethernet0 = &eth0;
 		ethernet1 = &eth1;
 		i2c0 = &i2c0;
+		mmc0 = &sdhi0;
 	};
 
 	memory@48000000 {
@@ -63,6 +68,19 @@ reg_3p3v: regulator-3p3v {
 		regulator-always-on;
 	};
 
+#if SW_SD0_DEV_SEL
+	vqmmc_sd0_pvdd: vqmmc-sd0-pvdd {
+		compatible = "regulator-gpio";
+		regulator-name = "SD0_PVDD";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&pinctrl RZG3L_GPIO(L, 4) GPIO_ACTIVE_HIGH>;
+		gpios-states = <0>;
+		states = <3300000 0>, <1800000 1>;
+		regulator-ramp-delay = <1200>;
+	};
+#endif
+
 	x2_clk: x2-clock {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -216,7 +234,100 @@ i2c0_pins: i2c0 {
 		pinmux = <RZG3L_PORT_PINMUX(L, 2, 4)>, /* RIIC0_SCL */
 			 <RZG3L_PORT_PINMUX(L, 3, 4)>; /* RIIC0_SDA */
 	};
+
+	sd0-pwr-en-hog {
+		gpio-hog;
+		gpios = <RZG3L_GPIO(5, 1) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd0_pwr_en";
+	};
+
+	sdhi0_emmc_pins: sd0-emmc {
+		sd0-ctrl {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source = <1800>;
+		};
+
+		sd0-data {
+			pins = "SD0_DAT0", "SD0_DAT1", "SD0_DAT2", "SD0_DAT3",
+			       "SD0_DAT4", "SD0_DAT5", "SD0_DAT6", "SD0_DAT7";
+			power-source = <1800>;
+		};
+
+		sd0-rst {
+			pins = "SD0_RST#";
+			power-source = <1800>;
+		};
+
+		sd0-ds {
+			pins = "SD0_DS";
+			power-source = <1800>;
+		};
+	};
+
+	sdhi0_usd_pins: sd0-usd {
+		sd0-cd {
+			pinmux = <RZG2L_PORT_PINMUX(5, 0, 8)>; /* SD0_CD */
+		};
+
+		sd0-ctrl {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source = <3300>;
+		};
+
+		sd0-data {
+			pins = "SD0_DAT0", "SD0_DAT1", "SD0_DAT2", "SD0_DAT3";
+			power-source = <3300>;
+		};
+	};
+
+	sdhi0_usd_uhs_pins: sd0-usd-uhs {
+		sd0-cd {
+			pinmux = <RZG2L_PORT_PINMUX(5, 0, 8)>; /* SD0_CD */
+		};
+
+		sd0-ctrl {
+			pins = "SD0_CLK", "SD0_CMD";
+			power-source = <1800>;
+		};
+
+		sd0-data {
+			pins = "SD0_DAT0", "SD0_DAT1", "SD0_DAT2", "SD0_DAT3";
+			power-source = <1800>;
+		};
+	};
+};
+
+#if (SW_SD0_DEV_SEL)
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_usd_pins>;
+	pinctrl-1 = <&sdhi0_usd_uhs_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vqmmc_sd0_pvdd>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+#else
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_emmc_pins>;
+	pinctrl-1 = <&sdhi0_emmc_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+	bus-width = <8>;
+	mmc-hs200-1_8v;
+	mmc-hs400-1_8v;
+	mmc-hs400-enhanced-strobe;
+	non-removable;
+	fixed-emmc-driver-type = <1>;
+	status = "okay";
 };
+#endif
 
 &wdt0 {
 	timeout-sec = <60>;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v18 4/4] arm64: dts: renesas: rzg3l-smarc-som: Enable SDHI2
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
  To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
	Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260622164819.184674-1-biju.das.jz@bp.renesas.com>

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

Enable SDHI2 on the RZ/G3L SMARC EVK platform using the internal
voltage regulator for voltage switching. SDHI2 signals are muxed
with I2S0; the selection is controlled by the SW_SD2_EN macro in
the board DTS, which must match the position of switch SYS.4 on
the SoM. By default, I2S0 is enabled.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
 * No change.
v1->v17:
 * No change.
---
 .../boot/dts/renesas/rzg3l-smarc-som.dtsi     | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
index 446c7780cb30..3d5e6b8489a9 100644
--- a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
@@ -42,6 +42,7 @@ aliases {
 		ethernet1 = &eth1;
 		i2c0 = &i2c0;
 		mmc0 = &sdhi0;
+		mmc2 = &sdhi2;
 	};
 
 	memory@48000000 {
@@ -296,6 +297,74 @@ sd0-data {
 			power-source = <1800>;
 		};
 	};
+
+	sdhi2_pins: sd2 {
+		sd2-cd {
+			pinmux = <RZG3L_PORT_PINMUX(K, 0, 1)>; /* SD2_CD */
+		};
+
+		sd2-clk {
+			pinmux = <RZG3L_PORT_PINMUX(H, 0, 1)>; /* SD2_CLK */
+			power-source = <3300>;
+		};
+
+		sd2-cmd {
+			pinmux = <RZG3L_PORT_PINMUX(H, 1, 1)>; /* SD2_CMD */
+			input-enable;
+			power-source = <3300>;
+		};
+
+		sd2-data {
+			pinmux = <RZG3L_PORT_PINMUX(H, 2, 1)>, /* SD2_DAT0 */
+				 <RZG3L_PORT_PINMUX(H, 3, 1)>, /* SD2_DAT1 */
+				 <RZG3L_PORT_PINMUX(H, 4, 1)>, /* SD2_DAT2 */
+				 <RZG3L_PORT_PINMUX(H, 5, 1)>; /* SD2_DAT3 */
+			input-enable;
+			power-source = <3300>;
+		};
+
+		sd2-iovs {
+			pinmux = <RZG3L_PORT_PINMUX(K, 1, 1)>; /* SD2_IOVS */
+		};
+
+		sd2-pwen {
+			pinmux = <RZG3L_PORT_PINMUX(K, 2, 1)>; /* SD2_PWEN */
+		};
+	};
+
+	sdhi2_pins_uhs: sd2-uhs {
+		sd2-cd {
+			pinmux = <RZG3L_PORT_PINMUX(K, 0, 1)>; /* SD2_CD */
+		};
+
+		sd2-clk {
+			pinmux = <RZG3L_PORT_PINMUX(H, 0, 1)>; /* SD2_CLK */
+			power-source = <1800>;
+		};
+
+		sd2-cmd {
+			pinmux = <RZG3L_PORT_PINMUX(H, 1, 1)>; /* SD2_CMD */
+			input-enable;
+			power-source = <1800>;
+		};
+
+		sd2-data {
+			pinmux = <RZG3L_PORT_PINMUX(H, 2, 1)>, /* SD2_DAT0 */
+				 <RZG3L_PORT_PINMUX(H, 3, 1)>, /* SD2_DAT1 */
+				 <RZG3L_PORT_PINMUX(H, 4, 1)>, /* SD2_DAT2 */
+				 <RZG3L_PORT_PINMUX(H, 5, 1)>; /* SD2_DAT3 */
+			input-enable;
+			power-source = <1800>;
+		};
+
+		sd2-iovs {
+			pinmux = <RZG3L_PORT_PINMUX(K, 1, 1)>; /* SD2_IOVS */
+		};
+
+		sd2-pwen {
+			pinmux = <RZG3L_PORT_PINMUX(K, 2, 1)>; /* SD2_PWEN */
+		};
+	};
 };
 
 #if (SW_SD0_DEV_SEL)
@@ -329,6 +398,25 @@ &sdhi0 {
 };
 #endif
 
+#if SW_SD2_EN
+&sdhi2 {
+	pinctrl-0 = <&sdhi2_pins>;
+	pinctrl-1 = <&sdhi2_pins_uhs>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&sdhi2_vqmmc>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+
+&sdhi2_vqmmc {
+	status = "okay";
+};
+#endif
+
 &wdt0 {
 	timeout-sec = <60>;
 	status = "okay";
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH RFC v5 5/6] iio: osf: add UART transport
From: Jonathan Cameron @ 2026-06-22 16:49 UTC (permalink / raw)
  To: Jinseob Kim
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
	Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
	linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260616072242.3942-6-kimjinseob88@gmail.com>

On Tue, 16 Jun 2026 16:22:41 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:

> Add the serdev UART transport and the initial OSF core receive path.
> 
> Enable the required vcc regulator with devm_regulator_get_enable()
> before opening the UART, keeping power handling limited to the simple
> probe-time requirement for this RFC.
> 
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
A few things inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/opensensorfusion/Kconfig b/drivers/iio/opensensorfusion/Kconfig
> new file mode 100644
> index 000000000..d393eb3aa
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config OPEN_SENSOR_FUSION
> +	tristate "Open Sensor Fusion UART IIO driver"
> +	depends on IIO
> +	depends on SERIAL_DEV_BUS
> +	select CRC32
> +	help
> +	  Build the Open Sensor Fusion UART receive path.
> +
> +	  The driver receives OSF protocol frames over a serdev UART.
> +	  Frames are decoded and validated before being passed to the
> +	  driver core.
> +	  This patch only adds the transport path.
> +	  IIO device registration is added separately.

Don't talk about a patch in here.  Talk about what is supported then
if you really want to add the other bits in later patches.  Mostly
this help is generic enough we don't need to modify it more than
once in a series.

> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> new file mode 100644
> index 000000000..137fb7166
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_core.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +
> +#include "osf_core.h"
> +#include "osf_protocol.h"
> +
> +#define OSF_RESERVED_MSG_FIRST		0x7f00
> +#define OSF_RESERVED_MSG_LAST		0x7fff
> +#define OSF_VENDOR_PRIVATE_FIRST	0x8000
> +
> +void osf_core_init(struct osf_device *osf, struct device *dev)
> +{
> +	memset(osf, 0, sizeof(*osf));
	*osf = (struct osf_device){
		.dev = dev,
	};

is guaranteed to also clear all other fields (new C spec as
well as the options the kernel has long been built with)
so is how I would always do cases of zero then set stuff like
this.

> +	osf->dev = dev;
> +}


> diff --git a/drivers/iio/opensensorfusion/osf_serdev.c b/drivers/iio/opensensorfusion/osf_serdev.c
> new file mode 100644
> index 000000000..624cb01fe
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_serdev.c

> +
> +static void osf_serdev_remove(struct serdev_device *serdev)
> +{
> +	struct osf_serdev *osf_uart = serdev_device_get_drvdata(serdev);
> +
> +	serdev_device_close(serdev);
> +	osf_stream_reset(&osf_uart->stream);
> +	osf_core_unregister_iio(&osf_uart->osf);

My gut feeling is this should be first to tear down the device
interfaces as soon as possible.  They will have been initialized
after the serdev was opened so should be unregistered before it is closed.
If there is a reason for this specific order add a comment.

> +}

> +
> +static struct serdev_device_driver osf_serdev_driver = {
> +	.probe = osf_serdev_probe,
> +	.remove = osf_serdev_remove,
> +	.driver = {
> +		.name = "open-sensor-fusion-uart",
> +		.of_match_table = osf_serdev_of_match,
> +	},
> +};
> +

No blank line here as the macro is extremely tightly coupled
with the structure and it is nice to have the visual cue.

> +module_serdev_device_driver(osf_serdev_driver);
> +
> +MODULE_DESCRIPTION("Open Sensor Fusion IIO driver");
> +MODULE_LICENSE("GPL");


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: clock: Drop incorrect usage of double '::'
From: Conor Dooley @ 2026-06-22 16:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Griffin, Alim Akhtar, Michael Turquette,
	Stephen Boyd, Brian Masney, Sylwester Nawrocki, Chanwoo Choi,
	Sam Protsenko, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
	Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Inki Dae, Seung-Woo Kim, Kyungmin Park,
	Andi Shyti, Georgi Djakov, Lee Jones, Pavel Machek, Hans Verkuil,
	Mauro Carvalho Chehab, Ulf Hansson, Peter Rosin, Vinod Koul,
	Neil Armstrong, Linus Walleij, Geert Uytterhoeven, Magnus Damm,
	Sebastian Reichel, Javier Martinez Canillas, Liam Girdwood,
	Mark Brown, Greg Kroah-Hartman, Jiri Slaby, Srinivas Kandagatla,
	Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Jonathan Marek, Taniya Das, Robert Marko,
	Christian Marangi, Stephan Gerhold, Adam Skladowski,
	Sireesh Kodali, Barnabas Czeman, Imran Shaik,
	Sricharan Ramabadhran, Anusha Rao, Luo Jie, Tomasz Figa,
	Chanho Park, Sunyeal Hong, Shin Son, Krishna Manikandan,
	Jacek Anaszewski, Jaehoon Chung, Marek Szyprowski, Alina Yu,
	Andy Gross, Niklas Söderlund, Wesley Cheng, linux-arm-msm,
	devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	linux-clk, dri-devel, freedreno, linux-i2c, linux-pm, linux-leds,
	linux-media, linux-mmc, linux-phy, linux-gpio, linux-renesas-soc,
	linux-serial, linux-sound, linux-usb
In-Reply-To: <20260622101606.485961-3-krzysztof.kozlowski@oss.qualcomm.com>

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

On Mon, Jun 22, 2026 at 12:16:07PM +0200, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
> 
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
> 
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

^ permalink raw reply

* Re: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Conor Dooley @ 2026-06-22 16:53 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: sashiko-reviews, neil.armstrong, vkoul, dri-devel, robh, conor+dt,
	linux-phy, olteanv, devicetree, wsa+renesas
In-Reply-To: <0a6f92ba-de8c-48ae-9025-a7570e53e453@oss.qualcomm.com>

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

On Mon, Jun 22, 2026 at 03:23:05PM +0200, Krzysztof Kozlowski wrote:
> On 22/06/2026 12:19, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] Unintended replacement of BNF notation `::=` with `:=`.
> > --
> > 
> > commit a9fdd0b065eb5b6394a9c33ffba0fd5194e80115
> > Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > 
> > dt-bindings: Drop incorrect usage of double '::'
> > 
> > This commit cleans up device tree binding schemas by removing the redundant
> > double colons at the end of descriptions inside YAML literal style blocks.
> > 
> > Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> > 
> >> diff --git a/Documentation/devicetree/bindings/mux/mux-consumer.yaml b/Documentation/devicetree/bindings/mux/mux-consumer.yaml
> >> index 9e2d78a78e409..769243a2bf046 100644
> >> --- a/Documentation/devicetree/bindings/mux/mux-consumer.yaml
> >> +++ b/Documentation/devicetree/bindings/mux/mux-consumer.yaml
> >> @@ -13,8 +13,8 @@ description: |
> >>    Mux controller consumers should specify a list of mux controllers that they
> >>    want to use with a property containing a 'mux-ctrl-list':
> >>  
> >> -    mux-ctrl-list ::= <single-mux-ctrl> [mux-ctrl-list]
> >> -    single-mux-ctrl ::= <mux-ctrl-phandle> [mux-ctrl-specifier]
> >> +    mux-ctrl-list := <single-mux-ctrl> [mux-ctrl-list]
> >> +    single-mux-ctrl := <mux-ctrl-phandle> [mux-ctrl-specifier]
> > 
> > [Severity: Low]
> > Does this unintentionally change the standard Backus-Naur Form (BNF) ::=
> > operator to :=? It looks like a global search-and-replace for :: might
> 
> 
> I wasn't sure about that... and indeed this should not be changed. I
> will send a v2.

Other than this one that I agree shouldn't be changed to a single :,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

(Although I find the syntax used here to be really confusing and a good
example in the text would be a lot clearer. OTOH, how to write consumers
is, I think, well understood, since they all work the same.)

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

^ permalink raw reply


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