This patch adds a regulator driver for Waveshare panels.
The regulator provides controlled power sequencing and is also used
to enable or disable the display backlight.

Features:
 - I2C interface to control panel-specific regulator registers
 - GPIO-based enable/disable support
 - Integration with the Linux regulator framework

This driver is required for proper initialization of Waveshare
MIPI-DSI panels supported by the companion DRM panel driver.

Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
---
 arch/arm64/configs/defconfig                  |   1 +
 drivers/regulator/Kconfig                     |  11 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/waveshare-panel-regulator.c | 294 ++++++++++++++++++
 4 files changed, 307 insertions(+)
 create mode 100644 drivers/regulator/waveshare-panel-regulator.c

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index e3a2d37bd104..a1e564024be2 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1823,3 +1823,4 @@ CONFIG_CORESIGHT_STM=m
 CONFIG_CORESIGHT_CPU_DEBUG=m
 CONFIG_CORESIGHT_CTI=m
 CONFIG_MEMTEST=y
+CONFIG_REGULATOR_WAVESHARE_TOUCHSCREEN=y
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index e6ea2d6f46a4..287ec02220f2 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -1184,6 +1184,17 @@ config REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2
 	  unit. The regulator is used to enable power to the display and to
 	  control backlight PWM.
 
+config REGULATOR_WAVESHARE_TOUCHSCREEN
+	tristate "Waveshare touchscreen panel regulator"
+	depends on BACKLIGHT_CLASS_DEVICE
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Enable support for Waveshare DSI touchscreen panels,
+	  This driver supports regulator on the waveshare
+	  touchscreen unit. The regulator is used to enable power to the
+	  display and to control backlight.
+
 config REGULATOR_RC5T583
 	tristate "RICOH RC5T583 Power regulators"
 	depends on MFD_RC5T583
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index b5befee45379..4c4011be74cd 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -138,6 +138,7 @@ obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
 obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
 obj-$(CONFIG_REGULATOR_RAA215300) += raa215300.o
 obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY)  += rpi-panel-attiny-regulator.o
+obj-$(CONFIG_REGULATOR_WAVESHARE_TOUCHSCREEN)  += waveshare-panel-regulator.o
 obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2)  += rpi-panel-v2-regulator.o
 obj-$(CONFIG_REGULATOR_RC5T583)  += rc5t583-regulator.o
 obj-$(CONFIG_REGULATOR_RK808)   += rk808-regulator.o
