[PATCH v2 0/9] regulator: rpi-panel: Support official Raspberry Pi 7 inches touchscreen

[PATCH v2 0/9] regulator: rpi-panel: Support official Raspberry Pi 7 inches touchscreen

[Detlev Casanova]

This patchset provides different fixes to the rpi-panel-attiny-regulator driver.

This is a preparation patchset for supporting the official Raspberry Pi 7
inches touchscreen.

It has been tested with a Raspberry Pi 4 B and the official Raspberry Pi 7
inches touchscreen.

Changes in v2:
 * Add a dependency on OF_GPIO for GPIO controlled resets

Dave Stevenson (9):
  regulator: rpi-panel: Register with a unique backlight name
  regulator: rpi-panel: Handle I2C errors/timing to the Atmel
  regulator: rpi-panel: Serialise operations.
  regulator: rpi-panel: Ensure the backlight is off during probe.
  regulator: rpi-panel: Convert to drive lines directly
  regulator: rpi-panel: Add GPIO control for panel and touch resets
  regulator: rpi-panel: Remove get_brightness hook
  regulator/rpi-panel-attiny: Don't read the LCD power status
  regulator/rpi-panel-attiny: Use two transactions for I2C read

 drivers/regulator/Kconfig                     |   1 +
 .../regulator/rpi-panel-attiny-regulator.c    | 285 +++++++++++++++---
 2 files changed, 237 insertions(+), 49 deletions(-)

-- 
2.34.1

[PATCH v2 1/9] regulator: rpi-panel: Register with a unique backlight name

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

There's no reason why 2 Raspberry Pi DSI displays can't be
attached to a Pi Compute Module, so the backlight names need to
be unique.

