Linux Documentation
 help / color / mirror / Atom feed
* [PATCH 0/3] iio: dac: Add support for AD5529R DAC
From: Janani Sunil @ 2026-05-07 16:28 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
	Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
	Janani Sunil

This patch series adds support for Analog Devices AD5529R, a 16 channel
16 and 12 bit voltage Digital-to-Analog Converter (DAC) with integrated
precision reference. The AD5529R operates from both unipolar and
bipolar supplies. The device communicates via SPI interface.

**Device Overview:**
The AD5529R features 16 independent DAC channels, with 16 or 12 bit
resolution, allowing independently programmable output ranges. The
internal 4.096V precision reference sets the accuracy of the output
voltage.

**Features Implemented:**
- Automatic detection of 12/16 bit generic with product ID read.
- Reset support via GPIO.
- Dual regmap configuration to handle 8 and 16 bit registers.

**Patch Summary:**
1. **dt-bindings**: Binding documentation with channel configuration.
2. **driver**: Implement IIO DAC Driver with regmap support.
3. **documentation**: Add driver documentation with usage examples.

**Testing:**
The driver was compiled and tested on the EVAL-AD5529R-ARDZ using a
coraZ7 with a mainline v7.0 kernel.

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
Janani Sunil (3):
      dt-bindings: iio: dac: Add AD5529R
      iio: dac: Add AD5529R DAC driver support
      Documentation: iio: Add AD5529R Documentation

 .../devicetree/bindings/iio/dac/adi,ad5529r.yaml   |  99 ++++
 Documentation/iio/ad5529r.rst                      | 214 ++++++++
 Documentation/iio/index.rst                        |   1 +
 MAINTAINERS                                        |   9 +
 drivers/iio/dac/Kconfig                            |  17 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/ad5529r.c                          | 564 +++++++++++++++++++++
 7 files changed, 905 insertions(+)
---
base-commit: 93df88612859e8e19dec93c69d563b4b73e9bd4b
change-id: 20260507-ad5529r-driver-866bbdd864de

Best regards,
-- 
Janani Sunil <janani.sunil@analog.com>


^ permalink raw reply

* [PATCH 1/3] dt-bindings: iio: dac: Add AD5529R
From: Janani Sunil @ 2026-05-07 16:28 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
	Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
	Janani Sunil
In-Reply-To: <20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com>

Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
buffered voltage output digital-to-analog converter (DAC) with an
integrated precision reference.

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
 .../devicetree/bindings/iio/dac/adi,ad5529r.yaml   | 99 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 ++
 2 files changed, 106 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
new file mode 100644
index 000000000000..ba559dde75fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/adi,ad5529r.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD5529R 16-Channel 12/16-bit High Voltage DAC
+
+maintainers:
+  - Janani Sunil <janani.sunil@analog.com>
+
+description: |
+  The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
+  digital-to-analog converter (DAC) with an integrated precision reference.
+  The device operates from unipolar and bipolar supplies. It is guaranteed
+  monotonic and has built-in rail-to-rail output buffers that can source or
+  sink up to 25mA.
+
+  Specifications:
+  * 16 independent 12-bit or 16-bit DAC channels
+  * Independently programmable output ranges: 0V to 5V, 0V to 10V, 0V to 20V,
+    0V to 40V, ±5V to ±15V, ±10V to ±20V
+  * 4.096V precision reference, 12ppm/°C maximum
+  * Built-in function generation: Toggle, Sinusoidal Dither, and Ramp waveforms
+  * Multiplexer for output voltage, load current sense and die temperature
+
+  Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
+
+properties:
+  compatible:
+    const: adi,ad5529r
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  spi-cpha:
+    description:
+      SPI clock phase (Optional). The AD5529R supports SPI mode 0 and mode 3.
+
+  spi-cpol:
+    description:
+      SPI clock polarity (Optional). The AD5529R supports SPI mode 0 and mode 3.
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the RESET pin. Active low. When asserted low,
+      performs a power-on reset and initializes the device to its default state.
+
+  vdd-supply:
+    description: Digital power supply (typically 3.3V)
+
+  avdd-supply:
+    description: Analog power supply (typically 5V)
+
+  hvdd-supply:
+    description: High voltage positive supply (up to 40V for output range)
+
+  hvss-supply:
+    description: High voltage negative supply (ground or negative voltage)
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - avdd-supply
+  - hvdd-supply
+  - hvss-supply
+
+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";
+            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>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index d6c3c7d22403..320e84765ce6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1507,6 +1507,13 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4851.yaml
 F:	drivers/iio/adc/ad4851.c
 
+ANALOG DEVICES INC AD5529R DRIVER
+M:	Janani Sunil <janani.sunil@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+
 ANALOG DEVICES INC AD5706R DRIVER
 M:	Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
 L:	linux-iio@vger.kernel.org

-- 
2.43.0


^ permalink raw reply related

* [PATCH 2/3] iio: dac: Add AD5529R DAC driver support
From: Janani Sunil @ 2026-05-07 16:28 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
	Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
	Janani Sunil
In-Reply-To: <20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com>

Add support for AD5529R 16-channel, 12/16 bit Digital to Analog Converter

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/dac/Kconfig   |  17 ++
 drivers/iio/dac/Makefile  |   1 +
 drivers/iio/dac/ad5529r.c | 564 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 583 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 320e84765ce6..143714e27d51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1513,6 +1513,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+F:	drivers/iio/dac/ad5529r.c
 
 ANALOG DEVICES INC AD5706R DRIVER
 M:	Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..bb1d59889a2a 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -134,6 +134,23 @@ config AD5449
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad5449.
 
+config AD5529R
+	tristate "Analog Devices AD5529R High Voltage DAC driver"
+	depends on SPI_MASTER
+	select REGMAP_SPI
+	help
+	  Say yes here to build support for Analog Devices AD5529R
+	  16-Channel, 12-Bit/16-Bit, 40V High Voltage Precision Digital to Analog
+	  Converter.
+
+	  The device features multiple output voltage ranges from -20V to +20V,
+	  built-in 4.096V voltage reference, and digital functions including
+	  toggle, dither, and ramp modes. Supports both 12-bit and 16-bit
+	  resolution variants.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad5529r.
+
 config AD5592R_BASE
 	tristate
 
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 003431798498..f35e060b3643 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_AD5446) += ad5446.o
 obj-$(CONFIG_AD5446_SPI) += ad5446-spi.o
 obj-$(CONFIG_AD5446_I2C) += ad5446-i2c.o
 obj-$(CONFIG_AD5449) += ad5449.o
+obj-$(CONFIG_AD5529R) += ad5529r.o
 obj-$(CONFIG_AD5592R_BASE) += ad5592r-base.o
 obj-$(CONFIG_AD5592R) += ad5592r.o
 obj-$(CONFIG_AD5593R) += ad5593r.o