diff --git a/drivers/regulator/waveshare-panel-regulator.c b/drivers/regulator/waveshare-panel-regulator.c
new file mode 100644
index 000000000000..eba068e9592a
--- /dev/null
+++ b/drivers/regulator/waveshare-panel-regulator.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Waveshare International Limited
+ *
+ * Based on rpi-panel-v2-regulator.c by Dave Stevenson <dave.stevenson@raspberrypi.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/of.h>
+
+/* I2C registers of the microcontroller. */
+#define REG_TP 0x94
+#define REG_LCD 0x95
+#define REG_PWM 0x96
+#define REG_SIZE 0x97
+#define REG_ID 0x98
+#define REG_VERSION 0x99
+
+#define NUM_GPIO 16 /* Treat BL_ENABLE, LCD_RESET, TP_RESET as GPIOs */
+
+struct waveshare_panel_lcd {
+	struct mutex dir_lock;
+	struct mutex pwr_lock;
+	struct regmap *regmap;
+	u16 poweron_state;
+	u16 direction_state;
+
+	struct gpio_chip gc;
+	struct gpio_desc *enable;
+};
+
+static const struct regmap_config waveshare_panel_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = REG_PWM,
+};
+
+static int waveshare_panel_gpio_direction_in(struct gpio_chip *gc,
+					     unsigned int offset)
+{
+	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
+
+	mutex_lock(&state->dir_lock);
+	state->direction_state |= BIT(offset);
+	mutex_unlock(&state->dir_lock);
+
+	return 0;
+}
+
+static int waveshare_panel_gpio_direction_out(struct gpio_chip *gc,
+					      unsigned int offset, int val)
+{
+	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
+	u16 last_val;
+
+	mutex_lock(&state->dir_lock);
+	state->direction_state &= ~BIT(offset);
+	mutex_unlock(&state->dir_lock);
+
+	mutex_lock(&state->pwr_lock);
+	last_val = state->poweron_state;
+	if (val)
+		last_val |= BIT(offset);
+	else
+		last_val &= ~BIT(offset);
+
+	state->poweron_state = last_val;
+	mutex_unlock(&state->pwr_lock);
+
+	regmap_write(state->regmap, REG_TP, last_val >> 8);
+	regmap_write(state->regmap, REG_LCD, last_val & 0xff);
+
+	return 0;
+}
+
+static int waveshare_panel_gpio_get_direction(struct gpio_chip *gc,
+					      unsigned int offset)
+{
+	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
+	u16 last_val;
+
+	mutex_lock(&state->dir_lock);
+	last_val = state->direction_state;
+	mutex_unlock(&state->dir_lock);
+
+	if (last_val & BIT(offset))
+		return GPIO_LINE_DIRECTION_IN;
+	else
+		return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int waveshare_panel_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
+	u16 pwr_state;
+
+	mutex_lock(&state->pwr_lock);
+	pwr_state = state->poweron_state & BIT(offset);
+	mutex_unlock(&state->pwr_lock);
+
+	return !!pwr_state;
+}
+
+static int waveshare_panel_gpio_set(struct gpio_chip *gc, unsigned int offset,
+				     int value)
+{
+	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
+	u16 last_val;
+
+	if (offset >= NUM_GPIO)
+		return 0;
+
+	mutex_lock(&state->pwr_lock);
+
+	last_val = state->poweron_state;
+	if (value)
+		last_val |= BIT(offset);
+	else
+		last_val &= ~BIT(offset);
+
+	state->poweron_state = last_val;
+
+	regmap_write(state->regmap, REG_TP, last_val >> 8);
+	regmap_write(state->regmap, REG_LCD, last_val & 0xff);
+
+	mutex_unlock(&state->pwr_lock);
+
+	return 0;
+}
+
+static int waveshare_panel_update_status(struct backlight_device *bl)
+{
+	struct waveshare_panel_lcd *state = bl_get_data(bl);
+	int brightness = bl->props.brightness;
+
+	if (bl->props.power != FB_BLANK_UNBLANK ||
+	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
+		brightness = 0;
+
+	if (state->enable)
+		gpiod_set_value_cansleep(state->enable, !!brightness);
+
+	return regmap_write(state->regmap, REG_PWM, brightness);
+}
+
+static const struct backlight_ops waveshare_panel_bl = {
+	.update_status = waveshare_panel_update_status,
+};
+
+static int waveshare_panel_i2c_read(struct i2c_client *client, u8 reg,
+				    unsigned int *buf)
+{
+	int val;
+
+	val = i2c_smbus_read_byte_data(client, reg);
+	if (val < 0)
+		return val;
+
+	*buf = val;
+
+	return 0;
+}
+
+static int waveshare_panel_i2c_probe(struct i2c_client *i2c)
+{
+	struct backlight_properties props = {};
+	struct backlight_device *bl;
+	struct waveshare_panel_lcd *state;
+	struct regmap *regmap;
+	unsigned int data;
+	int ret;
+
+	state = devm_kzalloc(&i2c->dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	mutex_init(&state->dir_lock);
+	mutex_init(&state->pwr_lock);
+
+	i2c_set_clientdata(i2c, state);
+
+	regmap = devm_regmap_init_i2c(i2c, &waveshare_panel_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
+			ret);
+		goto error;
+	}
+
+	ret = waveshare_panel_i2c_read(i2c, REG_ID, &data);
+	if (ret == 0)
+		dev_info(&i2c->dev, "waveshare panel hw id = 0x%x\n", data);
+
+	ret = waveshare_panel_i2c_read(i2c, REG_SIZE, &data);
+	if (ret == 0)
+		dev_info(&i2c->dev, "waveshare panel size = %d\n", data);
+
+	ret = waveshare_panel_i2c_read(i2c, REG_VERSION, &data);
+	if (ret == 0)
+		dev_info(&i2c->dev, "waveshare panel mcu version = 0x%x\n",
+			 data);
+
+	state->direction_state = 0;
+	state->poweron_state = BIT(9) | BIT(8); // Enable VCC
+	regmap_write(regmap, REG_TP, state->poweron_state >> 8);
+	regmap_write(regmap, REG_LCD, state->poweron_state & 0xff);
+	msleep(20);
+
+	state->regmap = regmap;
+	state->gc.parent = &i2c->dev;
+	state->gc.label = i2c->name;
+	state->gc.owner = THIS_MODULE;
+	state->gc.base = -1;
+	state->gc.ngpio = NUM_GPIO;
+
+	state->gc.get = waveshare_panel_gpio_get;
+	state->gc.set = waveshare_panel_gpio_set;
+	state->gc.direction_input = waveshare_panel_gpio_direction_in;
+	state->gc.direction_output = waveshare_panel_gpio_direction_out;
+	state->gc.get_direction = waveshare_panel_gpio_get_direction;
+	state->gc.can_sleep = true;
+
+	ret = devm_gpiochip_add_data(&i2c->dev, &state->gc, state);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to create gpiochip: %d\n", ret);
+		goto error;
+	}
+
+	state->enable =
+		devm_gpiod_get_optional(&i2c->dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(state->enable))
+		return dev_err_probe(&i2c->dev, PTR_ERR(state->enable),
+				     "Couldn't get enable GPIO\n");
+
+	props.type = BACKLIGHT_RAW;
+	props.max_brightness = 255;
+	bl = devm_backlight_device_register(&i2c->dev, dev_name(&i2c->dev),
+					    &i2c->dev, state,
+					    &waveshare_panel_bl, &props);
+	if (IS_ERR(bl))
+		return PTR_ERR(bl);
+
+	bl->props.brightness = 255;
+
+	return 0;
+
+error:
+	mutex_destroy(&state->dir_lock);
+	mutex_destroy(&state->pwr_lock);
+	return ret;
+}
+
+static void waveshare_panel_i2c_remove(struct i2c_client *client)
+{
+	struct waveshare_panel_lcd *state = i2c_get_clientdata(client);
+
+	mutex_destroy(&state->dir_lock);
+	mutex_destroy(&state->pwr_lock);
+}
+
+static void waveshare_panel_i2c_shutdown(struct i2c_client *client)
+{
+	struct waveshare_panel_lcd *state = i2c_get_clientdata(client);
+
+	regmap_write(state->regmap, REG_PWM, 0);
+}
+
+static const struct of_device_id waveshare_panel_dt_ids[] = {
+	{ .compatible = "waveshare,touchscreen-panel-regulator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, waveshare_panel_dt_ids);
+
+static struct i2c_driver waveshare_panel_regulator_driver = {
+	.driver = {
+		.name = "waveshare_touchscreen",
+		.of_match_table = of_match_ptr(waveshare_panel_dt_ids),
+	},
+	.probe = waveshare_panel_i2c_probe,
+	.remove	= waveshare_panel_i2c_remove,
+	.shutdown = waveshare_panel_i2c_shutdown,
+};
+
+module_i2c_driver(waveshare_panel_regulator_driver);
+
+MODULE_AUTHOR("Waveshare Team <support@waveshare.com>");
+MODULE_DESCRIPTION("Regulator device driver for Waveshare touchscreen");
+MODULE_LICENSE("GPL");
-- 
2.34.1

On Tue, Nov 11, 2025 at 04:13:20PM +0530, Sudarshan Shetty wrote:
> This patch adds a regulator driver for Waveshare panels.
> The regulator provides controlled power sequencing and is also used
> to enable or disable the display backlight.
> 
> Features:
>  - I2C interface to control panel-specific regulator registers
>  - GPIO-based enable/disable support
>  - Integration with the Linux regulator framework
> 
> This driver is required for proper initialization of Waveshare
> MIPI-DSI panels supported by the companion DRM panel driver.
> 

This patch should have been sent together with the binding patch
https://lore.kernel.org/all/20251111104245.3420041-1-tessolveupstream@gmail.com/

> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> ---
>  arch/arm64/configs/defconfig                  |   1 +
>  drivers/regulator/Kconfig                     |  11 +
>  drivers/regulator/Makefile                    |   1 +
>  drivers/regulator/waveshare-panel-regulator.c | 294 ++++++++++++++++++
>  4 files changed, 307 insertions(+)
>  create mode 100644 drivers/regulator/waveshare-panel-regulator.c
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index e3a2d37bd104..a1e564024be2 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -1823,3 +1823,4 @@ CONFIG_CORESIGHT_STM=m
>  CONFIG_CORESIGHT_CPU_DEBUG=m
>  CONFIG_CORESIGHT_CTI=m
>  CONFIG_MEMTEST=y
> +CONFIG_REGULATOR_WAVESHARE_TOUCHSCREEN=y

This is not in the right place (it shouldn't be in this patch and it
shouldn't be at this place in the defconfig).

> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index e6ea2d6f46a4..287ec02220f2 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1184,6 +1184,17 @@ config REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2
>  	  unit. The regulator is used to enable power to the display and to
>  	  control backlight PWM.
>  
> +config REGULATOR_WAVESHARE_TOUCHSCREEN
> +	tristate "Waveshare touchscreen panel regulator"
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Enable support for Waveshare DSI touchscreen panels,
> +	  This driver supports regulator on the waveshare
> +	  touchscreen unit. The regulator is used to enable power to the
> +	  display and to control backlight.
> +
>  config REGULATOR_RC5T583
>  	tristate "RICOH RC5T583 Power regulators"
>  	depends on MFD_RC5T583
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index b5befee45379..4c4011be74cd 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -138,6 +138,7 @@ obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
>  obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
>  obj-$(CONFIG_REGULATOR_RAA215300) += raa215300.o
>  obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY)  += rpi-panel-attiny-regulator.o
> +obj-$(CONFIG_REGULATOR_WAVESHARE_TOUCHSCREEN)  += waveshare-panel-regulator.o
>  obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2)  += rpi-panel-v2-regulator.o
>  obj-$(CONFIG_REGULATOR_RC5T583)  += rc5t583-regulator.o
>  obj-$(CONFIG_REGULATOR_RK808)   += rk808-regulator.o
> diff --git a/drivers/regulator/waveshare-panel-regulator.c b/drivers/regulator/waveshare-panel-regulator.c
> new file mode 100644
> index 000000000000..eba068e9592a
> --- /dev/null
> +++ b/drivers/regulator/waveshare-panel-regulator.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 Waveshare International Limited
> + *
> + * Based on rpi-panel-v2-regulator.c by Dave Stevenson <dave.stevenson@raspberrypi.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/of.h>
> +
> +/* I2C registers of the microcontroller. */
> +#define REG_TP 0x94
> +#define REG_LCD 0x95
> +#define REG_PWM 0x96
> +#define REG_SIZE 0x97
> +#define REG_ID 0x98
> +#define REG_VERSION 0x99
> +
> +#define NUM_GPIO 16 /* Treat BL_ENABLE, LCD_RESET, TP_RESET as GPIOs */

After a lot of digging, I determined that I need to drive GPIO 0, 1, 2
and 4 high to get my Waveshare display to light up, and I need GPIO 9
high to get the touchscreen to work.

What are these pins?!

Pin 0 and 4 are referred to by "regulator sounding names" in the
Waveshare "test" driver. So those sounds like regulators. The way you
toggle pin 2, also sounds like a regulator.

And then we have the 3.3V and 5V supply coming in to this thing...

> +
> +struct waveshare_panel_lcd {
> +	struct mutex dir_lock;
> +	struct mutex pwr_lock;
> +	struct regmap *regmap;
> +	u16 poweron_state;
> +	u16 direction_state;
> +
> +	struct gpio_chip gc;
> +	struct gpio_desc *enable;
> +};
> +
> +static const struct regmap_config waveshare_panel_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = REG_PWM,
> +};
> +
> +static int waveshare_panel_gpio_direction_in(struct gpio_chip *gc,
> +					     unsigned int offset)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +
> +	mutex_lock(&state->dir_lock);
> +	state->direction_state |= BIT(offset);

