From: Philipp Zabel <p.zabel@pengutronix.de>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Ma Haijun <mahaijuns@gmail.com>
Subject: Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver
Date: Thu, 03 Mar 2016 15:18:29 +0100 [thread overview]
Message-ID: <1457014709.3425.53.camel@pengutronix.de> (raw)
In-Reply-To: <1457005210-18485-7-git-send-email-narmstrong@baylibre.com>
Hi Neil,
Am Donnerstag, den 03.03.2016, 12:39 +0100 schrieb Neil Armstrong:
> Add System reset controller driver for PLX Technology OXNAS SoC Family.
>
> CC: Ma Haijun <mahaijuns@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/reset/Kconfig | 4 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-oxnas.c | 149 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 154 insertions(+)
> create mode 100644 drivers/reset/reset-oxnas.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index df37212..f0ea63b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,5 +12,9 @@ menuconfig RESET_CONTROLLER
>
> If unsure, say no.
>
> +config RESET_OXNAS
> + bool
> + select MFD_SYSCON
I'd prefer not to select MFD_SYSCON here, but rather let ARCH_OXNAS do
that.
> source "drivers/reset/sti/Kconfig"
> source "drivers/reset/hisilicon/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 4d7178e..97e04c5 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_STI) += sti/
> obj-$(CONFIG_ARCH_HISI) += hisilicon/
> obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
> obj-$(CONFIG_ATH79) += reset-ath79.o
> +obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/reset-oxnas.c b/drivers/reset/reset-oxnas.c
> new file mode 100644
> index 0000000..d0ab670
> --- /dev/null
> +++ b/drivers/reset/reset-oxnas.c
> @@ -0,0 +1,149 @@
> +/*
> + * drivers/reset/reset-oxnas.c
> + *
> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
> + * Copyright (C) 2014 Ma Haijun <mahaijuns@gmail.com>
> + * Copyright (C) 2009 Oxford Semiconductor Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/err.h>
> +#include <linux/io.h>
Is there any need to include linux/io.h ?
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/types.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +/* Regmap offsets */
> +#define RST_SET_REGOFFSET 0x34
> +#define RST_CLR_REGOFFSET 0x38
> +
> +struct oxnas_reset {
> + struct regmap *regmap;
> + struct reset_controller_dev rcdev;
> +};
> +
> +static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct oxnas_reset *data =
> + container_of(rcdev, struct oxnas_reset, rcdev);
> +
> + regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
> + msleep(50);
Is this the right delay for all of the resets in this register?
If not, I'd drop the .reset callback.
> + regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
> +
> + return 0;
> +}
> +
> +static int oxnas_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct oxnas_reset *data =
> + container_of(rcdev, struct oxnas_reset, rcdev);
> +
> + regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
> +
> + return 0;
> +}
> +
> +static int oxnas_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct oxnas_reset *data =
> + container_of(rcdev, struct oxnas_reset, rcdev);
> +
> + regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
> +
> + return 0;
> +}
> +
> +static struct reset_control_ops oxnas_reset_ops = {
const
> + .reset = oxnas_reset_reset,
> + .assert = oxnas_reset_assert,
> + .deassert = oxnas_reset_deassert,
> +};
> +
> +static const struct of_device_id oxnas_reset_dt_ids[] = {
> + { .compatible = "plxtech,nas782x-reset", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
> +
> +static int oxnas_reset_probe(struct platform_device *pdev)
> +{
> + struct oxnas_reset *data;
> + struct device *parent;
> +
> + parent = pdev->dev.parent;
> + if (!parent) {
> + dev_err(&pdev->dev, "no parent\n");
Can this even happen?
> + return -ENODEV;
> + }
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->regmap = syscon_node_to_regmap(parent->of_node);
> + if (IS_ERR(data->regmap)) {
> + dev_err(&pdev->dev, "failed to get parent regmap\n");
> + return -ENODEV;
Better print the error code and return it.
> + }
> +
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = 32;
> + data->rcdev.ops = &oxnas_reset_ops;
> + data->rcdev.of_node = pdev->dev.of_node;
> + reset_controller_register(&data->rcdev);
Move this down a bit:
> +
> + platform_set_drvdata(pdev, data);
> +
> + return 0;
and
return reset_controller_register(&data->rcdev);
here.
> +}
> +
> +static int oxnas_reset_remove(struct platform_device *pdev)
> +{
> + struct oxnas_reset *data = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&data->rcdev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver oxnas_reset_driver = {
> + .probe = oxnas_reset_probe,
> + .remove = oxnas_reset_remove,
> + .driver = {
> + .name = "oxnas-reset",
> + .owner = THIS_MODULE,
The .owner field is overwritten by __platform_driver_register() anyway,
just drop it.
> + .of_match_table = oxnas_reset_dt_ids,
> + },
> +};
> +
> +static int __init oxnas_reset_init(void)
> +{
> + return platform_driver_probe(&oxnas_reset_driver,
> + oxnas_reset_probe);
> +}
> +
> +/*
> + * Reset controller does not support probe deferral, so it has to be
> + * initialized before any user, in particular, PCIE uses subsys_initcall.
> + */
> +arch_initcall(oxnas_reset_init);
That doesn't sound right. (of_)reset_control_get return -EPROBE_DEFER if
the rcdev isn't found in the list. Could you elaborate on this?
regards
Philipp
next prev parent reply other threads:[~2016-03-03 14:18 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-03 11:39 [PATCH 00/17] Add Initial support for PLX Technology OX810SE Neil Armstrong
2016-03-03 11:39 ` [PATCH 01/17] dt-bindings: vendor-prefixes: Add PLX Technology Neil Armstrong
2016-03-03 15:02 ` Philipp Zabel
2016-03-05 4:29 ` Rob Herring
2016-03-07 9:55 ` Philipp Zabel
2016-03-03 11:39 ` [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Neil Armstrong
2016-03-03 13:01 ` Marc Zyngier
2016-03-03 13:08 ` Arnd Bergmann
2016-03-03 13:36 ` Russell King - ARM Linux
2016-03-03 17:32 ` Arnd Bergmann
2016-03-03 15:32 ` Ma Haijun
2016-03-03 16:56 ` Neil Armstrong
2016-03-03 17:17 ` Marc Zyngier
2016-03-04 11:10 ` Neil Armstrong
2016-03-03 11:39 ` [PATCH 03/17] dt-bindings: Add PLX Technology RPS IRQ Controller bindings Neil Armstrong
2016-03-03 14:53 ` Andrew Lunn
2016-03-03 14:57 ` Neil Armstrong
2016-03-03 15:06 ` Andrew Lunn
2016-03-03 11:39 ` [PATCH 04/17] clocksource: Add PLX Technology RPS Timer Neil Armstrong
2016-03-17 16:13 ` Daniel Lezcano
2016-03-03 11:39 ` [PATCH 05/17] dt-bindings: Add PLX Technology RPS Timer bindings Neil Armstrong
2016-03-03 11:39 ` [PATCH 06/17] reset: Add PLX Technology Reset Controller driver Neil Armstrong
2016-03-03 14:18 ` Philipp Zabel [this message]
2016-03-03 14:29 ` Neil Armstrong
2016-03-03 15:00 ` Philipp Zabel
2016-03-03 11:40 ` [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings Neil Armstrong
2016-03-03 14:21 ` Philipp Zabel
2016-03-03 14:24 ` Neil Armstrong
2016-03-03 14:31 ` Philipp Zabel
2016-03-03 11:40 ` [PATCH 08/17] clk: Add PLX Technology OXNAS Standard Clocks Neil Armstrong
2016-03-04 2:25 ` Stephen Boyd
2016-03-07 11:24 ` Neil Armstrong
2016-03-03 11:40 ` [PATCH 09/17] dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings Neil Armstrong
2016-03-03 11:40 ` [PATCH 10/17] pinctrl: Add PLX Technology OXNAS pinctrl and gpio driver Neil Armstrong
2016-03-15 14:56 ` Linus Walleij
2016-03-16 15:00 ` Neil Armstrong
2016-03-17 14:49 ` Linus Walleij
2016-03-03 11:40 ` [PATCH 11/17] dt-bindings: Add PLX Technology OXNAS pinctrl and gpio bindings Neil Armstrong
2016-03-15 14:30 ` Linus Walleij
2016-03-03 11:40 ` [PATCH 12/17] arm: Add new mach-oxnas Neil Armstrong
2016-03-03 11:49 ` Russell King - ARM Linux
2016-03-03 12:37 ` Neil Armstrong
2016-03-03 12:56 ` Arnd Bergmann
2016-03-03 13:29 ` Russell King - ARM Linux
2016-03-03 13:40 ` Arnd Bergmann
2016-03-03 11:40 ` [PATCH 13/17] arm: Add build support for mach-oxnas Neil Armstrong
2016-03-03 11:40 ` [PATCH 14/17] arm: boot: dts: Add PLX Technology OX810SE dtsi Neil Armstrong
2016-03-03 12:15 ` Arnd Bergmann
2016-03-03 13:39 ` Neil Armstrong
2016-03-03 11:40 ` [PATCH 15/17] dt-bindings: Add OXNAS bindings Neil Armstrong
2016-03-03 11:40 ` [PATCH 16/17] dt-bindings: Add Western Digital to vendor prefixes Neil Armstrong
2016-03-05 4:29 ` Rob Herring
2016-03-03 11:40 ` [PATCH 17/17] arm: boot: dts: Add Western Digital My Book World Edition device tree Neil Armstrong
2016-03-03 12:23 ` [PATCH 00/17] Add Initial support for PLX Technology OX810SE Arnd Bergmann
2016-03-03 12:36 ` Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 00/18] " Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 01/18] clocksource: sp804: Add support for non-32bit width counter Neil Armstrong
2016-03-17 16:40 ` Daniel Lezcano
2016-03-09 10:24 ` [PATCH v2 02/18] dt-bindings: timer: sp804: add timer-width property Neil Armstrong
2016-03-17 17:09 ` Rob Herring
2016-03-17 18:06 ` Robin Murphy
2016-03-17 19:00 ` Rob Herring
2016-03-17 19:21 ` Robin Murphy
2016-03-22 9:21 ` Neil Armstrong
2016-03-22 12:02 ` Robin Murphy
2016-03-22 14:29 ` Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 03/18] irqchip: versatile-fpga: add new arm,rps-irq compatible Neil Armstrong
2016-03-15 11:47 ` Marc Zyngier
2016-03-09 10:24 ` [PATCH v2 04/18] dt-bindings: irq: arm,versatile-fpga: add arm,rps-irq compatible string Neil Armstrong
2016-03-17 17:15 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 05/18] dt-bindings: vendor-prefixes: Add PLX Technology Neil Armstrong
2016-03-17 17:15 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 06/18] dt-bindings: Add Oxford Semiconductors to vendor prefixes Neil Armstrong
2016-03-17 17:16 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 07/18] reset: Add PLX Technology Reset Controller driver Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 08/18] dt-bindings: Add PLX Technology Reset Controller bindings Neil Armstrong
2016-03-17 17:18 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 09/18] clk: Add PLX Technology OXNAS Standard Clocks Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 10/18] dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings Neil Armstrong
2016-03-17 17:19 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 11/18] pinctrl: Add PLX Technology OXNAS pinctrl and gpio driver Neil Armstrong
2016-03-10 14:43 ` kbuild test robot
2016-03-09 10:24 ` [PATCH v2 12/18] dt-bindings: Add PLX Technology OXNAS pinctrl and gpio bindings Neil Armstrong
2016-03-17 17:25 ` Rob Herring
2016-03-09 10:24 ` [PATCH v2 13/18] arm: Add new mach-oxnas Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 14/18] arm: Add build support for mach-oxnas Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 15/18] arm: boot: dts: Add PLX Technology OX810SE dtsi Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 16/18] dt-bindings: Add OXNAS bindings Neil Armstrong
2016-03-17 17:27 ` Rob Herring
2016-03-23 8:37 ` Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 17/18] dt-bindings: Add Western Digital to vendor prefixes Neil Armstrong
2016-03-09 10:24 ` [PATCH v2 18/18] arm: boot: dts: Add Western Digital My Book World Edition device tree Neil Armstrong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1457014709.3425.53.camel@pengutronix.de \
--to=p.zabel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mahaijuns@gmail.com \
--cc=narmstrong@baylibre.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).