From: Philipp Zabel <p.zabel@pengutronix.de> To: Qin Jian <qinjian@cqplus1.com>, robh+dt@kernel.org Cc: mturquette@baylibre.com, sboyd@kernel.org, maz@kernel.org, broonie@kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, wells.lu@sunplus.com Subject: Re: [PATCH v3 4/8] reset: Add Sunplus SP7021 reset driver Date: Tue, 02 Nov 2021 13:22:59 +0100 [thread overview] Message-ID: <296d4a9fdbe2b60eea4d259f1e2e3fe8d67b3c07.camel@pengutronix.de> (raw) In-Reply-To: <c6f0aaef57b25705af988797ede5ab7119852a5c.1635737544.git.qinjian@cqplus1.com> On Mon, 2021-11-01 at 13:01 +0800, Qin Jian wrote: > Add reset driver for Sunplus SP7021 SoC. You don't mention Q645 here, it appears this driver supports both SoCs? [...] > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index be799a5ab..50695ab47 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -249,6 +249,14 @@ config RESET_TI_SYSCON > you wish to use the reset framework for such memory-mapped devices, > say Y here. Otherwise, say N. > > > +config RESET_SUNPLUS Please add these entries in alphabetical order. > + bool "Sunplus SoCs Reset Driver" Can this be made: depends SOC_SP7021 || SOC_Q645 || COMPILE_TEST ? > + help > + This enables the reset driver support for Sunplus SP7021 SoC family. > + Say Y if you want to control reset signals by the reset controller. > + Otherwise, say N. > + This driver is selected automatically by platform config. Which platform config? [...] > diff --git a/drivers/reset/reset-sunplus.c b/drivers/reset/reset-sunplus.c > new file mode 100644 > index 000000000..696efd75e > --- /dev/null > +++ b/drivers/reset/reset-sunplus.c > @@ -0,0 +1,159 @@ > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +/* > + * SP7021 reset driver > + * > + * Copyright (C) Sunplus Technology Co., Ltd. > + * All rights reserved. > + * > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. Drop this boilerplate, this is not required with the SPDX identifier above. > + */ > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/reset-controller.h> > +#include <linux/reboot.h> > + > +#if defined(CONFIG_SOC_SP7021) > +#include <dt-bindings/reset/sp-sp7021.h> > +#elif defined(CONFIG_SOC_Q645) > +#include <dt-bindings/reset/sp-q645.h> > +#endif I'd prefer if you added namespace prefixes to the defines and included both headers unconditionally. These are just required for RST_MAX, correct? > + > +#define BITASSERT(id, val) ((1 << (16 + id)) | (val << id)) > + > + > +struct sp_reset_data { > + struct reset_controller_dev rcdev; > + void __iomem *membase; > +} sp_reset; Please allocate this with devm_kzalloc in the probe function instead. > + > + > +static inline struct sp_reset_data * > +to_sp_reset_data(struct reset_controller_dev *rcdev) > +{ > + return container_of(rcdev, struct sp_reset_data, rcdev); > +} > + > +static int sp_reset_update(struct reset_controller_dev *rcdev, > + unsigned long id, bool assert) > +{ > + struct sp_reset_data *data = to_sp_reset_data(rcdev); > + int reg_width = sizeof(u32)/2; > + int bank = id / (reg_width * BITS_PER_BYTE); > + int offset = id % (reg_width * BITS_PER_BYTE); > + void __iomem *addr; > + > + addr = data->membase + (bank * 4); > + > + if (assert) > + writel(BITASSERT(offset, 1), addr); > + else > + writel(BITASSERT(offset, 0), addr); Could be writel(BITASSERT(offset, assert), addr); > + > + return 0; > +} > + > +static int sp_reset_assert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return sp_reset_update(rcdev, id, true); > +} > + > + > +static int sp_reset_deassert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return sp_reset_update(rcdev, id, false); > +} > + > +static int sp_reset_status(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct sp_reset_data *data = to_sp_reset_data(rcdev); > + int reg_width = sizeof(u32)/2; > + int bank = id / (reg_width * BITS_PER_BYTE); > + int offset = id % (reg_width * BITS_PER_BYTE); > + u32 reg; > + > + reg = readl(data->membase + (bank * 4)); > + > + return !!(reg & BIT(offset)); > +} > + > +static int sp_restart(struct notifier_block *this, unsigned long mode, > + void *cmd) > +{ > + sp_reset_assert(&sp_reset.rcdev, RST_SYSTEM); > + sp_reset_deassert(&sp_reset.rcdev, RST_SYSTEM); > + > + return NOTIFY_DONE; > +} > + > +static struct notifier_block sp_restart_nb = { > + .notifier_call = sp_restart, > + .priority = 192, > +}; > + > +static const struct reset_control_ops sp_reset_ops = { > + .assert = sp_reset_assert, > + .deassert = sp_reset_deassert, > + .status = sp_reset_status, > +}; > + > +static const struct of_device_id sp_reset_dt_ids[] = { > + { .compatible = "sunplus,sp7021-reset", }, > + { .compatible = "sunplus,q645-reset", }, > + { /* sentinel */ }, > +}; > + > +static int sp_reset_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct sp_reset_data *data = &sp_reset; > + void __iomem *membase; > + struct resource *res; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + membase = devm_ioremap(dev, res->start, resource_size(res)); > + if (IS_ERR(membase)) > + return PTR_ERR(membase); > + > + data->membase = membase; > + data->rcdev.owner = THIS_MODULE; > + data->rcdev.nr_resets = RST_MAX; Use of_device_get_match_data() to determine the correct number of resets. regards Philipp
WARNING: multiple messages have this Message-ID (diff)
From: Philipp Zabel <p.zabel@pengutronix.de> To: Qin Jian <qinjian@cqplus1.com>, robh+dt@kernel.org Cc: mturquette@baylibre.com, sboyd@kernel.org, maz@kernel.org, broonie@kernel.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, wells.lu@sunplus.com Subject: Re: [PATCH v3 4/8] reset: Add Sunplus SP7021 reset driver Date: Tue, 02 Nov 2021 13:22:59 +0100 [thread overview] Message-ID: <296d4a9fdbe2b60eea4d259f1e2e3fe8d67b3c07.camel@pengutronix.de> (raw) In-Reply-To: <c6f0aaef57b25705af988797ede5ab7119852a5c.1635737544.git.qinjian@cqplus1.com> On Mon, 2021-11-01 at 13:01 +0800, Qin Jian wrote: > Add reset driver for Sunplus SP7021 SoC. You don't mention Q645 here, it appears this driver supports both SoCs? [...] > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index be799a5ab..50695ab47 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -249,6 +249,14 @@ config RESET_TI_SYSCON > you wish to use the reset framework for such memory-mapped devices, > say Y here. Otherwise, say N. > > > +config RESET_SUNPLUS Please add these entries in alphabetical order. > + bool "Sunplus SoCs Reset Driver" Can this be made: depends SOC_SP7021 || SOC_Q645 || COMPILE_TEST ? > + help > + This enables the reset driver support for Sunplus SP7021 SoC family. > + Say Y if you want to control reset signals by the reset controller. > + Otherwise, say N. > + This driver is selected automatically by platform config. Which platform config? [...] > diff --git a/drivers/reset/reset-sunplus.c b/drivers/reset/reset-sunplus.c > new file mode 100644 > index 000000000..696efd75e > --- /dev/null > +++ b/drivers/reset/reset-sunplus.c > @@ -0,0 +1,159 @@ > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +/* > + * SP7021 reset driver > + * > + * Copyright (C) Sunplus Technology Co., Ltd. > + * All rights reserved. > + * > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. Drop this boilerplate, this is not required with the SPDX identifier above. > + */ > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/reset-controller.h> > +#include <linux/reboot.h> > + > +#if defined(CONFIG_SOC_SP7021) > +#include <dt-bindings/reset/sp-sp7021.h> > +#elif defined(CONFIG_SOC_Q645) > +#include <dt-bindings/reset/sp-q645.h> > +#endif I'd prefer if you added namespace prefixes to the defines and included both headers unconditionally. These are just required for RST_MAX, correct? > + > +#define BITASSERT(id, val) ((1 << (16 + id)) | (val << id)) > + > + > +struct sp_reset_data { > + struct reset_controller_dev rcdev; > + void __iomem *membase; > +} sp_reset; Please allocate this with devm_kzalloc in the probe function instead. > + > + > +static inline struct sp_reset_data * > +to_sp_reset_data(struct reset_controller_dev *rcdev) > +{ > + return container_of(rcdev, struct sp_reset_data, rcdev); > +} > + > +static int sp_reset_update(struct reset_controller_dev *rcdev, > + unsigned long id, bool assert) > +{ > + struct sp_reset_data *data = to_sp_reset_data(rcdev); > + int reg_width = sizeof(u32)/2; > + int bank = id / (reg_width * BITS_PER_BYTE); > + int offset = id % (reg_width * BITS_PER_BYTE); > + void __iomem *addr; > + > + addr = data->membase + (bank * 4); > + > + if (assert) > + writel(BITASSERT(offset, 1), addr); > + else > + writel(BITASSERT(offset, 0), addr); Could be writel(BITASSERT(offset, assert), addr); > + > + return 0; > +} > + > +static int sp_reset_assert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return sp_reset_update(rcdev, id, true); > +} > + > + > +static int sp_reset_deassert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + return sp_reset_update(rcdev, id, false); > +} > + > +static int sp_reset_status(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct sp_reset_data *data = to_sp_reset_data(rcdev); > + int reg_width = sizeof(u32)/2; > + int bank = id / (reg_width * BITS_PER_BYTE); > + int offset = id % (reg_width * BITS_PER_BYTE); > + u32 reg; > + > + reg = readl(data->membase + (bank * 4)); > + > + return !!(reg & BIT(offset)); > +} > + > +static int sp_restart(struct notifier_block *this, unsigned long mode, > + void *cmd) > +{ > + sp_reset_assert(&sp_reset.rcdev, RST_SYSTEM); > + sp_reset_deassert(&sp_reset.rcdev, RST_SYSTEM); > + > + return NOTIFY_DONE; > +} > + > +static struct notifier_block sp_restart_nb = { > + .notifier_call = sp_restart, > + .priority = 192, > +}; > + > +static const struct reset_control_ops sp_reset_ops = { > + .assert = sp_reset_assert, > + .deassert = sp_reset_deassert, > + .status = sp_reset_status, > +}; > + > +static const struct of_device_id sp_reset_dt_ids[] = { > + { .compatible = "sunplus,sp7021-reset", }, > + { .compatible = "sunplus,q645-reset", }, > + { /* sentinel */ }, > +}; > + > +static int sp_reset_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct sp_reset_data *data = &sp_reset; > + void __iomem *membase; > + struct resource *res; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + membase = devm_ioremap(dev, res->start, resource_size(res)); > + if (IS_ERR(membase)) > + return PTR_ERR(membase); > + > + data->membase = membase; > + data->rcdev.owner = THIS_MODULE; > + data->rcdev.nr_resets = RST_MAX; Use of_device_get_match_data() to determine the correct number of resets. regards Philipp _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-02 12:23 UTC|newest] Thread overview: 124+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-29 8:44 [PATCH v2 0/8] Add Sunplus SP7021 SoC Support Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 1/8] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 2/8] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 3/8] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 4/8] reset: Add Sunplus " Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-30 4:18 ` kernel test robot 2021-10-30 4:18 ` kernel test robot 2021-10-30 4:18 ` kernel test robot 2021-10-29 8:44 ` [PATCH v2 5/8] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 6/8] clk: Add Sunplus " Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 7/8] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 8:44 ` [PATCH v2 8/8] irqchip: Add support for Sunplus " Qin Jian 2021-10-29 8:44 ` Qin Jian 2021-10-29 15:25 ` Marc Zyngier 2021-10-29 15:25 ` Marc Zyngier 2021-11-01 5:01 ` [PATCH v3 0/8] Add Sunplus SP7021 SoC Support Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-01 5:01 ` [PATCH v3 1/8] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-02 17:44 ` Rob Herring 2021-11-02 17:44 ` Rob Herring 2021-11-01 5:01 ` [PATCH v3 2/8] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-01 19:58 ` Rob Herring 2021-11-01 19:58 ` Rob Herring 2021-11-01 5:01 ` [PATCH v3 3/8] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-02 11:51 ` Philipp Zabel 2021-11-02 11:51 ` Philipp Zabel 2021-11-03 1:20 ` 答复: " qinjian[覃健] 2021-11-03 1:20 ` qinjian[覃健] 2021-11-01 5:01 ` [PATCH v3 4/8] reset: Add Sunplus " Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-02 12:22 ` Philipp Zabel [this message] 2021-11-02 12:22 ` Philipp Zabel 2021-11-03 2:42 ` 答复: " qinjian[覃健] 2021-11-03 2:42 ` qinjian[覃健] 2021-11-01 5:01 ` [PATCH v3 5/8] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-01 19:59 ` Rob Herring 2021-11-01 19:59 ` Rob Herring 2021-11-01 5:01 ` [PATCH v3 6/8] clk: Add Sunplus " Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-01 10:16 ` kernel test robot 2021-11-01 10:16 ` kernel test robot 2021-11-01 10:16 ` kernel test robot 2021-11-01 5:01 ` [PATCH v3 7/8] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-02 17:45 ` Rob Herring 2021-11-02 17:45 ` Rob Herring 2021-11-01 5:01 ` [PATCH v3 8/8] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian 2021-11-01 5:01 ` Qin Jian 2021-11-01 8:27 ` kernel test robot 2021-11-01 8:27 ` kernel test robot 2021-11-01 8:27 ` kernel test robot 2021-11-01 10:26 ` kernel test robot 2021-11-01 10:26 ` kernel test robot 2021-11-01 10:26 ` kernel test robot 2021-10-30 15:30 ` [PATCH v2 8/8] irqchip: Add support for Sunplus SP7021 interrupt controller kernel test robot 2021-10-30 15:30 ` kernel test robot 2021-10-30 15:30 ` kernel test robot 2021-10-30 19:29 ` kernel test robot 2021-10-30 19:29 ` kernel test robot 2021-11-04 2:56 ` [PATCH v4 00/10] Add Sunplus SP7021 SoC Support Qin Jian 2021-11-04 2:56 ` Qin Jian 2021-11-04 2:56 ` [PATCH v4 01/10] dt-bindings: vendor-prefixes: Add Sunplus Qin Jian 2021-11-04 2:56 ` Qin Jian 2021-11-08 17:45 ` Rob Herring 2021-11-08 17:45 ` Rob Herring 2021-11-04 2:56 ` [PATCH v4 02/10] dt-bindings: arm: sunplus: Add bindings for Sunplus SP7021 SoC boards Qin Jian 2021-11-04 2:56 ` Qin Jian 2021-11-08 17:46 ` Rob Herring 2021-11-08 17:46 ` Rob Herring 2021-11-04 2:57 ` [PATCH v4 03/10] dt-bindings: reset: Add bindings for SP7021 reset driver Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-12 21:56 ` Rob Herring 2021-11-12 21:56 ` Rob Herring 2021-11-04 2:57 ` [PATCH v4 04/10] reset: Add Sunplus " Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-04 2:57 ` [PATCH v4 05/10] dt-bindings: clock: Add bindings for SP7021 clock driver Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-08 17:46 ` Rob Herring 2021-11-08 17:46 ` Rob Herring 2021-11-04 2:57 ` [PATCH v4 06/10] clk: Add Sunplus " Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 11:32 ` kernel test robot 2021-11-04 11:32 ` kernel test robot 2021-11-04 11:32 ` kernel test robot 2021-11-05 4:02 ` kernel test robot 2021-11-05 4:02 ` kernel test robot 2021-11-05 4:02 ` kernel test robot 2021-11-04 2:57 ` [PATCH v4 07/10] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-08 17:46 ` Rob Herring 2021-11-08 17:46 ` Rob Herring 2021-11-04 2:57 ` [PATCH v4 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 2:57 ` [PATCH v4 09/10] ARM: sunplus: Add initial support for Sunplus SP7021 SoC Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 5:09 ` Randy Dunlap 2021-11-04 15:23 ` kernel test robot 2021-11-04 15:23 ` kernel test robot 2021-11-04 15:23 ` kernel test robot 2021-11-04 2:57 ` [PATCH v4 10/10] ARM: sp7021_defconfig: Add Sunplus SP7021 defconfig Qin Jian 2021-11-04 2:57 ` Qin Jian 2021-11-04 8:22 ` [PATCH v4 00/10] Add Sunplus SP7021 SoC Support Marc Zyngier 2021-11-04 8:22 ` Marc Zyngier 2021-11-04 8:35 ` 答复: " qinjian[覃健] 2021-11-04 8:35 ` qinjian[覃健]
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=296d4a9fdbe2b60eea4d259f1e2e3fe8d67b3c07.camel@pengutronix.de \ --to=p.zabel@pengutronix.de \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maz@kernel.org \ --cc=mturquette@baylibre.com \ --cc=qinjian@cqplus1.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=wells.lu@sunplus.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.