| 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: <1456136612.5148.22.camel@pengutronix.de>
Date: Mon, 22 Feb 2016 11:23:32 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Chen Feng <puck.chen@...ilicon.com>
Cc: arnd@...db.de, xinliang.liu@...aro.org,
linux-kernel@...r.kernel.org, puck.chen@...mail.com, w.f@...wei.com
Subject: Re: [PATCH 1/2] reset: hisilicon: Add hi6220 media part of reset
Hi Chen,
Am Montag, den 22.02.2016, 10:45 +0800 schrieb Chen Feng:
> Signed-off-by: Chen Feng <puck.chen@...ilicon.com>
> Signed-off-by: Xinliang Liu <xinliang.liu@...aro.org>
> ---
> drivers/reset/hisilicon/hi6220_reset.c | 122 ++++++++++++++++++++++++---------
> 1 file changed, 90 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/reset/hisilicon/hi6220_reset.c b/drivers/reset/hisilicon/hi6220_reset.c
> index 7787a9b..a671390 100644
> --- a/drivers/reset/hisilicon/hi6220_reset.c
> +++ b/drivers/reset/hisilicon/hi6220_reset.c
> @@ -1,7 +1,7 @@
> /*
> * Hisilicon Hi6220 reset controller driver
> *
> - * Copyright (c) 2015 Hisilicon Limited.
> + * Copyright (c) 2015-2016 Hisilicon Limited.
> *
> * Author: Feng Chen <puck.chen@...ilicon.com>
> *
> @@ -15,46 +15,88 @@
> #include <linux/module.h>
> #include <linux/bitops.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/reset-controller.h>
> #include <linux/reset.h>
> #include <linux/platform_device.h>
>
> -#define ASSERT_OFFSET 0x300
> -#define DEASSERT_OFFSET 0x304
> -#define MAX_INDEX 0x509
> +/* peritheral ctrl regs
I assume this is supposed to mean "peripheral" ...
> + */
> +#define PERITH_ASSERT_OFFSET 0x300
... and this PERIPH_ASSERT_OFFSET, and so on.
> +#define PERITH_DEASSERT_OFFSET 0x304
> +#define PERITH_MAX_INDEX 0x509
> +
> +/* media ctrl regs
> + */
> +#define SC_MEDIA_RSTEN 0x052C
> +#define SC_MEDIA_RSTDIS 0x0530
> +#define MEDIA_MAX_INDEX 8
> +
> +enum hi6220_reset_ctrl_type {
> + PERITHERAL,
> + MEDIA,
> +};
>
> #define to_reset_data(x) container_of(x, struct hi6220_reset_data, rc_dev)
>
> struct hi6220_reset_data {
> - void __iomem *assert_base;
> - void __iomem *deassert_base;
> - struct reset_controller_dev rc_dev;
> + struct reset_controller_dev rc_dev;
> + enum hi6220_reset_ctrl_type type;
> + struct regmap *regmap;
> };
>
> +static int hi6220_media_assert(struct regmap *regmap, unsigned long idx)
> +{
> + return regmap_write(regmap, SC_MEDIA_RSTEN, BIT(idx));
> +}
> +
> +static int hi6220_media_deassert(struct regmap *regmap, unsigned long idx)
> +{
> + return regmap_write(regmap, SC_MEDIA_RSTDIS, BIT(idx));
> +}
> +
> +static int hi6220_peritheral_assert(struct regmap *regmap, unsigned long idx)
> +{
> + u32 bank = idx >> 8;
> + u32 offset = idx & 0xff;
> + u32 reg = PERITH_ASSERT_OFFSET + bank * 0x10;
> +
> + return regmap_write(regmap, reg, BIT(offset));
> +}
> +
> +static int hi6220_peritheral_deassert(struct regmap *regmap, unsigned long idx)
> +{
> + u32 bank = idx >> 8;
> + u32 offset = idx & 0xff;
> + u32 reg = PERITH_DEASSERT_OFFSET + bank * 0x10;
> +
> + return regmap_write(regmap, reg, BIT(offset));
> +}
If you make these take the struct reset_controller_dev *rc_dev as first
parameter, you can drop the two functions below an just add a second set
of const struct reset_control_ops for the media resets.
> static int hi6220_reset_assert(struct reset_controller_dev *rc_dev,
> unsigned long idx)
> {
> struct hi6220_reset_data *data = to_reset_data(rc_dev);
> + struct regmap *regmap = data->regmap;
>
> - int bank = idx >> 8;
> - int offset = idx & 0xff;
> -
> - writel(BIT(offset), data->assert_base + (bank * 0x10));
> -
> - return 0;
> + if (data->type == MEDIA)
> + return hi6220_media_assert(regmap, idx);
> + else
> + return hi6220_peritheral_assert(regmap, idx);
> }
>
> static int hi6220_reset_deassert(struct reset_controller_dev *rc_dev,
> unsigned long idx)
> {
> struct hi6220_reset_data *data = to_reset_data(rc_dev);
> + struct regmap *regmap = data->regmap;
>
> - int bank = idx >> 8;
> - int offset = idx & 0xff;
> -
> - writel(BIT(offset), data->deassert_base + (bank * 0x10));
> -
> - return 0;
> + if (data->type == MEDIA)
> + return hi6220_media_deassert(regmap, idx);
> + else
> + return hi6220_peritheral_deassert(regmap, idx);
> }
>
> static struct reset_control_ops hi6220_reset_ops = {
> @@ -64,24 +106,32 @@ static struct reset_control_ops hi6220_reset_ops = {
>
> static int hi6220_reset_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + enum hi6220_reset_ctrl_type type;
> struct hi6220_reset_data *data;
> - struct resource *res;
> - void __iomem *src_base;
> + struct regmap *regmap;
>
> - data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> return -ENOMEM;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - src_base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(src_base))
> - return PTR_ERR(src_base);
> + type = (enum hi6220_reset_ctrl_type)of_device_get_match_data(dev);
>
> - data->assert_base = src_base + ASSERT_OFFSET;
> - data->deassert_base = src_base + DEASSERT_OFFSET;
> - data->rc_dev.nr_resets = MAX_INDEX;
> + regmap = syscon_node_to_regmap(np);
Documentation/devicetree/bindings/reset/hisilicon,hi6220-reset.txt
should be changed to reflect that the "syscon" compatible is mandatory.
> + if (IS_ERR(regmap)) {
> + dev_err(dev, "failed to get reset controller regmap\n");
> + return PTR_ERR(regmap);
> + }
> +
> + data->type = type;
> + data->regmap = regmap;
> data->rc_dev.ops = &hi6220_reset_ops;
With media and periph resets in separate ops structures, you could chose
here already:
if (type == MEDIA)
data->rc_dev.ops = &hi6220_media_reset_ops;
else
data->rc_dev.ops = &hi6220_peripheral_reset_ops;
> - data->rc_dev.of_node = pdev->dev.of_node;
> + data->rc_dev.of_node = np;
> + if (type == MEDIA)
> + data->rc_dev.nr_resets = MEDIA_MAX_INDEX;
> + else
> + data->rc_dev.nr_resets = PERITH_MAX_INDEX;
>
> reset_controller_register(&data->rc_dev);
>
> @@ -89,9 +139,17 @@ static int hi6220_reset_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id hi6220_reset_match[] = {
> - { .compatible = "hisilicon,hi6220-sysctrl" },
> - { },
> + {
> + .compatible = "hisilicon,hi6220-sysctrl",
> + .data = (void *)PERITHERAL,
> + },
> + {
> + .compatible = "hisilicon,hi6220-mediactrl",
> + .data = (void *)MEDIA,
> + },
> + { /* sentinel */ },
> };
> +MODULE_DEVICE_TABLE(of, hi6220_reset_match);
>
> static struct platform_driver hi6220_reset_driver = {
> .probe = hi6220_reset_probe,
regards
Philipp
Powered by blists - more mailing lists