The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/4] drm/msm/dsi: Minor cleanups
@ 2025-01-06  8:49 Krzysztof Kozlowski
  2025-01-06  8:49 ` [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths Krzysztof Kozlowski
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-06  8:49 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Krzysztof Kozlowski

Few minor improvements/cleanups why browsing the code.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (4):
      drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
      drm/msm/dsi: Simplify with dev_err_probe()
      drm/msm/dsi: Minor whitespace and style cleanup
      drm/msm/dsi: Drop unnecessary -ENOMEM message

 drivers/gpu/drm/msm/dsi/dsi_host.c | 158 ++++++++++++++++---------------------
 1 file changed, 68 insertions(+), 90 deletions(-)
---
base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2
change-id: 20250106-drm-msm-cleanups-ddacf1fc3ba5

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
  2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
@ 2025-01-06  8:49 ` Krzysztof Kozlowski
  2025-01-07  2:43   ` Abhinav Kumar
  2025-01-06  8:49 ` [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe() Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-06  8:49 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Krzysztof Kozlowski

dsi_clk_init(), which gets the clocks, is called only through platform
driver probe and its failure is a failure of the probe.  Therefore
NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
value stored there won't be used/dereferenced afterwards.  What's more,
variant-specific clock init calls like dsi_clk_init_6g_v2() are not
doing this cleanup.  Dropping redundant code allows later to make this a
bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a98d24b7cb00b41d3bb371a965a80ceaa93775a6..86ac145076416fa7651d18820266a00d28e44b6f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -292,7 +292,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->byte_clk);
 		pr_err("%s: can't find dsi_byte clock. ret=%d\n",
 			__func__, ret);
-		msm_host->byte_clk = NULL;
 		goto exit;
 	}
 
@@ -301,7 +300,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->pixel_clk);
 		pr_err("%s: can't find dsi_pixel clock. ret=%d\n",
 			__func__, ret);
-		msm_host->pixel_clk = NULL;
 		goto exit;
 	}
 
@@ -310,7 +308,6 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 		ret = PTR_ERR(msm_host->esc_clk);
 		pr_err("%s: can't find dsi_esc clock. ret=%d\n",
 			__func__, ret);
