linux-kernel - Re: [PATCH v5 04/12] clk: actions: Add common clock driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <152150774389.254778.79582051178250322@swboyd.mtv.corp.google.com>
Date:   Mon, 19 Mar 2018 18:02:23 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        afaerber@...e.de, mark.rutland@....com, mturquette@...libre.com,
        robh+dt@...nel.org
Cc:     liuwei@...ions-semi.com, mp-cs@...ions-semi.com,
        96boards@...obotics.com, devicetree@...r.kernel.org,
        davem@...emloft.net, mchehab@...nel.org,
        daniel.thompson@...aro.org, amit.kucheria@...aro.org,
        viresh.kumar@...aro.org, hzhang@...obotics.com,
        bdong@...obotics.com, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        manivannanece23@...il.com,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Subject: Re: [PATCH v5 04/12] clk: actions: Add common clock driver support

Quoting Manivannan Sadhasivam (2018-03-17 03:09:44)
> diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
> new file mode 100644
> index 000000000000..a7cb698fdc86
> --- /dev/null
> +++ b/drivers/clk/actions/owl-common.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// OWL common clock driver
> +//
> +// Copyright (c) 2014 Actions Semi Inc.
> +// Author: David Liu <liuwei@...ions-semi.com>
> +//
> +// Copyright (c) 2018 Linaro Ltd.
> +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> +
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +
> +#include "owl-common.h"
> +
> +static const struct regmap_config owl_regmap_config = {
> +       .reg_bits       = 32,
> +       .reg_stride     = 4,
> +       .val_bits       = 32,
> +       .max_register   = 0xffff,

Is that actually the end?

> +       .fast_io        = true,
> +};
> +
> +static void owl_clk_set_regmap(const struct owl_clk_desc *desc,
> +                        struct regmap *regmap)
> +{
> +       int i;
> +       struct owl_clk_common *clks;
> +
> +       for (i = 0; i < desc->num_clks; i++) {
> +               clks = desc->clks[i];
> +               if (!clks)
> +                       continue;
> +
> +               clks->regmap = regmap;
> +       }
> +}
> +
> +int owl_clk_regmap_init(struct platform_device *pdev,
> +                        const struct owl_clk_desc *desc)
> +{
> +       void __iomem *base;
> +       struct device_node *node = pdev->dev.of_node;
> +       struct regmap *regmap;
> +
> +       base = of_iomap(node, 0);

Please use platform device apis to find and map the ioresources.

> +       regmap = devm_regmap_init_mmio(&pdev->dev, base, &owl_regmap_config);
> +       if (IS_ERR_OR_NULL(regmap)) {

Isn't it just IS_ERR() for regmap errors?

> +               pr_err("failed to init regmap\n");
> +               return PTR_ERR(regmap);
> +       }
> +
> +       owl_clk_set_regmap(desc, regmap);
> +
> +       return 0;
> +}
> +
> +int owl_clk_probe(struct device *dev, struct clk_hw_onecell_data *hw_clks)
> +{
> +       int i, ret;
> +       struct clk_hw *hw;
> +
> +       for (i = 0; i < hw_clks->num; i++) {
> +
> +               hw = hw_clks->hws[i];
> +
> +               if (!hw)

Style: Stick that if to the assignment before.

> +                       continue;
> +
> +               ret = devm_clk_hw_register(dev, hw);
> +               if (ret) {
> +                       dev_err(dev, "Couldn't register clock %d - %s\n",
> +                               i, hw->init->name);
> +                       return ret;
> +               }
> +       }
> +
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, hw_clks);
> +       if (ret)
> +               dev_err(dev, "Failed to add clock provider\n");
> +
> +       return ret;
> +}