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

Groups > linux.kernel > #1450422 > unrolled thread

[PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables

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 4/9] clk: sunxi-ng: mux: Add support for mux tables Chen-Yu Tsai <wens@csie.org> - 2016-07-26 09:10 +0200
    Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Jean-Francois Moine <moinejf@free.fr> - 2016-07-26 19:50 +0200
      Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-07-27 09:00 +0200
        Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Jean-Francois Moine <moinejf@free.fr> - 2016-07-27 09:20 +0200
          Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Chen-Yu Tsai <wens@csie.org> - 2016-07-27 09:40 +0200
          Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-07-27 09:50 +0200
            Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Jean-Francois Moine <moinejf@free.fr> - 2016-07-27 10:40 +0200
              Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-07-28 15:30 +0200
                Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Jean-Francois Moine <moinejf@free.fr> - 2016-07-28 16:30 +0200
    Re: [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-07-27 08:50 +0200

#1450422 — [PATCH 4/9] clk: sunxi-ng: mux: Add support for mux tables

Some clock muxes have holes, i.e. invalid or unconnected inputs,
between parent mux values.

Add support for specifying a mux table to map clock parents to
mux values.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu_mux.c | 12 ++++++++++++
 drivers/clk/sunxi-ng/ccu_mux.h | 12 ++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
index 1329b9ab481e..68b32f168a74 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.c
+++ b/drivers/clk/sunxi-ng/ccu_mux.c
@@ -107,6 +107,15 @@ u8 ccu_mux_helper_get_parent(struct ccu_common *common,
 	parent = reg >> cm->shift;
 	parent &= (1 << cm->width) - 1;
 
+	if (cm->table) {
+		int num_parents = clk_hw_get_num_parents(&common->hw);
+		int i;
+
+		for (i = 0; i < num_parents; i++)
+			if (cm->table[i] == parent)
+				return i;
+	}
+
 	return parent;
 }
 
@@ -117,6 +126,9 @@ int ccu_mux_helper_set_parent(struct ccu_common *common,
 	unsigned long flags;
 	u32 reg;
 
+	if (cm->table)
+		index = cm->table[index];
+
 	spin_lock_irqsave(common->lock, flags);
 
 	reg = readl(common->base + common->reg);
diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
index d35ce5e93840..f0078de78712 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.h
+++ b/drivers/clk/sunxi-ng/ccu_mux.h
@@ -6,8 +6,9 @@
 #include "ccu_common.h"
 
 struct ccu_mux_internal {
-	u8	shift;
-	u8	width;
+	u8		shift;
+	u8		width;
+	const u8	*table;
 
 	struct {
 		u8	index;
@@ -21,6 +22,13 @@ struct ccu_mux_internal {
 	} variable_prediv;
 };
 
+#define SUNXI_CLK_MUX_TABLE(_shift, _width, _table)	\
+	{						\
+		.shift	= _shift,			\
+		.width	= _width,			\
+		.table	= _table,			\
+	}
+
 #define SUNXI_CLK_MUX(_shift, _width)	\
 	{					\
 		.shift	= _shift,		\
-- 
2.8.1

[toc] | [next] | [standalone]

#1450721

On Tue, 26 Jul 2016 15:04:26 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

> Some clock muxes have holes, i.e. invalid or unconnected inputs,
> between parent mux values.
> 
> Add support for specifying a mux table to map clock parents to
> mux values.

Putting empty strings in the holes should work. No?
Ex:

static const char * const csi_mclk_parents[] =
	{ "pll-video0", "pll-video1", "", "", "", "osc24M" };

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

#1451073

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

On Tue, Jul 26, 2016 at 07:43:06PM +0200, Jean-Francois Moine wrote:
> On Tue, 26 Jul 2016 15:04:26 +0800
> Chen-Yu Tsai <wens@csie.org> wrote:
> 
> > Some clock muxes have holes, i.e. invalid or unconnected inputs,
> > between parent mux values.
> > 
> > Add support for specifying a mux table to map clock parents to
> > mux values.
> 
> Putting empty strings in the holes should work. No?
> Ex:
> 
> static const char * const csi_mclk_parents[] =
> 	{ "pll-video0", "pll-video1", "", "", "", "osc24M" };

Not really. The clock would be declared as orphan, while it's really
not.

Parenting functions would also not work as expected,
clk_hw_get_parent_by_index being the obvious example, in that case
returning the empty string for an invalid parent, while it should
really return NULL.

Maxime

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

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

#1451084

On Wed, 27 Jul 2016 08:59:34 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Tue, Jul 26, 2016 at 07:43:06PM +0200, Jean-Francois Moine wrote:
> > On Tue, 26 Jul 2016 15:04:26 +0800
> > Chen-Yu Tsai <wens@csie.org> wrote:
> > 
> > > Some clock muxes have holes, i.e. invalid or unconnected inputs,
> > > between parent mux values.
> > > 
> > > Add support for specifying a mux table to map clock parents to
> > > mux values.
> > 
> > Putting empty strings in the holes should work. No?
> > Ex:
> > 
> > static const char * const csi_mclk_parents[] =
> > 	{ "pll-video0", "pll-video1", "", "", "", "osc24M" };
> 
> Not really. The clock would be declared as orphan, while it's really
> not.
> 
> Parenting functions would also not work as expected,
> clk_hw_get_parent_by_index being the obvious example, in that case
> returning the empty string for an invalid parent, while it should
> really return NULL.

I don't see why the clock should be orphan.
Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

#1451092

On Wed, Jul 27, 2016 at 3:18 PM, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Wed, 27 Jul 2016 08:59:34 +0200
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
>> On Tue, Jul 26, 2016 at 07:43:06PM +0200, Jean-Francois Moine wrote:
>> > On Tue, 26 Jul 2016 15:04:26 +0800
>> > Chen-Yu Tsai <wens@csie.org> wrote:
>> >
>> > > Some clock muxes have holes, i.e. invalid or unconnected inputs,
>> > > between parent mux values.
>> > >
>> > > Add support for specifying a mux table to map clock parents to
>> > > mux values.
>> >
>> > Putting empty strings in the holes should work. No?
>> > Ex:
>> >
>> > static const char * const csi_mclk_parents[] =
>> >     { "pll-video0", "pll-video1", "", "", "", "osc24M" };
>>
>> Not really. The clock would be declared as orphan, while it's really
>> not.
>>
>> Parenting functions would also not work as expected,
>> clk_hw_get_parent_by_index being the obvious example, in that case
>> returning the empty string for an invalid parent, while it should
>> really return NULL.
>
> I don't see why the clock should be orphan.
> Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.

There's no requirement for parent clk indexes to match the actual
register value, is there?

ChenYu

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

#1451099

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

On Wed, Jul 27, 2016 at 09:18:27AM +0200, Jean-Francois Moine wrote:
> On Wed, 27 Jul 2016 08:59:34 +0200
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > On Tue, Jul 26, 2016 at 07:43:06PM +0200, Jean-Francois Moine wrote:
> > > On Tue, 26 Jul 2016 15:04:26 +0800
> > > Chen-Yu Tsai <wens@csie.org> wrote:
> > > 
> > > > Some clock muxes have holes, i.e. invalid or unconnected inputs,
> > > > between parent mux values.
> > > > 
> > > > Add support for specifying a mux table to map clock parents to
> > > > mux values.
> > > 
> > > Putting empty strings in the holes should work. No?
> > > Ex:
> > > 
> > > static const char * const csi_mclk_parents[] =
> > > 	{ "pll-video0", "pll-video1", "", "", "", "osc24M" };
> > 
> > Not really. The clock would be declared as orphan, while it's really
> > not.
> > 
> > Parenting functions would also not work as expected,
> > clk_hw_get_parent_by_index being the obvious example, in that case
> > returning the empty string for an invalid parent, while it should
> > really return NULL.
> 
> I don't see why the clock should be orphan.
> Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.

Why? It should return NULL when there's no parent, while you
explicitly registered a parent.

Maxime

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

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

#1451122

On Wed, 27 Jul 2016 09:40:20 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> > > Parenting functions would also not work as expected,
> > > clk_hw_get_parent_by_index being the obvious example, in that case
> > > returning the empty string for an invalid parent, while it should
> > > really return NULL.
> > 
> > I don't see why the clock should be orphan.
> > Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.
> 
> Why? It should return NULL when there's no parent, while you
> explicitly registered a parent.

"" is not an existing parent. It could be "none" / "dum" / "toto" / ...
with the same result: 'this index cannot be used in mux'.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

#1451930

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

On Wed, Jul 27, 2016 at 10:36:49AM +0200, Jean-Francois Moine wrote:
> On Wed, 27 Jul 2016 09:40:20 +0200
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> 
> > > > Parenting functions would also not work as expected,
> > > > clk_hw_get_parent_by_index being the obvious example, in that case
> > > > returning the empty string for an invalid parent, while it should
> > > > really return NULL.
> > > 
> > > I don't see why the clock should be orphan.
> > > Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.
> > 
> > Why? It should return NULL when there's no parent, while you
> > explicitly registered a parent.
> 
> "" is not an existing parent. It could be "none" / "dum" / "toto" / ...
> with the same result: 'this index cannot be used in mux'.

And the clock is marked as orphan, while it really isn't.

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

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

#1451960

On Thu, 28 Jul 2016 15:28:42 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Wed, Jul 27, 2016 at 10:36:49AM +0200, Jean-Francois Moine wrote:
> > On Wed, 27 Jul 2016 09:40:20 +0200
> > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > 
> > > > > Parenting functions would also not work as expected,
> > > > > clk_hw_get_parent_by_index being the obvious example, in that case
> > > > > returning the empty string for an invalid parent, while it should
> > > > > really return NULL.
> > > > 
> > > > I don't see why the clock should be orphan.
> > > > Then, when a parent is "", clk_hw_get_parent_by_index() returns NULL.
> > > 
> > > Why? It should return NULL when there's no parent, while you
> > > explicitly registered a parent.
> > 
> > "" is not an existing parent. It could be "none" / "dum" / "toto" / ...
> > with the same result: 'this index cannot be used in mux'.
> 
> And the clock is marked as orphan, while it really isn't.

Sorry for I don't follow you.

A clock is orphan when it has no parent. In our case, there are many
possible parents and, at startup time, the hardware or the boot sets
the mux to point to a real parent, with an index out of the usused
values.
Yes, the clock may be orphan, as the other clocks, but just the time
this real parent becomes visible.

So, how could such a clock stay marked as orphan?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

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

#1451068

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

On Tue, Jul 26, 2016 at 03:04:26PM +0800, Chen-Yu Tsai wrote:
> Some clock muxes have holes, i.e. invalid or unconnected inputs,
> between parent mux values.
> 
> Add support for specifying a mux table to map clock parents to
> mux values.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/clk/sunxi-ng/ccu_mux.c | 12 ++++++++++++
>  drivers/clk/sunxi-ng/ccu_mux.h | 12 ++++++++++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> index 1329b9ab481e..68b32f168a74 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -107,6 +107,15 @@ u8 ccu_mux_helper_get_parent(struct ccu_common *common,
>  	parent = reg >> cm->shift;
>  	parent &= (1 << cm->width) - 1;
>  
> +	if (cm->table) {
> +		int num_parents = clk_hw_get_num_parents(&common->hw);
> +		int i;
> +
> +		for (i = 0; i < num_parents; i++)
> +			if (cm->table[i] == parent)
> +				return i;
> +	}
> +
>  	return parent;
>  }
>  
> @@ -117,6 +126,9 @@ int ccu_mux_helper_set_parent(struct ccu_common *common,
>  	unsigned long flags;
>  	u32 reg;
>  
> +	if (cm->table)
> +		index = cm->table[index];
> +
>  	spin_lock_irqsave(common->lock, flags);
>  
>  	reg = readl(common->base + common->reg);
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
> index d35ce5e93840..f0078de78712 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.h
> +++ b/drivers/clk/sunxi-ng/ccu_mux.h
> @@ -6,8 +6,9 @@
>  #include "ccu_common.h"
>  
>  struct ccu_mux_internal {
> -	u8	shift;
> -	u8	width;
> +	u8		shift;
> +	u8		width;
> +	const u8	*table;
>  
>  	struct {
>  		u8	index;
> @@ -21,6 +22,13 @@ struct ccu_mux_internal {
>  	} variable_prediv;
>  };
>  
> +#define SUNXI_CLK_MUX_TABLE(_shift, _width, _table)	\
> +	{						\
> +		.shift	= _shift,			\
> +		.width	= _width,			\
> +		.table	= _table,			\
> +	}
> +

I basically had the exact same patch done a few days ago :)

This is in my A64 serie, together with some cleanup on that macro that
is not consistent with the other internal structures.

Maxime

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web