You don't actually do any direction handling, this is just kept in the
driver and returned upon request.

> +	mutex_unlock(&state->dir_lock);
> +
> +	return 0;
> +}
> +
> +static int waveshare_panel_gpio_direction_out(struct gpio_chip *gc,
> +					      unsigned int offset, int val)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 last_val;
> +
> +	mutex_lock(&state->dir_lock);
> +	state->direction_state &= ~BIT(offset);
> +	mutex_unlock(&state->dir_lock);
> +
> +	mutex_lock(&state->pwr_lock);
> +	last_val = state->poweron_state;
> +	if (val)
> +		last_val |= BIT(offset);
> +	else
> +		last_val &= ~BIT(offset);
> +
> +	state->poweron_state = last_val;
> +	mutex_unlock(&state->pwr_lock);
> +
> +	regmap_write(state->regmap, REG_TP, last_val >> 8);
> +	regmap_write(state->regmap, REG_LCD, last_val & 0xff);
> +
> +	return 0;
> +}
> +
> +static int waveshare_panel_gpio_get_direction(struct gpio_chip *gc,
> +					      unsigned int offset)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 last_val;
> +
> +	mutex_lock(&state->dir_lock);
> +	last_val = state->direction_state;
> +	mutex_unlock(&state->dir_lock);
> +
> +	if (last_val & BIT(offset))
> +		return GPIO_LINE_DIRECTION_IN;
> +	else
> +		return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int waveshare_panel_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 pwr_state;
> +
> +	mutex_lock(&state->pwr_lock);
> +	pwr_state = state->poweron_state & BIT(offset);

