This is a small cleanup of the mtk-dpi driver, switching it to devm
variants where possible and where it made sense, and reducing lines
while retaining and improving human readability.
AngeloGioacchino Del Regno (4):
drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()
drm/mediatek: mtk_dpi: Simplify with dev_err_probe()
drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()
drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()
drivers/gpu/drm/mediatek/mtk_dpi.c | 59 +++++++++++-------------------
1 file changed, 21 insertions(+), 38 deletions(-)
--
2.40.0
Change drm_bridge_add() to its devm variant to slightly simplify the
probe function.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 948a53f1f4b3..2d5f3fc34f61 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
dpi->bridge.of_node = dev->of_node;
dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
- drm_bridge_add(&dpi->bridge);
+ ret = devm_drm_bridge_add(dev, &dpi->bridge);
+ if (ret)
+ return ret;
ret = component_add(dev, &mtk_dpi_component_ops);
if (ret) {
- drm_bridge_remove(&dpi->bridge);
dev_err(dev, "Failed to add component: %d\n", ret);
return ret;
}
--
2.40.0
Use dev_err_probe() across the entire probe function of this driver
to shrink the size.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 44 ++++++++++--------------------
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 2d5f3fc34f61..6be65ea21f8f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1040,38 +1040,24 @@ static int mtk_dpi_probe(struct platform_device *pdev)
}
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
dpi->regs = devm_ioremap_resource(dev, mem);
- if (IS_ERR(dpi->regs)) {
- ret = PTR_ERR(dpi->regs);
- dev_err(dev, "Failed to ioremap mem resource: %d\n", ret);
- return ret;
- }
+ if (IS_ERR(dpi->regs))
+ return dev_err_probe(dev, PTR_ERR(dpi->regs),
+ "Failed to ioremap mem resource\n");
dpi->engine_clk = devm_clk_get(dev, "engine");
- if (IS_ERR(dpi->engine_clk)) {
- ret = PTR_ERR(dpi->engine_clk);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to get engine clock: %d\n", ret);
-
- return ret;
- }
+ if (IS_ERR(dpi->engine_clk))
+ return dev_err_probe(dev, PTR_ERR(dpi->engine_clk),
+ "Failed to get engine clock\n");
dpi->pixel_clk = devm_clk_get(dev, "pixel");
- if (IS_ERR(dpi->pixel_clk)) {
- ret = PTR_ERR(dpi->pixel_clk);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to get pixel clock: %d\n", ret);
-
- return ret;
- }
+ if (IS_ERR(dpi->pixel_clk))
+ return dev_err_probe(dev, PTR_ERR(dpi->pixel_clk),
+ "Failed to get pixel clock\n");
dpi->tvd_clk = devm_clk_get(dev, "pll");
- if (IS_ERR(dpi->tvd_clk)) {
- ret = PTR_ERR(dpi->tvd_clk);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
-
- return ret;
- }
+ if (IS_ERR(dpi->tvd_clk))
+ return dev_err_probe(dev, PTR_ERR(dpi->tvd_clk),
+ "Failed to get tvdpll clock\n");
dpi->irq = platform_get_irq(pdev, 0);
if (dpi->irq <= 0)
@@ -1095,10 +1081,8 @@ static int mtk_dpi_probe(struct platform_device *pdev)
return ret;
ret = component_add(dev, &mtk_dpi_component_ops);
- if (ret) {
- dev_err(dev, "Failed to add component: %d\n", ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add component.\n");
return 0;
}
--
2.40.0
Instead of the open-coded platform_get_resource, devm_ioremap_resource
switch to devm_platform_get_and_ioremap_resource(), doing exactly the
same.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 9025111013d3..45535dc7970f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
}
}
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dpi->regs = devm_ioremap_resource(dev, mem);
+ dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
if (IS_ERR(dpi->regs))
return dev_err_probe(dev, PTR_ERR(dpi->regs),
"Failed to ioremap mem resource\n");
--
2.40.0
Function drm_of_find_panel_or_bridge() is marked as deprecated: since
the usage of that in this driver exactly corresponds to the new function
devm_drm_of_get_bridge(), switch to it.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 6be65ea21f8f..9025111013d3 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
if (dpi->irq <= 0)
return -EINVAL;
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
- NULL, &dpi->next_bridge);
- if (ret)
- return ret;
+ dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
+ if (IS_ERR(dpi->next_bridge))
+ return PTR_ERR(dpi->next_bridge);
dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
--
2.40.0
Il 12/04/23 13:52, AngeloGioacchino Del Regno ha scritto:
> This is a small cleanup of the mtk-dpi driver, switching it to devm
> variants where possible and where it made sense, and reducing lines
> while retaining and improving human readability.
>
> AngeloGioacchino Del Regno (4):
> drm/mediatek: mtk_dpi: Simplify with devm_drm_bridge_add()
> drm/mediatek: mtk_dpi: Simplify with dev_err_probe()
> drm/mediatek: mtk_dpi: Switch to devm_drm_of_get_bridge()
> drm/mediatek: mtk_dpi: Use devm_platform_get_and_ioremap_resource()
>
> drivers/gpu/drm/mediatek/mtk_dpi.c | 59 +++++++++++-------------------
> 1 file changed, 21 insertions(+), 38 deletions(-)
>
Ping for a fully ready and well tested series that I've sent *3 months* ago.
Thanks,
Angelo
Hi Angelo,
On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Change drm_bridge_add() to its devm variant to slightly simplify the
> probe function.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 948a53f1f4b3..2d5f3fc34f61 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> dpi->bridge.of_node = dev->of_node;
> dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
>
> - drm_bridge_add(&dpi->bridge);
> + ret = devm_drm_bridge_add(dev, &dpi->bridge);
> + if (ret)
> + return ret;
And also drop the drm_bridge_remove() call in mtk_dpi_remove()?
Regards,
Fei
>
> ret = component_add(dev, &mtk_dpi_component_ops);
> if (ret) {
> - drm_bridge_remove(&dpi->bridge);
> dev_err(dev, "Failed to add component: %d\n", ret);
> return ret;
> }
> --
> 2.40.0
>
>
On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Use dev_err_probe() across the entire probe function of this driver
> to shrink the size.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Fei Shao <[email protected]>
Hi Angelo,
On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Instead of the open-coded platform_get_resource, devm_ioremap_resource
> switch to devm_platform_get_and_ioremap_resource(), doing exactly the
> same.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 9025111013d3..45535dc7970f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
> }
> }
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - dpi->regs = devm_ioremap_resource(dev, mem);
> + dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
"mem" appears to be unused elsewhere, so I assume
devm_platform_ioremap_resource() would be a better fit.
Regards,
Fei
> if (IS_ERR(dpi->regs))
> return dev_err_probe(dev, PTR_ERR(dpi->regs),
> "Failed to ioremap mem resource\n");
> --
> 2.40.0
>
>
Hi Angelo,
On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Function drm_of_find_panel_or_bridge() is marked as deprecated: since
> the usage of that in this driver exactly corresponds to the new function
> devm_drm_of_get_bridge(), switch to it.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
> index 6be65ea21f8f..9025111013d3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
> @@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
> if (dpi->irq <= 0)
> return -EINVAL;
>
> - ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> - NULL, &dpi->next_bridge);
> - if (ret)
> - return ret;
> + dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
> + if (IS_ERR(dpi->next_bridge))
> + return PTR_ERR(dpi->next_bridge);
The original code doesn't print any log so it's probably fine, but
given you're already at it, perhaps you want to also make use of
dev_err_probe() here?
devm_drm_of_get_bridge() can also pass -EPROBE_DEFER from its wrapped
drm_of_find_panel_or_bridge(). Furthermore, that will make the code
visually align with your previous patch.
But that's just optional, and since this patch works anyway,
Reviewed-by: Fei Shao <[email protected]>
>
> dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
>
> --
> 2.40.0
>
>
Il 13/07/23 10:34, Fei Shao ha scritto:
> Hi Angelo,
>
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Instead of the open-coded platform_get_resource, devm_ioremap_resource
>> switch to devm_platform_get_and_ioremap_resource(), doing exactly the
>> same.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>> ---
>> drivers/gpu/drm/mediatek/mtk_dpi.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 9025111013d3..45535dc7970f 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1038,8 +1038,7 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>> dev_dbg(&pdev->dev, "Cannot find pinctrl active!\n");
>> }
>> }
>> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - dpi->regs = devm_ioremap_resource(dev, mem);
>> + dpi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem);
>
> "mem" appears to be unused elsewhere, so I assume
> devm_platform_ioremap_resource() would be a better fit.
>
That's right. I'll switch to devm_platform_ioremap_resource() in v2.
Cheers,
Angelo
> Regards,
> Fei
>
>> if (IS_ERR(dpi->regs))
>> return dev_err_probe(dev, PTR_ERR(dpi->regs),
>> "Failed to ioremap mem resource\n");
>> --
>> 2.40.0
>>
>>
>
Il 13/07/23 10:24, Fei Shao ha scritto:
> Hi Angelo,
>
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Function drm_of_find_panel_or_bridge() is marked as deprecated: since
>> the usage of that in this driver exactly corresponds to the new function
>> devm_drm_of_get_bridge(), switch to it.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>> ---
>> drivers/gpu/drm/mediatek/mtk_dpi.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 6be65ea21f8f..9025111013d3 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1063,10 +1063,9 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>> if (dpi->irq <= 0)
>> return -EINVAL;
>>
>> - ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
>> - NULL, &dpi->next_bridge);
>> - if (ret)
>> - return ret;
>> + dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
>> + if (IS_ERR(dpi->next_bridge))
>> + return PTR_ERR(dpi->next_bridge);
>
> The original code doesn't print any log so it's probably fine, but
> given you're already at it, perhaps you want to also make use of
> dev_err_probe() here?
> devm_drm_of_get_bridge() can also pass -EPROBE_DEFER from its wrapped
> drm_of_find_panel_or_bridge(). Furthermore, that will make the code
> visually align with your previous patch.
>
> But that's just optional, and since this patch works anyway,
>
> Reviewed-by: Fei Shao <[email protected]>
>
Thanks. Yeah it's a good idea to add an error print... since I will have
to anyway send a v2, I'll add it.
Cheers,
Angelo
>>
>> dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
>>
>> --
>> 2.40.0
>>
>>
>
Il 13/07/23 09:55, Fei Shao ha scritto:
> Hi Angelo,
>
> On Wed, Apr 12, 2023 at 7:53 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Change drm_bridge_add() to its devm variant to slightly simplify the
>> probe function.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
>> ---
>> drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> index 948a53f1f4b3..2d5f3fc34f61 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dpi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
>> @@ -1090,11 +1090,12 @@ static int mtk_dpi_probe(struct platform_device *pdev)
>> dpi->bridge.of_node = dev->of_node;
>> dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
>>
>> - drm_bridge_add(&dpi->bridge);
>> + ret = devm_drm_bridge_add(dev, &dpi->bridge);
>> + if (ret)
>> + return ret;
>
> And also drop the drm_bridge_remove() call in mtk_dpi_remove()?
>
Yes. V2 will drop that.
Cheers,
Angelo
> Regards,
> Fei
>
>>
>> ret = component_add(dev, &mtk_dpi_component_ops);
>> if (ret) {
>> - drm_bridge_remove(&dpi->bridge);
>> dev_err(dev, "Failed to add component: %d\n", ret);
>> return ret;
>> }
>> --
>> 2.40.0
>>
>>
>