Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1256958 > unrolled thread

[PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Jens Kuske <jenskuske@gmail.com> - 2015-10-27 18:00 +0100
    Re: [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Philipp Zabel <p.zabel@pengutronix.de> - 2015-10-28 12:50 +0100
    Re: [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Arnd Bergmann <arnd@arndb.de> - 2015-10-30 09:30 +0100
      Re: [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Jens Kuske <jenskuske@gmail.com> - 2015-11-01 14:50 +0100
      Re: [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Maxime Ripard <maxime.ripard@free-electrons.com> - 2015-11-04 18:20 +0100
        Re: [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets Jean-Francois Moine <moinejf@free.fr> - 2015-11-05 08:00 +0100

#1256958 — [PATCH v4 4/6] reset: sunxi: Add Allwinner H3 bus resets

The H3 bus resets have some holes between the registers, so we add
an of_xlate() function to skip them according to the datasheet.

Signed-off-by: Jens Kuske <jenskuske@gmail.com>
---
 .../bindings/reset/allwinner,sunxi-clock-reset.txt |  1 +
 drivers/reset/reset-sunxi.c                        | 30 +++++++++++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
index c8f7757..e11f023 100644
--- a/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
+++ b/Documentation/devicetree/bindings/reset/allwinner,sunxi-clock-reset.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of the following:
   "allwinner,sun6i-a31-ahb1-reset"
   "allwinner,sun6i-a31-clock-reset"
+  "allwinner,sun8i-h3-bus-reset"
 - reg: should be register base and length as documented in the
   datasheet
 - #reset-cells: 1, see below
diff --git a/drivers/reset/reset-sunxi.c b/drivers/reset/reset-sunxi.c
index 3d95c87..c91e146 100644
--- a/drivers/reset/reset-sunxi.c
+++ b/drivers/reset/reset-sunxi.c
@@ -75,7 +75,9 @@ static struct reset_control_ops sunxi_reset_ops = {
 	.deassert	= sunxi_reset_deassert,
 };
 
-static int sunxi_reset_init(struct device_node *np)
+static int sunxi_reset_init(struct device_node *np,
+			    int (*of_xlate)(struct reset_controller_dev *rcdev,
+				    const struct of_phandle_args *reset_spec))
 {
 	struct sunxi_reset_data *data;
 	struct resource res;
@@ -108,6 +110,7 @@ static int sunxi_reset_init(struct device_node *np)
 	data->rcdev.nr_resets = size * 32;
 	data->rcdev.ops = &sunxi_reset_ops;
 	data->rcdev.of_node = np;
+	data->rcdev.of_xlate = of_xlate;
 	reset_controller_register(&data->rcdev);
 
 	return 0;
@@ -117,6 +120,21 @@ err_alloc:
 	return ret;
 };
 
+static int sun8i_h3_bus_reset_xlate(struct reset_controller_dev *rcdev,
+				    const struct of_phandle_args *reset_spec)
+{
+	unsigned int index = reset_spec->args[0];
+
+	if (index < 96)
+		return index;
+	else if (index < 128)
+		return index + 32;
+	else if (index < 160)
+		return index + 64;
+	else
+		return -EINVAL;
+}
+
 /*
  * These are the reset controller we need to initialize early on in
  * our system, before we can even think of using a regular device
@@ -124,15 +142,21 @@ err_alloc:
  */
 static const struct of_device_id sunxi_early_reset_dt_ids[] __initdata = {
 	{ .compatible = "allwinner,sun6i-a31-ahb1-reset", },
+	{ .compatible = "allwinner,sun8i-h3-bus-reset", .data = sun8i_h3_bus_reset_xlate, },
 	{ /* sentinel */ },
 };
 
 void __init sun6i_reset_init(void)
 {
 	struct device_node *np;
+	const struct of_device_id *match;
+	int (*of_xlate)(struct reset_controller_dev *rcdev,
+			const struct of_phandle_args *reset_spec);
 
-	for_each_matching_node(np, sunxi_early_reset_dt_ids)
-		sunxi_reset_init(np);
+	for_each_matching_node_and_match(np, sunxi_early_reset_dt_ids, &match) {
+		of_xlate = match->data;
+		sunxi_reset_init(np, of_xlate);
+	}
 }
 
 /*
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1257936

Hi Jens,

Am Dienstag, den 27.10.2015, 17:50 +0100 schrieb Jens Kuske:
[...]
> --- a/drivers/reset/reset-sunxi.c
> +++ b/drivers/reset/reset-sunxi.c
> @@ -75,7 +75,9 @@ static struct reset_control_ops sunxi_reset_ops = {
>  	.deassert	= sunxi_reset_deassert,
>  };
>  
> -static int sunxi_reset_init(struct device_node *np)
> +static int sunxi_reset_init(struct device_node *np,
> +			    int (*of_xlate)(struct reset_controller_dev *rcdev,
> +				    const struct of_phandle_args *reset_spec))

I'd add a tab to the indentation and drop the of_xlate parameter names.
If you agree to this change, I'll fix it up when I apply it.

best regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259300

On Tuesday 27 October 2015 17:50:24 Jens Kuske wrote:
> 
> +static int sun8i_h3_bus_reset_xlate(struct reset_controller_dev *rcdev,
> +                                   const struct of_phandle_args *reset_spec)
> +{
> +       unsigned int index = reset_spec->args[0];
> +
> +       if (index < 96)
> +               return index;
> +       else if (index < 128)
> +               return index + 32;
> +       else if (index < 160)
> +               return index + 64;
> +       else
> +               return -EINVAL;
> +}
> +
> 

This looks like you are doing something wrong and should either
put the actual number into DT, or use a two-cell representation,
with the first cell indicating the block (0, 1 or 2), and the
second cell the index.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1260166

Hi,

On 30/10/15 09:27, Arnd Bergmann wrote:
> On Tuesday 27 October 2015 17:50:24 Jens Kuske wrote:
>>
>> +static int sun8i_h3_bus_reset_xlate(struct reset_controller_dev *rcdev,
>> +                                   const struct of_phandle_args *reset_spec)
>> +{
>> +       unsigned int index = reset_spec->args[0];
>> +
>> +       if (index < 96)
>> +               return index;
>> +       else if (index < 128)
>> +               return index + 32;
>> +       else if (index < 160)
>> +               return index + 64;
>> +       else
>> +               return -EINVAL;
>> +}
>> +
>>
> 
> This looks like you are doing something wrong and should either
> put the actual number into DT, or use a two-cell representation,
> with the first cell indicating the block (0, 1 or 2), and the
> second cell the index.
> 

I tried to fix up the somewhat strange register layout here.

>From the datasheet:
BUS_SOFT_RST_REG0	0x02C0	Bus Software Reset Register 0
BUS_SOFT_RST_REG1	0x02C4	Bus Software Reset Register 1
BUS_SOFT_RST_REG2	0x02C8	Bus Software Reset Register 2
BUS_SOFT_RST_REG3	0x02D0	Bus Software Reset Register 3
BUS_SOFT_RST_REG4	0x02D8	Bus Software Reset Register 4

0x2cc and 0x2d4 are unused for some reason, but the regs are named 0-4,
so it lead to some confusion with the actual numbers in DT.
If we shouldn't do this I would be ok with putting the actual number
into DT too.

Jens
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1262451

[Multipart message — attachments visible in raw view] — view raw

Hi Arnd,

On Fri, Oct 30, 2015 at 09:27:03AM +0100, Arnd Bergmann wrote:
> On Tuesday 27 October 2015 17:50:24 Jens Kuske wrote:
> > 
> > +static int sun8i_h3_bus_reset_xlate(struct reset_controller_dev *rcdev,
> > +                                   const struct of_phandle_args *reset_spec)
> > +{
> > +       unsigned int index = reset_spec->args[0];
> > +
> > +       if (index < 96)
> > +               return index;
> > +       else if (index < 128)
> > +               return index + 32;
> > +       else if (index < 160)
> > +               return index + 64;
> > +       else
> > +               return -EINVAL;
> > +}
> > +
> > 
> 
> This looks like you are doing something wrong and should either
> put the actual number into DT,

This is the actual number, except that there's some useless registers
in between. Allwinner documents it like that:

0x0	Reset 0
0x4	Reset 1
0xc	Reset 2

So we have to adjust the offset to account with the blank register in
between (0x8).

> or use a two-cell representation, with the first cell indicating the
> block (0, 1 or 2), and the second cell the index.

And the missing register is not a block either.

That would also imply either changing the bindings of that driver (and
all the current DTS that are using it), or introducing a whole new
driver just to deal with some extraordinary offset calculation.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1262962

On Wed, 4 Nov 2015 08:30:14 -0800
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> Hi Arnd,
> 
> On Fri, Oct 30, 2015 at 09:27:03AM +0100, Arnd Bergmann wrote:
> > On Tuesday 27 October 2015 17:50:24 Jens Kuske wrote:
> > > 
> > > +static int sun8i_h3_bus_reset_xlate(struct reset_controller_dev *rcdev,
> > > +                                   const struct of_phandle_args *reset_spec)
> > > +{
> > > +       unsigned int index = reset_spec->args[0];
> > > +
> > > +       if (index < 96)
> > > +               return index;
> > > +       else if (index < 128)
> > > +               return index + 32;
> > > +       else if (index < 160)
> > > +               return index + 64;
> > > +       else
> > > +               return -EINVAL;
> > > +}
> > > +
> > > 
> > 
> > This looks like you are doing something wrong and should either
> > put the actual number into DT,
> 
> This is the actual number, except that there's some useless registers
> in between. Allwinner documents it like that:
> 
> 0x0	Reset 0
> 0x4	Reset 1
> 0xc	Reset 2
> 
> So we have to adjust the offset to account with the blank register in
> between (0x8).
> 
> > or use a two-cell representation, with the first cell indicating the
> > block (0, 1 or 2), and the second cell the index.
> 
> And the missing register is not a block either.
> 
> That would also imply either changing the bindings of that driver (and
> all the current DTS that are using it), or introducing a whole new
> driver just to deal with some extraordinary offset calculation.

In the H3, the holes are not used, but what would occur if these holes
would be used for some other purpose in future SoCs? Double mapping?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web