Is it possible to read this from the hardware?

> +	mutex_unlock(&state->pwr_lock);
> +
> +	return !!pwr_state;
> +}
> +
> +static int waveshare_panel_gpio_set(struct gpio_chip *gc, unsigned int offset,
> +				     int value)

This is exactly the same function as
waveshare_panel_gpio_direction_out()

> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 last_val;
> +
> +	if (offset >= NUM_GPIO)
> +		return 0;
> +
> +	mutex_lock(&state->pwr_lock);
> +
> +	last_val = state->poweron_state;
> +	if (value)
> +		last_val |= BIT(offset);
> +	else
> +		last_val &= ~BIT(offset);
> +
> +	state->poweron_state = last_val;
> +
> +	regmap_write(state->regmap, REG_TP, last_val >> 8);
> +	regmap_write(state->regmap, REG_LCD, last_val & 0xff);
> +
> +	mutex_unlock(&state->pwr_lock);
> +
> +	return 0;
> +}
> +
> +static int waveshare_panel_update_status(struct backlight_device *bl)
> +{
> +	struct waveshare_panel_lcd *state = bl_get_data(bl);
> +	int brightness = bl->props.brightness;
> +
> +	if (bl->props.power != FB_BLANK_UNBLANK ||

FB_BLANK_UNBLANK was renamed in v6.11, so you haven't compiled and/or
tested this on a modern kernel.

All contributions must be done base on the most recent kernel version,
or newer.

> +	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> +		brightness = 0;
> +
> +	if (state->enable)
> +		gpiod_set_value_cansleep(state->enable, !!brightness);
> +
> +	return regmap_write(state->regmap, REG_PWM, brightness);
> +}
> +
> +static const struct backlight_ops waveshare_panel_bl = {
> +	.update_status = waveshare_panel_update_status,
> +};
> +
> +static int waveshare_panel_i2c_read(struct i2c_client *client, u8 reg,
> +				    unsigned int *buf)
> +{
> +	int val;
> +
> +	val = i2c_smbus_read_byte_data(client, reg);
> +	if (val < 0)
> +		return val;
> +
> +	*buf = val;
> +
> +	return 0;
> +}
> +
> +static int waveshare_panel_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct backlight_properties props = {};
> +	struct backlight_device *bl;
> +	struct waveshare_panel_lcd *state;
> +	struct regmap *regmap;
> +	unsigned int data;
> +	int ret;
> +
> +	state = devm_kzalloc(&i2c->dev, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	mutex_init(&state->dir_lock);
> +	mutex_init(&state->pwr_lock);
> +
> +	i2c_set_clientdata(i2c, state);
> +
> +	regmap = devm_regmap_init_i2c(i2c, &waveshare_panel_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
> +			ret);
> +		goto error;
> +	}
> +
> +	ret = waveshare_panel_i2c_read(i2c, REG_ID, &data);

Why do you roll your own i2c_read() instead of just using the regmap?!

> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel hw id = 0x%x\n", data);

This is possibly interested to you while you work on bringing up this
driver, after that it's noise. dev_dbg() or perhaps even remove it.

> +
> +	ret = waveshare_panel_i2c_read(i2c, REG_SIZE, &data);
> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel size = %d\n", data);
> +
> +	ret = waveshare_panel_i2c_read(i2c, REG_VERSION, &data);
> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel mcu version = 0x%x\n",
> +			 data);
> +
> +	state->direction_state = 0;
> +	state->poweron_state = BIT(9) | BIT(8); // Enable VCC

