* [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