[1/3] clk: mediatek: clk-mtk: Grab iomem pointer for divider clocks

Message ID 20230615122051.546985-2-angelogioacchino.delregno@collabora.com
State New
Headers
Series MediaTek clocks: various fixes |

Commit Message

AngeloGioacchino Del Regno June 15, 2023, 12:20 p.m. UTC
  In the rare case in which one of the clock drivers has divider clocks
but not composite clocks, mtk_clk_simple_probe() would not io(re)map,
hence passing a NULL pointer to mtk_clk_register_dividers().

To fix this issue, extend the `if` conditional to also check if any
divider clocks are present. While at it, also make sure the iomem
pointer is NULL if no composite/divider clocks are declared, as we
are checking for that when iounmapping it in the error path.

This hasn't been seen on any MediaTek clock driver as the current ones
always declare composite clocks along with divider clocks, but this is
still an important fix for a future potential KP.

Fixes: 1fe074b1f112 ("clk: mediatek: Add divider clocks to mtk_clk_simple_{probe,remove}()")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/clk/mediatek/clk-mtk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Comments

Chen-Yu Tsai June 16, 2023, 3:10 a.m. UTC | #1
On Thu, Jun 15, 2023 at 8:21 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> In the rare case in which one of the clock drivers has divider clocks
> but not composite clocks, mtk_clk_simple_probe() would not io(re)map,
> hence passing a NULL pointer to mtk_clk_register_dividers().
>
> To fix this issue, extend the `if` conditional to also check if any
> divider clocks are present. While at it, also make sure the iomem
> pointer is NULL if no composite/divider clocks are declared, as we
> are checking for that when iounmapping it in the error path.
>
> This hasn't been seen on any MediaTek clock driver as the current ones
> always declare composite clocks along with divider clocks, but this is
> still an important fix for a future potential KP.
>
> Fixes: 1fe074b1f112 ("clk: mediatek: Add divider clocks to mtk_clk_simple_{probe,remove}()")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
  
Markus Schneider-Pargmann June 16, 2023, 8:43 a.m. UTC | #2
On Thu, Jun 15, 2023 at 02:20:49PM +0200, AngeloGioacchino Del Regno wrote:
> In the rare case in which one of the clock drivers has divider clocks
> but not composite clocks, mtk_clk_simple_probe() would not io(re)map,
> hence passing a NULL pointer to mtk_clk_register_dividers().
> 
> To fix this issue, extend the `if` conditional to also check if any
> divider clocks are present. While at it, also make sure the iomem
> pointer is NULL if no composite/divider clocks are declared, as we
> are checking for that when iounmapping it in the error path.
> 
> This hasn't been seen on any MediaTek clock driver as the current ones
> always declare composite clocks along with divider clocks, but this is
> still an important fix for a future potential KP.
> 
> Fixes: 1fe074b1f112 ("clk: mediatek: Add divider clocks to mtk_clk_simple_{probe,remove}()")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>

> ---
>  drivers/clk/mediatek/clk-mtk.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index cf3514c8e97e..b00ef4213335 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -469,7 +469,7 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
>  	const struct platform_device_id *id;
>  	const struct mtk_clk_desc *mcd;
>  	struct clk_hw_onecell_data *clk_data;
> -	void __iomem *base;
> +	void __iomem *base = NULL;
>  	int num_clks, r;
>  
>  	mcd = device_get_match_data(&pdev->dev);
> @@ -483,8 +483,8 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
>  			return -EINVAL;
>  	}
>  
> -	/* Composite clocks needs us to pass iomem pointer */
> -	if (mcd->composite_clks) {
> +	/* Composite and divider clocks needs us to pass iomem pointer */
> +	if (mcd->composite_clks || mcd->divider_clks) {
>  		if (!mcd->shared_io)
>  			base = devm_platform_ioremap_resource(pdev, 0);
>  		else
> -- 
> 2.40.1
>
  
Stephen Boyd June 16, 2023, 7:21 p.m. UTC | #3
Quoting AngeloGioacchino Del Regno (2023-06-15 05:20:49)
> In the rare case in which one of the clock drivers has divider clocks
> but not composite clocks, mtk_clk_simple_probe() would not io(re)map,
> hence passing a NULL pointer to mtk_clk_register_dividers().
> 
> To fix this issue, extend the `if` conditional to also check if any
> divider clocks are present. While at it, also make sure the iomem
> pointer is NULL if no composite/divider clocks are declared, as we
> are checking for that when iounmapping it in the error path.
> 
> This hasn't been seen on any MediaTek clock driver as the current ones
> always declare composite clocks along with divider clocks, but this is
> still an important fix for a future potential KP.
> 
> Fixes: 1fe074b1f112 ("clk: mediatek: Add divider clocks to mtk_clk_simple_{probe,remove}()")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---

Applied to clk-next
  

Patch

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index cf3514c8e97e..b00ef4213335 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -469,7 +469,7 @@  static int __mtk_clk_simple_probe(struct platform_device *pdev,
 	const struct platform_device_id *id;
 	const struct mtk_clk_desc *mcd;
 	struct clk_hw_onecell_data *clk_data;
-	void __iomem *base;
+	void __iomem *base = NULL;
 	int num_clks, r;
 
 	mcd = device_get_match_data(&pdev->dev);
@@ -483,8 +483,8 @@  static int __mtk_clk_simple_probe(struct platform_device *pdev,
 			return -EINVAL;
 	}
 
-	/* Composite clocks needs us to pass iomem pointer */
-	if (mcd->composite_clks) {
+	/* Composite and divider clocks needs us to pass iomem pointer */
+	if (mcd->composite_clks || mcd->divider_clks) {
 		if (!mcd->shared_io)
 			base = devm_platform_ioremap_resource(pdev, 0);
 		else