-		msm_host->esc_clk = NULL;
 		goto exit;
 	}
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe()
  2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
  2025-01-06  8:49 ` [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths Krzysztof Kozlowski
@ 2025-01-06  8:49 ` Krzysztof Kozlowski
  2025-01-07  2:56   ` Abhinav Kumar
  2025-01-06  8:49 ` [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-06  8:49 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Krzysztof Kozlowski

dsi_clk_init() and msm_dsi_host_init() are called only from platform
driver probe function, so using dev_err_probe is both appropriate and
beneficial:
 - Properly marks device deferred probe status,
 - Avoids dmesg flood on probe deferrals,
 - Already incorporates printing ERR value,
 - Shows device name (in contrast to pr_err()),
 - Makes code smaller and simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 100 +++++++++++++++----------------------
 1 file changed, 41 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 86ac145076416fa7651d18820266a00d28e44b6f..4a2ad04eea7359545a088bdc63907f6b3e5615bd 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -200,7 +200,8 @@ static const struct msm_dsi_cfg_handler *dsi_get_config(
 
 	ahb_clk = msm_clk_get(msm_host->pdev, "iface");
 	if (IS_ERR(ahb_clk)) {
-		pr_err("%s: cannot get interface clock\n", __func__);
+		dev_err_probe(dev, PTR_ERR(ahb_clk), "%s: cannot get interface clock\n",
+			      __func__);
 		goto exit;
 	}
 
@@ -208,13 +209,13 @@ static const struct msm_dsi_cfg_handler *dsi_get_config(
 
 	ret = clk_prepare_enable(ahb_clk);
 	if (ret) {
-		pr_err("%s: unable to enable ahb_clk\n", __func__);
+		dev_err_probe(dev, ret, "%s: unable to enable ahb_clk\n", __func__);
 		goto runtime_put;
 	}
 
 	ret = dsi_get_version(msm_host->ctrl_base, &major, &minor);
 	if (ret) {
-		pr_err("%s: Invalid version\n", __func__);
+		dev_err_probe(dev, ret, "%s: Invalid version\n", __func__);
 		goto disable_clks;
 	}
 
@@ -281,39 +282,31 @@ static int dsi_clk_init(struct msm_dsi_host *msm_host)
 	msm_host->num_bus_clks = cfg->num_bus_clks;
 
 	ret = devm_clk_bulk_get(&pdev->dev, msm_host->num_bus_clks, msm_host->bus_clks);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Unable to get clocks, ret = %d\n", ret);
-		goto exit;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "Unable to get clocks\n");
 
 	/* get link and source clocks */
 	msm_host->byte_clk = msm_clk_get(pdev, "byte");
-	if (IS_ERR(msm_host->byte_clk)) {
-		ret = PTR_ERR(msm_host->byte_clk);
-		pr_err("%s: can't find dsi_byte clock. ret=%d\n",
-			__func__, ret);
-		goto exit;
-	}
+	if (IS_ERR(msm_host->byte_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->byte_clk),
+				     "%s: can't find dsi_byte clock\n",
+				     __func__);
 
 	msm_host->pixel_clk = msm_clk_get(pdev, "pixel");
-	if (IS_ERR(msm_host->pixel_clk)) {
-		ret = PTR_ERR(msm_host->pixel_clk);
-		pr_err("%s: can't find dsi_pixel clock. ret=%d\n",
-			__func__, ret);
-		goto exit;
-	}
+	if (IS_ERR(msm_host->pixel_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->pixel_clk),
+				     "%s: can't find dsi_pixel clock\n",
+				     __func__);
 
 	msm_host->esc_clk = msm_clk_get(pdev, "core");
-	if (IS_ERR(msm_host->esc_clk)) {
-		ret = PTR_ERR(msm_host->esc_clk);
-		pr_err("%s: can't find dsi_esc clock. ret=%d\n",
-			__func__, ret);
-		goto exit;
-	}
+	if (IS_ERR(msm_host->esc_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->esc_clk),
+				     "%s: can't find dsi_esc clock\n",
+				     __func__);
 
 	if (cfg_hnd->ops->clk_init_ver)
 		ret = cfg_hnd->ops->clk_init_ver(msm_host);
-exit:
+
 	return ret;
 }
 
@@ -1879,31 +1872,28 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 	msm_dsi->host = &msm_host->base;
 
 	ret = dsi_host_parse_dt(msm_host);
-	if (ret) {
-		pr_err("%s: failed to parse dt\n", __func__);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "%s: failed to parse dt\n",
+				     __func__);
 
 	msm_host->ctrl_base = msm_ioremap_size(pdev, "dsi_ctrl", &msm_host->ctrl_size);
-	if (IS_ERR(msm_host->ctrl_base)) {
-		pr_err("%s: unable to map Dsi ctrl base\n", __func__);
-		return PTR_ERR(msm_host->ctrl_base);
-	}
+	if (IS_ERR(msm_host->ctrl_base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(msm_host->ctrl_base),
+				     "%s: unable to map Dsi ctrl base\n", __func__);
 
 	pm_runtime_enable(&pdev->dev);
 
 	msm_host->cfg_hnd = dsi_get_config(msm_host);
-	if (!msm_host->cfg_hnd) {
-		pr_err("%s: get config failed\n", __func__);
-		return -EINVAL;
-	}
+	if (!msm_host->cfg_hnd)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "%s: get config failed\n", __func__);
 	cfg = msm_host->cfg_hnd->cfg;
 
 	msm_host->id = dsi_host_get_id(msm_host);
-	if (msm_host->id < 0) {
-		pr_err("%s: unable to identify DSI host index\n", __func__);
-		return msm_host->id;
-	}
+	if (msm_host->id < 0)
+		return dev_err_probe(&pdev->dev, msm_host->id,
+				     "%s: unable to identify DSI host index\n",
+				     __func__);
 
 	/* fixup base address by io offset */
 	msm_host->ctrl_base += cfg->io_offset;
@@ -1915,10 +1905,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 		return ret;
 
 	ret = dsi_clk_init(msm_host);
-	if (ret) {
-		pr_err("%s: unable to initialize dsi clks\n", __func__);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "%s: unable to initialize dsi clks\n", __func__);
 
 	msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL);
 	if (!msm_host->rx_buf) {
@@ -1931,26 +1919,20 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 		return ret;
 	/* OPP table is optional */
 	ret = devm_pm_opp_of_add_table(&pdev->dev);
-	if (ret && ret != -ENODEV) {
-		dev_err(&pdev->dev, "invalid OPP table in device tree\n");
-		return ret;
-	}
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(&pdev->dev, ret, "invalid OPP table in device tree\n");
 
 	msm_host->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
-	if (!msm_host->irq) {
-		dev_err(&pdev->dev, "failed to get irq\n");
-		return -EINVAL;
-	}
+	if (!msm_host->irq)
+		return dev_err_probe(&pdev->dev, -EINVAL, "failed to get irq\n");
 
 	/* do not autoenable, will be enabled later */
 	ret = devm_request_irq(&pdev->dev, msm_host->irq, dsi_host_irq,
 			IRQF_TRIGGER_HIGH | IRQF_NO_AUTOEN,
 			"dsi_isr", msm_host);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to request IRQ%u: %d\n",
-				msm_host->irq, ret);
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "failed to request IRQ%u\n",
+				     msm_host->irq);
 
 	init_completion(&msm_host->dma_comp);
 	init_completion(&msm_host->video_comp);

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup
  2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
  2025-01-06  8:49 ` [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths Krzysztof Kozlowski
  2025-01-06  8:49 ` [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe() Krzysztof Kozlowski
@ 2025-01-06  8:49 ` Krzysztof Kozlowski
  2025-01-07  3:00   ` Abhinav Kumar
  2025-01-06  8:49 ` [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message Krzysztof Kozlowski
  2025-02-14 10:52 ` [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
  4 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-06  8:49 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Krzysztof Kozlowski

Cleanup few obvious kernel coding style violations: missing or
unnecessary braces in 'if-else', unnecessary break lines, incorrect
breaking of long function declarations, unnecessary 'else' after a
'return'.  No functional impact expected.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 51 +++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 4a2ad04eea7359545a088bdc63907f6b3e5615bd..a3c344b75fc2e90923cd6d26df87b7c05208f736 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -179,18 +179,18 @@ struct msm_dsi_host {
 	int irq;
 };
 
-
 static inline u32 dsi_read(struct msm_dsi_host *msm_host, u32 reg)
 {
 	return readl(msm_host->ctrl_base + reg);
 }
+
 static inline void dsi_write(struct msm_dsi_host *msm_host, u32 reg, u32 data)
 {
 	writel(data, msm_host->ctrl_base + reg);
 }
 
-static const struct msm_dsi_cfg_handler *dsi_get_config(
-						struct msm_dsi_host *msm_host)
+static const struct msm_dsi_cfg_handler *
+dsi_get_config(struct msm_dsi_host *msm_host)
 {
 	const struct msm_dsi_cfg_handler *cfg_hnd = NULL;
 	struct device *dev = &msm_host->pdev->dev;
@@ -370,7 +370,6 @@ int dsi_link_clk_set_rate_6g(struct msm_dsi_host *msm_host)
 	return 0;
 }
 
-
 int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host)
 {
 	int ret;
@@ -588,7 +587,6 @@ static void dsi_calc_pclk(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
 
 	DBG("pclk=%lu, bclk=%lu", msm_host->pixel_clk_rate,
 				msm_host->byte_clk_rate);
-
 }
 
 int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host, bool is_bonded_dsi)
@@ -677,8 +675,8 @@ static inline enum dsi_traffic_mode dsi_get_traffic_mode(const u32 mode_flags)
 	return NON_BURST_SYNCH_EVENT;
 }
 