Use the parent dev_name. It's not as readable, but is unique.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/regulator/rpi-panel-attiny-regulator.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index ee46bfbf5eee..370b9ae363dd 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -181,8 +181,7 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = 0xff;
-	bl = devm_backlight_device_register(&i2c->dev,
-					    "7inch-touchscreen-panel-bl",
+	bl = devm_backlight_device_register(&i2c->dev, dev_name(&i2c->dev),
 					    &i2c->dev, regmap, &attiny_bl,
 					    &props);
 	if (IS_ERR(bl))
-- 
2.34.1

Re: [PATCH v2 1/9] regulator: rpi-panel: Register with a unique backlight name

[Mark Brown]

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

On Fri, Jan 21, 2022 at 10:20:48AM -0500, Detlev Casanova wrote:
> 
> Use the parent dev_name. It's not as readable, but is unique.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
>  drivers/regulator/rpi-panel-attiny-regulator.c | 3 +--

You've not provided a Signed-off-by for this so I can't do anything with
it, please see Documentation/process/submitting-patches.rst for details
on what this is and why it's important.

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

Re: [PATCH v2 1/9] regulator: rpi-panel: Register with a unique backlight name

[Detlev Casanova]

On Friday, January 21, 2022 11:26:01 A.M. EST Mark Brown wrote:
> On Fri, Jan 21, 2022 at 10:20:48AM -0500, Detlev Casanova wrote:
> > Use the parent dev_name. It's not as readable, but is unique.
> > 
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > ---
> > 
> >  drivers/regulator/rpi-panel-attiny-regulator.c | 3 +--
> 
> You've not provided a Signed-off-by for this so I can't do anything with
> it, please see Documentation/process/submitting-patches.rst for details
> on what this is and why it's important.

Oh ok, I thought only the author had to be there but I'll add myself as well. 
Thanks for clarifying this.

[PATCH v2 2/9] regulator: rpi-panel: Handle I2C errors/timing to the Atmel

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The Atmel is doing some things in the I2C ISR, during which
period it will not respond to further commands. This is
particularly true of the POWERON command.

Increase delays appropriately, and retry should I2C errors be
reported.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 .../regulator/rpi-panel-attiny-regulator.c    | 56 +++++++++++++++----
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 370b9ae363dd..00fb69efcfa2 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -37,11 +37,24 @@ static const struct regmap_config attiny_regmap_config = {
 static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 {
 	unsigned int data;
+	int ret, i;
 
 	regmap_write(rdev->regmap, REG_POWERON, 1);
+	msleep(80);
+
 	/* Wait for nPWRDWN to go low to indicate poweron is done. */
-	regmap_read_poll_timeout(rdev->regmap, REG_PORTB, data,
-					data & BIT(0), 10, 1000000);
+	for (i = 0; i < 20; i++) {
+		ret = regmap_read(rdev->regmap, REG_PORTB, &data);
+		if (!ret) {
+			if (data & BIT(0))
+				break;
+		}
+		usleep_range(10000, 12000);
+	}
+	usleep_range(10000, 12000);
+
+	if (ret)
+		pr_err("%s: regmap_read_poll_timeout failed %d\n", __func__, ret);
 
 	/* Default to the same orientation as the closed source
 	 * firmware used for the panel.  Runtime rotation
@@ -57,23 +70,34 @@ static int attiny_lcd_power_disable(struct regulator_dev *rdev)
 {
 	regmap_write(rdev->regmap, REG_PWM, 0);
 	regmap_write(rdev->regmap, REG_POWERON, 0);
-	udelay(1);
+	msleep(30);
 	return 0;
 }
 
 static int attiny_lcd_power_is_enabled(struct regulator_dev *rdev)
 {
 	unsigned int data;
-	int ret;
+	int ret, i;
 
-	ret = regmap_read(rdev->regmap, REG_POWERON, &data);
+	for (i = 0; i < 10; i++) {
+		ret = regmap_read(rdev->regmap, REG_POWERON, &data);
+		if (!ret)
+			break;
+		usleep_range(10000, 12000);
+	}
 	if (ret < 0)
 		return ret;
 
 	if (!(data & BIT(0)))
 		return 0;
 
-	ret = regmap_read(rdev->regmap, REG_PORTB, &data);
+	for (i = 0; i < 10; i++) {
+		ret = regmap_read(rdev->regmap, REG_PORTB, &data);
+		if (!ret)
+			break;
+		usleep_range(10000, 12000);
+	}
+
 	if (ret < 0)
 		return ret;
 
@@ -103,20 +127,32 @@ static int attiny_update_status(struct backlight_device *bl)
 {
 	struct regmap *regmap = bl_get_data(bl);
 	int brightness = bl->props.brightness;
+	int ret, i;
 
 	if (bl->props.power != FB_BLANK_UNBLANK ||
 	    bl->props.fb_blank != FB_BLANK_UNBLANK)
 		brightness = 0;
 
-	return regmap_write(regmap, REG_PWM, brightness);
+	for (i = 0; i < 10; i++) {
+		ret = regmap_write(regmap, REG_PWM, brightness);
+		if (!ret)
+			break;
+	}
+
+	return ret;
 }
 
 static int attiny_get_brightness(struct backlight_device *bl)
 {
 	struct regmap *regmap = bl_get_data(bl);
-	int ret, brightness;
+	int ret, brightness, i;
+
+	for (i = 0; i < 10; i++) {
+		ret = regmap_read(regmap, REG_PWM, &brightness);
+		if (!ret)
+			break;
+	}
 
-	ret = regmap_read(regmap, REG_PWM, &brightness);
 	if (ret)
 		return ret;
 
@@ -166,7 +202,7 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 	}
 
 	regmap_write(regmap, REG_POWERON, 0);
-	mdelay(1);
+	msleep(30);
 
 	config.dev = &i2c->dev;
 	config.regmap = regmap;
-- 
2.34.1

[PATCH v2 3/9] regulator: rpi-panel: Serialise operations.

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The driver was using the regmap lock to serialise the
individual accesses, but we really need to protect the
timings of enabling the regulators, including any communication
with the Atmel.

Use a mutex within the driver to control overall accesses to
the Atmel, instead of the regmap lock.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 .../regulator/rpi-panel-attiny-regulator.c    | 91 ++++++++++++++++---
 1 file changed, 80 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 00fb69efcfa2..a4af7adad2b5 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -27,18 +27,28 @@
 #define REG_POWERON	0x85
 #define REG_PWM		0x86
 
+struct attiny_lcd {
+	/* lock to serialise overall accesses to the Atmel */
+	struct mutex	lock;
+	struct regmap	*regmap;
+};
+
 static const struct regmap_config attiny_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
+	.disable_locking = 1,
 	.max_register = REG_PWM,
 	.cache_type = REGCACHE_NONE,
 };
 
 static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 {
+	struct mutex *lock = rdev_get_drvdata(rdev);
 	unsigned int data;
 	int ret, i;
 
+	mutex_lock(lock);
+
 	regmap_write(rdev->regmap, REG_POWERON, 1);
 	msleep(80);
 
@@ -63,33 +73,49 @@ static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 	 */
 	regmap_write(rdev->regmap, REG_PORTA, BIT(2));
 
+	mutex_unlock(lock);
+
 	return 0;
 }
 
 static int attiny_lcd_power_disable(struct regulator_dev *rdev)
 {
+	struct mutex *lock = rdev_get_drvdata(rdev);
+
+	mutex_lock(lock);
+
 	regmap_write(rdev->regmap, REG_PWM, 0);
 	regmap_write(rdev->regmap, REG_POWERON, 0);
 	msleep(30);
+
+	mutex_unlock(lock);
+
 	return 0;
 }
 
 static int attiny_lcd_power_is_enabled(struct regulator_dev *rdev)
 {
+	struct mutex *lock = rdev_get_drvdata(rdev);
 	unsigned int data;
 	int ret, i;
 
+	mutex_lock(lock);
+
 	for (i = 0; i < 10; i++) {
 		ret = regmap_read(rdev->regmap, REG_POWERON, &data);
 		if (!ret)
 			break;
 		usleep_range(10000, 12000);
 	}
-	if (ret < 0)
+	if (ret < 0) {
+		mutex_unlock(lock);
 		return ret;
+	}
 
-	if (!(data & BIT(0)))
+	if (!(data & BIT(0))) {
+		mutex_unlock(lock);
 		return 0;
+	}
 
 	for (i = 0; i < 10; i++) {
 		ret = regmap_read(rdev->regmap, REG_PORTB, &data);
@@ -98,6 +124,8 @@ static int attiny_lcd_power_is_enabled(struct regulator_dev *rdev)
 		usleep_range(10000, 12000);
 	}
 
+	mutex_unlock(lock);
+
 	if (ret < 0)
 		return ret;
 
@@ -125,10 +153,13 @@ static const struct regulator_desc attiny_regulator = {
 
 static int attiny_update_status(struct backlight_device *bl)
 {
-	struct regmap *regmap = bl_get_data(bl);
+	struct attiny_lcd *state = bl_get_data(bl);
+	struct regmap *regmap = state->regmap;
 	int brightness = bl->props.brightness;
 	int ret, i;
 
+	mutex_lock(&state->lock);
+
 	if (bl->props.power != FB_BLANK_UNBLANK ||
 	    bl->props.fb_blank != FB_BLANK_UNBLANK)
 		brightness = 0;
@@ -139,20 +170,27 @@ static int attiny_update_status(struct backlight_device *bl)
 			break;
 	}
 
+	mutex_unlock(&state->lock);
+
 	return ret;
 }
 
 static int attiny_get_brightness(struct backlight_device *bl)
 {
-	struct regmap *regmap = bl_get_data(bl);
+	struct attiny_lcd *state = bl_get_data(bl);
+	struct regmap *regmap = state->regmap;
 	int ret, brightness, i;
 
+	mutex_lock(&state->lock);
+
 	for (i = 0; i < 10; i++) {
 		ret = regmap_read(regmap, REG_PWM, &brightness);
 		if (!ret)
 			break;
 	}
 
+	mutex_unlock(&state->lock);
+
 	if (ret)
 		return ret;
 
@@ -174,22 +212,30 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 	struct regulator_config config = { };
 	struct backlight_device *bl;
 	struct regulator_dev *rdev;
+	struct attiny_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->lock);
+	i2c_set_clientdata(i2c, state);
+
 	regmap = devm_regmap_init_i2c(i2c, &attiny_regmap_config);
 	if (IS_ERR(regmap)) {
 		ret = PTR_ERR(regmap);
 		dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
 			ret);
-		return ret;
+		goto error;
 	}
 
 	ret = regmap_read(regmap, REG_ID, &data);
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to read REG_ID reg: %d\n", ret);
-		return ret;
+		goto error;
 	}
 
 	switch (data) {
@@ -198,7 +244,8 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 		break;
 	default:
 		dev_err(&i2c->dev, "Unknown Atmel firmware revision: 0x%02x\n", data);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto error;
 	}
 
 	regmap_write(regmap, REG_POWERON, 0);
