From mboxrd@z Thu Jan 1 00:00:00 1970
From: Philipp Zabel
Subject: Re: [PATCH v3 10/16] reset: Add a reset controller driver for the
Lantiq XWAY based SoCs
Date: Mon, 29 May 2017 12:20:14 +0200
Message-ID: <1496053214.17695.49.camel@pengutronix.de>
References: <20170528184006.31668-1-hauke@hauke-m.de>
<20170528184006.31668-11-hauke@hauke-m.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Return-path:
In-Reply-To: <20170528184006.31668-11-hauke@hauke-m.de>
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
List-help:
List-unsubscribe:
List-software: Ecartis version 1.0.0
List-subscribe:
List-owner:
List-post:
List-archive:
To: Hauke Mehrtens
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org, linux-mtd@lists.infradead.org, linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org, martin.blumenstingl@googlemail.com, john@phrozen.org, linux-spi@vger.kernel.org, hauke.mehrtens@intel.com, robh@kernel.org, andy.shevchenko@gmail.com
List-Id: devicetree@vger.kernel.org
Hi Hauke,
On Sun, 2017-05-28 at 20:40 +0200, Hauke Mehrtens wrote:
[...]
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/lantiq,reset.txt
> @@ -0,0 +1,30 @@
> +Lantiq XWAY SoC RCU reset controller binding
> +============================================
> +
> +This binding describes a reset-controller found on the RCU module on Lantiq
> +XWAY SoCs.
> +
> +
> +-------------------------------------------------------------------------------
> +Required properties:
> +- compatible : Should be one of
> + "lantiq,reset-danube"
> + "lantiq,reset-xrx200"
> +- regmap : A phandle to the RCU syscon
> +- offset-set : Offset of the reset set register
> +- offset-status : Offset of the reset status register
> +- #reset-cells : Specifies the number of cells needed to encode the
> + reset line, should be 2.
> + The first cell takes the reset set bit and the
> + second cell takes the status bit.
> +
> +-------------------------------------------------------------------------------
> +Example for the reset-controllers on the xRX200 SoCs:
> + reset0: reset-controller@0 {
> + compatible = "lantiq,reset-xrx200";
> +
> + regmap = <&rcu0>;
Why not place these nodes as children of &rcu0 ? This property would be
superfluous then.
> + offset-set = <0x10>;
> + offset-status = <0x14>;
> + #reset-cells = <2>;
> + };
[...]
> --- /dev/null
> +++ b/drivers/reset/reset-lantiq.c
> @@ -0,0 +1,205 @@
[...]
> +static int lantiq_rcu_reset_update(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct lantiq_rcu_reset_priv *priv = to_lantiq_rcu_reset_priv(rcdev);
> + int ret, retry = LANTIQ_RCU_RESET_TIMEOUT;
> + unsigned int set = id & 0x1f;
> + u32 val;
> +
> + if (assert)
> + val = BIT(set);
> + else
> + val = 0;
> +
> + ret = regmap_update_bits(priv->regmap, priv->reset_offset, BIT(set),
> + val);
> + if (ret) {
> + dev_err(priv->dev, "Failed to set reset bit %u\n", set);
> + return ret;
> + }
> +
> + do {
> + ret = lantiq_rcu_reset_status(rcdev, id);
> + if (ret < 0)
> + return ret;
> + if (ret == assert)
> + break;
> + usleep_range(20, 40);
> + } while (--retry);
How about wrapping this in a lantiq_rcu_poll_status(_timeout) function?
I still think this is a case where regmap_read_poll_timeout could be
used.
[...]
> +static int lantiq_rcu_reset_of_probe(struct platform_device *pdev,
> + struct lantiq_rcu_reset_priv *priv)
> +{
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + priv->regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "regmap");
Why not use syscon_node_to_regmap(dev->of_node->parent) ?
regards
Philipp
From mboxrd@z Thu Jan 1 00:00:00 1970
Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33])
by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))
id 1dFHn1-0008LA-2M
for linux-mtd@lists.infradead.org; Mon, 29 May 2017 10:20:44 +0000
Message-ID: <1496053214.17695.49.camel@pengutronix.de>
Subject: Re: [PATCH v3 10/16] reset: Add a reset controller driver for the
Lantiq XWAY based SoCs
From: Philipp Zabel
To: Hauke Mehrtens
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org,
linux-mtd@lists.infradead.org, linux-watchdog@vger.kernel.org,
devicetree@vger.kernel.org, martin.blumenstingl@googlemail.com,
john@phrozen.org, linux-spi@vger.kernel.org, hauke.mehrtens@intel.com,
robh@kernel.org, andy.shevchenko@gmail.com
Date: Mon, 29 May 2017 12:20:14 +0200
In-Reply-To: <20170528184006.31668-11-hauke@hauke-m.de>
References: <20170528184006.31668-1-hauke@hauke-m.de>
<20170528184006.31668-11-hauke@hauke-m.de>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
List-Id: Linux MTD discussion mailing list
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
Hi Hauke,
On Sun, 2017-05-28 at 20:40 +0200, Hauke Mehrtens wrote:
[...]
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/lantiq,reset.txt
> @@ -0,0 +1,30 @@
> +Lantiq XWAY SoC RCU reset controller binding
> +============================================
> +
> +This binding describes a reset-controller found on the RCU module on Lantiq
> +XWAY SoCs.
> +
> +
> +-------------------------------------------------------------------------------
> +Required properties:
> +- compatible : Should be one of
> + "lantiq,reset-danube"
> + "lantiq,reset-xrx200"
> +- regmap : A phandle to the RCU syscon
> +- offset-set : Offset of the reset set register
> +- offset-status : Offset of the reset status register
> +- #reset-cells : Specifies the number of cells needed to encode the
> + reset line, should be 2.
> + The first cell takes the reset set bit and the
> + second cell takes the status bit.
> +
> +-------------------------------------------------------------------------------
> +Example for the reset-controllers on the xRX200 SoCs:
> + reset0: reset-controller@0 {
> + compatible = "lantiq,reset-xrx200";
> +
> + regmap = <&rcu0>;
Why not place these nodes as children of &rcu0 ? This property would be
superfluous then.
> + offset-set = <0x10>;
> + offset-status = <0x14>;
> + #reset-cells = <2>;
> + };
[...]
> --- /dev/null
> +++ b/drivers/reset/reset-lantiq.c
> @@ -0,0 +1,205 @@
[...]
> +static int lantiq_rcu_reset_update(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct lantiq_rcu_reset_priv *priv = to_lantiq_rcu_reset_priv(rcdev);
> + int ret, retry = LANTIQ_RCU_RESET_TIMEOUT;
> + unsigned int set = id & 0x1f;
> + u32 val;
> +
> + if (assert)
> + val = BIT(set);
> + else
> + val = 0;
> +
> + ret = regmap_update_bits(priv->regmap, priv->reset_offset, BIT(set),
> + val);
> + if (ret) {
> + dev_err(priv->dev, "Failed to set reset bit %u\n", set);
> + return ret;
> + }
> +
> + do {
> + ret = lantiq_rcu_reset_status(rcdev, id);
> + if (ret < 0)
> + return ret;
> + if (ret == assert)
> + break;
> + usleep_range(20, 40);
> + } while (--retry);
How about wrapping this in a lantiq_rcu_poll_status(_timeout) function?
I still think this is a case where regmap_read_poll_timeout could be
used.
[...]
> +static int lantiq_rcu_reset_of_probe(struct platform_device *pdev,
> + struct lantiq_rcu_reset_priv *priv)
> +{
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + priv->regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "regmap");
Why not use syscon_node_to_regmap(dev->of_node->parent) ?
regards
Philipp