-static inline enum dsi_vid_dst_format dsi_get_vid_fmt(
-				const enum mipi_dsi_pixel_format mipi_fmt)
+static inline enum dsi_vid_dst_format
+dsi_get_vid_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
 {
 	switch (mipi_fmt) {
 	case MIPI_DSI_FMT_RGB888:	return VID_DST_FORMAT_RGB888;
@@ -689,8 +687,8 @@ static inline enum dsi_vid_dst_format dsi_get_vid_fmt(
 	}
 }
 
-static inline enum dsi_cmd_dst_format dsi_get_cmd_fmt(
-				const enum mipi_dsi_pixel_format mipi_fmt)
+static inline enum dsi_cmd_dst_format
+dsi_get_cmd_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
 {
 	switch (mipi_fmt) {
 	case MIPI_DSI_FMT_RGB888:	return CMD_DST_FORMAT_RGB888;
@@ -1282,14 +1280,15 @@ static int dsi_cmd_dma_add(struct msm_dsi_host *msm_host,
 static int dsi_short_read1_resp(u8 *buf, const struct mipi_dsi_msg *msg)
 {
 	u8 *data = msg->rx_buf;
+
 	if (data && (msg->rx_len >= 1)) {
 		*data = buf[1]; /* strip out dcs type */
 		return 1;
-	} else {
-		pr_err("%s: read data does not match with rx_buf len %zu\n",
-			__func__, msg->rx_len);
-		return -EINVAL;
 	}
+
+	pr_err("%s: read data does not match with rx_buf len %zu\n",
+		__func__, msg->rx_len);
+	return -EINVAL;
 }
 
 /*
@@ -1298,15 +1297,16 @@ static int dsi_short_read1_resp(u8 *buf, const struct mipi_dsi_msg *msg)
 static int dsi_short_read2_resp(u8 *buf, const struct mipi_dsi_msg *msg)
 {
 	u8 *data = msg->rx_buf;
+
 	if (data && (msg->rx_len >= 2)) {
 		data[0] = buf[1]; /* strip out dcs type */
 		data[1] = buf[2];
 		return 2;
-	} else {
-		pr_err("%s: read data does not match with rx_buf len %zu\n",
-			__func__, msg->rx_len);
-		return -EINVAL;
 	}
+
+	pr_err("%s: read data does not match with rx_buf len %zu\n",
+		__func__, msg->rx_len);
+	return -EINVAL;
 }
 
 static int dsi_long_read_resp(u8 *buf, const struct mipi_dsi_msg *msg)
@@ -1366,8 +1366,9 @@ static int dsi_cmd_dma_tx(struct msm_dsi_host *msm_host, int len)
 			ret = -ETIMEDOUT;
 		else
 			ret = len;
-	} else
+	} else {
 		ret = len;
+	}
 
 	return ret;
 }