@@ -208,23 +255,44 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 	config.regmap = regmap;
 	config.of_node = i2c->dev.of_node;
 	config.init_data = &attiny_regulator_default;
+	config.driver_data = &state->lock;
 
 	rdev = devm_regulator_register(&i2c->dev, &attiny_regulator, &config);
 	if (IS_ERR(rdev)) {
 		dev_err(&i2c->dev, "Failed to register ATTINY regulator\n");
-		return PTR_ERR(rdev);
+		ret = PTR_ERR(rdev);
+		goto error;
 	}
 
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = 0xff;
+
+	state->regmap = regmap;
+
 	bl = devm_backlight_device_register(&i2c->dev, dev_name(&i2c->dev),
-					    &i2c->dev, regmap, &attiny_bl,
+					    &i2c->dev, state, &attiny_bl,
 					    &props);
-	if (IS_ERR(bl))
-		return PTR_ERR(bl);
+	if (IS_ERR(bl)) {
+		ret = PTR_ERR(bl);
+		goto error;
+	}
 
 	bl->props.brightness = 0xff;
 
+	return 0;
+
+error:
+	mutex_destroy(&state->lock);
+
+	return ret;
+}
+
+static int attiny_i2c_remove(struct i2c_client *client)
+{
+	struct attiny_lcd *state = i2c_get_clientdata(client);
+
+	mutex_destroy(&state->lock);
+
 	return 0;
 }
 
