Re: [PATCH V2 6/6] clk: sprd: add clocks support for SC9863A

From: Stephen Boyd
Date: Sun Jan 05 2020 - 02:20:15 EST


Quoting Chunyan Zhang (2019-12-16 04:19:32)
> diff --git a/drivers/clk/sprd/sc9863a-clk.c b/drivers/clk/sprd/sc9863a-clk.c
> new file mode 100644
> index 000000000000..145bb0a78740
> --- /dev/null
> +++ b/drivers/clk/sprd/sc9863a-clk.c
> @@ -0,0 +1,1835 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Unisoc SC9863A clock driver
> + *
> + * Copyright (C) 2019 Unisoc, Inc.
> + * Author: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

Is this include used?

> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/clock/sprd,sc9863a-clk.h>
> +
> +#include "common.h"
> +#include "composite.h"
> +#include "div.h"
> +#include "gate.h"
> +#include "mux.h"
> +#include "pll.h"
> +
[...]
> + 0x1000, BIT(12), 0, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(uart0_eb, "uart0-eb", "ext-26m", 0x0,
> + 0x1000, BIT(13), CLK_IGNORE_UNUSED, 0);

Why are we CLK_IGNORE_UNUSED marking these clks? Please add a comment to
explain why this should stay instead of being marked as critical.

> +static SPRD_SC_GATE_CLK_FW_NAME(uart1_eb, "uart1-eb", "ext-26m", 0x0,
> + 0x1000, BIT(14), CLK_IGNORE_UNUSED, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(uart2_eb, "uart2-eb", "ext-26m", 0x0,
> + 0x1000, BIT(15), CLK_IGNORE_UNUSED, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(uart3_eb, "uart3-eb", "ext-26m", 0x0,
> + 0x1000, BIT(16), CLK_IGNORE_UNUSED, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(uart4_eb, "uart4-eb", "ext-26m", 0x0,
> + 0x1000, BIT(17), CLK_IGNORE_UNUSED, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(sim0_32k_eb, "sim0_32k-eb", "ext-26m", 0x0,
> + 0x1000, BIT(18), 0, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(spi3_eb, "spi3-eb", "ext-26m", 0x0,
> + 0x1000, BIT(19), 0, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(i2c5_eb, "i2c5-eb", "ext-26m", 0x0,
> + 0x1000, BIT(20), 0, 0);
> +static SPRD_SC_GATE_CLK_FW_NAME(i2c6_eb, "i2c6-eb", "ext-26m", 0x0,
> + 0x1000, BIT(21), 0, 0);
> +
> +
> +static int sc9863a_clk_probe(struct platform_device *pdev)
> +{
> + const struct sprd_clk_desc *desc;
> +
> + desc = device_get_match_data(&pdev->dev);
> + if (!desc)
> + return -ENODEV;
> +
> + sprd_clk_regmap_init(pdev, desc);

Can this fail?

> +
> + return sprd_clk_probe(&pdev->dev, desc->hw_clks);
> +}
> +