On Wed, 2019-08-28 at 10:19 +0300, Tero Kristo wrote:
> Hardware reset signals are tightly coupled with associated clocks, and
> basically de-asserting a reset won't succeed properly if the clock is
> not enabled, and vice-versa. Also, disabling a clock won't fully succeed
> if the associated hardware resets are not asserted. Add status sync
> functionality between these two for TI drivers so that the situations
> can be handled properly without generating any timeouts.
>
> Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
> ---
> drivers/soc/ti/omap_prm.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c
> index 38998ce19c71..e876bad8f8d5 100644
> --- a/drivers/soc/ti/omap_prm.c
> +++ b/drivers/soc/ti/omap_prm.c
> @@ -15,6 +15,8 @@
> #include <linux/platform_device.h>
> #include <linux/reset-controller.h>
> #include <linux/delay.h>
> +#include <linux/clk.h>
> +#include <linux/clk/ti.h>
>
> #include <linux/platform_data/ti-prm.h>
>
> @@ -42,7 +44,9 @@ struct omap_reset_data {
> struct reset_controller_dev rcdev;
> struct omap_prm *prm;
> struct clockdomain *clkdm;
> + struct clk *clk;
> struct device *dev;
> + u32 mask;
> };
>
> #define to_omap_reset_data(p) container_of((p), struct omap_reset_data, rcdev)
> @@ -102,6 +106,8 @@ static int omap_reset_assert(struct reset_controller_dev *rcdev,
> v |= 1 << id;
> writel_relaxed(v, reset->prm->base + reset->prm->data->rstctrl);
>
> + ti_clk_notify_resets(reset->clk, v == reset->mask);
> +
> return 0;
> }
>
> @@ -163,9 +169,19 @@ static int omap_reset_deassert(struct reset_controller_dev *rcdev,
> v &= ~(1 << id);
> writel_relaxed(v, reset->prm->base + reset->prm->data->rstctrl);
>
> + ti_clk_notify_resets(reset->clk, v == reset->mask);
> +
> if (!has_rstst)
> goto exit;
>
> + /* If associated clock is disabled, we can't poll completion status */
> + if (reset->clk) {
> + struct clk_hw *hw = __clk_get_hw(reset->clk);
> +
> + if (!clk_hw_is_enabled(hw))
> + return ret;
> + }
> +
> /* wait for the status to be set */
> while (1) {
> v = readl_relaxed(reset->prm->base + reset->prm->data->rstst);
> @@ -199,8 +215,10 @@ static int omap_prm_reset_init(struct platform_device *pdev,
> struct omap_prm *prm)
> {
> struct omap_reset_data *reset;
> + const struct omap_rst_map *map;
> struct ti_prm_platform_data *pdata = dev_get_platdata(&pdev->dev);
> char buf[32];
> + u32 v;
>
> /*
> * Check if we have controllable resets. If either rstctrl is non-zero
> @@ -215,6 +233,10 @@ static int omap_prm_reset_init(struct platform_device *pdev,
> !pdata->clkdm_allow_idle)
> return -EINVAL;
>
> + map = prm->data->rstmap;
> + if (!map)
> + return -EINVAL;
Can this actually happen?
> +
> reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> if (!reset)
> return -ENOMEM;
> @@ -224,6 +246,10 @@ static int omap_prm_reset_init(struct platform_device *pdev,
> reset->rcdev.of_node = pdev->dev.of_node;
> reset->rcdev.nr_resets = OMAP_MAX_RESETS;
> reset->dev = &pdev->dev;
> + reset->clk = of_clk_get(pdev->dev.of_node, 0);
> +
> + if (IS_ERR(reset->clk))
> + reset->clk = NULL;
Maybe only ignore -ENOENT?
> reset->prm = prm;
>
> @@ -234,6 +260,16 @@ static int omap_prm_reset_init(struct platform_device *pdev,
> if (!reset->clkdm)
> return -EINVAL;
>
> + while (map->rst >= 0) {
> + reset->mask |= BIT(map->rst);
> + map++;
> + }
With this, you could use reset->mask to simplify _is_valid_reset.
regards
Philipp
