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.
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
.../regulator/rpi-panel-attiny-regulator.c | 285 +++++++++++++++---
1 file changed, 236 insertions(+), 49 deletions(-)
--
2.34.1
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
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
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
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]>
---
.../regulator/rpi-panel-attiny-regulator.c | 115 +++++++++++++++---
1 file changed, 97 insertions(+), 18 deletions(-)
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