@@ -240,6 +308,7 @@ static struct i2c_driver attiny_regulator_driver = {
 		.of_match_table = of_match_ptr(attiny_dt_ids),
 	},
 	.probe = attiny_i2c_probe,
+	.remove	= attiny_i2c_remove,
 };
 
 module_i2c_driver(attiny_regulator_driver);
-- 
2.34.1

[PATCH v2 4/9] regulator: rpi-panel: Ensure the backlight is off during probe.

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The initial state of the Atmel is not defined, so ensure the
backlight PWM is set to 0 by default.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/regulator/rpi-panel-attiny-regulator.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index a4af7adad2b5..b3629a1e0e50 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -250,6 +250,7 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 
 	regmap_write(regmap, REG_POWERON, 0);
 	msleep(30);
+	regmap_write(regmap, REG_PWM, 0);
 
 	config.dev = &i2c->dev;
 	config.regmap = regmap;
-- 
2.34.1

[PATCH v2 5/9] regulator: rpi-panel: Convert to drive lines directly

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The Atmel was doing a load of automatic sequencing of
control lines, however it was combining the touch controller's
reset with the bridge/panel control.

Change to control the control signals directly rather than
through the automatic POWERON control.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 .../regulator/rpi-panel-attiny-regulator.c    | 111 ++++++++++--------
 1 file changed, 60 insertions(+), 51 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index b3629a1e0e50..995915ca4a9b 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -21,11 +21,28 @@
 /* I2C registers of the Atmel microcontroller. */
 #define REG_ID		0x80
 #define REG_PORTA	0x81
-#define REG_PORTA_HF	BIT(2)
-#define REG_PORTA_VF	BIT(3)
 #define REG_PORTB	0x82
+#define REG_PORTC	0x83
 #define REG_POWERON	0x85
 #define REG_PWM		0x86