Are you sure? As I wrote above, Waveshare list pin 9 as "reset-gpios" in
the touchscreen node.

But, this means that we have mystery pins 0, 1, 2, 4, 8, and 9...

> +	regmap_write(regmap, REG_TP, state->poweron_state >> 8);
> +	regmap_write(regmap, REG_LCD, state->poweron_state & 0xff);

This is the 3rd time you write these two lines.

> +	msleep(20);
> +
> +	state->regmap = regmap;
> +	state->gc.parent = &i2c->dev;
> +	state->gc.label = i2c->name;
> +	state->gc.owner = THIS_MODULE;
> +	state->gc.base = -1;
> +	state->gc.ngpio = NUM_GPIO;
> +
> +	state->gc.get = waveshare_panel_gpio_get;
> +	state->gc.set = waveshare_panel_gpio_set;
> +	state->gc.direction_input = waveshare_panel_gpio_direction_in;
> +	state->gc.direction_output = waveshare_panel_gpio_direction_out;
> +	state->gc.get_direction = waveshare_panel_gpio_get_direction;
> +	state->gc.can_sleep = true;
> +
> +	ret = devm_gpiochip_add_data(&i2c->dev, &state->gc, state);

You should be able to follow drivers/regulator/rpi-panel-v2-regulator.c
and use devm_gpio_regmap_register() instead of writing your own
functions for all this.