diff --git a/drivers/iio/dac/ad5529r.c b/drivers/iio/dac/ad5529r.c
new file mode 100644
index 000000000000..b42e12da4de3
--- /dev/null
+++ b/drivers/iio/dac/ad5529r.c
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AD5529R Digital-to-Analog Converter Driver
+ * 16-Channel, 12/16-Bit, 40V High Voltage Precision DAC
+ *
+ * Copyright 2026 Analog Devices Inc.
+ * Author: Janani Sunil <janani.sunil@analog.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/spi/spi.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
+
+/* Register Map */
+#define AD5529R_REG_INTERFACE_CONFIG_A		0x00
+#define AD5529R_REG_INTERFACE_CONFIG_B		0x01
+#define AD5529R_REG_DEVICE_CONFIG		0x02
+#define AD5529R_REG_CHIP_TYPE			0x03
+#define AD5529R_REG_PRODUCT_ID_L		0x04
+#define AD5529R_REG_PRODUCT_ID_H		0x05
+#define AD5529R_REG_CHIP_GRADE			0x06
+#define AD5529R_REG_SCRATCH_PAD			0x0A
+#define AD5529R_REG_SPI_REVISION		0x0B
+#define AD5529R_REG_VENDOR_L			0x0C
+#define AD5529R_REG_VENDOR_H			0x0D
+#define AD5529R_REG_STREAM_MODE			0x0E
+#define AD5529R_REG_TRANSFER_CONFIG		0x0F
+#define AD5529R_REG_INTERFACE_CONFIG_C		0x10
+#define AD5529R_REG_INTERFACE_STATUS_A		0x11
+
+/* Configuration registers */
+#define AD5529R_REG_MULTI_DAC_CH_SEL		(0x14 + 1)
+#define AD5529R_REG_LDAC_SYNC_ASYNC		(0x16 + 1)
+#define AD5529R_REG_LDAC_HW_SW			(0x18 + 1)
+
+/* Hardware LDAC source and edge select registers (per channel, 16-bit) */
+#define AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE	(0x1A + 1)
+#define AD5529R_REG_LDAC_HW_SRC_EDGE_SEL(ch)	\
+	(AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE + (ch) * 2)
+
+/* Output configuration */
+#define AD5529R_REG_OUT_OPERATING_MODE		(0x3A + 1)
+#define AD5529R_REG_OUT_RANGE_BASE		(0x3C + 1)
+#define AD5529R_REG_OUT_RANGE(ch)		(AD5529R_REG_OUT_RANGE_BASE + (ch) * 2)
+
+/* Calibration registers */
+#define AD5529R_REG_CAL_GAIN_BASE		(0x5C + 1)
+#define AD5529R_REG_CAL_GAIN(ch)		(AD5529R_REG_CAL_GAIN_BASE + (ch) * 2)
+
+#define AD5529R_REG_CAL_OFFSET_BASE		(0x7C + 1)
+#define AD5529R_REG_CAL_OFFSET(ch)		(AD5529R_REG_CAL_OFFSET_BASE + (ch) * 2)
+
+/* Function generator registers */
+#define AD5529R_REG_FUNC_EN			(0x9C + 1)
+#define AD5529R_REG_FUNC_MODE_SEL_BASE		(0x9E + 1)
+#define AD5529R_REG_FUNC_MODE_SEL(ch)		\
+	(AD5529R_REG_FUNC_MODE_SEL_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DAC_INPUT_B_BASE	(0xBE + 1)
+#define AD5529R_REG_FUNC_DAC_INPUT_B(ch)	\
+	(AD5529R_REG_FUNC_DAC_INPUT_B_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DITHER_PERIOD_BASE	(0xDE + 1)
+#define AD5529R_REG_FUNC_DITHER_PERIOD(ch)	\
+	(AD5529R_REG_FUNC_DITHER_PERIOD_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_DITHER_PHASE_BASE	(0xFE + 1)
+#define AD5529R_REG_FUNC_DITHER_PHASE(ch)	\
+	(AD5529R_REG_FUNC_DITHER_PHASE_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_RAMP_STEP_BASE		(0x11E + 1)
+#define AD5529R_REG_FUNC_RAMP_STEP(ch)		\
+	(AD5529R_REG_FUNC_RAMP_STEP_BASE + (ch) * 2)
+
+#define AD5529R_REG_FUNC_INT_EN			(0x13E + 1)
+
+/* Multiplexer and main DAC registers */
+#define AD5529R_REG_MUX_OUT_SEL			(0x140 + 1)
+#define AD5529R_REG_MULTI_DAC_SW_LDAC		(0x142 + 1)
+#define AD5529R_REG_MULTI_DAC_INPUT_A		(0x144 + 1)
+#define AD5529R_REG_DAC_SW_LDAC			(0x146 + 1)
+
+#define AD5529R_REG_DAC_INPUT_A_BASE		(0x148 + 1)
+#define AD5529R_REG_DAC_INPUT_A(ch)		(AD5529R_REG_DAC_INPUT_A_BASE + (ch) * 2)
+
+/* Status and readback registers */
+#define AD5529R_REG_FUNC_INT_STAT		(0x168 + 1)
+#define AD5529R_REG_DAC_DATA_READBACK_BASE	(0x16A + 1)
+#define AD5529R_REG_DAC_DATA_READBACK(ch)	\
+	(AD5529R_REG_DAC_DATA_READBACK_BASE + (ch) * 2)
+
+/* Temperature sensor registers */
+#define AD5529R_REG_TSENS_EN			(0x18A + 1)
+#define AD5529R_REG_TSENS_ALERT_FLAG		(0x18C + 1)
+#define AD5529R_REG_TSENS_SHTD_FLAG		(0x18E + 1)
+#define AD5529R_REG_TSENS_ALERT_STAT		(0x190 + 1)
+#define AD5529R_REG_TSENS_SHTD_STAT		(0x192 + 1)
+#define AD5529R_REG_ALARMB_TSENS_EN		(0x194 + 1)
+#define AD5529R_REG_ALARMB_TSENS_SEL		(0x196 + 1)
+#define AD5529R_REG_TSENS_SHTD_EN_CH		(0x198 + 1)
+#define AD5529R_REG_DAC_DIS_DEGLITCH_CH		(0x19A + 1)
+#define AD5529R_REG_DAC_INT_EN			(0x19C + 1)
+#define AD5529R_REG_ALL_FUNC_INT_STAT		(0x19E + 1)
+#define AD5529R_REG_FUNC_BUSY			(0x1A0 + 1)
+#define AD5529R_REG_REF_SRC_SEL			(0x1A2 + 1)
+#define AD5529R_REG_INIT_CRC_ERR_STAT		(0x1A4 + 1)
+
+/* Hotpath registers for multi-device support */
+#define AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC		(0x1A8 + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_0	(0x1AA + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_1	(0x1AC + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_2	(0x1AE + 1)
+#define AD5529R_REG_MULTI_DAC_HOTPATH_INPUT_A_DIE_3	(0x1B0 + 1)
+#define AD5529R_REG_DAC_HOTPATH_SW_LDAC			(0x1B2 + 1)
+
+/* Hotpath per-channel DAC input registers for each die */
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE	(0x1B4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0(ch)	\
+	(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE	(0x1D4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1(ch)	\
+	(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE	(0x1F4 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2(ch)	\
+	(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE + (ch) * 2)
+
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE	(0x214 + 1)
+#define AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3(ch)	\
+	(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE + (ch) * 2)
+
+#define   AD5529R_INTERFACE_CONFIG_A_SW_RESET	(BIT(7) | BIT(0))
+#define   AD5529R_INTERFACE_CONFIG_A_ADDR_ASCENSION	BIT(5)
+#define   AD5529R_INTERFACE_CONFIG_A_SDO_ENABLE	BIT(4)
+#define   AD5529R_INTERFACE_CONFIG_A_DEFAULT	0x10
+#define   AD5529R_NUM_CHANNELS			16
+#define   AD5529R_MAX_CHANNEL_INDEX		(AD5529R_NUM_CHANNELS - 1)
+#define   AD5529R_MAX_REGISTER			(0x232 + 1)
+#define   AD5529R_8BIT_REG_MAX			0x13
+#define   AD5529R_ADDR(reg_addr)		((reg_addr) & 0xFFF)
+#define   AD5529R_RESET_PULSE_US		1000
+#define   AD5529R_RESET_DELAY_US		10000
+#define   AD5529R_SPI_BUF_SIZE			4
+#define   AD5529R_NUM_SUPPLIES			4
+#define   AD5529R_SPI_READ_FLAG			0x80
+
+/* Device identification values */
+#define   AD5529R_PRODUCT_ID_16BIT		0x4A
+#define   AD5529R_PRODUCT_ID_12BIT		0x49
+
+struct ad5529r_model_data {
+	const char *model_name;
+	unsigned int resolution;
+	const struct iio_chan_spec *channels;
+};
+
+#define AD5529R_DAC_CHANNEL(chan, bits) {			\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.output = 1,						\
+	.channel = (chan),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
+			      BIT(IIO_CHAN_INFO_SCALE),		\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = (bits),				\
+		.storagebits = 16,				\
+	},							\
+}
+
+static const char * const ad5529r_supply_names[AD5529R_NUM_SUPPLIES] = {
+	"vdd",
+	"avdd",
+	"hvdd",
+	"hvss",
+};
+
+static const struct iio_chan_spec ad5529r_channels_16bit[] = {
+	AD5529R_DAC_CHANNEL(0, 16),
+	AD5529R_DAC_CHANNEL(1, 16),
+	AD5529R_DAC_CHANNEL(2, 16),
+	AD5529R_DAC_CHANNEL(3, 16),
+	AD5529R_DAC_CHANNEL(4, 16),
+	AD5529R_DAC_CHANNEL(5, 16),
+	AD5529R_DAC_CHANNEL(6, 16),
+	AD5529R_DAC_CHANNEL(7, 16),
+	AD5529R_DAC_CHANNEL(8, 16),
+	AD5529R_DAC_CHANNEL(9, 16),
+	AD5529R_DAC_CHANNEL(10, 16),
+	AD5529R_DAC_CHANNEL(11, 16),
+	AD5529R_DAC_CHANNEL(12, 16),
+	AD5529R_DAC_CHANNEL(13, 16),
+	AD5529R_DAC_CHANNEL(14, 16),
+	AD5529R_DAC_CHANNEL(15, 16),
+};
+
+static const struct iio_chan_spec ad5529r_channels_12bit[] = {
+	AD5529R_DAC_CHANNEL(0, 12),
+	AD5529R_DAC_CHANNEL(1, 12),
+	AD5529R_DAC_CHANNEL(2, 12),
+	AD5529R_DAC_CHANNEL(3, 12),
+	AD5529R_DAC_CHANNEL(4, 12),
+	AD5529R_DAC_CHANNEL(5, 12),
+	AD5529R_DAC_CHANNEL(6, 12),
+	AD5529R_DAC_CHANNEL(7, 12),
+	AD5529R_DAC_CHANNEL(8, 12),
+	AD5529R_DAC_CHANNEL(9, 12),
+	AD5529R_DAC_CHANNEL(10, 12),
+	AD5529R_DAC_CHANNEL(11, 12),
+	AD5529R_DAC_CHANNEL(12, 12),
+	AD5529R_DAC_CHANNEL(13, 12),
+	AD5529R_DAC_CHANNEL(14, 12),
+	AD5529R_DAC_CHANNEL(15, 12),
+};
+
+static const struct ad5529r_model_data ad5529r_16bit_model_data = {
+	.model_name = "ad5529r-16",
+	.resolution = 16,
+	.channels = ad5529r_channels_16bit,
+};
+
+static const struct ad5529r_model_data ad5529r_12bit_model_data = {
+	.model_name = "ad5529r-12",
+	.resolution = 12,
+	.channels = ad5529r_channels_12bit,
+};
+
+struct ad5529r_state {
+	struct spi_device *spi;
+	const struct ad5529r_model_data *model_data;
+	struct regmap *regmap_8bit;
+	struct regmap *regmap_16bit;
+};
+
+static const struct regmap_range ad5529r_8bit_readable_ranges[] = {
+	regmap_reg_range(AD5529R_REG_INTERFACE_CONFIG_A, AD5529R_REG_CHIP_GRADE),
+	regmap_reg_range(AD5529R_REG_SCRATCH_PAD, AD5529R_REG_VENDOR_H),
+	regmap_reg_range(AD5529R_REG_STREAM_MODE, AD5529R_REG_INTERFACE_STATUS_A),
+};
+
+static const struct regmap_range ad5529r_16bit_readable_ranges[] = {
+	regmap_reg_range(AD5529R_REG_MULTI_DAC_CH_SEL, AD5529R_REG_LDAC_HW_SW),
+	regmap_reg_range(AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE,
+			 AD5529R_REG_LDAC_HW_SRC_EDGE_SEL_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_OUT_OPERATING_MODE, AD5529R_REG_OUT_OPERATING_MODE),
+	regmap_reg_range(AD5529R_REG_OUT_RANGE_BASE,
+			 AD5529R_REG_OUT_RANGE_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_CAL_GAIN_BASE,
+			 AD5529R_REG_CAL_GAIN_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_CAL_OFFSET_BASE,
+			 AD5529R_REG_CAL_OFFSET_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_EN, AD5529R_REG_FUNC_EN),
+	regmap_reg_range(AD5529R_REG_FUNC_MODE_SEL_BASE,
+			 AD5529R_REG_FUNC_MODE_SEL_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_DAC_INPUT_B_BASE,
+			 AD5529R_REG_FUNC_DAC_INPUT_B_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_DITHER_PERIOD_BASE,
+			 AD5529R_REG_FUNC_DITHER_PERIOD_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_DITHER_PHASE_BASE,
+			 AD5529R_REG_FUNC_DITHER_PHASE_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_RAMP_STEP_BASE,
+			 AD5529R_REG_FUNC_RAMP_STEP_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_INT_EN, AD5529R_REG_DAC_SW_LDAC),
+	regmap_reg_range(AD5529R_REG_DAC_INPUT_A_BASE,
+			 AD5529R_REG_DAC_INPUT_A_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_FUNC_INT_STAT, AD5529R_REG_FUNC_INT_STAT),
+	regmap_reg_range(AD5529R_REG_DAC_DATA_READBACK_BASE,
+			 AD5529R_REG_DAC_DATA_READBACK_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_TSENS_EN, AD5529R_REG_INIT_CRC_ERR_STAT),
+	regmap_reg_range(AD5529R_REG_MULTI_DAC_HOTPATH_SW_LDAC, AD5529R_REG_DAC_HOTPATH_SW_LDAC),
+	regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE,
+			 AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_0_BASE +
+			 AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE,
+			 AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_1_BASE +
+			 AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE,
+			 AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_2_BASE +
+			 AD5529R_MAX_CHANNEL_INDEX * 2),
+	regmap_reg_range(AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE,
+			 AD5529R_REG_DAC_HOTPATH_INPUT_A_DIE_3_BASE +
+			 AD5529R_MAX_CHANNEL_INDEX * 2),
+};
+
+static const struct regmap_access_table ad5529r_8bit_readable_table = {
+	.yes_ranges = ad5529r_8bit_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad5529r_8bit_readable_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_readable_table = {
+	.yes_ranges = ad5529r_16bit_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(ad5529r_16bit_readable_ranges),
+};
+
+static const struct regmap_range ad5529r_8bit_read_only_ranges[] = {
+	regmap_reg_range(AD5529R_REG_CHIP_TYPE, AD5529R_REG_CHIP_GRADE),
+	regmap_reg_range(AD5529R_REG_SPI_REVISION, AD5529R_REG_VENDOR_H),
+	regmap_reg_range(AD5529R_REG_DEVICE_CONFIG, AD5529R_REG_DEVICE_CONFIG),
+};
+
+static const struct regmap_range ad5529r_16bit_read_only_ranges[] = {
+	regmap_reg_range(AD5529R_REG_TSENS_ALERT_FLAG, AD5529R_REG_TSENS_SHTD_STAT),
+	regmap_reg_range(AD5529R_REG_ALL_FUNC_INT_STAT, AD5529R_REG_FUNC_BUSY),
+	regmap_reg_range(AD5529R_REG_INIT_CRC_ERR_STAT, AD5529R_REG_INIT_CRC_ERR_STAT),
+	regmap_reg_range(AD5529R_REG_DAC_DATA_READBACK_BASE,
+			 AD5529R_REG_DAC_DATA_READBACK_BASE + AD5529R_MAX_CHANNEL_INDEX * 2),
+};
+
+static const struct regmap_access_table ad5529r_8bit_writeable_table = {
+	.no_ranges = ad5529r_8bit_read_only_ranges,
+	.n_no_ranges = ARRAY_SIZE(ad5529r_8bit_read_only_ranges),
+};
+
+static const struct regmap_access_table ad5529r_16bit_writeable_table = {
+	.no_ranges = ad5529r_16bit_read_only_ranges,
+	.n_no_ranges = ARRAY_SIZE(ad5529r_16bit_read_only_ranges),
+};
+
+static const struct regmap_config ad5529r_regmap_8bit_config = {
+	.name = "ad5529r-8bit",
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = AD5529R_8BIT_REG_MAX,
+	.read_flag_mask = AD5529R_SPI_READ_FLAG,
+	.rd_table = &ad5529r_8bit_readable_table,
+	.wr_table = &ad5529r_8bit_writeable_table,
+};
+
+static const struct regmap_config ad5529r_regmap_16bit_config = {
+	.name = "ad5529r-16bit",
+	.reg_bits = 16,
+	.val_bits = 16,
+	.max_register = AD5529R_MAX_REGISTER,
+	.read_flag_mask = AD5529R_SPI_READ_FLAG,
+	.rd_table = &ad5529r_16bit_readable_table,
+	.wr_table = &ad5529r_16bit_writeable_table,
+};
+
+static struct regmap *ad5529r_get_regmap(struct ad5529r_state *st, unsigned int reg)
+{
+	if (reg <= AD5529R_8BIT_REG_MAX)
+		return st->regmap_8bit;
+
+	return st->regmap_16bit;
+}
+
+static int ad5529r_debugfs_reg_read(struct ad5529r_state *st, unsigned int reg,
+				    unsigned int *val)
+{
+	return regmap_read(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_debugfs_reg_write(struct ad5529r_state *st, unsigned int reg,
+				     unsigned int val)
+{
+	return regmap_write(ad5529r_get_regmap(st, reg), reg, val);
+}
+
+static int ad5529r_detect_device(struct ad5529r_state *st)
+{
+	unsigned int product_id;
+	int ret;
+
+	ret = regmap_read(st->regmap_8bit, AD5529R_REG_PRODUCT_ID_L, &product_id);
+	if (ret)
+		return ret;
+
+	switch (product_id) {
+	case AD5529R_PRODUCT_ID_16BIT:
+		st->model_data = &ad5529r_16bit_model_data;
+		break;
+	case AD5529R_PRODUCT_ID_12BIT:
+		st->model_data = &ad5529r_12bit_model_data;
+		break;
+	default:
+		dev_err(&st->spi->dev, "Unknown product ID: 0x%02X\n", product_id);
+		return -ENODEV;
+	}
+
+	dev_dbg(&st->spi->dev, "Detected %s variant (Product ID: 0x%02X)\n",
+		st->model_data->model_name, product_id);
+
+	return 0;
+}
+
+static int ad5529r_reset(struct ad5529r_state *st)
+{
+	struct reset_control *rst;
+	int ret;
+
+	rst = devm_reset_control_get_optional_exclusive(&st->spi->dev, NULL);
+	if (IS_ERR(rst))
+		return PTR_ERR(rst);
+
+	if (rst) {
+		ret = reset_control_deassert(rst);
+		if (ret)
+			return ret;
+	} else {
+		ret = regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+				   AD5529R_INTERFACE_CONFIG_A_SW_RESET);
+		if (ret)
+			return ret;
+	}
+
+	fsleep(AD5529R_RESET_DELAY_US);
+
+	return regmap_write(st->regmap_8bit, AD5529R_REG_INTERFACE_CONFIG_A,
+			   AD5529R_INTERFACE_CONFIG_A_DEFAULT);
+}
+
+static int ad5529r_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct ad5529r_state *st = iio_priv(indio_dev);
+	unsigned int reg_addr;
+	unsigned int reg_val_h;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		reg_addr = AD5529R_REG_DAC_INPUT_A(chan->channel);
+		ret = regmap_read(st->regmap_16bit, reg_addr, &reg_val_h);
+		if (ret)
+			return ret;
+
+		*val = reg_val_h;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Using default 0-5V range: VOUTn = A × D/2^N + B
+		 * where A = 5V, B = 0V, D = digital code, N = resolution
+		 * Scale = 5V / 2^resolution
+		 */
+		*val = 5;
+		*val2 = st->model_data->resolution;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5529r_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct ad5529r_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (val < 0 || val > GENMASK(st->model_data->resolution - 1, 0))
+			return -EINVAL;
+
+		return regmap_write(st->regmap_16bit, AD5529R_REG_DAC_INPUT_A(chan->channel), val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad5529r_reg_access(struct iio_dev *indio_dev,
+			      unsigned int reg,
+			      unsigned int writeval,
+			      unsigned int *readval)
+{
+	struct ad5529r_state *st = iio_priv(indio_dev);
+
+	if (!readval)
+		return ad5529r_debugfs_reg_write(st, reg, writeval);
+
+	return ad5529r_debugfs_reg_read(st, reg, readval);
+}
+
+static const struct iio_info ad5529r_info = {
+	.read_raw = ad5529r_read_raw,
+	.write_raw = ad5529r_write_raw,
+	.debugfs_reg_access = ad5529r_reg_access,
+};
+
+static int ad5529r_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ad5529r_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	st->spi = spi;
+
+	ret = devm_regulator_bulk_get_enable(dev, AD5529R_NUM_SUPPLIES,
+					     ad5529r_supply_names);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get and enable regulators\n");
+
+	st->regmap_8bit = devm_regmap_init_spi(spi, &ad5529r_regmap_8bit_config);
+	if (IS_ERR(st->regmap_8bit))
+		return dev_err_probe(dev, PTR_ERR(st->regmap_8bit),
+				     "Failed to initialize 8-bit regmap\n");
+
+	st->regmap_16bit = devm_regmap_init_spi(spi, &ad5529r_regmap_16bit_config);
+	if (IS_ERR(st->regmap_16bit))
+		return dev_err_probe(dev, PTR_ERR(st->regmap_16bit),
+				     "Failed to initialize 16-bit regmap\n");
+
+	ret = ad5529r_reset(st);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to reset device\n");
+
+	ret = ad5529r_detect_device(st);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to detect device variant\n");
+
+	indio_dev->name = st->model_data->model_name;
+	indio_dev->info = &ad5529r_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = st->model_data->channels;
+	indio_dev->num_channels = AD5529R_NUM_CHANNELS;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id ad5529r_of_match[] = {
+	{ .compatible = "adi,ad5529r" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad5529r_of_match);
+
+static const struct spi_device_id ad5529r_id[] = {
+	{ "ad5529r" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad5529r_id);
+
+static struct spi_driver ad5529r_driver = {
+	.driver = {
+		.name = "ad5529r",
+		.of_match_table = ad5529r_of_match,
+	},
+	.probe = ad5529r_probe,
+	.id_table = ad5529r_id,
+};
+module_spi_driver(ad5529r_driver);
+
+MODULE_AUTHOR("Janani Sunil <janani.sunil@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD5529R 12/16-bit DAC driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0


^ permalink raw reply related

* [PATCH 3/3] Documentation: iio: Add AD5529R Documentation
From: Janani Sunil @ 2026-05-07 16:28 UTC (permalink / raw)
  To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, Jonathan Corbet,
	Shuah Khan
  Cc: linux-iio, devicetree, linux-kernel, linux-doc, Janani Sunil,
	Janani Sunil
In-Reply-To: <20260507-ad5529r-driver-v1-0-b4460f3cb44f@analog.com>

Add documentation for AD5529R high voltage, 16-channel 12/16 bit DAC

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
---
 Documentation/iio/ad5529r.rst | 214 ++++++++++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst   |   1 +
 MAINTAINERS                   |   1 +
 3 files changed, 216 insertions(+)

diff --git a/Documentation/iio/ad5529r.rst b/Documentation/iio/ad5529r.rst
new file mode 100644
index 000000000000..c9760945eee7
--- /dev/null
+++ b/Documentation/iio/ad5529r.rst
@@ -0,0 +1,214 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==============
+AD5529R driver
+==============
+
+Device driver for Analog Devices Inc. AD5529R 16-Channel 12/16-bit High Voltage DAC.
+The module name is ``ad5529r``.
+
+Supported devices
+=================
+
+* `AD5529R <https://www.analog.com/en/products/ad5529r.html>`_
+
+Description
+===========
+
+The AD5529R is a 16-channel, 12-bit or 16-bit, high voltage, buffered voltage output
+digital-to-analog converter (DAC) with an integrated precision reference.
+The device operates from unipolar and bipolar supplies and is guaranteed
+monotonic. It has built-in rail-to-rail output buffers that can source or
+sink up to 25mA.
+
+Hardware Features
+=================
+
+* 16 independent 12-bit or 16-bit DAC channels
+* Independently programmable output ranges:
+
+  - 0V to 5V (current driver default)
+  - 0V to 10V
+  - 0V to 20V
+  - 0V to 40V
+  - ±5V to ±15V
+  - ±10V to ±20V
+
+* 4.096V precision reference (12ppm/°C maximum)
+* Built-in function generation capabilities (hardware support)
+* Output voltage and current monitoring (hardware support)
+* Temperature monitoring with 8 on-chip sensors (hardware support)
+* Over-temperature protection
+* SPI interface with CRC error detection support
+
+Current Driver Implementation
+=============================
+
+The current driver provides basic DAC functionality with the following features:
+
+* Basic DAC output control for all 16 channels
+* Scale attributes for voltage conversion (0-5V default range)
+* SPI communication with regmap support
+* Reset control framework support
+* Automatic hardware variant detection (16-bit vs 12-bit) based on product ID
+* Debugfs register access for development
+
+SPI Configuration:
+
+* **Mode**: Supports SPI mode 0 and mode 3 (default: mode 0)
+* **Frequency**: Up to 50 MHz (typically tested at lower frequencies)
+* **Word Size**: 16-bit transactions
+
+.. note::
+   The device default configuration uses address decrement mode (ADDR_ASCENSION=0)
+   for multi-byte SPI transactions. Therefore, all 16-bit register addresses are
+   incremented by 1 in the driver to access the last byte first, allowing the
+   hardware to decrement and access the complete multi-byte register correctly.
+
+IIO Attributes (Currently Implemented)
+======================================
+
+Basic DAC Control
+-----------------
+
+For each of the 16 channels (0-15):
+
+**out_voltageY_raw**
+  Raw DAC code (12-bit: 0-4095, 16-bit: 0-65535)
+
+  * Read: Returns the current DAC register value
+  * Write: Sets the DAC output code
+
+**out_voltageY_scale**
+  Scale factor for voltage conversion (microvolts per LSB)
+
+  Based on the formula: VOUTn = A × D/2^N + B, where A=5V, B=0V, N=resolution
+
+  * 16-bit: 76.294 µV/LSB (5V ÷ 2^16 = 5V ÷ 65536 = 76.294µV)
+  * 12-bit: 1220.703 µV/LSB (5V ÷ 2^12 = 5V ÷ 4096 = 1.220703mV)
+  * Read-only attribute
+
+Debug Interface
+===============
+
+**Register Access**
+
+The driver provides debugfs register access for debugging and development:
+
+``/sys/kernel/debug/iio/iio:deviceX/direct_reg_access``
+  Direct register read/write access. Format:
+
+  * Read: ``echo <register_address> > direct_reg_access; cat direct_reg_access``
+  * Write: ``echo <register_address> <value> > direct_reg_access``
+
+Usage examples
+==============
+
+Basic DAC Output Control
+------------------------
+
+.. code-block:: bash
+
+    # Set channel 0 to mid-scale (approximately 2.048V with 4.096V reference)
+    echo "32768" > /sys/bus/iio/devices/iio:device0/out_voltage0_raw
+
+    # Set channel 15 to full scale
+    echo "65535" > /sys/bus/iio/devices/iio:device0/out_voltage15_raw
+
+    # Read current value from channel 5
+    cat /sys/bus/iio/devices/iio:device0/out_voltage5_raw
+
+Scale Attributes
+----------------
+
+.. code-block:: bash
+
+    # Read scale factor (microvolts per LSB)
+    cat /sys/bus/iio/devices/iio:device0/out_voltage0_scale
+    # Output: 0.076294 (for 16-bit) or 1.220703 (for 12-bit)
+
+    # Convert raw to voltage: voltage_uv = raw * scale
+    # Formula: VOUTn = A × D/2^N + B where A=5V, B=0V
+
+Register Access for Development
+-------------------------------
+
+.. code-block:: bash
+
+    # Navigate to debugfs directory
+    cd /sys/kernel/debug/iio/iio:device0/
+
+    # Read device product ID (register 0x04)
+    echo 4 > direct_reg_access
+    cat direct_reg_access
+
+    # Write to a 16-bit configuration register (example: LDAC_HW_SW register 0x19)
+    echo "0x019 0xAA11" > direct_reg_access
+    cat direct_reg_access
+    # Output: 0xAA11
+
+    # Write to DAC channel registers (16-bit values)
+    echo "0x149 32768" > direct_reg_access   # DAC channel 0 mid-scale
+    echo "0x14B 65535" > direct_reg_access   # DAC channel 1 full-scale
+
+    # Read back DAC register values
+    echo 0x149 > direct_reg_access && cat direct_reg_access  # Read channel 0
+    echo 0x14B > direct_reg_access && cat direct_reg_access  # Read channel 1
+
+.. note::
+   For 16-bit registers, use hexadecimal format for addresses (0x019, 0x149, etc.).
+   Values can be decimal (32768) or hexadecimal (0xAA11). Register addresses shown
+   include the +1 offset required for decrement mode operation.
+
+Device Tree Configuration
+=========================
+
+Basic configuration example:
+
+.. code-block:: devicetree
+
+    &spi0 {
+        status = "okay";
+
+        ad5529r@0 {
+            compatible = "adi,ad5529r";
+            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>;
+        };
+    };
+
+For complete device tree binding documentation, see:
+``Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml``
+
+Driver Architecture
+===================
+
+The driver is structured as follows:
+
+* **Core**: Basic SPI communication and device initialization
+* **IIO Interface**: Standard IIO DAC channel interface with scale attributes
+* **Dual Regmap**: Uses standard regmap-spi for both 8-bit and 16-bit register access
+* **Reset Framework**: Reset control support
+
+Development Notes
+=================
+
+* The driver uses standard regmap-spi for both 8-bit and 16-bit register access
+* SPI mode 0 (CPOL=0, CPHA=0) is typically used
+* Reset control framework support for device initialization
+* Register addresses are incremented by 1 for 16-bit registers due to decrement mode addressing
+* Scale attributes provide voltage conversion for 0-5V range
+* Automatic regmap selection based on register address (≤0x13: 8-bit, >0x13: 16-bit)
+
+References
+==========
+
+* AD5529R Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad5529r.pdf
+* Linux IIO Subsystem: https://www.kernel.org/doc/html/latest/driver-api/iio/index.html
\ No newline at end of file
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 007e0a1fcc5a..4b1d0432a040 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -21,6 +21,7 @@ Industrial I/O Kernel Drivers
 
    ad3552r
    ad4000
+   ad5529r
    ad4030
    ad4062
    ad4691
diff --git a/MAINTAINERS b/MAINTAINERS
index 143714e27d51..41f42eb1adf2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1513,6 +1513,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
+F:	Documentation/iio/ad5529r.rst
 F:	drivers/iio/dac/ad5529r.c
 
 ANALOG DEVICES INC AD5706R DRIVER

-- 
2.43.0


^ permalink raw reply related

* [PATCH v5 00/28] mtd: spi-nor: Enhance software protection
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal, stable

Hello,

As recently raised on the mailing-list (link below), it seems that the
"locking" support in SPI NOR could benefit from some enhancements. As I
myself had to dig into it recently, here is a proposal.

First issue that I see, the MEMLOCK ioctl is not behaving correctly
in some cases, as addressed in:

    mtd: spi-nor: swp: Improve locking user experience

Then there is no clear explanation of the shortcuts taken by the kernel
in terms of uAPI, so there is an attempt to list them in:

    mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour

Plus, Tudor also asked if we could cover locking in the testing
procedure, which is done in:

    mtd: spi-nor: Add steps for testing locking support

In order to simplify this procedure, and because it got very helpful
during my testing/development, I want to propose additions to the
debugfs output:

    mtd: spi-nor: debugfs: Add locking support TODO: make the captures again

Finally, I am providing an implementation for the complement (CMP)
feature in order to allow finer control of the regions locked. This
feature is for instance available on Winbond chips:

    [core] mtd: spi-nor: swp: Add support for the complement feature
    [doc]  mtd: spi-nor: Add steps for testing locking with CMP
    [use]  mtd: spi-nor: winbond: Add CMP locking support

Disclaimer: it was much less straightforward than I initially thought to
get the CMP feature working correctly. I tested it with as much focus as
I could, and I am improving the test coverage for the new cases, I am
also providing extra test cases in the metadata of the commit (which do
not make sense to test for chip additions, but may be sensible show when
making core additions like this one), but honestly there are so many
possibilities, I may still be missing corner cases. I hope this will
anyway be helpful to others!

All the other patches are misc improvements or style fixes which I faced
and fixed during my development.

Link: https://lore.kernel.org/linux-mtd/92e99a96-5582-48a5-a4f9-e9b33fcff171@linux.dev/

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Changes in v5:
- Fixed the writing of SR2 in the locking ops for devices without RDCR
  capability.
- Moved the ioctl comments to an existing comment as advised by
  Pratyush.
- Changed the documentation to use the concept of lock sector instead of
  block because on small density devices, the two amounts are typically
  different.
- Renamed the debugfs output "#blocks" -> "#sectors" for the same
  reason.
- Added a patch to make sure the QE bit is not badly overwritten when
  using the core's SR/CR status write helpers.
- Link to v4: https://lore.kernel.org/r/20260403-winbond-v6-18-rc1-spi-nor-swp-v4-0-833dab5e7288@bootlin.com

Changes in v4:
- Make sure we don't try to show the (SR specific) debugfs info if the
  chip does not support an SR based locking scheme. For this, add a new
  helper to derive whether we are using the default ops or not.
- Fix compilation issue on arm32, by using div_u64.
- Link to v3: https://lore.kernel.org/r/20260317-winbond-v6-18-rc1-spi-nor-swp-v3-0-2ca9ea4e7b9b@bootlin.com

Changes in v3:
- No change at all, just rebased on top of v7.0-rc1.
- Collected 2 R-by from M. Walle.
- Link to v2: https://lore.kernel.org/r/20260108-winbond-v6-18-rc1-spi-nor-swp-v2-0-c462ef806130@bootlin.com

Changes in v2:
- Collect tags.
- Add missing Fixes/Cc: stable tags.
- Add a comment explaining why can_be_top && can_be_bottom is a specific
  condition.
- Fix commit logs following Michael Walle's reviews.
- Amend the documentation following our discussion with Michael Walle as
  well.
- Cache the SR register for debugfs use.
- Create a locked sector map file instead of dumping it as part of the
  `params` file output.
- Improved greatly the output of the map as suggested by Michael.
- Add a patch fixing a duplicate dependency in Kconfig.
- Add an important comment in the doc about the small 4kiB erase size
  choice.
- Add test runs for each and every chip for which the CMP feature is
  added. This prove me that testing of each and every chip was needed,
  as some of them seem to feature a broken BFPT table which does not
  advertise a working 35h (Read CR) command.
- Added a condition on which the CMP feature is enabled: RDCR must be
  possible.
- Link to v1: https://lore.kernel.org/r/20251114-winbond-v6-18-rc1-spi-nor-swp-v1-0-487bc7129931@bootlin.com

---
Miquel Raynal (28):
      mtd: spi-nor: Drop duplicate Kconfig dependency
      mtd: spi-nor: debugfs: Fix the flags list
      mtd: spi-nor: Make sure the QE bit is kept enabled if useful
      mtd: spi-nor: swp: Improve locking user experience
      mtd: spi-nor: Improve opcodes documentation
      mtd: spi-nor: debugfs: Align variable access with the rest of the file
      mtd: spi-nor: debugfs: Enhance output
      mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
      mtd: spi-nor: swp: Clarify a comment
      mtd: spi-nor: swp: Use a pointer for SR instead of a single byte
      mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
      mtd: spi-nor: swp: Rename a mask
      mtd: spi-nor: swp: Create a TB intermediate variable
      mtd: spi-nor: swp: Create helpers for building the SR register
      mtd: spi-nor: swp: Simplify checking the locked/unlocked range
      mtd: spi-nor: swp: Cosmetic changes
      mtd: spi-nor: Create a local SR cache
      mtd: spi-nor: debugfs: Add locking support
      mtd: spi-nor: debugfs: Add a locked sectors map
      mtd: spi-nor: Add steps for testing locking support
      mtd: spi-nor: swp: Add support for the complement feature
      mtd: spi-nor: Add steps for testing locking with CMP
      mtd: spi-nor: winbond: Add W25H512NWxxAM CMP locking support
      mtd: spi-nor: winbond: Add W25H01NWxxAM CMP locking support
      mtd: spi-nor: winbond: Add W25H02NWxxAM CMP locking support
      mtd: spi-nor: winbond: Add W25H01NWxxIQ CMP locking support
      mtd: spi-nor: winbond: Add W25Q01NWxxIM CMP locking support
      mtd: spi-nor: winbond: Add W25Q02NWxxIM CMP locking support

 Documentation/driver-api/mtd/spi-nor.rst | 169 +++++++++++++++
 drivers/mtd/spi-nor/Kconfig              |   1 -
 drivers/mtd/spi-nor/core.c               |  78 ++++++-
 drivers/mtd/spi-nor/core.h               |  25 ++-
 drivers/mtd/spi-nor/debugfs.c            |  69 +++++-
 drivers/mtd/spi-nor/swp.c                | 359 ++++++++++++++++++++++++-------
 drivers/mtd/spi-nor/winbond.c            |  41 +++-
 include/linux/mtd/spi-nor.h              |   7 +-
 8 files changed, 656 insertions(+), 93 deletions(-)
---
base-commit: 2df04c01437d44ea7cac641cb4d0c9fd1275df45
change-id: 20251114-winbond-v6-18-rc1-spi-nor-swp-865d36f4f695

Best regards,
-- 
Miquel Raynal <miquel.raynal@bootlin.com>


^ permalink raw reply

* [PATCH v5 01/28] mtd: spi-nor: Drop duplicate Kconfig dependency
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

I do not think the MTD dependency is needed twice. This is likely a
duplicate coming from a former rebase when the spi-nor core got cleaned
up a while ago. Remove the extra line.

Fixes: b35b9a10362d ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 24cd25de2b8b..fd05a24d64a9 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig MTD_SPI_NOR
 	tristate "SPI NOR device support"
-	depends on MTD
 	depends on MTD && SPI_MASTER
 	select SPI_MEM
 	help

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 02/28] mtd: spi-nor: debugfs: Fix the flags list
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

As mentioned above the spi_nor_option_flags enumeration in core.h, this
list should be kept in sync with the one in the core.

Add the missing flag.

Fixes: 6a42bc97ccda ("mtd: spi-nor: core: Allow specifying the byte order in Octal DTR mode")
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/debugfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index fa6956144d2e..d700e0b27182 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -28,6 +28,7 @@ static const char *const snor_f_names[] = {
 	SNOR_F_NAME(RWW),
 	SNOR_F_NAME(ECC),
 	SNOR_F_NAME(NO_WP),
+	SNOR_F_NAME(SWAP16),
 };
 #undef SNOR_F_NAME
 

-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v5 07/11] leds: flash: add support for Samsung S2M series PMIC flash LED device
From: Lee Jones @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Kaustabh Chakraborty
  Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	MyungJoo Ham, Chanwoo Choi, Sebastian Reichel,
	Krzysztof Kozlowski, André Draszik, Alexandre Belloni,
	Jonathan Corbet, Shuah Khan, Nam Tran,
	Łukasz Lebiedziński, linux-leds, devicetree,
	linux-kernel, linux-pm, linux-samsung-soc, linux-rtc, linux-doc
In-Reply-To: <20260424-s2mu005-pmic-v5-7-fcbc9da5a004@disroot.org>

On Fri, 24 Apr 2026, Kaustabh Chakraborty wrote:

> Add support for flash LEDs in certain Samsung S2M series PMICs.
> The device has two channels for LEDs, typically for the back and front
> cameras in mobile devices. Both channels can be independently
> controlled, and can be operated in torch or flash modes.
> 
> The driver includes initial support for the S2MU005 PMIC flash LEDs.
> 
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/leds/flash/Kconfig          |  12 ++
>  drivers/leds/flash/Makefile         |   1 +
>  drivers/leds/flash/leds-s2m-flash.c | 358 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 371 insertions(+)
> 
> diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
> index 5e08102a67841..be62e05277429 100644
> --- a/drivers/leds/flash/Kconfig
> +++ b/drivers/leds/flash/Kconfig
> @@ -114,6 +114,18 @@ config LEDS_RT8515
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called leds-rt8515.
>  
> +config LEDS_S2M_FLASH
> +	tristate "Samsung S2M series PMICs flash/torch LED support"
> +	depends on LEDS_CLASS
> +	depends on MFD_SEC_CORE
> +	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS

The `|| !V4L2_FLASH_LED_CLASS` part of this dependency makes it
unconditionally true. Was this intended? Perhaps this dependency can be
removed entirely.

> +	select REGMAP_IRQ
> +	help
> +	  This option enables support for the flash/torch LEDs found in
> +	  certain Samsung S2M series PMICs, such as the S2MU005. It has
> +	  a LED channel dedicated for every physical LED. The LEDs can
> +	  be controlled in flash and torch modes.
> +
>  config LEDS_SGM3140
>  	tristate "LED support for the SGM3140"
>  	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
> diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile
> index 712fb737a428e..44e6c1b4beb37 100644
> --- a/drivers/leds/flash/Makefile
> +++ b/drivers/leds/flash/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_LEDS_MAX77693)	+= leds-max77693.o
>  obj-$(CONFIG_LEDS_QCOM_FLASH)	+= leds-qcom-flash.o
>  obj-$(CONFIG_LEDS_RT4505)	+= leds-rt4505.o
>  obj-$(CONFIG_LEDS_RT8515)	+= leds-rt8515.o
> +obj-$(CONFIG_LEDS_S2M_FLASH)	+= leds-s2m-flash.o
>  obj-$(CONFIG_LEDS_SGM3140)	+= leds-sgm3140.o
>  obj-$(CONFIG_LEDS_SY7802)	+= leds-sy7802.o
>  obj-$(CONFIG_LEDS_TPS6131X)	+= leds-tps6131x.o
> diff --git a/drivers/leds/flash/leds-s2m-flash.c b/drivers/leds/flash/leds-s2m-flash.c
> new file mode 100644
> index 0000000000000..177d23b432ce6
> --- /dev/null
> +++ b/drivers/leds/flash/leds-s2m-flash.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Flash and Torch LED Driver for Samsung S2M series PMICs.
> + *
> + * Copyright (c) 2015 Samsung Electronics Co., Ltd
> + * Copyright (c) 2026 Kaustabh Chakraborty <kauschluss@disroot.org>
> + */
> +
> +#include <linux/container_of.h>
> +#include <linux/led-class-flash.h>
> +#include <linux/mfd/samsung/core.h>
> +#include <linux/mfd/samsung/s2mu005.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <media/v4l2-flash-led-class.h>
> +
> +#define MAX_CHANNELS	2
> +
> +struct s2m_led {
> +	struct regmap *regmap;
> +	struct led_classdev_flash fled;
> +	struct v4l2_flash *v4l2_flash;
> +	/*
> +	 * The mutex object prevents the concurrent access of flash control
> +	 * registers by the LED and V4L2 subsystems.
> +	 */
> +	struct mutex lock;
> +	unsigned int reg_enable;
> +	u8 channel;
> +	u8 flash_brightness;
> +	u8 flash_timeout;
> +};
> +
> +static struct s2m_led *to_s2m_led(struct led_classdev_flash *fled)
> +{
> +	return container_of(fled, struct s2m_led, fled);
> +}
> +
> +static struct led_classdev_flash *to_s2m_fled(struct led_classdev *cdev)
> +{
> +	return container_of(cdev, struct led_classdev_flash, led_cdev);
> +}
> +
> +static int s2m_fled_flash_brightness_set(struct led_classdev_flash *fled, u32 brightness)
> +{
> +	struct s2m_led *led = to_s2m_led(fled);
> +	struct led_flash_setting *setting = &fled->brightness;
> +
> +	led->flash_brightness = (brightness - setting->min) / setting->step;
> +
> +	return 0;
> +}
> +
> +static int s2m_fled_flash_timeout_set(struct led_classdev_flash *fled, u32 timeout)
> +{
> +	struct s2m_led *led = to_s2m_led(fled);
> +	struct led_flash_setting *setting = &fled->timeout;
> +
> +	led->flash_timeout = (timeout - setting->min) / setting->step;
> +
> +	return 0;
> +}
> +
> +#if IS_ENABLED(CONFIG_V4L2_FLASH_LED_CLASS)
> +static int s2m_fled_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable)
> +{
> +	struct s2m_led *led = to_led(v4l2_flash->fled_cdev);

What is to_led()?

Was this tested?

> +	mutex_lock(&led->lock);
> +
> +	led->fled.ops->strobe_set(&led->fled, enable);
> +
> +	mutex_unlock(&led->lock);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops = {
> +	.external_strobe_set = s2m_fled_flash_external_strobe_set,
> +};
> +#else
> +static const struct v4l2_flash_ops s2m_fled_v4l2_flash_ops;
> +#endif
> +
> +static void s2m_fled_v4l2_flash_release(void *v4l2_flash)
> +{
> +	v4l2_flash_release(v4l2_flash);
> +}
> +
> +static int s2mu005_fled_torch_brightness_set(struct led_classdev *cdev, enum led_brightness value)
> +{
> +	struct s2m_led *led = to_s2m_led(to_s2m_fled(cdev));
> +	int ret;
> +
> +	mutex_lock(&led->lock);
> +
> +	if (!value) {
> +		ret = regmap_clear_bits(led->regmap, led->reg_enable,
> +					S2MU005_FLED_TORCH_EN(led->channel));
> +		if (ret < 0)
> +			dev_err(cdev->dev, "failed to disable torch LED\n");
> +		goto unlock;
> +	}
> +
> +	ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL1(led->channel),
> +				 S2MU005_FLED_TORCH_IOUT,
> +				 FIELD_PREP(S2MU005_FLED_TORCH_IOUT, value - 1));
> +	if (ret < 0) {

Is a positive number even possible?

> +		dev_err(cdev->dev, "failed to set torch current\n");
> +		goto unlock;
> +	}
> +
> +	ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_TORCH_EN(led->channel));
> +	if (ret < 0) {
> +		dev_err(cdev->dev, "failed to enable torch LED\n");
> +		goto unlock;
> +	}
> +
> +unlock:
> +	mutex_unlock(&led->lock);
> +
> +	return ret;
> +}
> +
> +static int s2mu005_fled_flash_strobe_set(struct led_classdev_flash *fled, bool state)
> +{
> +	struct s2m_led *led = to_s2m_led(fled);
> +	int ret;
> +
> +	mutex_lock(&led->lock);
> +
> +	ret = regmap_clear_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
> +	if (ret < 0) {
> +		dev_err(fled->led_cdev.dev, "failed to disable flash LED\n");
> +		goto unlock;
> +	}
> +
> +	if (!state)
> +		goto unlock;
> +
> +	ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL0(led->channel),
> +				 S2MU005_FLED_FLASH_IOUT,
> +				 FIELD_PREP(S2MU005_FLED_FLASH_IOUT, led->flash_brightness));
> +	if (ret < 0) {
> +		dev_err(fled->led_cdev.dev, "failed to set flash brightness\n");
> +		goto unlock;
> +	}
> +
> +	ret = regmap_update_bits(led->regmap, S2MU005_REG_FLED_CH_CTRL3(led->channel),
> +				 S2MU005_FLED_FLASH_TIMEOUT,
> +				 FIELD_PREP(S2MU005_FLED_FLASH_TIMEOUT, led->flash_timeout));
> +	if (ret < 0) {
> +		dev_err(fled->led_cdev.dev, "failed to set flash timeout\n");
> +		goto unlock;
> +	}
> +
> +	ret = regmap_set_bits(led->regmap, led->reg_enable, S2MU005_FLED_FLASH_EN(led->channel));
> +	if (ret < 0) {
> +		dev_err(fled->led_cdev.dev, "failed to enable flash LED\n");
> +		goto unlock;
> +	}
> +
> +unlock:
> +	mutex_unlock(&led->lock);
> +
> +	return 0;

It seems like this function swallows error codes.

Better if they're propagated properly.

> +}
> +
> +static int s2mu005_fled_flash_strobe_get(struct led_classdev_flash *fled, bool *state)
> +{
> +	struct s2m_led *led = to_s2m_led(fled);
> +	u32 val;
> +	int ret;
> +
> +	mutex_lock(&led->lock);
> +
> +	ret = regmap_read(led->regmap, S2MU005_REG_FLED_STATUS, &val);
> +	if (ret < 0) {
> +		dev_err(fled->led_cdev.dev, "failed to fetch LED status");

Missed '/n'.

> +		goto unlock;
> +	}
> +
> +	*state = !!(val & S2MU005_FLED_FLASH_STATUS(led->channel));
> +
> +unlock:
> +	mutex_unlock(&led->lock);
> +
> +	return ret;
> +}
> +
> +static const struct led_flash_ops s2mu005_fled_flash_ops = {
> +	.flash_brightness_set = s2m_fled_flash_brightness_set,
> +	.timeout_set = s2m_fled_flash_timeout_set,
> +	.strobe_set = s2mu005_fled_flash_strobe_set,
> +	.strobe_get = s2mu005_fled_flash_strobe_get,
> +};
> +
> +static int s2mu005_fled_init(struct s2m_led *led, struct device *dev, struct regmap *regmap,
> +			     unsigned int nr_channels)
> +{
> +	unsigned int val;
> +	int i, ret;
> +
> +	/* Enable the LED channels. */
> +	ret = regmap_set_bits(regmap, S2MU005_REG_FLED_CTRL1, S2MU005_FLED_CH_EN);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to enable LED channels\n");
> +
> +	ret = regmap_read(regmap, S2MU005_REG_ID, &val);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to read revision\n");
> +
> +	for (i = 0; i < nr_channels; i++) {

for (int i = 0; i < nr_channels; i++)

> +		/*
> +		 * Read the revision register. Revision EVT0 has the register
> +		 * at CTRL4, while EVT1 and higher have it at CTRL6.
> +		 */
> +		if (FIELD_GET(S2MU005_ID_MASK, val) == 0)

Why not remove the " == 0" and reverse the branches?

> +			led[i].reg_enable = S2MU005_REG_FLED_CTRL4;
> +		else
> +			led[i].reg_enable = S2MU005_REG_FLED_CTRL6;
> +	}
> +
> +	return 0;
> +}
> +
> +static int s2mu005_fled_init_channel(struct s2m_led *led, struct device *dev,
> +				     struct fwnode_handle *fwnp)
> +{
> +	struct led_classdev *cdev = &led->fled.led_cdev;
> +	struct led_init_data init_data = {};
> +	struct v4l2_flash_config v4l2_cfg = {};
> +	int ret;
> +
> +	cdev->max_brightness = 16;
> +	cdev->brightness_set_blocking = s2mu005_fled_torch_brightness_set;
> +	cdev->flags |= LED_DEV_CAP_FLASH;
> +
> +	led->fled.timeout.min = 62000;
> +	led->fled.timeout.step = 62000;
> +	led->fled.timeout.max = 992000;
> +	led->fled.timeout.val = 992000;
> +
> +	led->fled.brightness.min = 25000;
> +	led->fled.brightness.step = 25000;
> +	led->fled.brightness.max = 375000; /* 400000 causes flickering */
> +	led->fled.brightness.val = 375000;
> +
> +	s2m_fled_flash_timeout_set(&led->fled, led->fled.timeout.val);
> +	s2m_fled_flash_brightness_set(&led->fled, led->fled.brightness.val);
> +
> +	led->fled.ops = &s2mu005_fled_flash_ops;
> +
> +	init_data.fwnode = fwnp;
> +	ret = devm_led_classdev_flash_register_ext(dev, &led->fled, &init_data);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to create LED flash device\n");
> +
> +	v4l2_cfg.intensity.min = led->fled.timeout.min;
> +	v4l2_cfg.intensity.step = led->fled.timeout.step;
> +	v4l2_cfg.intensity.max = led->fled.timeout.max;
> +	v4l2_cfg.intensity.val = led->fled.timeout.val;

Is it correct to configure the V4L2 intensity settings from the timeout
values?  I would expect these to be based on the brightness settings.

> +
> +	v4l2_cfg.has_external_strobe = true;
> +
> +	led->v4l2_flash = v4l2_flash_init(dev, fwnp, &led->fled, &s2m_fled_v4l2_flash_ops,
> +					  &v4l2_cfg);
> +	if (IS_ERR(led->v4l2_flash)) {
> +		v4l2_flash_release(led->v4l2_flash);

So you're going to try and release an error?

> +		return dev_err_probe(dev, PTR_ERR(led->v4l2_flash),
> +				     "failed to create V4L2 flash device\n");
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, (void *)s2m_fled_v4l2_flash_release, led->v4l2_flash);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to add cleanup action\n");
> +
> +	return 0;
> +}
> +
> +static int s2m_fled_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sec_pmic_dev *ddata = dev_get_drvdata(dev->parent);
> +	struct s2m_led *led;
> +	int ret;
> +
> +	led = devm_kzalloc(dev, sizeof(*led) * MAX_CHANNELS, GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	switch (platform_get_device_id(pdev)->driver_data) {
> +	case S2MU005:
> +		ret = s2mu005_fled_init(led, dev, ddata->regmap_pmic, MAX_CHANNELS);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		return dev_err_probe(dev, -ENODEV, "device type %d is not supported by driver\n",
> +				     ddata->device_type);
> +	}

Will this be expanded in the very near future?

If not, having a switch () with only one entry seems odd.


if (platform_get_device_id(pdev)->driver_data != S2MU005)
	dev_err_probe()

> +	device_for_each_child_node_scoped(dev, child) {
> +		u32 reg;
> +
> +		if (fwnode_property_read_u32(child, "reg", &reg))
> +			continue;
> +
> +		if (led[reg].regmap) {
> +			dev_warn(dev, "duplicate node for channel %d\n", reg);
> +			continue;
> +		}

If reg > MAX_CHANNELS, you just created an OOB condition.

> +
> +		led[reg].regmap = ddata->regmap_pmic;
> +		led[reg].channel = (u8)reg;
> +
> +		ret = devm_mutex_init(dev, &led[reg].lock);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to create mutex\n");
> +
> +		switch (platform_get_device_id(pdev)->driver_data) {
> +		case S2MU005:
> +			ret = s2mu005_fled_init_channel(led + reg, dev, child);
> +			if (ret < 0)
> +				return ret;
> +			break;
> +		}

This is even more odd!

> +	}
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id s2m_fled_id_table[] = {
> +	{ "s2mu005-flash", S2MU005 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(platform, s2m_fled_id_table);
> +
> +static const struct of_device_id s2m_fled_of_match_table[] = {
> +	{ .compatible = "samsung,s2mu005-flash", .data = (void *)S2MU005 },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, s2m_fled_of_match_table);
> +
> +static struct platform_driver s2m_fled_driver = {
> +	.driver = {
> +		.name = "s2m-flash",
> +	},
> +	.probe = s2m_fled_probe,
> +	.id_table = s2m_fled_id_table,
> +};
> +module_platform_driver(s2m_fled_driver);
> +
> +MODULE_DESCRIPTION("Flash/Torch LED Driver For Samsung S2M Series PMICs");
> +MODULE_AUTHOR("Kaustabh Chakraborty <kauschluss@disroot.org>");
> +MODULE_LICENSE("GPL");

-- 
Lee Jones

^ permalink raw reply

* [PATCH v5 03/28] mtd: spi-nor: Make sure the QE bit is kept enabled if useful
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

Not all chips implement the 4BAIT table which typically indicates the
program capability, while many of them do implement the relevant SFDP
parts indicating the read capabilities. In such a situation, programs
can happen in single mode (1-1-1) and reads in quad mode (1-1-4 or
1-4-4). For the reads to work in such condition, the QE bit must be set.
In case we later use the spi_nor_write_16bit_sr_and_check() helper with
a chip with such configuration, the QE bit would get incorrectly
cleared.

Make sure this doesn't happen by keeping the QE bit under a simpler
condition:
- the quad enable hook is there (no change)
- and at least one of the two protocols is based on quad I/O cycles

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5dd0b3cb5250..394c27de02d6 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -869,8 +869,8 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
 		ret = spi_nor_read_cr(nor, &sr_cr[1]);
 		if (ret)
 			return ret;
-	} else if (spi_nor_get_protocol_width(nor->read_proto) == 4 &&
-		   spi_nor_get_protocol_width(nor->write_proto) == 4 &&
+	} else if ((spi_nor_get_protocol_width(nor->read_proto) == 4 ||
+		    spi_nor_get_protocol_width(nor->write_proto) == 4) &&
 		   nor->params->quad_enable) {
 		/*
 		 * If the Status Register 2 Read command (35h) is not

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 04/28] mtd: spi-nor: swp: Improve locking user experience
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal, stable
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

In the case of the first block being locked (or the few first blocks),
if the user want to fully unlock the device it has two possibilities:
- either it asks to unlock the entire device, and this works;
- or it asks to unlock just the block(s) that are currently locked,
  which fails.

It fails because the conditions "can_be_top" and "can_be_bottom" are
true. Indeed, in this case, we unlock everything, so the TB bit does not
matter. However in the current implementation, use_top would be true (as
this is the favourite option) and lock_len, which in practice should be
reduced down to 0, is set to "nor->params->size - (ofs + len)" which is
a positive number. This is wrong.

An easy way is to simply add an extra condition. In the unlock() path,
if we can achieve the same result from both sides, it means we unlock
everything and lock_len must simply be 0. A comment is added to clarify
that logic.

Fixes: 3dd8012a8eeb ("mtd: spi-nor: add TB (Top/Bottom) protect support")
Cc: stable@kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/swp.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index e67a81dbb6bf..d5f4bf555cfc 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -282,8 +282,15 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	/* Prefer top, if both are valid */
 	use_top = can_be_top;
 
-	/* lock_len: length of region that should remain locked */
-	if (use_top)
+	/*
+	 * lock_len: length of region that should remain locked.
+	 *
+	 * When can_be_top and can_be_bottom booleans are true, both adjacent
+	 * regions are unlocked, thus the entire flash can be unlocked.
+	 */
+	if (can_be_top && can_be_bottom)
+		lock_len = 0;
+	else if (use_top)
 		lock_len = nor->params->size - (ofs + len);
 	else
 		lock_len = ofs;

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 05/28] mtd: spi-nor: Improve opcodes documentation
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

There are two status registers, named 1 and 2. The current wording is
misleading as "1" may refer to the status register ID as well as the
number of bytes required (which, in this case can be 1 or 2).

Clarify the comments by aligning them on the same pattern:
"{read,write} status {1,2} register"

Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/mtd/spi-nor.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index cdcfe0fd2e7d..90a0cf583512 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -21,8 +21,8 @@
 /* Flash opcodes. */
 #define SPINOR_OP_WRDI		0x04	/* Write disable */
 #define SPINOR_OP_WREN		0x06	/* Write enable */
-#define SPINOR_OP_RDSR		0x05	/* Read status register */
-#define SPINOR_OP_WRSR		0x01	/* Write status register 1 byte */
+#define SPINOR_OP_RDSR		0x05	/* Read status register 1 */
+#define SPINOR_OP_WRSR		0x01	/* Write status register 1 */
 #define SPINOR_OP_RDSR2		0x3f	/* Read status register 2 */
 #define SPINOR_OP_WRSR2		0x3e	/* Write status register 2 */
 #define SPINOR_OP_READ		0x03	/* Read data bytes (low frequency) */

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 06/28] mtd: spi-nor: debugfs: Align variable access with the rest of the file
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

The "params" variable is used everywhere else, align this particular
line of the file to use "params" directly rather than the "nor" pointer.

Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index d700e0b27182..69830ad43990 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -139,7 +139,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 
 	if (!(nor->flags & SNOR_F_NO_OP_CHIP_ERASE)) {
 		string_get_size(params->size, 1, STRING_UNITS_2, buf, sizeof(buf));
-		seq_printf(s, " %02x (%s)\n", nor->params->die_erase_opcode, buf);
+		seq_printf(s, " %02x (%s)\n", params->die_erase_opcode, buf);
 	}
 
 	seq_puts(s, "\nsector map\n");

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 07/28] mtd: spi-nor: debugfs: Enhance output
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

Align the number of dashes to the bigger column width (the title in this
case) to make the output more pleasant and aligned with what is done
in the "params" file output.

Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 69830ad43990..d0191eb9f879 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -144,7 +144,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 
 	seq_puts(s, "\nsector map\n");
 	seq_puts(s, " region (in hex)   | erase mask | overlaid\n");
-	seq_puts(s, " ------------------+------------+----------\n");
+	seq_puts(s, " ------------------+------------+---------\n");
 	for (i = 0; i < erase_map->n_regions; i++) {
 		u64 start = region[i].offset;
 		u64 end = start + region[i].size - 1;

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 08/28] mtd: spi-nor: swp: Explain the MEMLOCK ioctl implementation behaviour
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

Add more details about how these requests are actually handled in the
SPI NOR core. Their behaviour was not entirely clear to me at first, and
explaining them in plain English sounds the way to go.

Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/core.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index e838c40a2589..22f21497c720 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -279,9 +279,14 @@ struct spi_nor_erase_map {
 
 /**
  * struct spi_nor_locking_ops - SPI NOR locking methods
- * @lock:	lock a region of the SPI NOR.
- * @unlock:	unlock a region of the SPI NOR.
- * @is_locked:	check if a region of the SPI NOR is completely locked
+ * @lock:	lock a region of the SPI NOR, never locks more than what is
+ *		requested, ie. may lock less.
+ * @unlock:	unlock a region of the SPI NOR, never unlocks more than what is
+ *		requested, ie. may unlock less.
+ * @is_locked:	check if a region of the SPI NOR is completely locked, returns
+ *		false otherwise. This feeback may be misleading because users
+ *		may get an "unlocked" status even though a subpart of the region
+ *		is effectively locked.
  */
 struct spi_nor_locking_ops {
 	int (*lock)(struct spi_nor *nor, loff_t ofs, u64 len);

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 09/28] mtd: spi-nor: swp: Clarify a comment
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

The comment states that some power of two sizes are not supported. This
is very device dependent (based on the size), so modulate a bit the
sentence to make it more accurate.

Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index d5f4bf555cfc..f221d6361b57 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -305,7 +305,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 		if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
 			val = (val & ~SR_BP3) | SR_BP3_BIT6;
 
-		/* Some power-of-two sizes are not supported */
+		/* Some power-of-two sizes may not be supported */
 		if (val & ~mask)
 			return -EINVAL;
 	}

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 10/28] mtd: spi-nor: swp: Use a pointer for SR instead of a single byte
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

At this stage, the Status Register is most often seen as a single
byte. This is subject to change when we will need to read the CMP bit
which is located in the Control Register (kind of secondary status
register). Both will need to be carried.

Change a few prototypes to carry a u8 pointer. This way it also makes it
very clear where we access the first register, and where we will access
the second.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 48 ++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index f221d6361b57..61830f18a147 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -55,13 +55,13 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
 		return sector_size;
 }
 
-static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
+static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
 					u64 *len)
 {
 	u64 min_prot_len;
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
-	u8 bp, val = sr & mask;
+	u8 bp, val = sr[0] & mask;
 
 	if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
 		val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -81,7 +81,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
 	if (*len > nor->params->size)
 		*len = nor->params->size;
 
-	if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask)
+	if (nor->flags & SNOR_F_HAS_SR_TB && sr[0] & tb_mask)
 		*ofs = 0;
 	else
 		*ofs = nor->params->size - *len;
@@ -92,7 +92,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
  * (if @locked is false); false otherwise.
  */
 static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
-					 u64 len, u8 sr, bool locked)
+					 u64 len, const u8 *sr, bool locked)
 {
 	loff_t lock_offs, lock_offs_max, offs_max;
 	u64 lock_len;
@@ -113,13 +113,13 @@ static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
 		return (ofs >= lock_offs_max) || (offs_max <= lock_offs);
 }
 
-static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, u8 sr)
+static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
 {
 	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true);
 }
 
 static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
