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