> +	if (ret) {
> +		dev_err(&i2c->dev, "Failed to create gpiochip: %d\n", ret);
> +		goto error;
> +	}
> +
> +	state->enable =
> +		devm_gpiod_get_optional(&i2c->dev, "enable", GPIOD_OUT_LOW);

No, there's no point in having the driver reach for DeviceTree in order
to get to pin 2 in its own GPIO controller.

Perhaps pin 2 is one of the master switches? In which case it shouldn't
be exposed through the gpio-controller, but handled internally, when the
PWM or other regulators are enabled?

> +	if (IS_ERR(state->enable))
> +		return dev_err_probe(&i2c->dev, PTR_ERR(state->enable),
> +				     "Couldn't get enable GPIO\n");
> +
> +	props.type = BACKLIGHT_RAW;
> +	props.max_brightness = 255;
> +	bl = devm_backlight_device_register(&i2c->dev, dev_name(&i2c->dev),
> +					    &i2c->dev, state,
> +					    &waveshare_panel_bl, &props);
> +	if (IS_ERR(bl))
> +		return PTR_ERR(bl);
> +
> +	bl->props.brightness = 255;
> +
> +	return 0;
> +
> +error:
> +	mutex_destroy(&state->dir_lock);
> +	mutex_destroy(&state->pwr_lock);
> +	return ret;
> +}
> +
> +static void waveshare_panel_i2c_remove(struct i2c_client *client)
> +{
> +	struct waveshare_panel_lcd *state = i2c_get_clientdata(client);
> +
> +	mutex_destroy(&state->dir_lock);
> +	mutex_destroy(&state->pwr_lock);
> +}
> +
> +static void waveshare_panel_i2c_shutdown(struct i2c_client *client)
> +{
> +	struct waveshare_panel_lcd *state = i2c_get_clientdata(client);
> +
> +	regmap_write(state->regmap, REG_PWM, 0);

What about the regulators?

Regards,
Bjorn

> +}
> +
> +static const struct of_device_id waveshare_panel_dt_ids[] = {
> +	{ .compatible = "waveshare,touchscreen-panel-regulator" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, waveshare_panel_dt_ids);
> +
> +static struct i2c_driver waveshare_panel_regulator_driver = {
> +	.driver = {
> +		.name = "waveshare_touchscreen",
> +		.of_match_table = of_match_ptr(waveshare_panel_dt_ids),
> +	},
> +	.probe = waveshare_panel_i2c_probe,
> +	.remove	= waveshare_panel_i2c_remove,
> +	.shutdown = waveshare_panel_i2c_shutdown,
> +};
> +
> +module_i2c_driver(waveshare_panel_regulator_driver);
> +
> +MODULE_AUTHOR("Waveshare Team <support@waveshare.com>");
> +MODULE_DESCRIPTION("Regulator device driver for Waveshare touchscreen");
> +MODULE_LICENSE("GPL");
> -- 
> 2.34.1
>

On 11/11/2025 11:43, Sudarshan Shetty wrote:
> This patch adds a regulator driver for Waveshare panels.
> The regulator provides controlled power sequencing and is also used
> to enable or disable the display backlight.
> 
> Features:
>  - I2C interface to control panel-specific regulator registers
>  - GPIO-based enable/disable support
>  - Integration with the Linux regulator framework
> 
> This driver is required for proper initialization of Waveshare
> MIPI-DSI panels supported by the companion DRM panel driver.
> 
> Signed-off-by: Sudarshan Shetty <tessolveupstream@gmail.com>
> ---
>  arch/arm64/configs/defconfig                  |   1 +

That is not regulator subsystem, don't mix.

>  drivers/regulator/Kconfig                     |  11 +
>  drivers/regulator/Makefile                    |   1 +
>  drivers/regulator/waveshare-panel-regulator.c | 294 ++++++++++++++++++
>  4 files changed, 307 insertions(+)
You did not implement feedback from your internal review. Please go back
to that first, so you will not ask maintainers to review obvious issues
which were already pointed out.


Best regards,
Krzysztof

On Tue, Nov 11, 2025 at 04:13:20PM +0530, Sudarshan Shetty wrote:

> This patch adds a regulator driver for Waveshare panels.
> The regulator provides controlled power sequencing and is also used
> to enable or disable the display backlight.

> +++ b/drivers/regulator/Makefile
> @@ -138,6 +138,7 @@ obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
>  obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
>  obj-$(CONFIG_REGULATOR_RAA215300) += raa215300.o
>  obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY)  += rpi-panel-attiny-regulator.o
> +obj-$(CONFIG_REGULATOR_WAVESHARE_TOUCHSCREEN)  += waveshare-panel-regulator.o
>  obj-$(CONFIG_REGULATOR_RASPBERRYPI_TOUCHSCREEN_V2)  += rpi-panel-v2-regulator.o