-				   u8 sr)
+				   const u8 *sr)
 {
 	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
 }
@@ -160,7 +160,8 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
 static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
 	u64 min_prot_len;
-	int ret, status_old, status_new;
+	int ret;
+	u8 status_old[1] = {}, status_new[1] = {};
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
 	u8 pow, val;
@@ -172,7 +173,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (ret)
 		return ret;
 
-	status_old = nor->bouncebuf[0];
+	status_old[0] = nor->bouncebuf[0];
 
 	/* If nothing in our range is unlocked, we don't need to do anything */
 	if (spi_nor_is_locked_sr(nor, ofs, len, status_old))
@@ -217,7 +218,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~tb_mask) | val;
+	status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
 
 	/*
 	 * Disallow further writes if WP# pin is neither left floating nor
@@ -225,20 +226,20 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	 * WP# pin hard strapped to GND can be a valid use case.
 	 */
 	if (!(nor->flags & SNOR_F_NO_WP))
-		status_new |= SR_SRWD;
+		status_new[0] |= SR_SRWD;
 
 	if (!use_top)
-		status_new |= tb_mask;
+		status_new[0] |= tb_mask;
 
 	/* Don't bother if they're the same */
-	if (status_new == status_old)
+	if (status_new[0] == status_old[0])
 		return 0;
 
 	/* Only modify protection if it will not unlock other areas */
