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
| ||
|
Message-ID: <9b56df24-7906-f539-51ec-b9f8b785e020@amlogic.com> Date: Sun, 8 Apr 2018 11:00:57 +0800 From: Yixun Lan <yixun.lan@...ogic.com> To: Stephen Boyd <sboyd@...nel.org>, Carlo Caione <carlo@...one.org>, Jerome Brunet <jbrunet@...libre.com>, Kevin Hilman <khilman@...libre.com>, Neil Armstrong <narmstrong@...libre.com> CC: <yixun.lan@...ogic.com>, Rob Herring <robh@...nel.org>, Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...eaurora.org>, Philipp Zabel <p.zabel@...gutronix.de>, Qiufang Dai <qiufang.dai@...ogic.com>, <linux-amlogic@...ts.infradead.org>, <linux-clk@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v3 1/6] clk: meson: aoclk: refactor common code into dedicated file HI Stephen thanks for the review On 04/07/18 02:39, Stephen Boyd wrote: > Quoting Yixun Lan (2018-03-27 19:50:45) >> diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c >> index 9ec23ae9a219..5a922639a264 100644 >> --- a/drivers/clk/meson/gxbb-aoclk.c >> +++ b/drivers/clk/meson/gxbb-aoclk.c >> @@ -165,38 +135,39 @@ static int gxbb_aoclkc_probe(struct platform_device *pdev) >> return -ENODEV; >> } >> >> - /* Reset Controller */ >> - rstc->regmap = regmap; >> - rstc->data = gxbb_aoclk_reset; >> - rstc->reset.ops = &gxbb_aoclk_reset_ops; >> - rstc->reset.nr_resets = ARRAY_SIZE(gxbb_aoclk_reset); >> - rstc->reset.of_node = dev->of_node; >> - ret = devm_reset_controller_register(dev, &rstc->reset); >> - >> - /* >> - * Populate regmap and register all clks >> - */ >> - for (clkid = 0; clkid < ARRAY_SIZE(gxbb_aoclk_gate); clkid++) { >> - gxbb_aoclk_gate[clkid]->map = regmap; >> - >> - ret = devm_clk_hw_register(dev, >> - gxbb_aoclk_onecell_data.hws[clkid]); >> - if (ret) >> - return ret; >> - } >> - >> /* Specific clocks */ >> cec_32k_ao.regmap = regmap; >> ret = devm_clk_hw_register(dev, &cec_32k_ao.hw); >> - if (ret) >> + if (ret) { >> + dev_err(&pdev->dev, "clk cec_32k_ao register failed.\n"); >> return ret; >> + } >> + >> + return 0; >> +} >> + >> +static struct meson_aoclk_data gxbb_aoclkc_data = { > > Can this be const? > sure, I'll update at v4 >> + .reset_reg = AO_RTI_GEN_CNTL_REG0, >> + .num_reset = ARRAY_SIZE(gxbb_aoclk_reset), >> + .reset = gxbb_aoclk_reset, >> + .num_clks = ARRAY_SIZE(gxbb_aoclk_gate), >> + .clks = gxbb_aoclk_gate, >> + .hw_data = &gxbb_aoclk_onecell_data, >> +}; >> + >> diff --git a/drivers/clk/meson/meson-aoclk.c b/drivers/clk/meson/meson-aoclk.c >> new file mode 100644 >> index 000000000000..36067c801f7b >> --- /dev/null >> +++ b/drivers/clk/meson/meson-aoclk.c >> @@ -0,0 +1,79 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Amlogic Meson-AXG Clock Controller Driver >> + * >> + * Copyright (c) 2016 BayLibre, SAS. >> + * Author: Neil Armstrong <narmstrong@...libre.com> >> + * >> + * Copyright (c) 2018 Amlogic, inc. >> + * Author: Qiufang Dai <qiufang.dai@...ogic.com> >> + * Author: Yixun Lan <yixun.lan@...ogic.com> >> + */ >> + >> +#include <linux/platform_device.h> >> +#include <linux/reset-controller.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/init.h> >> +#include <linux/of_device.h> >> +#include "clk-regmap.h" >> +#include "meson-aoclk.h" >> + >> +static int meson_aoclk_do_reset(struct reset_controller_dev *rcdev, >> + unsigned long id) >> +{ >> + struct meson_aoclk_reset_controller *rstc = >> + container_of(rcdev, struct meson_aoclk_reset_controller, reset); >> + >> + return regmap_write(rstc->regmap, rstc->data->reset_reg, >> + BIT(rstc->data->reset[id])); >> +} >> + >> +static const struct reset_control_ops meson_aoclk_reset_ops = { >> + .reset = meson_aoclk_do_reset, >> +}; >> + >> +int meson_aoclkc_probe(struct platform_device *pdev) >> +{ >> + struct meson_aoclk_reset_controller *rstc; >> + struct meson_aoclk_data *data; >> + struct device *dev = &pdev->dev; >> + struct regmap *regmap; >> + int ret, clkid; >> + >> + data = (struct meson_aoclk_data *) >> + of_device_get_match_data(dev); >> + if (!data) >> + return -ENODEV; >> + >> + rstc = devm_kzalloc(dev, sizeof(*rstc), GFP_KERNEL); >> + if (!rstc) >> + return -ENOMEM; >> + >> + regmap = syscon_node_to_regmap(of_get_parent(dev->of_node)); >> + if (IS_ERR(regmap)) { >> + dev_err(dev, "failed to get regmap\n"); >> + return -ENODEV; >> + } >> + >> + /* Reset Controller */ >> + rstc->data = data; >> + rstc->regmap = regmap; >> + rstc->reset.ops = &meson_aoclk_reset_ops; >> + rstc->reset.nr_resets = data->num_reset, >> + rstc->reset.of_node = dev->of_node; >> + ret = devm_reset_controller_register(dev, &rstc->reset); > > Please check the return value here. > will do, thanks for pointing out this >> + >> + /* >> + * Populate regmap and register all clks >> + */ >> + for (clkid = 0; clkid < data->num_clks; clkid++) { >> + data->clks[clkid]->map = regmap; >> + >> + ret = devm_clk_hw_register(dev, data->hw_data->hws[clkid]); >> + if (ret) >> + return ret; >> + } >> + >> + return of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, >> + data->hw_data); > > Use devm_ variant? > sure > I suppose because code is moving in this patch that it would be better > to make improvements in another patch so that this patch is a pure code > movement change instead of a movement + improvement change. > I think it's a good idea to fix this in another independent patch also I will address the comments you raised in another mail thread Yixun
Powered by blists - more mailing lists