Please keep this and the Kconfig sorted.

> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 Waveshare International Limited
> + *

Please make the entire comment block a C++ one so things look more
intentional.

> +static const struct regmap_config waveshare_panel_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = REG_PWM,
> +};

Perhaps worth enabling caching to cut down on reads and hence improve
performance?

> +static int waveshare_panel_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 pwr_state;
> +
> +	mutex_lock(&state->pwr_lock);
> +	pwr_state = state->poweron_state & BIT(offset);
> +	mutex_unlock(&state->pwr_lock);
> +
> +	return !!pwr_state;
> +}

This will only read the value that has been set but there's support in
the driver for configuring the GPIO as an input, how would that work?

> +static int waveshare_panel_gpio_set(struct gpio_chip *gc, unsigned int offset,
> +				     int value)
> +{
> +	struct waveshare_panel_lcd *state = gpiochip_get_data(gc);
> +	u16 last_val;
> +
> +	if (offset >= NUM_GPIO)
> +		return 0;

Shouldn't this be an error?

> +	ret = waveshare_panel_i2c_read(i2c, REG_ID, &data);
> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel hw id = 0x%x\n", data);
> +
> +	ret = waveshare_panel_i2c_read(i2c, REG_SIZE, &data);
> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel size = %d\n", data);
> +
> +	ret = waveshare_panel_i2c_read(i2c, REG_VERSION, &data);
> +	if (ret == 0)
> +		dev_info(&i2c->dev, "waveshare panel mcu version = 0x%x\n",
> +			 data);

Shouldn't we take I/O errors on these registers more seriously?

> +	state->direction_state = 0;
> +	state->poweron_state = BIT(9) | BIT(8); // Enable VCC
> +	regmap_write(regmap, REG_TP, state->poweron_state >> 8);
> +	regmap_write(regmap, REG_LCD, state->poweron_state & 0xff);
> +	msleep(20);

Generally the default for regulators is to leave the hardware alone
unless explicitly configured to control it.

> +static const struct of_device_id waveshare_panel_dt_ids[] = {
> +	{ .compatible = "waveshare,touchscreen-panel-regulator" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, waveshare_panel_dt_ids);

This DT binding needs to be documented.

> +static struct i2c_driver waveshare_panel_regulator_driver = {
> +	.driver = {
> +		.name = "waveshare_touchscreen",
> +		.of_match_table = of_match_ptr(waveshare_panel_dt_ids),
> +	},
> +	.probe = waveshare_panel_i2c_probe,
> +	.remove	= waveshare_panel_i2c_remove,
> +	.shutdown = waveshare_panel_i2c_shutdown,
> +};

This doesn't actually seem to register a regulator, why put it in the
regulator subsystem?