-	if ((status_new & mask) < (status_old & mask))
+	if ((status_new[0] & mask) < (status_old[0] & mask))
 		return -EINVAL;
 
-	return spi_nor_write_sr_and_check(nor, status_new);
+	return spi_nor_write_sr_and_check(nor, status_new[0]);
 }
 
 /*
@@ -249,7 +250,8 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
 	u64 min_prot_len;
-	int ret, status_old, status_new;
+	int ret;
+	u8 status_old[1], status_new[1];
 	u8 mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
 	u8 pow, val;
@@ -261,7 +263,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (ret)
 		return ret;
 
-	status_old = nor->bouncebuf[0];
+	status_old[0] = nor->bouncebuf[0];
 
 	/* If nothing in our range is locked, we don't need to do anything */
 	if (spi_nor_is_unlocked_sr(nor, ofs, len, status_old))
@@ -310,24 +312,24 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 			return -EINVAL;
 	}
 
-	status_new = (status_old & ~mask & ~tb_mask) | val;
+	status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
-		status_new &= ~SR_SRWD;
+		status_new[0] &= ~SR_SRWD;
 
 	if (!use_top)
-		status_new |= tb_mask;
+		status_new[0] |= tb_mask;
 
 	/* Don't bother if they're the same */
-	if (status_new == status_old)
+	if (status_new[0] == status_old[0])
 		return 0;
 
 	/* Only modify protection if it will not lock other areas */
