2022-01-22 01:53:27

by Detlev Casanova

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

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


2022-01-22 01:53:41

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
.../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

2022-01-22 01:53:53

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
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

2022-01-22 01:54:04

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
Signed-off-by: Detlev Casanova <[email protected]>
---
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

2022-01-22 01:54:07

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
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

2022-01-22 01:54:38

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
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

2022-01-22 01:54:47

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
.../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

2022-01-22 01:54:59

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
.../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

2022-01-22 01:55:00

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
.../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

2022-01-22 01:55:09

by Detlev Casanova

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

From: Dave Stevenson <[email protected]>

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 <[email protected]>
---
.../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

2022-01-22 01:56:47

by Mark Brown

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

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 <[email protected]>
> ---
> 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.


Attachments:
(No filename) (461.00 B)
signature.asc (499.00 B)
Download all attachments

2022-01-22 01:58:13

by Mark Brown

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

On Fri, Jan 21, 2022 at 10:20:55AM -0500, Detlev Casanova wrote:
> From: Dave Stevenson <[email protected]>
>
> 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.


Attachments:
(No filename) (501.00 B)
signature.asc (499.00 B)
Download all attachments

2022-01-22 02:10:03

by Detlev Casanova

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

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 <[email protected]>
> > ---
> >
> > 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.



2022-01-24 21:17:17

by Detlev Casanova

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

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 <[email protected]>
> >
> > 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.



2022-01-24 23:17:54

by Mark Brown

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

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.


Attachments:
(No filename) (598.00 B)
signature.asc (499.00 B)
Download all attachments