No longer produce error messages when dphy is deferred
Subsystems (4)
cover No longer produce error messages when dphy is deferred
This is a small series that makes the deferred dphy no longer emit error messages to the kernel log, which may even be repeated multiple times, to avoid possible false impression of some issues.
This series also performs a few small, rather trivial code cleanups, to make the code a bit easier to read and a bit more consistent.
Dragan Simic (2):
drm/rockchip: dsi: Perform trivial code cleanups drm/rockchip: dsi: Don't log errors on deferred dphy
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
PATCH 1/2 drm/rockchip: dsi: Perform trivial code cleanups
Perform a few trivial code cleanups, to make one logged message a bit more consistent with the other logged messages by capitalizing its first word, and to avoid line wrapping by using the 100-column width better.
No intended functional changes are introduced by these code cleanups.
--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +4 -8
@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);return -EINVAL ; }
@@ -1408,19 +1408,16 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->pllref_clk =NULL ; }else { ret =PTR_ERR (dsi->pllref_clk); -DRM_DEV_ERROR (dev, -"Unable to get pll reference clock: %d\n" , - ret); +DRM_DEV_ERROR (dev,"Unable to get pll reference clock: %d\n" , ret);return ret; } }if (dsi->cdata->flags &DW_MIPI_NEEDS_PHY_CFG_CLK ) { dsi->phy_cfg_clk =devm_clk_get (dev,"phy_cfg" );if (IS_ERR (dsi->phy_cfg_clk)) { ret =PTR_ERR (dsi->phy_cfg_clk); -DRM_DEV_ERROR (dev, -"Unable to get phy_cfg_clk: %d\n" , ret); +DRM_DEV_ERROR (dev,"Unable to get phy_cfg_clk: %d\n" , ret);return ret; } }
@@ -1465,8 +1462,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)if (IS_ERR (dsi->dmd)) { ret =PTR_ERR (dsi->dmd);if (ret != -EPROBE_DEFER ) -DRM_DEV_ERROR (dev, -"Failed to probe dw_mipi_dsi: %d\n" , ret); +DRM_DEV_ERROR (dev,"Failed to probe dw_mipi_dsi: %d\n" , ret);return ret; }
Heiko Stübner
maintainerRe: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Hey Dragan,
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 16 lines
Perform a few trivial code cleanups, to make one logged message a bit more consistent with the other logged messages by capitalizing its first word, and to avoid line wrapping by using the 100-column width better.
No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 5 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);
this is all probe-related, why not convert to dev_err_probe?
As the doc states [0], DRM_DEV_ERROR is deprecated in favor of dev_err. So dev_err_probe would be the correct way to go?
[0] https://elixir.bootlin.com/linux/v6.11.6/source/include/drm/drm_print.h#L431
Quoted 36 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +3
-7return -EINVAL ; }@@ -1408,19 +1408,16 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->pllref_clk =NULL ; }else { ret =PTR_ERR (dsi->pllref_clk); -DRM_DEV_ERROR (dev, -"Unable to get pll reference clock: %d\n" , - ret); +DRM_DEV_ERROR (dev,"Unable to get pll reference clock: %d\n" , ret);return ret; } }if (dsi->cdata->flags &DW_MIPI_NEEDS_PHY_CFG_CLK ) { dsi->phy_cfg_clk =devm_clk_get (dev,"phy_cfg" );if (IS_ERR (dsi->phy_cfg_clk)) { ret =PTR_ERR (dsi->phy_cfg_clk); -DRM_DEV_ERROR (dev, -"Unable to get phy_cfg_clk: %d\n" , ret); +DRM_DEV_ERROR (dev,"Unable to get phy_cfg_clk: %d\n" , ret);return ret; } }@@ -1465,8 +1462,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)if (IS_ERR (dsi->dmd)) { ret =PTR_ERR (dsi->dmd);if (ret != -EPROBE_DEFER ) -DRM_DEV_ERROR (dev, -"Failed to probe dw_mipi_dsi: %d\n" , ret); +DRM_DEV_ERROR (dev,"Failed to probe dw_mipi_dsi: %d\n" , ret);return ret; }
Re: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Hello Heiko,
Quoted 20 lines · Heiko Stübner
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 19 lines
Perform a few trivial code cleanups, to make one logged message a bit more consistent with the other logged messages by capitalizing its first word, and to avoid line wrapping by using the 100-column width better.
No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }
Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?
Thanks for your quick response! Seeing that DRM_DEV_ERROR() is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages that DRM_DEV_ERROR() produces.
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete DRM_DEV_ERROR() --> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)
Quoted 41 lines
[0] https://elixir.bootlin.com/linux/v6.11.6/source/include/drm/drm_print.h#L431
Quoted 39 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +2
-5return -EINVAL ; }@@ -1408,19 +1408,16 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->pllref_clk = NULL; }else { ret =PTR_ERR (dsi->pllref_clk); -DRM_DEV_ERROR (dev, -"Unable to get pll reference clock: %d\n" , - ret); +DRM_DEV_ERROR (dev,"Unable to get pll reference clock: %d\n" , ret);return ret; } }if (dsi->cdata->flags &DW_MIPI_NEEDS_PHY_CFG_CLK ) { dsi->phy_cfg_clk =devm_clk_get (dev,"phy_cfg" );if (IS_ERR (dsi->phy_cfg_clk)) { ret =PTR_ERR (dsi->phy_cfg_clk); -DRM_DEV_ERROR (dev, -"Unable to get phy_cfg_clk: %d\n" , ret); +DRM_DEV_ERROR (dev,"Unable to get phy_cfg_clk: %d\n" , ret);return ret;drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-2} }@@ -1465,8 +1462,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)if (IS_ERR (dsi->dmd)) { ret =PTR_ERR (dsi->dmd);if (ret != -EPROBE_DEFER ) -DRM_DEV_ERROR (dev, -"Failed to probe dw_mipi_dsi: %d\n" , ret); +DRM_DEV_ERROR (dev,"Failed to probe dw_mipi_dsi: %d\n" , ret);return ret; }
Heiko Stübner
maintainerRe: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Am Freitag, 8. November 2024, 15:05:02 CET schrieb Dragan Simic:
Quoted 40 lines
Hello Heiko,
Quoted 23 lines · Heiko Stübner
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 22 lines
Perform a few trivial code cleanups, to make one logged message a bit more
consistent with the other logged messages by capitalizing its first word, and
to avoid line wrapping by using the 100-column width better.No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?Thanks for your quick response! Seeing that
DRM_DEV_ERROR()is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages thatDRM_DEV_ERROR()produces.
Quoted 6 lines
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete
DRM_DEV_ERROR()--> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)
But your patch-2 really just open-codes, what dev_err_probe is meant to fix. So with going this way, you're sort of making things worse first, until that second step happens.
Similarly, reflowing lines for things that get removed in a week do not serve a purpose - those line-breaks have been that way for years already.
Re: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Quoted 46 lines · Heiko Stübner
Am Freitag, 8. November 2024, 15:05:02 CET schrieb Dragan Simic:
Quoted 38 lines · Heiko Stübner
Quoted 23 lines
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 22 lines
Perform a few trivial code cleanups, to make one logged message a bit more
consistent with the other logged messages by capitalizing its first word, and
to avoid line wrapping by using the 100-column width better.No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?Thanks for your quick response! Seeing that
DRM_DEV_ERROR()is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages thatDRM_DEV_ERROR()produces.Quoted 6 lines
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete
DRM_DEV_ERROR()--> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)
Quoted 7 lines
But your patch-2 really just open-codes, what
dev_err_probeis meant to fix. So with going this way, you're sort of making things worse first, until that second step happens.Similarly, reflowing lines for things that get removed in a week do not serve a purpose - those line-breaks have been that way for years already.
Hmm, it makes sense when described that way. I'll see to perform the complete conversion in the next few days.
Heiko Stübner
maintainerRe: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Am Freitag, 8. November 2024, 15:13:33 CET schrieb Dragan Simic:
Quoted 52 lines · Heiko Stübner
Quoted 51 lines
Am Freitag, 8. November 2024, 15:05:02 CET schrieb Dragan Simic:
Quoted 38 lines · Heiko Stübner
Quoted 23 lines
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 22 lines
Perform a few trivial code cleanups, to make one logged message a bit more
consistent with the other logged messages by capitalizing its first word, and
to avoid line wrapping by using the 100-column width better.No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?Thanks for your quick response! Seeing that
DRM_DEV_ERROR()is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages thatDRM_DEV_ERROR()produces.Quoted 6 lines
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete
DRM_DEV_ERROR()--> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)But your patch-2 really just open-codes, what
dev_err_probeis meant to fix. So with going this way, you're sort of making things worse first,
until that second step happens.
Quoted 6 lines
Quoted 3 lines
Similarly, reflowing lines for things that get removed in a week do not serve a purpose - those line-breaks have been that way for years already.
Hmm, it makes sense when described that way. I'll see to perform the complete conversion in the next few days.
just a note, as written on IRC earlier, I am sitting on a dev_err_probe conversion for dw-dsi-rockchip.
I was waiting to see if more cleanups turned up, so didn't sent that yet.
Don't want to steal your spotlight though, so not sure if I should send that or wait for your conversion ;-)
Heiko
Re: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Quoted 60 lines · Heiko Stübner
Am Freitag, 8. November 2024, 15:13:33 CET schrieb Dragan Simic:
Quoted 52 lines · Heiko Stübner
Quoted 51 lines
Am Freitag, 8. November 2024, 15:05:02 CET schrieb Dragan Simic:
Quoted 38 lines · Heiko Stübner
Quoted 23 lines
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 22 lines
Perform a few trivial code cleanups, to make one logged message a bit more
consistent with the other logged messages by capitalizing its first word, and
to avoid line wrapping by using the 100-column width better.No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) · line 1382
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?Thanks for your quick response! Seeing that
DRM_DEV_ERROR()is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages thatDRM_DEV_ERROR()produces.Quoted 6 lines
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete
DRM_DEV_ERROR()--> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)But your patch-2 really just open-codes, what
dev_err_probeis meant to fix. So with going this way, you're sort of making things worse first,
until that second step happens.Quoted 6 lines
Quoted 3 lines
Similarly, reflowing lines for things that get removed in a week do not serve a purpose - those line-breaks have been that way for years already.
Hmm, it makes sense when described that way. I'll see to perform the complete conversion in the next few days.
Quoted 7 lines
just a note, as written on IRC earlier, I am sitting on a
dev_err_probeconversion for dw-dsi-rockchip.I was waiting to see if more cleanups turned up, so didn't sent that yet.
Don't want to steal your spotlight though, so not sure if I should send that or wait for your conversion ;-)
I see no reasons why should we duplicate some effort. :) If you're already nearing the file-level conversion to its completion, please feel free to send it, and we can drop this series. :)
Re: [PATCH 1/2] drm/rockchip: dsi: Perform trivial code cleanups
Quoted 64 lines · Dragan Simic
Quoted 63 lines · Heiko Stübner
Am Freitag, 8. November 2024, 15:13:33 CET schrieb Dragan Simic:
Quoted 52 lines · Heiko Stübner
Quoted 51 lines
Am Freitag, 8. November 2024, 15:05:02 CET schrieb Dragan Simic:
Quoted 38 lines · Heiko Stübner
Quoted 23 lines
Am Freitag, 8. November 2024, 14:53:57 CET schrieb Dragan Simic:
Quoted 22 lines
Perform a few trivial code cleanups, to make one logged message a bit more
consistent with the other logged messages by capitalizing its first word, and
to avoid line wrapping by using the 100-column width better.No intended functional changes are introduced by these code cleanups.
Signed-off-by Dragan Simic <[email protected]>--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +0
-0@@ -1379,7 +1379,7 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) }Quoted 8 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c static int dw_mipi_dsi_rockchip_probe(struct · line 1383
Quoted 3 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +1
-1if (!dsi->cdata) { -DRM_DEV_ERROR (dev,"no dsi-config for %s node\n" , np->name); +DRM_DEV_ERROR (dev,"No dsi-config for %s node\n" , np->name);this is all probe-related, why not convert to
dev_err_probe?As the doc states [0],
DRM_DEV_ERRORis deprecated in favor ofdev_err. Sodev_err_probewould be the correct way to go?Thanks for your quick response! Seeing that
DRM_DEV_ERROR()is now deprecated (which I originally missed, in all honesty) makes me very happy. :) I've never been a huge fan of the format of the messages thatDRM_DEV_ERROR()produces.Quoted 6 lines
However, perhaps it would be better to keep these patches as-is, as some kind of an intermediate, limited-scope cleanup + bugfix combo, and leave the complete
DRM_DEV_ERROR()--> dev_err()/dev_err_probe() conversion to separate patches. I think it would be better to avoid a partial conversion, and I'll be more than happy to put the complete conversion on my TODO list. :)But your patch-2 really just open-codes, what
dev_err_probeis meant to fix. So with going this way, you're sort of making things worse first,
until that second step happens.Quoted 6 lines
Quoted 3 lines
Similarly, reflowing lines for things that get removed in a week do not serve a purpose - those line-breaks have been that way for years already.
Hmm, it makes sense when described that way. I'll see to perform the complete conversion in the next few days.
just a note, as written on IRC earlier, I am sitting on a
dev_err_probeconversion for dw-dsi-rockchip.
Quoted 8 lines
Quoted 4 lines
I was waiting to see if more cleanups turned up, so didn't sent that yet.
Don't want to steal your spotlight though, so not sure if I should send that or wait for your conversion ;-)
I see no reasons why should we duplicate some effort. :) If you're already nearing the file-level conversion to its completion, please feel free to send it, and we can drop this series. :)
Sorry, I wasn't clear enough. When I wrote "we can drop this series", I actually referred to what this series might have turned into, i.e. into my file-level conversion. :)
PATCH 2/2 drm/rockchip: dsi: Don't log errors on deferred dphy
In reply to Dragan SimicDeferred driver probing shouldn't result in errors or warnings being logged, because their presence in the kernel log provides no value and may actually cause false impression that some issues exist. Thus, let's no longer produce error messages when getting the dphy results in deferred probing.
This prevents misleading error messages like the following one, which was observed on a Pine64 PineTab2, from appearing in the kernel log. To make matters worse, the following error message was observed appearing multiple times in the kernel log of a single PineTab2 boot:
dw-mipi-dsi-rockchip fe060000.dsi: [drm:dw_mipi_dsi_rockchip_probe \
[rockchipdrm]] *ERROR* failed to get mipi dphy: -517
At the same time, make the adjusted logged message a bit more consistent with the other logged messages by capitalizing its first word.
--- drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +2 -1
@@ -1387,7 +1387,8 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->phy =devm_phy_optional_get (dev,"dphy" );if (IS_ERR (dsi->phy)) { ret =PTR_ERR (dsi->phy); -DRM_DEV_ERROR (dev,"failed to get mipi dphy: %d\n" , ret); +if (ret != -EPROBE_DEFER ) +DRM_DEV_ERROR (dev,"Failed to get mipi dphy: %d\n" , ret);return ret; }
Re: [PATCH 2/2] drm/rockchip: dsi: Don't log errors on deferred dphy
Hi,
Quoted 13 lines · Dragan Simic
Deferred driver probing shouldn't result in errors or warnings being logged, because their presence in the kernel log provides no value and may actually cause false impression that some issues exist. Thus, let's no longer produce error messages when getting the dphy results in deferred probing.
This prevents misleading error messages like the following one, which was observed on a Pine64 PineTab2, from appearing in the kernel log. To make matters worse, the following error message was observed appearing multiple times in the kernel log of a single PineTab2 boot:
dw-mipi-dsi-rockchip fe060000.dsi: [drm:dw_mipi_dsi_rockchip_probe \
[rockchipdrm]] *ERROR* failed to get mipi dphy: -517
Quoted 6 lines
At the same time, make the adjusted logged message a bit more consistent with the other logged messages by capitalizing its first word.
Reported-by Diederik de Haas <[email protected]>Signed-off-by Dragan Simic <[email protected]>---
From include/drm/drm_print.h:
* DRM_DEV_ERROR() - Error output.
*
* NOTE: this is deprecated in favor of drm_err() or dev_err().
The recommended way to do this nowadays looks like this:
return dev_err_probe(dev, PTR_ERR(dsi->phy), "Failed to get mipi dphy");
That will not print anything for -EPROBE_DEFER, but capture the reason and make it available through /sys/kernel/debug/devices_deferred if the device never probes.
Greetings,
-- Sebastian
Quoted 17 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +2
-1@@ -1387,7 +1387,8 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->phy =devm_phy_optional_get (dev,"dphy" );if (IS_ERR (dsi->phy)) { ret =PTR_ERR (dsi->phy); -DRM_DEV_ERROR (dev,"failed to get mipi dphy: %d\n" , ret); +if (ret != -EPROBE_DEFER ) +DRM_DEV_ERROR (dev,"Failed to get mipi dphy: %d\n" , ret);return ret; }
Re: [PATCH 2/2] drm/rockchip: dsi: Don't log errors on deferred dphy
Hello Sebastian,
Quoted 27 lines · Sebastian Reichel
Quoted 13 lines · Dragan Simic
Deferred driver probing shouldn't result in errors or warnings being logged, because their presence in the kernel log provides no value and may actually cause false impression that some issues exist. Thus, let's no longer produce error messages when getting the dphy results in deferred probing.
This prevents misleading error messages like the following one, which was observed on a Pine64 PineTab2, from appearing in the kernel log. To make matters worse, the following error message was observed appearing multiple times in the kernel log of a single PineTab2 boot:
dw-mipi-dsi-rockchip fe060000.dsi: [drm:dw_mipi_dsi_rockchip_probe \
[rockchipdrm]] *ERROR* failed to get mipi dphy: -517Quoted 6 lines
At the same time, make the adjusted logged message a bit more consistent with the other logged messages by capitalizing its first word.
Reported-by Diederik de Haas <[email protected]>Signed-off-by Dragan Simic <[email protected]>---
From
include/drm/drm_print.h:*
DRM_DEV_ERROR()- Error output.
*
* NOTE: this is deprecated in favor ofdrm_err()ordev_err().
Quoted 7 lines
The recommended way to do this nowadays looks like this:
return
dev_err_probe(dev, PTR_ERR(dsi->phy), "Failed to get mipi dphy");That will not print anything for -EPROBE_DEFER, but capture the reason and make it available through
/sys/kernel/debug/devices_deferredif the device never probes.
Thanks for your quick response! As already discussed with Heiko, I'll move forward with implementing a complete file-level conversion. At first, I thought that a partial bugfix would be beneficial, [1] but now I agree that performing a complete file-level coversion is the way to go. [2]
I've got to admit, I love seeing that DRM_DEV_ERROR() is deprecated, because I've never been a big fan of the format of the messages it produces.
[1] https://lore.kernel.org/dri-devel/[email protected]/
[2] https://lore.kernel.org/dri-devel/[email protected]/
Quoted 18 lines
Quoted 18 lines
drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c +2
-1@@ -1387,7 +1387,8 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) dsi->phy = devm_phy_optional_get(dev, "dphy");if (IS_ERR (dsi->phy)) { ret =PTR_ERR (dsi->phy); -DRM_DEV_ERROR (dev,"failed to get mipi dphy: %d\n" , ret); +if (ret != -EPROBE_DEFER ) +DRM_DEV_ERROR (dev,"Failed to get mipi dphy: %d\n" , ret);return ret; }