-	if ((status_new & mask) > (status_old & mask))
+	if ((status_new[0] & mask) > (status_old[0] & mask))
 		return -EINVAL;
 
-	return spi_nor_write_sr_and_check(nor, status_new);
+	return spi_nor_write_sr_and_check(nor, status_new[0]);
 }
 
 /*
@@ -345,7 +347,7 @@ static int spi_nor_sr_is_locked(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (ret)
 		return ret;
 
-	return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf[0]);
+	return spi_nor_is_locked_sr(nor, ofs, len, nor->bouncebuf);
 }
 
 static const struct spi_nor_locking_ops spi_nor_sr_locking_ops = {

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 11/28] mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

There are many helpers already to either read and/or write SR and/or CR,
as well as sometimes check the returned values. In order to be able to
switch from a 1 byte status register to a 2 bytes status register while
keeping the same level of verification, let's introduce a new helper
that writes them both (atomically) and then reads them back (separated)
to compare the values.

In case 2 bytes registers are not supported, we still have the usual
fallback available in the helper being exported to the rest of the core.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/core.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h |  1 +
 2 files changed, 66 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 394c27de02d6..2799c21d0b67 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -976,6 +976,54 @@ int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
 	return 0;
 }
 
+/**
+ * spi_nor_write_16bit_sr_cr_and_check() - Write the Status Register 1 and the
+ * Configuration Register in one shot. Ensure that the bytes written in both
+ * registers match the received value.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @regs:	two-byte array with values to be written to the status and
+ *		configuration registers.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
+{
+	u8 written_regs[2];
+	int ret;
+
+	written_regs[0] = regs[0];
+	written_regs[1] = regs[1];
+	nor->bouncebuf[0] = regs[0];
+	nor->bouncebuf[1] = regs[1];
+
+	ret = spi_nor_write_sr(nor, nor->bouncebuf, 2);
+	if (ret)
+		return ret;
+
+	ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
+	if (ret)
+		return ret;
+
+	if (written_regs[0] != nor->bouncebuf[0]) {
+		dev_dbg(nor->dev, "SR: Read back test failed\n");
+		return -EIO;
+	}
+
+	if (nor->flags & SNOR_F_NO_READ_CR)
+		return 0;
+
+	ret = spi_nor_read_cr(nor, &nor->bouncebuf[1]);
+	if (ret)
+		return ret;
+
+	if (written_regs[1] != nor->bouncebuf[1]) {
+		dev_dbg(nor->dev, "CR: read back test failed\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 /**
  * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
  * the byte written match the received value without affecting other bits in the
@@ -993,6 +1041,23 @@ int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
 	return spi_nor_write_sr1_and_check(nor, sr1);
 }
 
+/**
+ * spi_nor_write_sr_cr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value. Same for the Control Register if
+ * available.
+ * @nor:	pointer to a 'struct spi_nor'.
+ * @regs:	byte array to be written to the registers.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs)
+{
+	if (nor->flags & SNOR_F_HAS_16BIT_SR)
+		return spi_nor_write_16bit_sr_cr_and_check(nor, regs);
+
+	return spi_nor_write_sr1_and_check(nor, regs[0]);
+}
+
 /**
  * spi_nor_write_sr2() - Write the Status Register 2 using the
  * SPINOR_OP_WRSR2 (3eh) command.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 22f21497c720..7f92586b326b 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -637,6 +637,7 @@ int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
 int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len);
 int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1);
 int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr);
+int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, const u8 *regs);
 
 ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
 			  u8 *buf);

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 12/28] mtd: spi-nor: swp: Rename a mask
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

"mask" is not very descriptive when we already manipulate two masks, and
soon will manipulate three. Rename it "bp_mask" to align with the
existing "tb_mask" and soon "cmp_mask".

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 61830f18a147..07269e09370a 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -59,9 +59,9 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
 					u64 *len)
 {
 	u64 min_prot_len;
-	u8 mask = spi_nor_get_sr_bp_mask(nor);
+	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
-	u8 bp, val = sr[0] & mask;
+	u8 bp, val = sr[0] & bp_mask;
 
 	if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
 		val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -162,7 +162,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	u64 min_prot_len;
 	int ret;
 	u8 status_old[1] = {}, status_new[1] = {};
-	u8 mask = spi_nor_get_sr_bp_mask(nor);
+	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
 	u8 pow, val;
 	loff_t lock_len;
@@ -201,7 +201,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 		lock_len = ofs + len;
 
 	if (lock_len == nor->params->size) {
-		val = mask;
+		val = bp_mask;
 	} else {
 		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 		pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
@@ -210,15 +210,15 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 		if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
 			val = (val & ~SR_BP3) | SR_BP3_BIT6;
 
-		if (val & ~mask)
+		if (val & ~bp_mask)
 			return -EINVAL;
 
 		/* Don't "lock" with no region! */