+#define REG_ADDR_L	0x8c
+#define REG_ADDR_H	0x8d
+#define REG_WRITE_DATA_H	0x90
+#define REG_WRITE_DATA_L	0x91
+
+#define PA_LCD_DITHB		BIT(0)
+#define PA_LCD_MODE		BIT(1)
+#define PA_LCD_LR		BIT(2)
+#define PA_LCD_UD		BIT(3)
+
+#define PB_BRIDGE_PWRDNX_N	BIT(0)
+#define PB_LCD_VCC_N		BIT(1)
+#define PB_LCD_MAIN		BIT(7)
+
+#define PC_LED_EN		BIT(0)
+#define PC_RST_TP_N		BIT(1)
+#define PC_RST_LCD_N		BIT(2)
+#define PC_RST_BRIDGE_N		BIT(3)
 
 struct attiny_lcd {
 	/* lock to serialise overall accesses to the Atmel */
@@ -37,99 +54,91 @@ static const struct regmap_config attiny_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.disable_locking = 1,
-	.max_register = REG_PWM,
+	.max_register = REG_WRITE_DATA_L,
 	.cache_type = REGCACHE_NONE,
 };
 
 static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 {
-	struct mutex *lock = rdev_get_drvdata(rdev);
-	unsigned int data;
-	int ret, i;
-
-	mutex_lock(lock);
-
-	regmap_write(rdev->regmap, REG_POWERON, 1);
-	msleep(80);
+	struct attiny_lcd *state = rdev_get_drvdata(rdev);
 
-	/* Wait for nPWRDWN to go low to indicate poweron is done. */
-	for (i = 0; i < 20; i++) {
-		ret = regmap_read(rdev->regmap, REG_PORTB, &data);
-		if (!ret) {
-			if (data & BIT(0))
-				break;
-		}
-		usleep_range(10000, 12000);
-	}
-	usleep_range(10000, 12000);
+	mutex_lock(&state->lock);
 
-	if (ret)
-		pr_err("%s: regmap_read_poll_timeout failed %d\n", __func__, ret);
+	/* Ensure bridge, and tp stay in reset */
+	regmap_write(rdev->regmap, REG_PORTC, 0);
+	usleep_range(5000, 10000);
 
 	/* Default to the same orientation as the closed source
 	 * firmware used for the panel.  Runtime rotation
 	 * configuration will be supported using VC4's plane
 	 * orientation bits.
 	 */
-	regmap_write(rdev->regmap, REG_PORTA, BIT(2));
+	regmap_write(rdev->regmap, REG_PORTA, PA_LCD_LR);
+	usleep_range(5000, 10000);
+	regmap_write(rdev->regmap, REG_PORTB, PB_LCD_MAIN);
+	usleep_range(5000, 10000);
+	/* Bring controllers out of reset */
+	regmap_write(rdev->regmap, REG_PORTC,
+		     PC_LED_EN | PC_RST_BRIDGE_N | PC_RST_LCD_N | PC_RST_TP_N);
+
+	msleep(80);
+
+	regmap_write(rdev->regmap, REG_ADDR_H, 0x04);
+	usleep_range(5000, 8000);
+	regmap_write(rdev->regmap, REG_ADDR_L, 0x7c);
+	usleep_range(5000, 8000);
+	regmap_write(rdev->regmap, REG_WRITE_DATA_H, 0x00);
+	usleep_range(5000, 8000);
+	regmap_write(rdev->regmap, REG_WRITE_DATA_L, 0x00);
+
+	msleep(100);
 
-	mutex_unlock(lock);
+	mutex_unlock(&state->lock);
 
 	return 0;
 }
 
 static int attiny_lcd_power_disable(struct regulator_dev *rdev)
 {
-	struct mutex *lock = rdev_get_drvdata(rdev);
+	struct attiny_lcd *state = rdev_get_drvdata(rdev);
 
-	mutex_lock(lock);
+	mutex_lock(&state->lock);
 
 	regmap_write(rdev->regmap, REG_PWM, 0);
-	regmap_write(rdev->regmap, REG_POWERON, 0);
+	usleep_range(5000, 10000);
+	regmap_write(rdev->regmap, REG_PORTA, 0);
+	usleep_range(5000, 10000);
+	regmap_write(rdev->regmap, REG_PORTB, PB_LCD_VCC_N);
+	usleep_range(5000, 10000);
+	regmap_write(rdev->regmap, REG_PORTC, 0);
 	msleep(30);
 
-	mutex_unlock(lock);
+	mutex_unlock(&state->lock);
 
 	return 0;
 }
 
 static int attiny_lcd_power_is_enabled(struct regulator_dev *rdev)
 {
-	struct mutex *lock = rdev_get_drvdata(rdev);
+	struct attiny_lcd *state = rdev_get_drvdata(rdev);
 	unsigned int data;
 	int ret, i;
 
-	mutex_lock(lock);
-
-	for (i = 0; i < 10; i++) {
-		ret = regmap_read(rdev->regmap, REG_POWERON, &data);
-		if (!ret)
-			break;
-		usleep_range(10000, 12000);
-	}
-	if (ret < 0) {
-		mutex_unlock(lock);
-		return ret;
-	}
-
-	if (!(data & BIT(0))) {
-		mutex_unlock(lock);
-		return 0;
-	}
+	mutex_lock(&state->lock);
 
 	for (i = 0; i < 10; i++) {
-		ret = regmap_read(rdev->regmap, REG_PORTB, &data);
+		ret = regmap_read(rdev->regmap, REG_PORTC, &data);
 		if (!ret)
 			break;
 		usleep_range(10000, 12000);
 	}
 
-	mutex_unlock(lock);
+	mutex_unlock(&state->lock);
 
 	if (ret < 0)
 		return ret;
 
-	return data & BIT(0);
+	return data & PC_RST_BRIDGE_N;
 }
 
 static const struct regulator_init_data attiny_regulator_default = {
@@ -256,7 +265,7 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 	config.regmap = regmap;
 	config.of_node = i2c->dev.of_node;
 	config.init_data = &attiny_regulator_default;
-	config.driver_data = &state->lock;
+	config.driver_data = state;
 
 	rdev = devm_regulator_register(&i2c->dev, &attiny_regulator, &config);
 	if (IS_ERR(rdev)) {
-- 
2.34.1

[PATCH v2 6/9] regulator: rpi-panel: Add GPIO control for panel and touch resets

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

We need independent control of the resets for the panel&bridge,
vs the touch controller.

Expose the reset lines that are on the Atmel's port C via the GPIO
API so that they can be controlled appropriately.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 drivers/regulator/Kconfig                     |   1 +
 .../regulator/rpi-panel-attiny-regulator.c    | 115 +++++++++++++++---
 2 files changed, 98 insertions(+), 18 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 1c35fed20d34..22503e4f5327 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -984,6 +984,7 @@ config REGULATOR_RASPBERRYPI_TOUCHSCREEN_ATTINY
 	tristate "Raspberry Pi 7-inch touchscreen panel ATTINY regulator"
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on I2C
+	depends on OF_GPIO
 	select REGMAP_I2C
 	help
 	  This driver supports ATTINY regulator on the Raspberry Pi 7-inch
diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 995915ca4a9b..998233f14085 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -8,6 +8,7 @@
 #include <linux/backlight.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -44,10 +45,30 @@
 #define PC_RST_LCD_N		BIT(2)
 #define PC_RST_BRIDGE_N		BIT(3)
 
+enum gpio_signals {
+	RST_BRIDGE_N,	/* TC358762 bridge reset */
+	RST_TP_N,	/* Touch controller reset */
+	NUM_GPIO
+};
+
+struct gpio_signal_mappings {
+	unsigned int reg;
+	unsigned int mask;
+};
+
+static const struct gpio_signal_mappings mappings[NUM_GPIO] = {
+	[RST_BRIDGE_N] = { REG_PORTC, PC_RST_BRIDGE_N | PC_RST_LCD_N  },
+	[RST_TP_N] = { REG_PORTC, PC_RST_TP_N },
+};
+
 struct attiny_lcd {
 	/* lock to serialise overall accesses to the Atmel */
 	struct mutex	lock;
 	struct regmap	*regmap;
+	bool gpio_states[NUM_GPIO];
+	u8 port_states[3];
+
+	struct gpio_chip gc;
 };
 
 static const struct regmap_config attiny_regmap_config = {
@@ -58,6 +79,17 @@ static const struct regmap_config attiny_regmap_config = {
 	.cache_type = REGCACHE_NONE,
 };
 
+static int attiny_set_port_state(struct attiny_lcd *state, int reg, u8 val)
+{
+	state->port_states[reg - REG_PORTA] = val;
+	return regmap_write(state->regmap, reg, val);
+};
+
+static u8 attiny_get_port_state(struct attiny_lcd *state, int reg)
+{
+	return state->port_states[reg - REG_PORTA];
+};
+
 static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 {
 	struct attiny_lcd *state = rdev_get_drvdata(rdev);
@@ -65,7 +97,7 @@ static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 	mutex_lock(&state->lock);
 
 	/* Ensure bridge, and tp stay in reset */
-	regmap_write(rdev->regmap, REG_PORTC, 0);
+	attiny_set_port_state(state, REG_PORTC, 0);
 	usleep_range(5000, 10000);
 
 	/* Default to the same orientation as the closed source
@@ -73,26 +105,16 @@ static int attiny_lcd_power_enable(struct regulator_dev *rdev)
 	 * configuration will be supported using VC4's plane
 	 * orientation bits.
 	 */
-	regmap_write(rdev->regmap, REG_PORTA, PA_LCD_LR);
+	attiny_set_port_state(state, REG_PORTA, PA_LCD_LR);
 	usleep_range(5000, 10000);
-	regmap_write(rdev->regmap, REG_PORTB, PB_LCD_MAIN);
+	/* Main regulator on, and power to the panel (LCD_VCC_N) */
+	attiny_set_port_state(state, REG_PORTB, PB_LCD_MAIN);
 	usleep_range(5000, 10000);
 	/* Bring controllers out of reset */
-	regmap_write(rdev->regmap, REG_PORTC,
-		     PC_LED_EN | PC_RST_BRIDGE_N | PC_RST_LCD_N | PC_RST_TP_N);
+	attiny_set_port_state(state, REG_PORTC, PC_LED_EN);
 
 	msleep(80);
 
-	regmap_write(rdev->regmap, REG_ADDR_H, 0x04);
-	usleep_range(5000, 8000);
-	regmap_write(rdev->regmap, REG_ADDR_L, 0x7c);
-	usleep_range(5000, 8000);
-	regmap_write(rdev->regmap, REG_WRITE_DATA_H, 0x00);
-	usleep_range(5000, 8000);
-	regmap_write(rdev->regmap, REG_WRITE_DATA_L, 0x00);
-
-	msleep(100);
-
 	mutex_unlock(&state->lock);
 
 	return 0;
@@ -106,11 +128,12 @@ static int attiny_lcd_power_disable(struct regulator_dev *rdev)
 
 	regmap_write(rdev->regmap, REG_PWM, 0);
 	usleep_range(5000, 10000);
-	regmap_write(rdev->regmap, REG_PORTA, 0);
+
+	attiny_set_port_state(state, REG_PORTA, 0);
 	usleep_range(5000, 10000);
-	regmap_write(rdev->regmap, REG_PORTB, PB_LCD_VCC_N);
+	attiny_set_port_state(state, REG_PORTB, PB_LCD_VCC_N);
 	usleep_range(5000, 10000);
-	regmap_write(rdev->regmap, REG_PORTC, 0);
+	attiny_set_port_state(state, REG_PORTC, 0);
 	msleep(30);
 
 	mutex_unlock(&state->lock);
@@ -211,6 +234,45 @@ static const struct backlight_ops attiny_bl = {
 	.get_brightness	= attiny_get_brightness,
 };
 
+static int attiny_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
+{
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static void attiny_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
+{
+	struct attiny_lcd *state = gpiochip_get_data(gc);
+	u8 last_val;
+
+	if (off >= NUM_GPIO)
+		return;
+
+	mutex_lock(&state->lock);
+
+	last_val = attiny_get_port_state(state, mappings[off].reg);
+	if (val)
+		last_val |= mappings[off].mask;
+	else
+		last_val &= ~mappings[off].mask;
+
+	attiny_set_port_state(state, mappings[off].reg, last_val);
+
+	if (off == RST_BRIDGE_N && val) {
+		usleep_range(5000, 8000);
+		regmap_write(state->regmap, REG_ADDR_H, 0x04);
+		usleep_range(5000, 8000);
+		regmap_write(state->regmap, REG_ADDR_L, 0x7c);
+		usleep_range(5000, 8000);
+		regmap_write(state->regmap, REG_WRITE_DATA_H, 0x00);
+		usleep_range(5000, 8000);
+		regmap_write(state->regmap, REG_WRITE_DATA_L, 0x00);
+
+		msleep(100);
+	}
+
+	mutex_unlock(&state->lock);
+}
+
 /*
  * I2C driver interface functions
  */
@@ -289,6 +351,23 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 
 	bl->props.brightness = 0xff;
 
+	state->gc.parent = &i2c->dev;
+	state->gc.label = i2c->name;
+	state->gc.owner = THIS_MODULE;
+	state->gc.of_node = i2c->dev.of_node;
+	state->gc.base = -1;
+	state->gc.ngpio = NUM_GPIO;
+
+	state->gc.set = attiny_gpio_set;
+	state->gc.get_direction = attiny_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;
+	}
+
 	return 0;
 
 error:
-- 
2.34.1

[PATCH v2 7/9] regulator: rpi-panel: Remove get_brightness hook

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The driver was implementing a get_brightness function that
tried to read back the PWM setting of the display to report
as the current brightness.
The controller on the display does not support that, therefore
we end up reporting a brightness of 0, and that confuses
systemd's backlight service.

Remove the hook so that the framework returns the current
brightness automatically.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 .../regulator/rpi-panel-attiny-regulator.c    | 23 -------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 998233f14085..8090b9a485b5 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -207,31 +207,8 @@ static int attiny_update_status(struct backlight_device *bl)
 	return ret;
 }
 
-static int attiny_get_brightness(struct backlight_device *bl)
-{
-	struct attiny_lcd *state = bl_get_data(bl);
-	struct regmap *regmap = state->regmap;
-	int ret, brightness, i;
-
-	mutex_lock(&state->lock);
-
-	for (i = 0; i < 10; i++) {
-		ret = regmap_read(regmap, REG_PWM, &brightness);
-		if (!ret)
-			break;
-	}
-
-	mutex_unlock(&state->lock);
-
-	if (ret)
-		return ret;
-
-	return brightness;
-}
-
 static const struct backlight_ops attiny_bl = {
 	.update_status	= attiny_update_status,
-	.get_brightness	= attiny_get_brightness,
 };
 
 static int attiny_gpio_get_direction(struct gpio_chip *gc, unsigned int off)
-- 
2.34.1

[PATCH v2 8/9] regulator/rpi-panel-attiny: Don't read the LCD power status

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The I2C to the Atmel is very fussy, and locks up easily on
Pi0-3 particularly on reads.

The LCD power status is controlled solely by this driver, so
rather than reading it back from the Atmel, use the cached
status last set.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/regulator/rpi-panel-attiny-regulator.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 8090b9a485b5..8b80c0de1022 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -144,24 +144,8 @@ static int attiny_lcd_power_disable(struct regulator_dev *rdev)
 static int attiny_lcd_power_is_enabled(struct regulator_dev *rdev)
 {
 	struct attiny_lcd *state = rdev_get_drvdata(rdev);
-	unsigned int data;
-	int ret, i;
-
-	mutex_lock(&state->lock);
-
-	for (i = 0; i < 10; i++) {
-		ret = regmap_read(rdev->regmap, REG_PORTC, &data);
-		if (!ret)
-			break;
-		usleep_range(10000, 12000);
-	}
-
-	mutex_unlock(&state->lock);
-
-	if (ret < 0)
-		return ret;
 
-	return data & PC_RST_BRIDGE_N;
+	return state->port_states[REG_PORTC - REG_PORTA] & PC_RST_BRIDGE_N;
 }
 
 static const struct regulator_init_data attiny_regulator_default = {
-- 
2.34.1

Re: [PATCH v2 8/9] regulator/rpi-panel-attiny: Don't read the LCD power status

[Mark Brown]

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

On Fri, Jan 21, 2022 at 10:20:55AM -0500, Detlev Casanova wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> The I2C to the Atmel is very fussy, and locks up easily on
> Pi0-3 particularly on reads.
> 
> The LCD power status is controlled solely by this driver, so
> rather than reading it back from the Atmel, use the cached
> status last set.

If that's the case since you're using regmap would it not make sense to
enable caching in order to minimise reads in general.

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

Re: [PATCH v2 8/9] regulator/rpi-panel-attiny: Don't read the LCD power status

[Detlev Casanova]

On Friday, January 21, 2022 11:30:39 A.M. EST Mark Brown wrote:
> On Fri, Jan 21, 2022 at 10:20:55AM -0500, Detlev Casanova wrote:
> > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > 
> > The I2C to the Atmel is very fussy, and locks up easily on
> > Pi0-3 particularly on reads.
> > 
> > The LCD power status is controlled solely by this driver, so
> > rather than reading it back from the Atmel, use the cached
> > status last set.
> 
> If that's the case since you're using regmap would it not make sense to
> enable caching in order to minimise reads in general.

Would caching avoid all reads if the values are written at least once before? 
The idea here is to make sure they are never read, I'm not sure if the regmap 
cache ensures no read in this situation.

Re: [PATCH v2 8/9] regulator/rpi-panel-attiny: Don't read the LCD power status

[Mark Brown]

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

On Mon, Jan 24, 2022 at 03:22:35PM -0500, Detlev Casanova wrote:
> On Friday, January 21, 2022 11:30:39 A.M. EST Mark Brown wrote:

> > If that's the case since you're using regmap would it not make sense to
> > enable caching in order to minimise reads in general.

> Would caching avoid all reads if the values are written at least once before? 
> The idea here is to make sure they are never read, I'm not sure if the regmap 
> cache ensures no read in this situation.

All reads for which the value in the hardware is known, either from a
register default or from a previous write.

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

[PATCH v2 9/9] regulator/rpi-panel-attiny: Use two transactions for I2C read

[Detlev Casanova]

From: Dave Stevenson <dave.stevenson@raspberrypi.com>

The I2C to the Atmel is very fussy, and locks up easily on
Pi0-3 particularly on reads.
If running at 100kHz on Pi3, reading the ID register generally
locks up the Atmel, but splitting the register select write and
read into two transactions is reliable.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 .../regulator/rpi-panel-attiny-regulator.c    | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/rpi-panel-attiny-regulator.c b/drivers/regulator/rpi-panel-attiny-regulator.c
index 8b80c0de1022..e3decc419814 100644
--- a/drivers/regulator/rpi-panel-attiny-regulator.c
+++ b/drivers/regulator/rpi-panel-attiny-regulator.c
@@ -234,6 +234,39 @@ static void attiny_gpio_set(struct gpio_chip *gc, unsigned int off, int val)
 	mutex_unlock(&state->lock);
 }
 
+static int attiny_i2c_read(struct i2c_client *client, u8 reg, unsigned int *buf)
+{
+	struct i2c_msg msgs[1];
+	u8 addr_buf[1] = { reg };
+	u8 data_buf[1] = { 0, };
+	int ret;
+
+	/* Write register address */
+	msgs[0].addr = client->addr;
+	msgs[0].flags = 0;
+	msgs[0].len = ARRAY_SIZE(addr_buf);
+	msgs[0].buf = addr_buf;
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
+
+	usleep_range(5000, 10000);
+
+	/* Read data from register */
+	msgs[0].addr = client->addr;
+	msgs[0].flags = I2C_M_RD;
+	msgs[0].len = 1;
+	msgs[0].buf = data_buf;
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret != ARRAY_SIZE(msgs))
+		return -EIO;
+
+	*buf = data_buf[0];
+	return 0;
+}
+
 /*
  * I2C driver interface functions
  */
@@ -264,7 +297,7 @@ static int attiny_i2c_probe(struct i2c_client *i2c,
 		goto error;
 	}
 
-	ret = regmap_read(regmap, REG_ID, &data);
+	ret = attiny_i2c_read(i2c, REG_ID, &data);
 	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to read REG_ID reg: %d\n", ret);
 		goto error;
-- 
2.34.1