Hi,
this small series improves debugging the tc358767 driver by using
dev_err_probe for additional information (patch 1) and print IRQ
debug output only if hotplug status actually changed.
Changes in v2:
* Added patch for supporting write-only registers
Best regards,
Alexander
Alexander Stein (3):
drm/bridge: tc358767: Use dev_err_probe
drm/bridge: tc358767: Only print GPIO debug output if they actually
occur
drm/bridge: tc358767: Support write-only registers
drivers/gpu/drm/bridge/tc358767.c | 56 +++++++++++++++++++------------
1 file changed, 35 insertions(+), 21 deletions(-)
--
2.34.1
The function calls preceding these returns can return -EPROBE_DEFER. So
use dev_err_probe to add some information to
/sys/kernel/debug/devices_deferred
Signed-off-by: Alexander Stein <[email protected]>
---
drivers/gpu/drm/bridge/tc358767.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 99e8a8cf29f5b..de62a7e2eafec 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2215,7 +2215,8 @@ static int tc_probe_dpi_bridge_endpoint(struct tc_data *tc)
/* port@1 is the DPI input/output port */
ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, &panel, &bridge);
if (ret && ret != -ENODEV)
- return ret;
+ return dev_err_probe(dev, ret,
+ "Could not find DPI panel or bridge\n");
if (panel) {
bridge = devm_drm_panel_bridge_add(dev, panel);
@@ -2243,7 +2244,8 @@ static int tc_probe_edp_bridge_endpoint(struct tc_data *tc)
/* port@2 is the output port */
ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, NULL);
if (ret && ret != -ENODEV)
- return ret;
+ return dev_err_probe(dev, ret,
+ "Could not find DSI panel or bridge\n");
if (panel) {
struct drm_bridge *panel_bridge;
@@ -2455,7 +2457,7 @@ static int tc_probe(struct i2c_client *client)
ret = tc_mipi_dsi_host_attach(tc);
if (ret) {
drm_bridge_remove(&tc->bridge);
- return ret;
+ return dev_err_probe(dev, ret, "Failed to attach DSI host\n");
}
}
--
2.34.1
Most registers are read-writable, but some are only RO or even WO.
regmap does not support using readable_reg and wr_table when outputting
in debugfs, so switch to writeable_reg.
First check for RO or WO registers and fallback tc_readable_reg() for the
leftover RW registers.
Signed-off-by: Alexander Stein <[email protected]>
---
drivers/gpu/drm/bridge/tc358767.c | 40 ++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 8874713bdd4a4..04c98ab1991bd 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2086,19 +2086,31 @@ static const struct regmap_access_table tc_precious_table = {
.n_yes_ranges = ARRAY_SIZE(tc_precious_ranges),
};
-static const struct regmap_range tc_non_writeable_ranges[] = {
- regmap_reg_range(PPI_BUSYPPI, PPI_BUSYPPI),
- regmap_reg_range(DSI_BUSYDSI, DSI_BUSYDSI),
- regmap_reg_range(DSI_LANESTATUS0, DSI_INTSTATUS),
- regmap_reg_range(TC_IDREG, SYSSTAT),
- regmap_reg_range(GPIOI, GPIOI),
- regmap_reg_range(DP0_LTSTAT, DP0_SNKLTCHGREQ),
-};
-
-static const struct regmap_access_table tc_writeable_table = {
- .no_ranges = tc_non_writeable_ranges,
- .n_no_ranges = ARRAY_SIZE(tc_non_writeable_ranges),
-};
+static bool tc_writeable_reg(struct device *dev, unsigned int reg)
+{
+ /* RO reg */
+ switch (reg) {
+ case PPI_BUSYPPI:
+ case DSI_BUSYDSI:
+ case DSI_LANESTATUS0:
+ case DSI_LANESTATUS1:
+ case DSI_INTSTATUS:
+ case TC_IDREG:
+ case SYSBOOT:
+ case SYSSTAT:
+ case GPIOI:
+ case DP0_LTSTAT:
+ case DP0_SNKLTCHGREQ:
+ return false;
+ }
+ /* WO reg */
+ switch (reg) {
+ case DSI_STARTDSI:
+ case DSI_INTCLR:
+ return true;
+ }
+ return tc_readable_reg(dev, reg);
+}
static const struct regmap_config tc_regmap_config = {
.name = "tc358767",
@@ -2108,9 +2120,9 @@ static const struct regmap_config tc_regmap_config = {
.max_register = PLL_DBG,
.cache_type = REGCACHE_MAPLE,
.readable_reg = tc_readable_reg,
+ .writeable_reg = tc_writeable_reg,
.volatile_table = &tc_volatile_table,
.precious_table = &tc_precious_table,
- .wr_table = &tc_writeable_table,
.reg_format_endian = REGMAP_ENDIAN_BIG,
.val_format_endian = REGMAP_ENDIAN_LITTLE,
};
--
2.34.1
Currently the output the following output is printed upon each interrupt:
tc358767 1-000f: GPIO0:
This spams the kernel log while debugging an IRQ storm from the bridge.
Only print the debug output if the GPIO hotplug event actually happened.
Signed-off-by: Alexander Stein <[email protected]>
---
drivers/gpu/drm/bridge/tc358767.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index de62a7e2eafec..8874713bdd4a4 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -2146,11 +2146,11 @@ static irqreturn_t tc_irq_handler(int irq, void *arg)
bool h = val & INT_GPIO_H(tc->hpd_pin);
bool lc = val & INT_GPIO_LC(tc->hpd_pin);
- dev_dbg(tc->dev, "GPIO%d: %s %s\n", tc->hpd_pin,
- h ? "H" : "", lc ? "LC" : "");
-
- if (h || lc)
+ if (h || lc) {
+ dev_dbg(tc->dev, "GPIO%d: %s %s\n", tc->hpd_pin,
+ h ? "H" : "", lc ? "LC" : "");
drm_kms_helper_hotplug_event(tc->bridge.dev);
+ }
}
regmap_write(tc->regmap, INTSTS_G, val);
--
2.34.1
On Thu, May 16, 2024 at 08:24:53AM +0200, Alexander Stein wrote:
> The function calls preceding these returns can return -EPROBE_DEFER. So
> use dev_err_probe to add some information to
> /sys/kernel/debug/devices_deferred
>
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358767.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Thu, May 16, 2024 at 8:25 AM Alexander Stein
<[email protected]> wrote:
>
> Currently the output the following output is printed upon each interrupt:
> tc358767 1-000f: GPIO0:
> This spams the kernel log while debugging an IRQ storm from the bridge.
> Only print the debug output if the GPIO hotplug event actually happened.
>
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358767.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index de62a7e2eafec..8874713bdd4a4 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -2146,11 +2146,11 @@ static irqreturn_t tc_irq_handler(int irq, void *arg)
> bool h = val & INT_GPIO_H(tc->hpd_pin);
> bool lc = val & INT_GPIO_LC(tc->hpd_pin);
>
> - dev_dbg(tc->dev, "GPIO%d: %s %s\n", tc->hpd_pin,
> - h ? "H" : "", lc ? "LC" : "");
> -
> - if (h || lc)
> + if (h || lc) {
> + dev_dbg(tc->dev, "GPIO%d: %s %s\n", tc->hpd_pin,
> + h ? "H" : "", lc ? "LC" : "");
> drm_kms_helper_hotplug_event(tc->bridge.dev);
> + }
> }
>
> regmap_write(tc->regmap, INTSTS_G, val);
> --
> 2.34.1
>
>
Reviewed-by: Robert Foss <[email protected]>
On Thu, May 16, 2024 at 8:25 AM Alexander Stein
<[email protected]> wrote:
>
> Most registers are read-writable, but some are only RO or even WO.
> regmap does not support using readable_reg and wr_table when outputting
> in debugfs, so switch to writeable_reg.
> First check for RO or WO registers and fallback tc_readable_reg() for the
> leftover RW registers.
>
> Signed-off-by: Alexander Stein <[email protected]>
> ---
> drivers/gpu/drm/bridge/tc358767.c | 40 ++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 8874713bdd4a4..04c98ab1991bd 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -2086,19 +2086,31 @@ static const struct regmap_access_table tc_precious_table = {
> .n_yes_ranges = ARRAY_SIZE(tc_precious_ranges),
> };
>
> -static const struct regmap_range tc_non_writeable_ranges[] = {
> - regmap_reg_range(PPI_BUSYPPI, PPI_BUSYPPI),
> - regmap_reg_range(DSI_BUSYDSI, DSI_BUSYDSI),
> - regmap_reg_range(DSI_LANESTATUS0, DSI_INTSTATUS),
> - regmap_reg_range(TC_IDREG, SYSSTAT),
> - regmap_reg_range(GPIOI, GPIOI),
> - regmap_reg_range(DP0_LTSTAT, DP0_SNKLTCHGREQ),
> -};
> -
> -static const struct regmap_access_table tc_writeable_table = {
> - .no_ranges = tc_non_writeable_ranges,
> - .n_no_ranges = ARRAY_SIZE(tc_non_writeable_ranges),
> -};
> +static bool tc_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + /* RO reg */
> + switch (reg) {
> + case PPI_BUSYPPI:
> + case DSI_BUSYDSI:
> + case DSI_LANESTATUS0:
> + case DSI_LANESTATUS1:
> + case DSI_INTSTATUS:
> + case TC_IDREG:
> + case SYSBOOT:
> + case SYSSTAT:
> + case GPIOI:
> + case DP0_LTSTAT:
> + case DP0_SNKLTCHGREQ:
> + return false;
> + }
> + /* WO reg */
> + switch (reg) {
> + case DSI_STARTDSI:
> + case DSI_INTCLR:
> + return true;
> + }
> + return tc_readable_reg(dev, reg);
> +}
>
> static const struct regmap_config tc_regmap_config = {
> .name = "tc358767",
> @@ -2108,9 +2120,9 @@ static const struct regmap_config tc_regmap_config = {
> .max_register = PLL_DBG,
> .cache_type = REGCACHE_MAPLE,
> .readable_reg = tc_readable_reg,
> + .writeable_reg = tc_writeable_reg,
> .volatile_table = &tc_volatile_table,
> .precious_table = &tc_precious_table,
> - .wr_table = &tc_writeable_table,
> .reg_format_endian = REGMAP_ENDIAN_BIG,
> .val_format_endian = REGMAP_ENDIAN_LITTLE,
> };
> --
> 2.34.1
>
Reviewed-by: Robert Foss <[email protected]>