-		if (!(val & mask))
+		if (!(val & bp_mask))
 			return -EINVAL;
 	}
 
-	status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
+	status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
 
 	/*
 	 * Disallow further writes if WP# pin is neither left floating nor
@@ -236,7 +236,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 		return 0;
 
 	/* Only modify protection if it will not unlock other areas */
-	if ((status_new[0] & mask) < (status_old[0] & mask))
+	if ((status_new[0] & bp_mask) < (status_old[0] & bp_mask))
 		return -EINVAL;
 
 	return spi_nor_write_sr_and_check(nor, status_new[0]);
@@ -252,7 +252,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	u64 min_prot_len;
 	int ret;
 	u8 status_old[1], status_new[1];
-	u8 mask = spi_nor_get_sr_bp_mask(nor);
+	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
 	u8 pow, val;
 	loff_t lock_len;
@@ -308,11 +308,11 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 			val = (val & ~SR_BP3) | SR_BP3_BIT6;
 
 		/* Some power-of-two sizes may not be supported */
-		if (val & ~mask)
+		if (val & ~bp_mask)
 			return -EINVAL;
 	}
 
-	status_new[0] = (status_old[0] & ~mask & ~tb_mask) | val;
+	status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
@@ -326,7 +326,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 		return 0;
 
 	/* Only modify protection if it will not lock other areas */
-	if ((status_new[0] & mask) > (status_old[0] & mask))
+	if ((status_new[0] & bp_mask) > (status_old[0] & bp_mask))
 		return -EINVAL;
 
 	return spi_nor_write_sr_and_check(nor, status_new[0]);

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 13/28] mtd: spi-nor: swp: Create a TB intermediate variable
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

Ease the future reuse of the tb (Top/Bottom) boolean by creating an
intermediate variable.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 07269e09370a..540cd221c455 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -62,6 +62,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
 	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
 	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
 	u8 bp, val = sr[0] & bp_mask;
+	bool tb = (nor->flags & SNOR_F_HAS_SR_TB) ? sr[0] & tb_mask : 0;
 
 	if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3_BIT6)
 		val = (val & ~SR_BP3_BIT6) | SR_BP3;
@@ -81,7 +82,7 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_
 	if (*len > nor->params->size)
 		*len = nor->params->size;
 
-	if (nor->flags & SNOR_F_HAS_SR_TB && sr[0] & tb_mask)
+	if (tb)
 		*ofs = 0;
 	else
 		*ofs = nor->params->size - *len;

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 14/28] mtd: spi-nor: swp: Create helpers for building the SR register
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

The status register contains 3 or 4 BP (Block Protect) bits, 0 or 1
TB (Top/Bottom) bit, soon 0 or 1 CMP (Complement) bit. The last BP bit
and the TB bit locations change between vendors. The whole logic of
buildling the content of the status register based on some input
conditions is used two times and soon will be used 4 times.

Create dedicated helpers for these steps.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 83 +++++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 540cd221c455..8aa0fe297188 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -125,6 +125,43 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len,
 	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false);
 }
 
+static int spi_nor_sr_set_bp_mask(struct spi_nor *nor, u8 *sr, u8 pow)
+{
+	u8 mask = spi_nor_get_sr_bp_mask(nor);
+	u8 val = pow << SR_BP_SHIFT;
+
+	if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
+		val = (val & ~SR_BP3) | SR_BP3_BIT6;
+
+	if (val & ~mask)
+		return -EINVAL;
+
+	sr[0] = val;
+
+	return 0;
+}
+
+static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
+			    u8 pow, bool use_top)
+{
+	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+	int ret;
+
+	new_sr[0] = old_sr[0] & ~bp_mask & ~tb_mask;
+
+	/* Build BP field */
+	ret = spi_nor_sr_set_bp_mask(nor, &new_sr[0], pow);
+	if (ret)
+		return ret;
+
+	/* Build TB field */
+	if (!use_top)
+		new_sr[0] |= tb_mask;
+
+	return 0;
+}
+
 /*
  * Lock a region of the flash. Compatible with ST Micro and similar flash.
  * Supports the block protection bits BP{0,1,2}/BP{0,1,2,3} in the status
@@ -164,11 +201,10 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	int ret;
 	u8 status_old[1] = {}, status_new[1] = {};
 	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
-	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
-	u8 pow, val;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
+	u8 pow;
 
 	ret = spi_nor_read_sr(nor, nor->bouncebuf);
 	if (ret)
@@ -202,24 +238,19 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 		lock_len = ofs + len;
 
 	if (lock_len == nor->params->size) {
-		val = bp_mask;
+		pow = (nor->flags & SNOR_F_HAS_4BIT_BP) ? GENMASK(3, 0) : GENMASK(2, 0);
 	} else {
 		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 		pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
-		val = pow << SR_BP_SHIFT;
-
-		if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
-			val = (val & ~SR_BP3) | SR_BP3_BIT6;
-
-		if (val & ~bp_mask)
-			return -EINVAL;
-
-		/* Don't "lock" with no region! */
-		if (!(val & bp_mask))
-			return -EINVAL;
 	}
 
