* [PATCH 1/7] drm/msm/hdmi: Simplify with dev_of_node()
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 2/7] drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure Krzysztof Kozlowski
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
Simplify chain of pointer dereferences with dev_of_node() which also
checks if 'dev' argument is non-NULL.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 5afac09c0d33..b1db1b597901 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -91,7 +91,7 @@ static int msm_hdmi_get_phy(struct hdmi *hdmi)
struct platform_device *phy_pdev;
struct device_node *phy_node;
- phy_node = of_parse_phandle(pdev->dev.of_node, "phys", 0);
+ phy_node = of_parse_phandle(dev_of_node(&pdev->dev), "phys", 0);
if (!phy_node) {
DRM_DEV_ERROR(&pdev->dev, "cannot find phy device\n");
return -ENXIO;
@@ -287,7 +287,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
spin_lock_init(&hdmi->reg_lock);
mutex_init(&hdmi->state_mutex);
- ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge);
+ ret = drm_of_find_panel_or_bridge(dev_of_node(dev), 1, 0, NULL, &hdmi->next_bridge);
if (ret && ret != -ENODEV)
return ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 2/7] drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
2026-03-11 20:17 ` [PATCH 1/7] drm/msm/hdmi: Simplify with dev_of_node() Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 3/7] drm/msm/hdmi: Simplify with local 'dev' variable Krzysztof Kozlowski
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
msm_hdmi_get_phy() already prints error messages on each error path
using dev_err_probe(), so final DRM_DEV_ERROR() would duplicate it and
possibly flood the dmesg on probe deferrals.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index b1db1b597901..02a87bccab77 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -358,10 +358,8 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
ret = msm_hdmi_get_phy(hdmi);
- if (ret) {
- DRM_DEV_ERROR(&pdev->dev, "failed to get phy\n");
+ if (ret)
return ret;
- }
ret = devm_pm_runtime_enable(&pdev->dev);
if (ret)
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure
2026-03-11 20:17 ` [PATCH 2/7] drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure Krzysztof Kozlowski
@ 2026-03-11 20:45 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:45 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Correct. Looking at `msm_hdmi_get_phy()` (hdmi.c:88-115), every error path already prints via `DRM_DEV_ERROR` or `dev_err_probe()`, so the caller's `DRM_DEV_ERROR("failed to get phy\n")` was indeed redundant. Additionally, `dev_err_probe()` with `-EPROBE_DEFER` correctly suppresses the message during deferred probing, while the removed `DRM_DEV_ERROR` would have printed unconditionally — so this also fixes a minor log spam issue.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] drm/msm/hdmi: Simplify with local 'dev' variable
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
2026-03-11 20:17 ` [PATCH 1/7] drm/msm/hdmi: Simplify with dev_of_node() Krzysztof Kozlowski
2026-03-11 20:17 ` [PATCH 2/7] drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 4/7] drm/msm/hdmi: Consistently use u32 instead of uint32_t Krzysztof Kozlowski
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
msm_hdmi_dev_probe() function already stores pdev->dev in local
variable, so use it directly to make code simpler.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 02a87bccab77..03aa29dbb2f5 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -278,7 +278,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
if (!config)
return -EINVAL;
- hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+ hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL);
if (!hdmi)
return -ENOMEM;
@@ -304,7 +304,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical");
if (IS_ERR(hdmi->qfprom_mmio)) {
- DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n");
+ DRM_DEV_INFO(dev, "can't find qfprom resource\n");
hdmi->qfprom_mmio = NULL;
}
@@ -312,8 +312,7 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
if (hdmi->irq < 0)
return hdmi->irq;
- hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
- config->pwr_reg_cnt,
+ hdmi->pwr_regs = devm_kcalloc(dev, config->pwr_reg_cnt,
sizeof(hdmi->pwr_regs[0]),
GFP_KERNEL);
if (!hdmi->pwr_regs)
@@ -322,12 +321,11 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
for (i = 0; i < config->pwr_reg_cnt; i++)
hdmi->pwr_regs[i].supply = config->pwr_reg_names[i];
- ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt, hdmi->pwr_regs);
+ ret = devm_regulator_bulk_get(dev, config->pwr_reg_cnt, hdmi->pwr_regs);
if (ret)
return dev_err_probe(dev, ret, "failed to get pwr regulators\n");
- hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
- config->pwr_clk_cnt,
+ hdmi->pwr_clks = devm_kcalloc(dev, config->pwr_clk_cnt,
sizeof(hdmi->pwr_clks[0]),
GFP_KERNEL);
if (!hdmi->pwr_clks)
@@ -336,16 +334,16 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
for (i = 0; i < config->pwr_clk_cnt; i++)
hdmi->pwr_clks[i].id = config->pwr_clk_names[i];
- ret = devm_clk_bulk_get(&pdev->dev, config->pwr_clk_cnt, hdmi->pwr_clks);
+ ret = devm_clk_bulk_get(dev, config->pwr_clk_cnt, hdmi->pwr_clks);
if (ret)
return ret;
- hdmi->extp_clk = devm_clk_get_optional(&pdev->dev, "extp");
+ hdmi->extp_clk = devm_clk_get_optional(dev, "extp");
if (IS_ERR(hdmi->extp_clk))
return dev_err_probe(dev, PTR_ERR(hdmi->extp_clk),
"failed to get extp clock\n");
- hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
+ hdmi->hpd_gpiod = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
/* This will catch e.g. -EPROBE_DEFER */
if (IS_ERR(hdmi->hpd_gpiod))
return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
@@ -361,13 +359,13 @@ static int msm_hdmi_dev_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = devm_pm_runtime_enable(&pdev->dev);
+ ret = devm_pm_runtime_enable(dev);
if (ret)
goto err_put_phy;
platform_set_drvdata(pdev, hdmi);
- ret = component_add(&pdev->dev, &msm_hdmi_ops);
+ ret = component_add(dev, &msm_hdmi_ops);
if (ret)
goto err_put_phy;
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/7] drm/msm/hdmi: Consistently use u32 instead of uint32_t
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
` (2 preceding siblings ...)
2026-03-11 20:17 ` [PATCH 3/7] drm/msm/hdmi: Simplify with local 'dev' variable Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 5/7] drm/msm/hdmi: Drop redundant 'int' for longs Krzysztof Kozlowski
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
Linux coding style asks to use kernel types like u32 instead of uint32_t
and code already has it in other places, so unify the remaining pieces.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_audio.c | 5 ++---
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++--
drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 4 ++--
drivers/gpu/drm/msm/hdmi/hdmi_i2c.c | 12 ++++++------
5 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 03aa29dbb2f5..06cb0247eb7e 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -20,7 +20,7 @@
void msm_hdmi_set_mode(struct hdmi *hdmi, bool power_on)
{
- uint32_t ctrl = 0;
+ u32 ctrl = 0;
unsigned long flags;
spin_lock_irqsave(&hdmi->reg_lock, flags);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_audio.c b/drivers/gpu/drm/msm/hdmi/hdmi_audio.c
index d9a8dc9dae8f..249c167ab04d 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_audio.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_audio.c
@@ -17,8 +17,7 @@ int msm_hdmi_audio_update(struct hdmi *hdmi)
{
struct hdmi_audio *audio = &hdmi->audio;
bool enabled = audio->enabled;
- uint32_t acr_pkt_ctrl, vbi_pkt_ctrl, aud_pkt_ctrl;
- uint32_t audio_config;
+ u32 acr_pkt_ctrl, vbi_pkt_ctrl, aud_pkt_ctrl, audio_config;
if (!hdmi->connector->display_info.is_hdmi)
return -EINVAL;
@@ -43,7 +42,7 @@ int msm_hdmi_audio_update(struct hdmi *hdmi)
acr_pkt_ctrl &= ~HDMI_ACR_PKT_CTRL_SELECT__MASK;
if (enabled) {
- uint32_t n, cts, multiplier;
+ u32 n, cts, multiplier;
enum hdmi_acr_cts select;
drm_hdmi_acr_get_n_cts(hdmi->pixclock, audio->rate, &n, &cts);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 9b7012692ece..a9eb6489c520 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -356,7 +356,7 @@ static void msm_hdmi_set_timings(struct hdmi *hdmi,
const struct drm_display_mode *mode)
{
int hstart, hend, vstart, vend;
- uint32_t frame_ctrl;
+ u32 frame_ctrl;
hstart = mode->htotal - mode->hsync_start;
hend = mode->htotal - mode->hsync_start + mode->hdisplay;
@@ -409,7 +409,7 @@ static const struct drm_edid *msm_hdmi_bridge_edid_read(struct drm_bridge *bridg
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
const struct drm_edid *drm_edid;
- uint32_t hdmi_ctrl;
+ u32 hdmi_ctrl;
hdmi_ctrl = hdmi_read(hdmi, REG_HDMI_CTRL);
hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl | HDMI_CTRL_ENABLE);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
index 114b0d507700..2cccd9062584 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
@@ -65,7 +65,7 @@ void msm_hdmi_hpd_enable(struct drm_bridge *bridge)
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
struct device *dev = &hdmi->pdev->dev;
- uint32_t hpd_ctrl;
+ u32 hpd_ctrl;
int ret;
unsigned long flags;
@@ -125,7 +125,7 @@ void msm_hdmi_hpd_irq(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
- uint32_t hpd_int_status, hpd_int_ctrl;
+ u32 hpd_int_status, hpd_int_ctrl;
/* Process HPD: */
hpd_int_status = hdmi_read(hdmi, REG_HDMI_HPD_INT_STATUS);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
index 6b9265159195..4a4a2cf90234 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_i2c.c
@@ -40,8 +40,8 @@ static int ddc_clear_irq(struct hdmi_i2c_adapter *hdmi_i2c)
{
struct hdmi *hdmi = hdmi_i2c->hdmi;
struct drm_device *dev = hdmi->dev;
- uint32_t retry = 0xffff;
- uint32_t ddc_int_ctrl;
+ u32 retry = 0xffff;
+ u32 ddc_int_ctrl;
do {
--retry;
@@ -71,7 +71,7 @@ static bool sw_done(struct hdmi_i2c_adapter *hdmi_i2c)
struct hdmi *hdmi = hdmi_i2c->hdmi;
if (!hdmi_i2c->sw_done) {
- uint32_t ddc_int_ctrl;
+ u32 ddc_int_ctrl;
ddc_int_ctrl = hdmi_read(hdmi, REG_HDMI_DDC_INT_CTRL);
@@ -92,13 +92,13 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
struct hdmi_i2c_adapter *hdmi_i2c = to_hdmi_i2c_adapter(i2c);
struct hdmi *hdmi = hdmi_i2c->hdmi;
struct drm_device *dev = hdmi->dev;
- static const uint32_t nack[] = {
+ static const u32 nack[] = {
HDMI_DDC_SW_STATUS_NACK0, HDMI_DDC_SW_STATUS_NACK1,
HDMI_DDC_SW_STATUS_NACK2, HDMI_DDC_SW_STATUS_NACK3,
};
int indices[MAX_TRANSACTIONS];
int ret, i, j, index = 0;
- uint32_t ddc_status, ddc_data, i2c_trans;
+ u32 ddc_status, ddc_data, i2c_trans;
num = min(num, MAX_TRANSACTIONS);
@@ -119,7 +119,7 @@ static int msm_hdmi_i2c_xfer(struct i2c_adapter *i2c,
for (i = 0; i < num; i++) {
struct i2c_msg *p = &msgs[i];
- uint32_t raw_addr = p->addr << 1;
+ u32 raw_addr = p->addr << 1;
if (p->flags & I2C_M_RD)
raw_addr |= 1;
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/msm/hdmi: Consistently use u32 instead of uint32_t
2026-03-11 20:17 ` [PATCH 4/7] drm/msm/hdmi: Consistently use u32 instead of uint32_t Krzysztof Kozlowski
@ 2026-03-11 20:45 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:45 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Standard kernel coding style cleanup. The changes are mechanical `uint32_t` → `u32` replacements across hdmi.c, hdmi_audio.c, hdmi_bridge.c, hdmi_hpd.c, and hdmi_i2c.c.
One minor note: in `hdmi_audio.c`, two separate declaration lines are merged into one:
```c
- uint32_t acr_pkt_ctrl, vbi_pkt_ctrl, aud_pkt_ctrl;
- uint32_t audio_config;
+ u32 acr_pkt_ctrl, vbi_pkt_ctrl, aud_pkt_ctrl, audio_config;
```
This is fine but slightly beyond the stated scope of "type changes only." Not a problem.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/7] drm/msm/hdmi: Drop redundant 'int' for longs
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
` (3 preceding siblings ...)
2026-03-11 20:17 ` [PATCH 4/7] drm/msm/hdmi: Consistently use u32 instead of uint32_t Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 6/7] drm/msm/hdmi_bridge: Simplify register bit updates Krzysztof Kozlowski
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
'long' type is already an integer, so 'int' is redundant.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.h | 6 +++---
drivers/gpu/drm/msm/hdmi/hdmi_phy.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c | 2 +-
drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 02cfd46df594..49433f7727c3 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -43,7 +43,7 @@ struct hdmi {
bool power_on;
bool hpd_enabled;
struct mutex state_mutex; /* protects two booleans */
- unsigned long int pixclock;
+ unsigned long pixclock;
void __iomem *mmio;
void __iomem *qfprom_mmio;
@@ -132,7 +132,7 @@ enum hdmi_phy_type {
struct hdmi_phy_cfg {
enum hdmi_phy_type type;
- void (*powerup)(struct hdmi_phy *phy, unsigned long int pixclock);
+ void (*powerup)(struct hdmi_phy *phy, unsigned long pixclock);
void (*powerdown)(struct hdmi_phy *phy);
const char * const *reg_names;
int num_regs;
@@ -167,7 +167,7 @@ static inline u32 hdmi_phy_read(struct hdmi_phy *phy, u32 reg)
int msm_hdmi_phy_resource_enable(struct hdmi_phy *phy);
void msm_hdmi_phy_resource_disable(struct hdmi_phy *phy);
-void msm_hdmi_phy_powerup(struct hdmi_phy *phy, unsigned long int pixclock);
+void msm_hdmi_phy_powerup(struct hdmi_phy *phy, unsigned long pixclock);
void msm_hdmi_phy_powerdown(struct hdmi_phy *phy);
void __init msm_hdmi_phy_driver_register(void);
void __exit msm_hdmi_phy_driver_unregister(void);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
index 667573f1db7c..73fd5a47022d 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy.c
@@ -94,7 +94,7 @@ void msm_hdmi_phy_resource_disable(struct hdmi_phy *phy)
pm_runtime_put_sync(dev);
}
-void msm_hdmi_phy_powerup(struct hdmi_phy *phy, unsigned long int pixclock)
+void msm_hdmi_phy_powerup(struct hdmi_phy *phy, unsigned long pixclock)
{
if (!phy || !phy->cfg->powerup)
return;
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c
index cf90a0c1f822..cfa8fc494199 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8960.c
@@ -7,7 +7,7 @@
#include "hdmi.h"
static void hdmi_phy_8960_powerup(struct hdmi_phy *phy,
- unsigned long int pixclock)
+ unsigned long pixclock)
{
DBG("pixclock: %lu", pixclock);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c
index 1d97640d8c24..10ee91818364 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x60.c
@@ -9,7 +9,7 @@
#include "hdmi.h"
static void hdmi_phy_8x60_powerup(struct hdmi_phy *phy,
- unsigned long int pixclock)
+ unsigned long pixclock)
{
/* De-serializer delay D/C for non-lbk mode: */
hdmi_phy_write(phy, REG_HDMI_8x60_PHY_REG0,
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c
index a2a6940e195a..6f40820d9071 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_phy_8x74.c
@@ -7,7 +7,7 @@
#include "hdmi.h"
static void hdmi_phy_8x74_powerup(struct hdmi_phy *phy,
- unsigned long int pixclock)
+ unsigned long pixclock)
{
hdmi_phy_write(phy, REG_HDMI_8x74_ANA_CFG0, 0x1b);
hdmi_phy_write(phy, REG_HDMI_8x74_ANA_CFG1, 0xf2);
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 6/7] drm/msm/hdmi_bridge: Simplify register bit updates
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
` (4 preceding siblings ...)
2026-03-11 20:17 ` [PATCH 5/7] drm/msm/hdmi: Drop redundant 'int' for longs Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:17 ` [PATCH 7/7] drm/msm/hdmi_hdcp: " Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: drm/msm/hdmi: Simplify the code Claude Code Review Bot
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
Simplify reister updates (read, apply mask, write) with a wrapper to
make code more obvious and avoid possible errors of reading and writing
to different registers.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi.h | 19 +++++++
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 98 ++++++++++++++--------------------
2 files changed, 60 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index 49433f7727c3..436d4f9fe346 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -112,6 +112,25 @@ static inline u32 hdmi_read(struct hdmi *hdmi, u32 reg)
return readl(hdmi->mmio + reg);
}
+static inline void hdmi_clear_bits(struct hdmi *hdmi, u32 reg, u32 mask)
+{
+ u32 val;
+
+ val = hdmi_read(hdmi, reg);
+ val &= ~mask;
+ hdmi_write(hdmi, reg, val);
+}
+
+static inline void hdmi_update_bits(struct hdmi *hdmi, u32 reg, u32 mask, u32 data)
+{
+ u32 val;
+
+ val = hdmi_read(hdmi, reg);
+ val &= ~mask;
+ val |= data & mask;
+ hdmi_write(hdmi, reg, val);
+}
+
static inline u32 hdmi_qfprom_read(struct hdmi *hdmi, u32 reg)
{
return readl(hdmi->qfprom_mmio + reg);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index a9eb6489c520..b6ca334fb9fe 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -58,16 +58,13 @@ static int msm_hdmi_bridge_clear_avi_infoframe(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
- u32 val;
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
- val &= ~(HDMI_INFOFRAME_CTRL0_AVI_SEND |
- HDMI_INFOFRAME_CTRL0_AVI_CONT);
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_INFOFRAME_CTRL0,
+ HDMI_INFOFRAME_CTRL0_AVI_SEND |
+ HDMI_INFOFRAME_CTRL0_AVI_CONT);
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL1);
- val &= ~HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE__MASK;
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_INFOFRAME_CTRL1,
+ HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE__MASK);
return 0;
}
@@ -76,18 +73,15 @@ static int msm_hdmi_bridge_clear_audio_infoframe(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
- u32 val;
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
- val &= ~(HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE);
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_INFOFRAME_CTRL0,
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE);
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL1);
- val &= ~HDMI_INFOFRAME_CTRL1_AUDIO_INFO_LINE__MASK;
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_INFOFRAME_CTRL1,
+ HDMI_INFOFRAME_CTRL1_AUDIO_INFO_LINE__MASK);
return 0;
}
@@ -96,13 +90,11 @@ static int msm_hdmi_bridge_clear_spd_infoframe(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
- u32 val;
- val = hdmi_read(hdmi, REG_HDMI_GEN_PKT_CTRL);
- val &= ~(HDMI_GEN_PKT_CTRL_GENERIC1_SEND |
- HDMI_GEN_PKT_CTRL_GENERIC1_CONT |
- HDMI_GEN_PKT_CTRL_GENERIC1_LINE__MASK);
- hdmi_write(hdmi, REG_HDMI_GEN_PKT_CTRL, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_GEN_PKT_CTRL,
+ HDMI_GEN_PKT_CTRL_GENERIC1_SEND |
+ HDMI_GEN_PKT_CTRL_GENERIC1_CONT |
+ HDMI_GEN_PKT_CTRL_GENERIC1_LINE__MASK);
return 0;
}
@@ -111,14 +103,12 @@ static int msm_hdmi_bridge_clear_hdmi_infoframe(struct drm_bridge *bridge)
{
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
- u32 val;
- val = hdmi_read(hdmi, REG_HDMI_GEN_PKT_CTRL);
- val &= ~(HDMI_GEN_PKT_CTRL_GENERIC0_SEND |
- HDMI_GEN_PKT_CTRL_GENERIC0_CONT |
- HDMI_GEN_PKT_CTRL_GENERIC0_UPDATE |
- HDMI_GEN_PKT_CTRL_GENERIC0_LINE__MASK);
- hdmi_write(hdmi, REG_HDMI_GEN_PKT_CTRL, val);
+ hdmi_clear_bits(hdmi, REG_HDMI_GEN_PKT_CTRL,
+ HDMI_GEN_PKT_CTRL_GENERIC0_SEND |
+ HDMI_GEN_PKT_CTRL_GENERIC0_CONT |
+ HDMI_GEN_PKT_CTRL_GENERIC0_UPDATE |
+ HDMI_GEN_PKT_CTRL_GENERIC0_LINE__MASK);
return 0;
}
@@ -129,7 +119,6 @@ static int msm_hdmi_bridge_write_avi_infoframe(struct drm_bridge *bridge,
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
struct hdmi *hdmi = hdmi_bridge->hdmi;
u32 buf[4] = {};
- u32 val;
int i;
if (len != HDMI_INFOFRAME_SIZE(AVI) || len - 3 > sizeof(buf)) {
@@ -153,15 +142,13 @@ static int msm_hdmi_bridge_write_avi_infoframe(struct drm_bridge *bridge,
for (i = 0; i < ARRAY_SIZE(buf); i++)
hdmi_write(hdmi, REG_HDMI_AVI_INFO(i), buf[i]);
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
- val |= HDMI_INFOFRAME_CTRL0_AVI_SEND |
- HDMI_INFOFRAME_CTRL0_AVI_CONT;
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
+ hdmi_update_bits(hdmi, REG_HDMI_INFOFRAME_CTRL0,
+ HDMI_INFOFRAME_CTRL0_AVI_SEND | HDMI_INFOFRAME_CTRL0_AVI_CONT,
+ HDMI_INFOFRAME_CTRL0_AVI_SEND | HDMI_INFOFRAME_CTRL0_AVI_CONT);
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL1);
- val &= ~HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE__MASK;
- val |= HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE(AVI_IFRAME_LINE_NUMBER);
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL1, val);
+ hdmi_update_bits(hdmi, REG_HDMI_INFOFRAME_CTRL1,
+ HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE__MASK,
+ HDMI_INFOFRAME_CTRL1_AVI_INFO_LINE(AVI_IFRAME_LINE_NUMBER));
return 0;
}
@@ -193,12 +180,11 @@ static int msm_hdmi_bridge_write_audio_infoframe(struct drm_bridge *bridge,
buffer[9] << 16 |
buffer[10] << 24);
- val = hdmi_read(hdmi, REG_HDMI_INFOFRAME_CTRL0);
- val |= HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
- HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE;
- hdmi_write(hdmi, REG_HDMI_INFOFRAME_CTRL0, val);
+ val = HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SEND |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_CONT |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_SOURCE |
+ HDMI_INFOFRAME_CTRL0_AUDIO_INFO_UPDATE;
+ hdmi_update_bits(hdmi, REG_HDMI_INFOFRAME_CTRL0, val, val);
return 0;
}
@@ -231,11 +217,10 @@ static int msm_hdmi_bridge_write_spd_infoframe(struct drm_bridge *bridge,
for (i = 0; i < ARRAY_SIZE(buf); i++)
hdmi_write(hdmi, REG_HDMI_GENERIC1(i), buf[i]);
- val = hdmi_read(hdmi, REG_HDMI_GEN_PKT_CTRL);
- val |= HDMI_GEN_PKT_CTRL_GENERIC1_SEND |
- HDMI_GEN_PKT_CTRL_GENERIC1_CONT |
- HDMI_GEN_PKT_CTRL_GENERIC1_LINE(SPD_IFRAME_LINE_NUMBER);
- hdmi_write(hdmi, REG_HDMI_GEN_PKT_CTRL, val);
+ val = HDMI_GEN_PKT_CTRL_GENERIC1_SEND |
+ HDMI_GEN_PKT_CTRL_GENERIC1_CONT |
+ HDMI_GEN_PKT_CTRL_GENERIC1_LINE(SPD_IFRAME_LINE_NUMBER);
+ hdmi_update_bits(hdmi, REG_HDMI_GEN_PKT_CTRL, val, val);
return 0;
}
@@ -269,12 +254,11 @@ static int msm_hdmi_bridge_write_hdmi_infoframe(struct drm_bridge *bridge,
for (i = 0; i < ARRAY_SIZE(buf); i++)
hdmi_write(hdmi, REG_HDMI_GENERIC0(i), buf[i]);
- val = hdmi_read(hdmi, REG_HDMI_GEN_PKT_CTRL);
- val |= HDMI_GEN_PKT_CTRL_GENERIC0_SEND |
- HDMI_GEN_PKT_CTRL_GENERIC0_CONT |
- HDMI_GEN_PKT_CTRL_GENERIC0_UPDATE |
- HDMI_GEN_PKT_CTRL_GENERIC0_LINE(VENSPEC_IFRAME_LINE_NUMBER);
- hdmi_write(hdmi, REG_HDMI_GEN_PKT_CTRL, val);
+ val = HDMI_GEN_PKT_CTRL_GENERIC0_SEND |
+ HDMI_GEN_PKT_CTRL_GENERIC0_CONT |
+ HDMI_GEN_PKT_CTRL_GENERIC0_UPDATE |
+ HDMI_GEN_PKT_CTRL_GENERIC0_LINE(VENSPEC_IFRAME_LINE_NUMBER);
+ hdmi_update_bits(hdmi, REG_HDMI_GEN_PKT_CTRL, val, val);
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/msm/hdmi_bridge: Simplify register bit updates
2026-03-11 20:17 ` [PATCH 6/7] drm/msm/hdmi_bridge: Simplify register bit updates Krzysztof Kozlowski
@ 2026-03-11 20:45 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:45 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This patch introduces two new helpers in `hdmi.h`:
```c
static inline void hdmi_clear_bits(struct hdmi *hdmi, u32 reg, u32 mask)
static inline void hdmi_update_bits(struct hdmi *hdmi, u32 reg, u32 mask, u32 data)
```
The `hdmi_update_bits` correctly applies `val |= data & mask`, ensuring only bits within the mask are modified. The conversions in `hdmi_bridge.c` are all correct.
One observation: in the "set bits" cases (e.g., SPD and HDMI infoframe writes), the pattern `hdmi_update_bits(hdmi, reg, val, val)` is used where mask == data. This is actually *slightly better* than the original `|=` pattern because it clears the `_LINE` field before setting the new value, whereas the original code relied on the field being zero before the `|=`. In the clear functions that precede these writes, the LINE field would already be zeroed, so it's not a bug fix, but the new code is more defensive.
Minor nit: the commit message has a typo — "reister" should be "register." This also appears in patch 7.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 7/7] drm/msm/hdmi_hdcp: Simplify register bit updates
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
` (5 preceding siblings ...)
2026-03-11 20:17 ` [PATCH 6/7] drm/msm/hdmi_bridge: Simplify register bit updates Krzysztof Kozlowski
@ 2026-03-11 20:17 ` Krzysztof Kozlowski
2026-03-11 20:45 ` Claude review: " Claude Code Review Bot
2026-03-11 20:45 ` Claude review: drm/msm/hdmi: Simplify the code Claude Code Review Bot
7 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-11 20:17 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
Krzysztof Kozlowski
Simplify reister updates (read, apply mask, write) with a wrapper to
make code more obvious and avoid possible errors of reading and writing
to different registers.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c | 85 ++++++++++++------------------------
1 file changed, 28 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c b/drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c
index 8fb5497aac9f..7862bd67d154 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c
@@ -306,9 +306,9 @@ static int msm_reset_hdcp_ddc_failures(struct hdmi_hdcp_ctrl *hdcp_ctrl)
HDMI_HDCP_DDC_CTRL_0_DISABLE);
/* ACK the Failure to Clear it */
- reg_val = hdmi_read(hdmi, REG_HDMI_HDCP_DDC_CTRL_1);
- reg_val |= HDMI_HDCP_DDC_CTRL_1_FAILED_ACK;
- hdmi_write(hdmi, REG_HDMI_HDCP_DDC_CTRL_1, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_HDCP_DDC_CTRL_1,
+ HDMI_HDCP_DDC_CTRL_1_FAILED_ACK,
+ HDMI_HDCP_DDC_CTRL_1_FAILED_ACK);
/* Check if the FAILURE got Cleared */
reg_val = hdmi_read(hdmi, REG_HDMI_HDCP_DDC_STATUS);
@@ -324,28 +324,22 @@ static int msm_reset_hdcp_ddc_failures(struct hdmi_hdcp_ctrl *hdcp_ctrl)
DBG("Before: HDMI_DDC_SW_STATUS=0x%08x",
hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS));
/* Reset HDMI DDC software status */
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_CTRL);
- reg_val |= HDMI_DDC_CTRL_SW_STATUS_RESET;
- hdmi_write(hdmi, REG_HDMI_DDC_CTRL, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_DDC_CTRL, HDMI_DDC_CTRL_SW_STATUS_RESET,
+ HDMI_DDC_CTRL_SW_STATUS_RESET);
rc = msm_hdmi_hdcp_msleep(hdcp_ctrl, 20, AUTH_ABORT_EV);
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_CTRL);
- reg_val &= ~HDMI_DDC_CTRL_SW_STATUS_RESET;
- hdmi_write(hdmi, REG_HDMI_DDC_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_DDC_CTRL, HDMI_DDC_CTRL_SW_STATUS_RESET);
/* Reset HDMI DDC Controller */
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_CTRL);
- reg_val |= HDMI_DDC_CTRL_SOFT_RESET;
- hdmi_write(hdmi, REG_HDMI_DDC_CTRL, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_DDC_CTRL, HDMI_DDC_CTRL_SOFT_RESET,
+ HDMI_DDC_CTRL_SOFT_RESET);
/* If previous msleep is aborted, skip this msleep */
if (!rc)
rc = msm_hdmi_hdcp_msleep(hdcp_ctrl, 20, AUTH_ABORT_EV);
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_CTRL);
- reg_val &= ~HDMI_DDC_CTRL_SOFT_RESET;
- hdmi_write(hdmi, REG_HDMI_DDC_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_DDC_CTRL, HDMI_DDC_CTRL_SOFT_RESET);
DBG("After: HDMI_DDC_SW_STATUS=0x%08x",
hdmi_read(hdmi, REG_HDMI_DDC_SW_STATUS));
}
@@ -399,7 +393,6 @@ static void msm_hdmi_hdcp_reauth_work(struct work_struct *work)
struct hdmi_hdcp_ctrl, hdcp_reauth_work);
struct hdmi *hdmi = hdcp_ctrl->hdmi;
unsigned long flags;
- u32 reg_val;
DBG("HDCP REAUTH WORK");
/*
@@ -409,9 +402,7 @@ static void msm_hdmi_hdcp_reauth_work(struct work_struct *work)
* AN1_READY bits in HDMI_HDCP_LINK0_STATUS register
*/
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_HPD_CTRL);
- reg_val &= ~HDMI_HPD_CTRL_ENABLE;
- hdmi_write(hdmi, REG_HDMI_HPD_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_HPD_CTRL, HDMI_HPD_CTRL_ENABLE);
/* Disable HDCP interrupts */
hdmi_write(hdmi, REG_HDMI_HDCP_INT_CTRL, 0);
@@ -431,9 +422,8 @@ static void msm_hdmi_hdcp_reauth_work(struct work_struct *work)
/* Enable HPD circuitry */
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_HPD_CTRL);
- reg_val |= HDMI_HPD_CTRL_ENABLE;
- hdmi_write(hdmi, REG_HDMI_HPD_CTRL, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_HPD_CTRL, HDMI_HPD_CTRL_ENABLE,
+ HDMI_HPD_CTRL_ENABLE);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
/*
@@ -456,7 +446,6 @@ static int msm_hdmi_hdcp_auth_prepare(struct hdmi_hdcp_ctrl *hdcp_ctrl)
{
struct hdmi *hdmi = hdcp_ctrl->hdmi;
u32 link0_status;
- u32 reg_val;
unsigned long flags;
int rc;
@@ -472,14 +461,11 @@ static int msm_hdmi_hdcp_auth_prepare(struct hdmi_hdcp_ctrl *hdcp_ctrl)
spin_lock_irqsave(&hdmi->reg_lock, flags);
/* disable HDMI Encrypt */
- reg_val = hdmi_read(hdmi, REG_HDMI_CTRL);
- reg_val &= ~HDMI_CTRL_ENCRYPTED;
- hdmi_write(hdmi, REG_HDMI_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_CTRL, HDMI_CTRL_ENCRYPTED);
/* Enabling Software DDC */
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_ARBITRATION);
- reg_val &= ~HDMI_DDC_ARBITRATION_HW_ARBITRATION;
- hdmi_write(hdmi, REG_HDMI_DDC_ARBITRATION, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_DDC_ARBITRATION,
+ HDMI_DDC_ARBITRATION_HW_ARBITRATION);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
/*
@@ -498,9 +484,8 @@ static int msm_hdmi_hdcp_auth_prepare(struct hdmi_hdcp_ctrl *hdcp_ctrl)
hdmi_write(hdmi, REG_HDMI_HDCP_ENTROPY_CTRL1, 0xF00DFACE);
/* Disable the RngCipher state */
- reg_val = hdmi_read(hdmi, REG_HDMI_HDCP_DEBUG_CTRL);
- reg_val &= ~HDMI_HDCP_DEBUG_CTRL_RNG_CIPHER;
- hdmi_write(hdmi, REG_HDMI_HDCP_DEBUG_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_HDCP_DEBUG_CTRL,
+ HDMI_HDCP_DEBUG_CTRL_RNG_CIPHER);
DBG("HDCP_DEBUG_CTRL=0x%08x",
hdmi_read(hdmi, REG_HDMI_HDCP_DEBUG_CTRL));
@@ -537,15 +522,12 @@ static int msm_hdmi_hdcp_auth_prepare(struct hdmi_hdcp_ctrl *hdcp_ctrl)
static void msm_hdmi_hdcp_auth_fail(struct hdmi_hdcp_ctrl *hdcp_ctrl)
{
struct hdmi *hdmi = hdcp_ctrl->hdmi;
- u32 reg_val;
unsigned long flags;
DBG("hdcp auth failed, queue reauth work");
/* clear HDMI Encrypt */
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_CTRL);
- reg_val &= ~HDMI_CTRL_ENCRYPTED;
- hdmi_write(hdmi, REG_HDMI_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_CTRL, HDMI_CTRL_ENCRYPTED);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
hdcp_ctrl->hdcp_state = HDCP_STATE_AUTH_FAILED;
@@ -555,7 +537,6 @@ static void msm_hdmi_hdcp_auth_fail(struct hdmi_hdcp_ctrl *hdcp_ctrl)
static void msm_hdmi_hdcp_auth_done(struct hdmi_hdcp_ctrl *hdcp_ctrl)
{
struct hdmi *hdmi = hdcp_ctrl->hdmi;
- u32 reg_val;
unsigned long flags;
/*
@@ -563,16 +544,15 @@ static void msm_hdmi_hdcp_auth_done(struct hdmi_hdcp_ctrl *hdcp_ctrl)
* there is no Arbitration between software and hardware for DDC
*/
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_DDC_ARBITRATION);
- reg_val |= HDMI_DDC_ARBITRATION_HW_ARBITRATION;
- hdmi_write(hdmi, REG_HDMI_DDC_ARBITRATION, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_DDC_ARBITRATION,
+ HDMI_DDC_ARBITRATION_HW_ARBITRATION,
+ HDMI_DDC_ARBITRATION_HW_ARBITRATION);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
/* enable HDMI Encrypt */
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_CTRL);
- reg_val |= HDMI_CTRL_ENCRYPTED;
- hdmi_write(hdmi, REG_HDMI_CTRL, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_CTRL, HDMI_CTRL_ENCRYPTED,
+ HDMI_CTRL_ENCRYPTED);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
hdcp_ctrl->hdcp_state = HDCP_STATE_AUTHENTICATED;
@@ -1304,7 +1284,6 @@ static void msm_hdmi_hdcp_auth_work(struct work_struct *work)
void msm_hdmi_hdcp_on(struct hdmi_hdcp_ctrl *hdcp_ctrl)
{
struct hdmi *hdmi = hdcp_ctrl->hdmi;
- u32 reg_val;
unsigned long flags;
if ((HDCP_STATE_INACTIVE != hdcp_ctrl->hdcp_state) ||
@@ -1315,9 +1294,7 @@ void msm_hdmi_hdcp_on(struct hdmi_hdcp_ctrl *hdcp_ctrl)
/* clear HDMI Encrypt */
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_CTRL);
- reg_val &= ~HDMI_CTRL_ENCRYPTED;
- hdmi_write(hdmi, REG_HDMI_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_CTRL, HDMI_CTRL_ENCRYPTED);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
hdcp_ctrl->auth_event = 0;
@@ -1330,7 +1307,6 @@ void msm_hdmi_hdcp_off(struct hdmi_hdcp_ctrl *hdcp_ctrl)
{
struct hdmi *hdmi = hdcp_ctrl->hdmi;
unsigned long flags;
- u32 reg_val;
if ((HDCP_STATE_INACTIVE == hdcp_ctrl->hdcp_state) ||
(HDCP_STATE_NO_AKSV == hdcp_ctrl->hdcp_state)) {
@@ -1345,9 +1321,7 @@ void msm_hdmi_hdcp_off(struct hdmi_hdcp_ctrl *hdcp_ctrl)
* AN1_READY bits in HDMI_HDCP_LINK0_STATUS register
*/
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_HPD_CTRL);
- reg_val &= ~HDMI_HPD_CTRL_ENABLE;
- hdmi_write(hdmi, REG_HDMI_HPD_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_HPD_CTRL, HDMI_HPD_CTRL_ENABLE);
/*
* Disable HDCP interrupts.
@@ -1375,14 +1349,11 @@ void msm_hdmi_hdcp_off(struct hdmi_hdcp_ctrl *hdcp_ctrl)
hdmi_write(hdmi, REG_HDMI_HDCP_CTRL, 0);
spin_lock_irqsave(&hdmi->reg_lock, flags);
- reg_val = hdmi_read(hdmi, REG_HDMI_CTRL);
- reg_val &= ~HDMI_CTRL_ENCRYPTED;
- hdmi_write(hdmi, REG_HDMI_CTRL, reg_val);
+ hdmi_clear_bits(hdmi, REG_HDMI_CTRL, HDMI_CTRL_ENCRYPTED);
/* Enable HPD circuitry */
- reg_val = hdmi_read(hdmi, REG_HDMI_HPD_CTRL);
- reg_val |= HDMI_HPD_CTRL_ENABLE;
- hdmi_write(hdmi, REG_HDMI_HPD_CTRL, reg_val);
+ hdmi_update_bits(hdmi, REG_HDMI_HPD_CTRL, HDMI_HPD_CTRL_ENABLE,
+ HDMI_HPD_CTRL_ENABLE);
spin_unlock_irqrestore(&hdmi->reg_lock, flags);
hdcp_ctrl->hdcp_state = HDCP_STATE_INACTIVE;
--
2.51.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Claude review: drm/msm/hdmi_hdcp: Simplify register bit updates
2026-03-11 20:17 ` [PATCH 7/7] drm/msm/hdmi_hdcp: " Krzysztof Kozlowski
@ 2026-03-11 20:45 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:45 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Continues the same pattern from patch 6, applying `hdmi_clear_bits()` and `hdmi_update_bits()` to `hdmi_hdcp.c`. All conversions are straightforward and correct.
Same typo in the commit message: "reister" → "register."
**Thread safety note**: The HDCP code uses these helpers within `spin_lock_irqsave`/`spin_unlock_irqrestore` regions (e.g., around `REG_HDMI_CTRL` and `REG_HDMI_HPD_CTRL` accesses). The helpers perform read-modify-write via `readl`/`writel`, which is safe under the existing spinlock. The refactoring preserves the locking boundaries correctly.
No functional issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: drm/msm/hdmi: Simplify the code
2026-03-11 20:17 [PATCH 0/7] drm/msm/hdmi: Simplify the code Krzysztof Kozlowski
` (6 preceding siblings ...)
2026-03-11 20:17 ` [PATCH 7/7] drm/msm/hdmi_hdcp: " Krzysztof Kozlowski
@ 2026-03-11 20:45 ` Claude Code Review Bot
7 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-11 20:45 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/msm/hdmi: Simplify the code
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Patches: 8
Reviewed: 2026-03-12T06:45:26.014914
---
This is a straightforward 7-patch cleanup series for the MSM HDMI driver from Krzysztof Kozlowski. All patches are purely cosmetic/style improvements with no functional changes. The series is well-organized with each patch addressing a distinct cleanup concern: using `dev_of_node()`, removing duplicate error messages, introducing a local `dev` variable, switching to kernel-preferred `u32` types, removing redundant `int` from `unsigned long int`, and introducing `hdmi_clear_bits()`/`hdmi_update_bits()` helpers.
The series is clean and ready to merge. One minor nit on the cover letter and one subtle improvement worth noting are called out below.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread