[PATCH 0/7] drm/msm/hdmi: Simplify the code

[PATCH 0/7] drm/msm/hdmi: Simplify the code

[Krzysztof Kozlowski]

Few code simplifications, without functional impact.

Context based (dependency) on:
https://lore.kernel.org/all/20260311191620.245394-2-krzysztof.kozlowski@oss.qualcomm.com/

Best regards,
Krzysztof

---
Krzysztof Kozlowski (7):
      drm/msm/hdmi: Simplify with dev_of_node()
      drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure
      drm/msm/hdmi: Simplify with local 'dev' variable
      drm/msm/hdmi: Consistently use u32 instead of uint32_t
      drm/msm/hdmi: Drop redundant 'int' for longs
      drm/msm/hdmi_bridge: Simplify register bit updates
      drm/msm/hdmi_hdcp: Simplify register bit updates

 drivers/gpu/drm/msm/hdmi/hdmi.c          |  32 +++++-----
 drivers/gpu/drm/msm/hdmi/hdmi.h          |  25 +++++++-
 drivers/gpu/drm/msm/hdmi/hdmi_audio.c    |   5 +-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c   | 102 +++++++++++++------------------
 drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c     |  85 +++++++++-----------------
 drivers/gpu/drm/msm/hdmi/hdmi_hpd.c      |   4 +-
 drivers/gpu/drm/msm/hdmi/hdmi_i2c.c      |  12 ++--
 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 +-
 11 files changed, 121 insertions(+), 152 deletions(-)
---
base-commit: 5138081b838d92e1bfcddc7b72b9215cca6e83f0
change-id: 20260311-drm-msm-hdmi-cleanup-035d0ca56c9f
prerequisite-message-id: 20260311191620.245394-2-krzysztof.kozlowski@oss.qualcomm.com
prerequisite-patch-id: 2720d6338c556d78897568eba7c7f3a457f690f4

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

[PATCH 1/7] drm/msm/hdmi: Simplify with dev_of_node()

[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

[PATCH 2/7] drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure

[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

[PATCH 3/7] drm/msm/hdmi: Simplify with local 'dev' variable

[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

[PATCH 4/7] drm/msm/hdmi: Consistently use u32 instead of uint32_t

[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

[PATCH 5/7] drm/msm/hdmi: Drop redundant 'int' for longs

[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

[PATCH 6/7] drm/msm/hdmi_bridge: Simplify register bit updates

[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

[PATCH 7/7] drm/msm/hdmi_hdcp: Simplify register bit updates

[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

Claude review: drm/msm/hdmi: Simplify the code

[Claude Code Review Bot]

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

Claude review: drm/msm/hdmi: Simplify with dev_of_node()

[Claude Code Review Bot]

Patch Review

Straightforward replacement of `pdev->dev.of_node` with `dev_of_node(&pdev->dev)` / `dev_of_node(dev)`. The `dev_of_node()` wrapper additionally provides a NULL check on the `dev` argument, which is a minor safety improvement.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/hdmi: Avoid double error print on msm_hdmi_get_phy() failure

[Claude Code Review Bot]

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

Claude review: drm/msm/hdmi: Simplify with local 'dev' variable

[Claude Code Review Bot]

Patch Review

The function already had `struct device *dev = &pdev->dev;` declared at the top, so replacing the remaining `&pdev->dev` references with `dev` is mechanical and correct.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/hdmi: Consistently use u32 instead of uint32_t

[Claude Code Review Bot]

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

Claude review: drm/msm/hdmi: Drop redundant 'int' for longs

[Claude Code Review Bot]

Patch Review

Correct. `unsigned long int` and `unsigned long` are identical types in C. The changes across hdmi.h, hdmi_phy.c, and the three phy variant files are consistent.

Note: the patch also fixes alignment in `hdmi_phy_8x60.c` and `hdmi_phy_8x74.c` (aligning the `pixclock` parameter to match the opening paren), which is a minor bonus cleanup.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/msm/hdmi_bridge: Simplify register bit updates

[Claude Code Review Bot]

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

Claude review: drm/msm/hdmi_hdcp: Simplify register bit updates

[Claude Code Review Bot]

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