-	status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
+	ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+	if (ret)
+		return ret;
+
+	/* Don't "lock" with no region! */
+	if (!(status_new[0] & bp_mask))
+		return -EINVAL;
 
 	/*
 	 * Disallow further writes if WP# pin is neither left floating nor
@@ -229,9 +260,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (!(nor->flags & SNOR_F_NO_WP))
 		status_new[0] |= SR_SRWD;
 
-	if (!use_top)
-		status_new[0] |= tb_mask;
-
 	/* Don't bother if they're the same */
 	if (status_new[0] == status_old[0])
 		return 0;
@@ -254,11 +282,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	int ret;
 	u8 status_old[1], status_new[1];
 	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
-	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
-	u8 pow, val;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
+	u8 pow;
 
 	ret = spi_nor_read_sr(nor, nor->bouncebuf);
 	if (ret)
@@ -299,29 +326,21 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 		lock_len = ofs;
 
 	if (lock_len == 0) {
-		val = 0; /* fully unlocked */
+		pow = 0; /* fully unlocked */
 	} else {
 		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 		pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
-		val = pow << SR_BP_SHIFT;
 
-		if (nor->flags & SNOR_F_HAS_SR_BP3_BIT6 && val & SR_BP3)
-			val = (val & ~SR_BP3) | SR_BP3_BIT6;
-
-		/* Some power-of-two sizes may not be supported */
-		if (val & ~bp_mask)
-			return -EINVAL;
 	}
 
-	status_new[0] = (status_old[0] & ~bp_mask & ~tb_mask) | val;
+	ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
+	if (ret)
+		return ret;
 
 	/* Don't protect status register if we're fully unlocked */
 	if (lock_len == 0)
 		status_new[0] &= ~SR_SRWD;
 
-	if (!use_top)
-		status_new[0] |= tb_mask;
-
 	/* Don't bother if they're the same */
 	if (status_new[0] == status_old[0])
 		return 0;

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 15/28] mtd: spi-nor: swp: Simplify checking the locked/unlocked range
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

In both the locking/unlocking steps, at the end we verify whether we do
not lock/unlock more than requested (in which case an error must be
returned).

While being possible to do that with very simple mask comparisons, it
does not scale when adding extra locking features such as the CMP
possibility. In order to make these checks slightly easier to read and
more future proof, use existing helpers to read the (future) status
register, extract the covered range, and compare it with very usual
algebric comparisons.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 8aa0fe297188..a45627380363 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -200,7 +200,8 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	u64 min_prot_len;
 	int ret;
 	u8 status_old[1] = {}, status_new[1] = {};
-	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+	loff_t ofs_old, ofs_new;
+	u64 len_old, len_new;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
@@ -248,10 +249,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (ret)
 		return ret;
 
-	/* Don't "lock" with no region! */
-	if (!(status_new[0] & bp_mask))
-		return -EINVAL;
-
 	/*
 	 * Disallow further writes if WP# pin is neither left floating nor
 	 * wrongly tied to GND (that includes internal pull-downs).
@@ -264,8 +261,16 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	if (status_new[0] == status_old[0])
 		return 0;
 
+	spi_nor_get_locked_range_sr(nor, status_old, &ofs_old, &len_old);
+	spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+
+	/* Don't "lock" with no region! */
+	if (!len_new)
+		return -EINVAL;
+
 	/* Only modify protection if it will not unlock other areas */
-	if ((status_new[0] & bp_mask) < (status_old[0] & bp_mask))
+	if (len_old &&
+	    (ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
 		return -EINVAL;
 
 	return spi_nor_write_sr_and_check(nor, status_new[0]);
@@ -281,7 +286,8 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	u64 min_prot_len;
 	int ret;
 	u8 status_old[1], status_new[1];
-	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+	loff_t ofs_old, ofs_new;
+	u64 len_old, len_new;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
@@ -346,7 +352,10 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 		return 0;
 
 	/* Only modify protection if it will not lock other areas */
-	if ((status_new[0] & bp_mask) > (status_old[0] & bp_mask))
+	spi_nor_get_locked_range_sr(nor, status_old, &ofs_old, &len_old);
+	spi_nor_get_locked_range_sr(nor, status_new, &ofs_new, &len_new);
+	if (len_old && len_new &&
+	    (ofs_new < ofs_old || (ofs_old + len_old) < (ofs_new + len_new)))
 		return -EINVAL;
 
 	return spi_nor_write_sr_and_check(nor, status_new[0]);

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 16/28] mtd: spi-nor: swp: Cosmetic changes
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

As a final preparation step for the introduction of CMP support, make
a few more cosmetic changes to simplify the reading of the diff when
adding the CMP feature. In particular, define "min_prot_len" earlier as
it will be reused and move the definition of the "ret" variable at the
end of the stack just because it looks better.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/swp.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index a45627380363..c22cb094b66a 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -197,14 +197,14 @@ static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
  */
 static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
-	u64 min_prot_len;
-	int ret;
+	u64 min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 	u8 status_old[1] = {}, status_new[1] = {};
 	loff_t ofs_old, ofs_new;
 	u64 len_old, len_new;
 	loff_t lock_len;
 	bool can_be_top = true, can_be_bottom = nor->flags & SNOR_F_HAS_SR_TB;
 	bool use_top;
+	int ret;
 	u8 pow;
 
 	ret = spi_nor_read_sr(nor, nor->bouncebuf);
@@ -238,12 +238,10 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	else
 		lock_len = ofs + len;
 
-	if (lock_len == nor->params->size) {
+	if (lock_len == nor->params->size)
 		pow = (nor->flags & SNOR_F_HAS_4BIT_BP) ? GENMASK(3, 0) : GENMASK(2, 0);
-	} else {
-		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
+	else
 		pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
-	}
 
 	ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
 	if (ret)
@@ -283,7 +281,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
  */
 static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
-	u64 min_prot_len;
+	u64 min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 	int ret;
 	u8 status_old[1], status_new[1];
 	loff_t ofs_old, ofs_new;
@@ -331,14 +329,11 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	else
 		lock_len = ofs;
 
-	if (lock_len == 0) {
+	if (lock_len == 0)
 		pow = 0; /* fully unlocked */
-	} else {
-		min_prot_len = spi_nor_get_min_prot_length_sr(nor);
+	else
 		pow = ilog2(lock_len) - ilog2(min_prot_len) + 1;
 
-	}
-
 	ret = spi_nor_build_sr(nor, status_old, status_new, pow, use_top);
 	if (ret)
 		return ret;

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 17/28] mtd: spi-nor: Create a local SR cache
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

In order to be able to generate debugfs output without having to
actually reach the flash, create a SPI NOR local cache of the status
registers. What matters in our case are all the bits related to sector
locking. As such, in order to make it clear that this cache is not
intended to be used anywhere else, we zero the irrelevant bits.

The cache is initialized once during the early init, and then maintained
every time the write protection scheme is updated.

Suggested-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/spi-nor/core.c  |  6 +++++-
 drivers/mtd/spi-nor/core.h  |  1 +
 drivers/mtd/spi-nor/swp.c   | 35 +++++++++++++++++++++++++++++++++--
 include/linux/mtd/spi-nor.h |  2 ++
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 2799c21d0b67..cdca0fd881a3 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3328,8 +3328,12 @@ static int spi_nor_init(struct spi_nor *nor)
 	 */
 	if (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE) ||
 	    (IS_ENABLED(CONFIG_MTD_SPI_NOR_SWP_DISABLE_ON_VOLATILE) &&
-	     nor->flags & SNOR_F_SWP_IS_VOLATILE))
+	     nor->flags & SNOR_F_SWP_IS_VOLATILE)) {
 		spi_nor_try_unlock_all(nor);
+	} else {
+		/* In the other cases, make sure the debugfs SR cache is up to date */
+		spi_nor_cache_sr_lock_bits(nor, NULL);
+	}
 
 	if (nor->addr_nbytes == 4 &&
 	    nor->read_proto != SNOR_PROTO_8_8_8_DTR &&
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 7f92586b326b..16bf6190e55a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -679,6 +679,7 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
 
 void spi_nor_init_default_locking_ops(struct spi_nor *nor);
 void spi_nor_try_unlock_all(struct spi_nor *nor);
+void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr);
 void spi_nor_set_mtd_locking_ops(struct spi_nor *nor);
 void spi_nor_set_mtd_otp_ops(struct spi_nor *nor);
 
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index c22cb094b66a..2455f1f6fdf1 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -162,6 +162,25 @@ static int spi_nor_build_sr(struct spi_nor *nor, const u8 *old_sr, u8 *new_sr,
 	return 0;
 }
 
+/*
+ * Keep a local cache containing all lock-related bits for debugfs use only.
+ * This way, debugfs never needs to access the flash directly.
+ */
+void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr)
+{
+	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
+	u8 tb_mask = spi_nor_get_sr_tb_mask(nor);
+
+	if (!sr) {
+		if (spi_nor_read_sr(nor, nor->bouncebuf))
+			return;
+
+		sr = nor->bouncebuf;
+	}
+
+	nor->dfs_sr_cache[0] = sr[0] & (bp_mask | tb_mask | SR_SRWD);
+}
+
 /*
  * Lock a region of the flash. Compatible with ST Micro and similar flash.
  * Supports the block protection bits BP{0,1,2}/BP{0,1,2,3} in the status
@@ -271,7 +290,13 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 	    (ofs_old < ofs_new || (ofs_new + len_new) < (ofs_old + len_old)))
 		return -EINVAL;
 
-	return spi_nor_write_sr_and_check(nor, status_new[0]);
+	ret = spi_nor_write_sr_and_check(nor, status_new[0]);
+	if (ret)
+		return ret;
+
+	spi_nor_cache_sr_lock_bits(nor, status_new);
+
+	return 0;
 }
 
 /*
@@ -353,7 +378,13 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len)
 	    (ofs_new < ofs_old || (ofs_old + len_old) < (ofs_new + len_new)))
 		return -EINVAL;
 
-	return spi_nor_write_sr_and_check(nor, status_new[0]);
+	ret = spi_nor_write_sr_and_check(nor, status_new[0]);
+	if (ret)
+		return ret;
+
+	spi_nor_cache_sr_lock_bits(nor, status_new);
+
+	return 0;
 }
 
 /*
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 90a0cf583512..9ad77f9e76c2 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -371,6 +371,7 @@ struct spi_nor_flash_parameter;
  * @reg_proto:		the SPI protocol for read_reg/write_reg/erase operations
  * @sfdp:		the SFDP data of the flash
  * @debugfs_root:	pointer to the debugfs directory
+ * @dfs_sr_cache:	Status Register cached value for debugfs use only
  * @controller_ops:	SPI NOR controller driver specific operations.
  * @params:		[FLASH-SPECIFIC] SPI NOR flash parameters and settings.
  *                      The structure includes legacy flash parameters and
@@ -409,6 +410,7 @@ struct spi_nor {
 	enum spi_nor_cmd_ext	cmd_ext_type;
 	struct sfdp		*sfdp;
 	struct dentry		*debugfs_root;
+	u8			dfs_sr_cache[2];
 
 	const struct spi_nor_controller_ops *controller_ops;
 

-- 
2.53.0


^ permalink raw reply related

* [PATCH v5 18/28] mtd: spi-nor: debugfs: Add locking support
From: Miquel Raynal @ 2026-05-07 16:46 UTC (permalink / raw)
  To: Pratyush Yadav, Michael Walle, Takahiro Kuwano,
	Richard Weinberger, Vignesh Raghavendra, Jonathan Corbet,
	Tudor Ambarus, Shuah Khan
  Cc: Sean Anderson, Thomas Petazzoni, Steam Lin, linux-mtd,
	linux-kernel, linux-doc, Miquel Raynal
In-Reply-To: <20260507-winbond-v6-18-rc1-spi-nor-swp-v5-0-93453e1a9597@bootlin.com>

The ioctl output may be counter intuitive in some cases. Asking for a
"locked status" over a region that is only partially locked will return
"unlocked" whereas in practice maybe the biggest part is actually
locked.

Knowing what is the real software locking state through debugfs would be
very convenient for development/debugging purposes, hence this proposal
for adding an extra block at the end of the file: a "locked sectors"
array which lists every section, if it is locked or not, showing both
the address ranges and the sizes in numbers of "lock sectors" (which on
small density devices is typically different than erase blocks).

Here is an example of output, what is after the "sector map" is new.

$ cat /sys/kernel/debug/spi-nor/spi0.0/params
name		(null)
id		ef a0 20 00 00 00
size		64.0 MiB
write size	1
page size	256
address nbytes	4
flags		HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_16BIT_SR | HAS_SR_TB_BIT6 | HAS_4BIT_BP | SOFT_RESET | NO_WP

opcodes
 read		0xec
  dummy cycles	6
 erase		0xdc
 program	0x34
 8D extension	none

protocols
 read		1S-4S-4S
 write		1S-1S-4S
 register	1S-1S-1S

erase commands
 21 (4.00 KiB) [1]
 dc (64.0 KiB) [3]
 c7 (64.0 MiB)

sector map
 region (in hex)   | erase mask | overlaid
 ------------------+------------+---------
 00000000-03ffffff |     [   3] | no

locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-03ffffff | unlocked | 1024

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
Here are below more examples of output with various situations. The full
output of the "params" content has been manually removed to only show
what has been added and how it behaves.

$ flash_lock -l /dev/mtd0 0x3f00000 16
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-03efffff | unlocked | 1008
 03f00000-03ffffff |   locked | 16
$
$ flash_lock -u /dev/mtd0 0x3f00000 8
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-03f7ffff | unlocked | 1016
 03f80000-03ffffff |   locked | 8
$
$ flash_lock -u /dev/mtd0
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-03ffffff | unlocked | 1024
$
$ flash_lock -l /dev/mtd0
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-03ffffff |   locked | 1024
$
$ flash_lock -u /dev/mtd0 0x20000 1022
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
locked sectors
 region (in hex)   | status   | #sectors
 ------------------+----------+---------
 00000000-0001ffff |   locked | 2
 00020000-03ffffff | unlocked | 1022
---
 drivers/mtd/spi-nor/core.h    |  8 ++++++++
 drivers/mtd/spi-nor/debugfs.c | 27 +++++++++++++++++++++++++++
 drivers/mtd/spi-nor/swp.c     | 13 +++++++++----
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 16bf6190e55a..552e734c7107 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -287,6 +287,9 @@ struct spi_nor_erase_map {
  *		false otherwise. This feeback may be misleading because users
  *		may get an "unlocked" status even though a subpart of the region
  *		is effectively locked.
+ *
+ * If in doubt during development, check-out the debugfs output which tries to
+ * be more user friendly.
  */
 struct spi_nor_locking_ops {
 	int (*lock)(struct spi_nor *nor, loff_t ofs, u64 len);
@@ -678,6 +681,7 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
 			     const struct sfdp_bfpt *bfpt);
 
 void spi_nor_init_default_locking_ops(struct spi_nor *nor);
+bool spi_nor_has_default_locking_ops(struct spi_nor *nor);
 void spi_nor_try_unlock_all(struct spi_nor *nor);
 void spi_nor_cache_sr_lock_bits(struct spi_nor *nor, u8 *sr);
 void spi_nor_set_mtd_locking_ops(struct spi_nor *nor);
@@ -712,6 +716,10 @@ static inline bool spi_nor_needs_sfdp(const struct spi_nor *nor)
 	return !nor->info->size;
 }
 
+u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor);
+void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs, u64 *len);
+bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr);
+
 #ifdef CONFIG_DEBUG_FS
 void spi_nor_debugfs_register(struct spi_nor *nor);
 void spi_nor_debugfs_shutdown(void);
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index d0191eb9f879..36ec35d6b2dc 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <linux/debugfs.h>
+#include <linux/math64.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
@@ -77,10 +78,12 @@ static void spi_nor_print_flags(struct seq_file *s, unsigned long flags,
 static int spi_nor_params_show(struct seq_file *s, void *data)
 {
 	struct spi_nor *nor = s->private;
+	unsigned int min_prot_len = spi_nor_get_min_prot_length_sr(nor);
 	struct spi_nor_flash_parameter *params = nor->params;
 	struct spi_nor_erase_map *erase_map = &params->erase_map;
 	struct spi_nor_erase_region *region = erase_map->regions;
 	const struct flash_info *info = nor->info;
+	loff_t lock_start, lock_length;
 	char buf[16], *str;
 	unsigned int i;
 
@@ -159,6 +162,30 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 			   region[i].overlaid ? "yes" : "no");
 	}
 
+	if (!spi_nor_has_default_locking_ops(nor))
+		return 0;
+
+	seq_puts(s, "\nlocked sectors\n");
+	seq_puts(s, " region (in hex)   | status   | #sectors\n");
+	seq_puts(s, " ------------------+----------+---------\n");
+
+	spi_nor_get_locked_range_sr(nor, nor->dfs_sr_cache, &lock_start, &lock_length);
+	if (!lock_length || lock_length == params->size) {
+		seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, params->size - 1,
+			   lock_length ? "  locked" : "unlocked",
+			   div_u64(params->size, min_prot_len));
+	} else if (!lock_start) {
+		seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, lock_length - 1,
+			   "  locked", div_u64(lock_length, min_prot_len));
+		seq_printf(s, " %08llx-%08llx | %s | %llu\n", lock_length, params->size - 1,
+			   "unlocked", div_u64(params->size - lock_length, min_prot_len));
+	} else {
+		seq_printf(s, " %08llx-%08llx | %s | %llu\n", 0ULL, lock_start - 1,
+			   "unlocked", div_u64(lock_start, min_prot_len));
+		seq_printf(s, " %08llx-%08llx | %s | %llu\n", lock_start, params->size - 1,
+			   "  locked", div_u64(lock_length, min_prot_len));
+	}
+
 	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(spi_nor_params);
diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
index 2455f1f6fdf1..5d6c3afa36e3 100644
--- a/drivers/mtd/spi-nor/swp.c
+++ b/drivers/mtd/spi-nor/swp.c
@@ -34,7 +34,7 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
 		return 0;
 }
 
-static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
+u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
 {
 	unsigned int bp_slots, bp_slots_needed;
 	/*
@@ -55,8 +55,8 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
 		return sector_size;
 }
 
-static void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
-					u64 *len)
+void spi_nor_get_locked_range_sr(struct spi_nor *nor, const u8 *sr, loff_t *ofs,
+				 u64 *len)
 {
 	u64 min_prot_len;
 	u8 bp_mask = spi_nor_get_sr_bp_mask(nor);
@@ -114,7 +114,7 @@ static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs,
 		return (ofs >= lock_offs_max) || (offs_max <= lock_offs);
 }
 
-static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
+bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, u64 len, const u8 *sr)
 {
 	return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true);
 }
@@ -416,6 +416,11 @@ void spi_nor_init_default_locking_ops(struct spi_nor *nor)
 	nor->params->locking_ops = &spi_nor_sr_locking_ops;
 }
 
+bool spi_nor_has_default_locking_ops(struct spi_nor *nor)
+{
+	return nor->params->locking_ops == &spi_nor_sr_locking_ops;
+}
+
 static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, u64 len)
 {
 	struct spi_nor *nor = mtd_to_spi_nor(mtd);

-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v5 00/13] ima: Introduce staging mechanism
From: steven chen @ 2026-05-07 16:47 UTC (permalink / raw)
  To: Roberto Sassu, corbet, skhan, zohar, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge
  Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
	gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <20260429160319.4162918-1-roberto.sassu@huaweicloud.com>

On 4/29/2026 9:03 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduction
> ============
>
> The IMA measurements list is currently stored in the kernel memory.
> Memory occupation grows linearly with the number of entries, and can
> become a problem especially in environments with reduced resources.
>
> While there is an advantage in keeping the IMA measurements list in
> kernel memory, so that it is always available for reading from the
> securityfs interfaces, storing it elsewhere would make it possible to
> free precious memory for other kernel components.
>
> Storing the IMA measurements list outside the kernel does not introduce
> security issues, since its integrity is anyway protected by the TPM.
>
> Hence, the new IMA staging mechanism is introduced to allow user space
> to remove the desired portion of the measurements list from the kernel.
>
>
> Usage
> =====
>
> The IMA staging mechanism can be enabled from the kernel configuration
> with the CONFIG_IMA_STAGING option.
>
> If it is enabled, IMA duplicates the current measurements interfaces
> (both binary and ASCII), by adding the _staged file suffix. Both the
> original and the staging interfaces gain the write permission for the
> root user and group, but require the process to have CAP_SYS_ADMIN set.
>
> The staging mechanism supports two flavors.
>
> Staging with prompt
> ~~~~~~~~~~~~~~~~~~~
>
> The current measurements list is moved to a temporary staging area, and
> staged measurements are deleted upon confirmation.
>
> This staging process is achieved with the following steps.
>
>    1.  echo A > <original interface>: the user requests IMA to stage the
>        entire measurements list;
>    2.  cat <_staged interface>: the user reads the staged measurements;
>    3.  echo D > <_staged interface>: the user requests IMA to delete
>        staged measurements.
>
> Staging and deleting
> ~~~~~~~~~~~~~~~~~~~~
>
> N measurements are staged to a temporary staging area, and immediately
> deleted without further confirmation.
>
> This staging process is achieved with the following steps.
>
>    1.  cat <original interface>: the user reads the current measurements
>        list and determines what the value N for staging should be;
>    2.  echo N > <original interface>: the user requests IMA to delete N
>        measurements from the current measurements list.

This submission proposes two ways for log trimming:

*Favour 1:* Staging with prompt
*Favour 2:* stage and delete N

Functionally, both approaches address the same problem, but *Favour 2 
*is the
stronger design and should be preferred. There is no good reason to keep 
*Favour 1.*

 From a kernel implementation perspective, *Favour 2 *is more efficient 
because it
minimizes the time spent holding the list lock (can’t be shorter). It 
also substantially
reduces the amount of kernel-side logic, removing nearly half of the 
code required
by the alternative approach.

 From a user-space perspective, *Favour 2 *results in a much cleaner 
model. It avoids
the need to track and reconcile both old and staged lists in user space 
as well as
two lists (cur and staged) in the kernel space, which simplifies log 
trimming logic
and reduces maintenance overhead. In addition, it preserves the existing 
external
behavior by not exposing any staged list to user space.

Overall, *Favour 2 *provides the same functional result with lower 
kernel complexity,
shorter kernel list lock hold time, and a simpler user-space interface. 
For those
reasons, it is the preferable approach and *Favour 1* does not appear to 
offer sufficient
justification to keep both implementations.

Steven
>
> Management of Staged Measurements
> =================================
>
> Since with the staging mechanism measurement entries are removed from
> the kernel, the user needs to save the staged ones in a storage and
> concatenate them together, so that it can present them to remote
> attestation agents as if staging was never done.
>
>
> Patch set content
> =================
>
> Patches 1-8 are preparatory patches to quickly replace the hash table,
> maintain separate counters for the different measurements list types,
> mediate access to the measurements list interface, and simplify the staging
> patches.
>
> Patch 9 introduces the staging with prompt flavor. Patch 10 makes it
> possible to flush the hash table when deleting all the staged measurements.
> Patch 11 introduces the staging and deleting flavor. Patch 12 avoids
> measurements entries to be stored twice if there is contention between the
> measurements interfaces and kexec. Patch 13 adds the documentation of the
> staging mechanism.
>
>
> Changelog
> =========
>
> v4:
>   - Add write permission to the original measurement interface, and move
>     the A and N staging commands to that interface
>   - Explain better the two staging flavors and highlight that the staging
>     and delete only stages measurements internally
>   - Rename ima_queue_staged_delete_partial() to ima_queue_delete_partial()
>   - Replace ima_staged_measurements_prepended with per measurements list
>     flag to avoid copying staged and active list measurements twice
>   - Optimize the staging and deleting flavor by locklessly determining the
>     cut position in the active list, and immediately deleting entries
>     without explicit staging and splicing (suggested by Steven Chen)
>
> v3:
>   - Add Kconfig option to enable the staging mechanism (suggested by Mimi)
>   - Change the meaning of BINARY_STAGED to be just the staged measurements
>   - Separate the two staging flavors in two different functions:
>     ima_queue_staged_delete_all() for staging with prompt,
>     ima_queue_staged_delete_partial() for staging and deleting
>   - Delete N entries without staging first (suggested by Mimi)
>   - Avoid duplicate staged entries if there is contention between the
>     measurements list interfaces and kexec
>
> v2:
>   - New patch to move measurements and violation counters outside the
>     ima_h_table structure
>   - New patch to quickly replace the hash table
>   - Forbid partial deletion when flushing hash table (suggested by Mimi)
>   - Ignore ima_flush_htable if CONFIG_IMA_DISABLE_HTABLE is enabled
>   - BINARY_SIZE_* renamed to BINARY_* for better clarity
>   - Removed ima_measurements_staged_exist and testing list empty instead
>   - ima_queue_stage_trim() and ima_queue_delete_staged_trimmed() renamed to
>     ima_queue_stage() and ima_queue_delete_staged()
>   - New delete interval [1, ULONG_MAX - 1]
>   - Rename ima_measure_lock to ima_measure_mutex
>   - Move seq_open() and seq_release() outside the ima_measure_mutex lock
>   - Drop ima_measurements_staged_read() and use seq_read() instead
>   - Optimize create_securityfs_measurement_lists() changes
>   - New file name format with _staged suffix at the end of the file name
>   - Use _rcu list variant in ima_dump_measurement_list()
>   - Remove support for direct trimming and splice the remaining entries to
>     the active list (suggested by Mimi)
>   - Hot swap the hash table if flushing is requested
>
> v1:
>   - Support for direct trimming without staging
>   - Support unstaging on kexec (requested by Gregory Lumen)
>
> Roberto Sassu (13):
>    ima: Remove ima_h_table structure
>    ima: Replace static htable queue with dynamically allocated array
>    ima: Introduce per binary measurements list type ima_num_entries
>      counter
>    ima: Introduce per binary measurements list type binary_runtime_size
>      value
>    ima: Introduce _ima_measurements_start() and _ima_measurements_next()
>    ima: Mediate open/release method of the measurements list
>    ima: Use snprintf() in create_securityfs_measurement_lists
>    ima: Introduce ima_dump_measurement()
>    ima: Add support for staging measurements with prompt
>    ima: Add support for flushing the hash table when staging measurements
>    ima: Support staging and deleting N measurements entries
>    ima: Return error on deleting measurements already copied during kexec
>    doc: security: Add documentation of the IMA staging mechanism
>
>   .../admin-guide/kernel-parameters.txt         |   4 +
>   Documentation/security/IMA-staging.rst        | 163 +++++++++
>   Documentation/security/index.rst              |   1 +
>   MAINTAINERS                                   |   2 +
>   security/integrity/ima/Kconfig                |  16 +
>   security/integrity/ima/ima.h                  |  32 +-
>   security/integrity/ima/ima_api.c              |   2 +-
>   security/integrity/ima/ima_fs.c               | 315 ++++++++++++++++--
>   security/integrity/ima/ima_init.c             |   5 +
>   security/integrity/ima/ima_kexec.c            |  53 ++-
>   security/integrity/ima/ima_queue.c            | 283 ++++++++++++++--
>   11 files changed, 803 insertions(+), 73 deletions(-)
>   create mode 100644 Documentation/security/IMA-staging.rst
>


^ 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