@@ -1435,11 +1436,12 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
 		return len;
 	}
 
-	/* for video mode, do not send cmds more than
-	* one pixel line, since it only transmit it
-	* during BLLP.
-	*/
-	/* TODO: if the command is sent in LP mode, the bit rate is only
+	/*
+	 * for video mode, do not send cmds more than
+	 * one pixel line, since it only transmit it
+	 * during BLLP.
+	 *
+	 * TODO: if the command is sent in LP mode, the bit rate is only
 	 * half of esc clk rate. In this case, if the video is already
 	 * actively streaming, we need to check more carefully if the
 	 * command can be fit into one BLLP.
@@ -1864,9 +1866,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 	int ret;
 
 	msm_host = devm_kzalloc(&pdev->dev, sizeof(*msm_host), GFP_KERNEL);
-	if (!msm_host) {
+	if (!msm_host)
 		return -ENOMEM;
-	}
 
 	msm_host->pdev = pdev;
 	msm_dsi->host = &msm_host->base;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message
  2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2025-01-06  8:49 ` [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup Krzysztof Kozlowski
@ 2025-01-06  8:49 ` Krzysztof Kozlowski
  2025-01-07  3:11   ` Abhinav Kumar
  2025-02-14 10:52 ` [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
  4 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-06  8:49 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	Krzysztof Kozlowski

Kernel core already prints detailed report about memory allocation
failures, so drivers should not have their own error messages.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a3c344b75fc2e90923cd6d26df87b7c05208f736..ab5d6fd53972f62307b0c5b801cca8b2394c5556 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1910,10 +1910,8 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi)
 		return dev_err_probe(&pdev->dev, ret, "%s: unable to initialize dsi clks\n", __func__);
 
 	msm_host->rx_buf = devm_kzalloc(&pdev->dev, SZ_4K, GFP_KERNEL);
-	if (!msm_host->rx_buf) {
-		pr_err("%s: alloc rx temp buf failed\n", __func__);
+	if (!msm_host->rx_buf)
 		return -ENOMEM;
-	}
 
 	ret = devm_pm_opp_set_clkname(&pdev->dev, "byte");
 	if (ret)

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths
  2025-01-06  8:49 ` [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths Krzysztof Kozlowski
@ 2025-01-07  2:43   ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2025-01-07  2:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel



On 1/6/2025 12:49 AM, Krzysztof Kozlowski wrote:
> dsi_clk_init(), which gets the clocks, is called only through platform
> driver probe and its failure is a failure of the probe.  Therefore
> NULL-ifying specific clocks is pointless and redundant - the PTR_ERR
> value stored there won't be used/dereferenced afterwards.  What's more,
> variant-specific clock init calls like dsi_clk_init_6g_v2() are not
> doing this cleanup.  Dropping redundant code allows later to make this a
> bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ---
>   1 file changed, 3 deletions(-)
> 

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe()
  2025-01-06  8:49 ` [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe() Krzysztof Kozlowski
@ 2025-01-07  2:56   ` Abhinav Kumar
  2025-01-07  6:08     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Abhinav Kumar @ 2025-01-07  2:56 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel



On 1/6/2025 12:49 AM, Krzysztof Kozlowski wrote:
> dsi_clk_init() and msm_dsi_host_init() are called only from platform

dsi_get_config() as well? OR you didnt want to explicitly mention that 
since its called from msm_dsi_host_init().

> driver probe function, so using dev_err_probe is both appropriate and
> beneficial:
>   - Properly marks device deferred probe status,
>   - Avoids dmesg flood on probe deferrals,
>   - Already incorporates printing ERR value,
>   - Shows device name (in contrast to pr_err()),
>   - Makes code smaller and simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 100 +++++++++++++++----------------------
>   1 file changed, 41 insertions(+), 59 deletions(-)
> 

Change LGTM,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup
  2025-01-06  8:49 ` [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup Krzysztof Kozlowski
@ 2025-01-07  3:00   ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2025-01-07  3:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel



On 1/6/2025 12:49 AM, Krzysztof Kozlowski wrote:
> Cleanup few obvious kernel coding style violations: missing or
> unnecessary braces in 'if-else', unnecessary break lines, incorrect
> breaking of long function declarations, unnecessary 'else' after a
> 'return'.  No functional impact expected.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 51 +++++++++++++++++++-------------------
>   1 file changed, 26 insertions(+), 25 deletions(-)
> 

Thanks, all formatting / style fixes are good

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message
  2025-01-06  8:49 ` [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message Krzysztof Kozlowski
@ 2025-01-07  3:11   ` Abhinav Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2025-01-07  3:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel



On 1/6/2025 12:49 AM, Krzysztof Kozlowski wrote:
> Kernel core already prints detailed report about memory allocation
> failures, so drivers should not have their own error messages.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 


Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe()
  2025-01-07  2:56   ` Abhinav Kumar
@ 2025-01-07  6:08     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-01-07  6:08 UTC (permalink / raw)
  To: Abhinav Kumar, Rob Clark, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel

On 07/01/2025 03:56, Abhinav Kumar wrote:
> 
> 
> On 1/6/2025 12:49 AM, Krzysztof Kozlowski wrote:
>> dsi_clk_init() and msm_dsi_host_init() are called only from platform
> 
> dsi_get_config() as well? OR you didnt want to explicitly mention that 
> since its called from msm_dsi_host_init().
> 

As well, but I forgot to mention it.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/4] drm/msm/dsi: Minor cleanups
  2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2025-01-06  8:49 ` [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message Krzysztof Kozlowski
@ 2025-02-14 10:52 ` Krzysztof Kozlowski
  2025-02-14 11:30   ` Dmitry Baryshkov
  4 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-02-14 10:52 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel

On 06/01/2025 09:49, Krzysztof Kozlowski wrote:
> Few minor improvements/cleanups why browsing the code.
> 
> Best regards,
> Krzysztof
> 

5 weeks on the list. Any more comments from DRM side? Can it be merged?

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/4] drm/msm/dsi: Minor cleanups
  2025-02-14 10:52 ` [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
@ 2025-02-14 11:30   ` Dmitry Baryshkov
  2025-02-14 11:39     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-02-14 11:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel

On Fri, Feb 14, 2025 at 11:52:14AM +0100, Krzysztof Kozlowski wrote:
> On 06/01/2025 09:49, Krzysztof Kozlowski wrote:
> > Few minor improvements/cleanups why browsing the code.
> > 
> > Best regards,
> > Krzysztof
> > 
> 
> 5 weeks on the list. Any more comments from DRM side? Can it be merged?

Are you going to repost for the patch #2 commit message update?

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/4] drm/msm/dsi: Minor cleanups
  2025-02-14 11:30   ` Dmitry Baryshkov
@ 2025-02-14 11:39     ` Krzysztof Kozlowski
  2025-02-14 11:47       ` Dmitry Baryshkov
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2025-02-14 11:39 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel

On 14/02/2025 12:30, Dmitry Baryshkov wrote:
> On Fri, Feb 14, 2025 at 11:52:14AM +0100, Krzysztof Kozlowski wrote:
>> On 06/01/2025 09:49, Krzysztof Kozlowski wrote:
>>> Few minor improvements/cleanups why browsing the code.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> 5 weeks on the list. Any more comments from DRM side? Can it be merged?
> 
> Are you going to repost for the patch #2 commit message update?

I did not plan, but I can send v2 with that update.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/4] drm/msm/dsi: Minor cleanups
  2025-02-14 11:39     ` Krzysztof Kozlowski
@ 2025-02-14 11:47       ` Dmitry Baryshkov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-02-14 11:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, linux-arm-msm, dri-devel, freedreno, linux-kernel

On Fri, Feb 14, 2025 at 12:39:30PM +0100, Krzysztof Kozlowski wrote:
> On 14/02/2025 12:30, Dmitry Baryshkov wrote:
> > On Fri, Feb 14, 2025 at 11:52:14AM +0100, Krzysztof Kozlowski wrote:
> >> On 06/01/2025 09:49, Krzysztof Kozlowski wrote:
> >>> Few minor improvements/cleanups why browsing the code.
> >>>
> >>> Best regards,
> >>> Krzysztof
> >>>
> >>
> >> 5 weeks on the list. Any more comments from DRM side? Can it be merged?
> > 
> > Are you going to repost for the patch #2 commit message update?
> 
> I did not plan, but I can send v2 with that update.

Yes, please.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-02-14 11:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-06  8:49 [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
2025-01-06  8:49 ` [PATCH 1/4] drm/msm/dsi: Drop redundant NULL-ifying of clocks on error paths Krzysztof Kozlowski
2025-01-07  2:43   ` Abhinav Kumar
2025-01-06  8:49 ` [PATCH 2/4] drm/msm/dsi: Simplify with dev_err_probe() Krzysztof Kozlowski
2025-01-07  2:56   ` Abhinav Kumar
2025-01-07  6:08     ` Krzysztof Kozlowski
2025-01-06  8:49 ` [PATCH 3/4] drm/msm/dsi: Minor whitespace and style cleanup Krzysztof Kozlowski
2025-01-07  3:00   ` Abhinav Kumar
2025-01-06  8:49 ` [PATCH 4/4] drm/msm/dsi: Drop unnecessary -ENOMEM message Krzysztof Kozlowski
2025-01-07  3:11   ` Abhinav Kumar
2025-02-14 10:52 ` [PATCH 0/4] drm/msm/dsi: Minor cleanups Krzysztof Kozlowski
2025-02-14 11:30   ` Dmitry Baryshkov
2025-02-14 11:39     ` Krzysztof Kozlowski
2025-02-14 11:47       ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox