[PATCH] ARM: fix regression in IXP4xx clocksource

[PATCH] ARM: fix regression in IXP4xx clocksource

[Krzysztof Halasa]

From: Richard Cochran <richardcochran@gmail.com>

Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868

   clocksource: convert ARM 32-bit up counting clocksources

broke the build for ixp4xx and made big endian operation impossible.
This commit restores the original behaviour.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Signed-off-by: Krzysztof Ha?asa <khc@pm.waw.pl>

diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 74ed81a..0777257 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
 /*
  * clocksource
  */
+
+static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
+{
+	return *IXP4XX_OSTS;
+}
+
 unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
 EXPORT_SYMBOL(ixp4xx_timer_freq);
 static void __init ixp4xx_clocksource_init(void)
 {
 	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
 
-	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
-			clocksource_mmio_readl_up);
+	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
+			ixp4xx_clocksource_read);
 }
 
 /*

[PATCH] ARM: fix regression in IXP4xx clocksource

[Russell King - ARM Linux]

Did you see Thomas' reply to your last patch?  Would you mind responding
to that please, before asking Linus to merge this?

On Wed, Jul 06, 2011 at 11:02:17PM +0200, Krzysztof Halasa wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> 
> Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> 
>    clocksource: convert ARM 32-bit up counting clocksources
> 
> broke the build for ixp4xx and made big endian operation impossible.
> This commit restores the original behaviour.
> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> Signed-off-by: Krzysztof Ha?asa <khc@pm.waw.pl>
> 
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 74ed81a..0777257 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
>  /*
>   * clocksource
>   */
> +
> +static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
> +{
> +	return *IXP4XX_OSTS;
> +}
> +
>  unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
>  EXPORT_SYMBOL(ixp4xx_timer_freq);
>  static void __init ixp4xx_clocksource_init(void)
>  {
>  	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
>  
> -	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
> -			clocksource_mmio_readl_up);
> +	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
> +			ixp4xx_clocksource_read);
>  }
>  
>  /*
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: fix regression in IXP4xx clocksource

[Krzysztof Halasa]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> Did you see Thomas' reply to your last patch?  Would you mind responding
> to that please, before asking Linus to merge this?

I like the BE (or endian-neutral value-preserving) accessor idea.
Nevertheless Linux on IXP4xx doesn't even build at this time and I think
fixing this before 3.0 is well worth it.

I assume changing the common ARM code to use new accessors will happen
post-3.0.

(BTW that was Richard Cochran's patch)
-- 
Krzysztof Halasa

[PATCH] ARM: fix regression in IXP4xx clocksource

[Richard Cochran]

On Wed, Jul 06, 2011 at 11:02:17PM +0200, Krzysztof Halasa wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> 
> Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> 
>    clocksource: convert ARM 32-bit up counting clocksources
> 
> broke the build for ixp4xx and made big endian operation impossible.
> This commit restores the original behaviour.

I know I nag, but can we *please* have this patch in 3.0?
Big endian IXP really, really does not work without it.

Thanks,

Richard

> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> Signed-off-by: Krzysztof Ha?asa <khc@pm.waw.pl>
> 
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 74ed81a..0777257 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
>  /*
>   * clocksource
>   */
> +
> +static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
> +{
> +	return *IXP4XX_OSTS;
> +}
> +
>  unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
>  EXPORT_SYMBOL(ixp4xx_timer_freq);
>  static void __init ixp4xx_clocksource_init(void)
>  {
>  	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
>  
> -	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
> -			clocksource_mmio_readl_up);
> +	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
> +			ixp4xx_clocksource_read);
>  }
>  
>  /*
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: fix regression in IXP4xx clocksource

[Russell King - ARM Linux]

On Tue, Jul 12, 2011 at 10:05:13AM +0200, Richard Cochran wrote:
> On Wed, Jul 06, 2011 at 11:02:17PM +0200, Krzysztof Halasa wrote:
> > From: Richard Cochran <richardcochran@gmail.com>
> > 
> > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> > 
> >    clocksource: convert ARM 32-bit up counting clocksources
> > 
> > broke the build for ixp4xx and made big endian operation impossible.
> > This commit restores the original behaviour.
> 
> I know I nag, but can we *please* have this patch in 3.0?
> Big endian IXP really, really does not work without it.

Thomas Glexnier said:
> Thinking more about it we should add BE accessor functions to the mmio
> clocksource as this might be useful for other architectures as well.

It seems that I can't rely on others, so I'm going to have to fit
this into my busy schedule at some point.

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868

   clocksource: convert ARM 32-bit up counting clocksources

broke the build for ixp4xx and made big endian operation impossible.
This commit restores the original behaviour.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 arch/arm/mach-ixp4xx/common.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
index 74ed81a..0777257 100644
--- a/arch/arm/mach-ixp4xx/common.c
+++ b/arch/arm/mach-ixp4xx/common.c
@@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
 /*
  * clocksource
  */
+
+static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
+{
+	return *IXP4XX_OSTS;
+}
+
 unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
 EXPORT_SYMBOL(ixp4xx_timer_freq);
 static void __init ixp4xx_clocksource_init(void)
 {
 	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
 
-	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
-			clocksource_mmio_readl_up);
+	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
+			ixp4xx_clocksource_read);
 }
 
 /*
-- 
1.7.0.4

[PATCH] arm: fix regression in ixp4xx clocksource

[Krzysztof Halasa]

Richard Cochran <richardcochran@gmail.com> writes:

> Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
>
>    clocksource: convert ARM 32-bit up counting clocksources
>
> broke the build for ixp4xx and made big endian operation impossible.
> This commit restores the original behaviour.

Thanks.

I can't ATM do anything Linux-related but will do ASAP. Two-three weeks
I hope.
-- 
Krzysztof Halasa

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

On Wed, Jun 01, 2011 at 05:08:59PM +0200, Krzysztof Halasa wrote:
> Richard Cochran <richardcochran@gmail.com> writes:
> 
> > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> >
> >    clocksource: convert ARM 32-bit up counting clocksources
> >
> > broke the build for ixp4xx and made big endian operation impossible.
> > This commit restores the original behaviour.
> 
> Thanks.
> 
> I can't ATM do anything Linux-related but will do ASAP. Two-three weeks
> I hope.

I don't complain that you have a lack of time for this issue. Still,
it is a serious bug for IXP, and it should be fixed in the 3.0
release.

Russell, can you please, please take a look?

Thanks,
Richard

[PATCH] arm: fix regression in ixp4xx clocksource

[Krzysztof Halasa]

Richard Cochran <richardcochran@gmail.com> writes:

> I don't complain that you have a lack of time for this issue. Still,
> it is a serious bug for IXP, and it should be fixed in the 3.0
> release.

Sure, shouldn't be a problem though.
-- 
Krzysztof Halasa

[PATCH] arm: fix regression in ixp4xx clocksource

[Russell King - ARM Linux]

On Mon, May 30, 2011 at 10:43:07AM +0200, Richard Cochran wrote:
> Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> 
>    clocksource: convert ARM 32-bit up counting clocksources
> 
> broke the build for ixp4xx and made big endian operation impossible.
> This commit restores the original behaviour.

I'm really not happy about using the MMIO clocksource stuff with random
other read functions like this - it defeats the entire purpose of the
MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
and treat it as "special" for the time being.

Thomas - do you have any other views?

> 
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
>  arch/arm/mach-ixp4xx/common.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 74ed81a..0777257 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
>  /*
>   * clocksource
>   */
> +
> +static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
> +{
> +	return *IXP4XX_OSTS;
> +}
> +
>  unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
>  EXPORT_SYMBOL(ixp4xx_timer_freq);
>  static void __init ixp4xx_clocksource_init(void)
>  {
>  	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
>  
> -	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
> -			clocksource_mmio_readl_up);
> +	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
> +			ixp4xx_clocksource_read);
>  }
>  
>  /*
> -- 
> 1.7.0.4
> 

[PATCH] arm: fix regression in ixp4xx clocksource

[Thomas Gleixner]

Russell,

On Wed, 1 Jun 2011, Russell King - ARM Linux wrote:

> On Mon, May 30, 2011 at 10:43:07AM +0200, Richard Cochran wrote:
> > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> > 
> >    clocksource: convert ARM 32-bit up counting clocksources
> > 
> > broke the build for ixp4xx and made big endian operation impossible.
> > This commit restores the original behaviour.
> 
> I'm really not happy about using the MMIO clocksource stuff with random
> other read functions like this - it defeats the entire purpose of the
> MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
> and treat it as "special" for the time being.
> 
> Thomas - do you have any other views?

I have no objections to have special cased read functions as long as
all the other copied code is gone. We have the same problem with the
generic irq chip and I did not come up with a good decision function
where to draw the line. As for everything we come up with in the
consolidation space we need to apply common sense and keep an eye on
the real abusers.

Though in the mmio clocksource case we might ask the question whether
read[l|w]() is really necessary in the generic implemetation or
not. [too tired to answer that now ]
 
Thanks,

	tglx

[PATCH] arm: fix regression in ixp4xx clocksource

[Thomas Gleixner]

On Thu, 2 Jun 2011, Thomas Gleixner wrote:
> > I'm really not happy about using the MMIO clocksource stuff with random
> > other read functions like this - it defeats the entire purpose of the
> > MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
> > and treat it as "special" for the time being.
> > 
> > Thomas - do you have any other views?
> 
> I have no objections to have special cased read functions as long as
> all the other copied code is gone. We have the same problem with the
> generic irq chip and I did not come up with a good decision function
> where to draw the line. As for everything we come up with in the
> consolidation space we need to apply common sense and keep an eye on
> the real abusers.
> 
> Though in the mmio clocksource case we might ask the question whether
> read[l|w]() is really necessary in the generic implemetation or
> not. [too tired to answer that now ]

Thinking more about it we should add BE accessor functions to the mmio
clocksource as this might be useful for other architectures as well.

Thanks,

	tglx

[PATCH] arm: fix regression in ixp4xx clocksource

[Krzysztof Halasa]

Thomas Gleixner <tglx@linutronix.de> writes:

> Thinking more about it we should add BE accessor functions to the mmio
> clocksource as this might be useful for other architectures as well.

Why not simply use a value-preserving accessors like readl/writel?
Aren't they value-preserving on all ARM CPUs?

IMHO the CPU/platform code should supply certain accessors (value- and
order-preserving), and the generic code should simply use the correct
one.

Perhaps we should have (for ARM only) another set of register_readl()
etc.? (PCI and register endianness may differ, a separate set would mean
no runtime overhead in such case).
-- 
Krzysztof Halasa

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

On Wed, Jun 01, 2011 at 11:58:26PM +0100, Russell King - ARM Linux wrote:
> On Mon, May 30, 2011 at 10:43:07AM +0200, Richard Cochran wrote:
> > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> > 
> >    clocksource: convert ARM 32-bit up counting clocksources
> > 
> > broke the build for ixp4xx and made big endian operation impossible.
> > This commit restores the original behaviour.
> 
> I'm really not happy about using the MMIO clocksource stuff with random
> other read functions like this - it defeats the entire purpose of the
> MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
> and treat it as "special" for the time being.

I took another look at the whole MMIO clocksource implementation and
can't understand why all of the accessors use le_to_cpu. This seems to
imply that all peripheral buses are little endian (except for ixp,
which no one cares about, anyhow).

I understand (or have been told) that almost all arm implementations
out there are little endian. Why not use use a normal load to read the
timer? Are there machines with LE peripheral buses but BE cores?

Thanks,
Richard

[PATCH] arm: fix regression in ixp4xx clocksource

[Mikael Pettersson]

Richard Cochran writes:
 > On Wed, Jun 01, 2011 at 11:58:26PM +0100, Russell King - ARM Linux wrote:
 > > On Mon, May 30, 2011 at 10:43:07AM +0200, Richard Cochran wrote:
 > > > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
 > > > 
 > > >    clocksource: convert ARM 32-bit up counting clocksources
 > > > 
 > > > broke the build for ixp4xx and made big endian operation impossible.
 > > > This commit restores the original behaviour.
 > > 
 > > I'm really not happy about using the MMIO clocksource stuff with random
 > > other read functions like this - it defeats the entire purpose of the
 > > MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
 > > and treat it as "special" for the time being.
 > 
 > I took another look at the whole MMIO clocksource implementation and
 > can't understand why all of the accessors use le_to_cpu. This seems to
 > imply that all peripheral buses are little endian (except for ixp,
 > which no one cares about, anyhow).

"no one"? ixp4xx is still being used.

 > I understand (or have been told) that almost all arm implementations
 > out there are little endian. Why not use use a normal load to read the
 > timer? Are there machines with LE peripheral buses but BE cores?

ARM cores often have software-controllable endianess (see the ARM ARM).
Off the top of my head:
- ixp4xx: natively BE core and peripherals, but some people like to
  switch the core to LE for user-space SW compatibility reasons
  (I run my ixp4xx in BE as intended though)
- Kirkwood: natively LE core and peripherals, but the core is switchable
  to BE, although driver support for that appears to be incomplete

And then there were the old chips where the integer core and the FPA
(ancient FPU) could run with different endianesses ... lots of software
had problems with that; I'm glad EABI and VFP put an end to that insanity.

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

On Sat, Jun 11, 2011 at 01:22:18PM +0200, Mikael Pettersson wrote:
> Richard Cochran writes:
>  > I took another look at the whole MMIO clocksource implementation and
>  > can't understand why all of the accessors use le_to_cpu. This seems to
>  > imply that all peripheral buses are little endian (except for ixp,
>  > which no one cares about, anyhow).
> 
> "no one"? ixp4xx is still being used.

Yes, I know, but I was trying and failed to sound sacastic.

>  > I understand (or have been told) that almost all arm implementations
>  > out there are little endian. Why not use use a normal load to read the
>  > timer? Are there machines with LE peripheral buses but BE cores?
> 
> ARM cores often have software-controllable endianess (see the ARM ARM).
> Off the top of my head:
> - ixp4xx: natively BE core and peripherals, but some people like to
>   switch the core to LE for user-space SW compatibility reasons
>   (I run my ixp4xx in BE as intended though)
> - Kirkwood: natively LE core and peripherals, but the core is switchable
>   to BE, although driver support for that appears to be incomplete

So, would you say that using le_to_cpu in the MMIO clocksource reading
functions makes good sense, or not?

Thanks,
Richard

[PATCH] arm: fix regression in ixp4xx clocksource

[Mikael Pettersson]

Richard Cochran writes:
 > >  > I understand (or have been told) that almost all arm implementations
 > >  > out there are little endian. Why not use use a normal load to read the
 > >  > timer? Are there machines with LE peripheral buses but BE cores?
 > > 
 > > ARM cores often have software-controllable endianess (see the ARM ARM).
 > > Off the top of my head:
 > > - ixp4xx: natively BE core and peripherals, but some people like to
 > >   switch the core to LE for user-space SW compatibility reasons
 > >   (I run my ixp4xx in BE as intended though)
 > > - Kirkwood: natively LE core and peripherals, but the core is switchable
 > >   to BE, although driver support for that appears to be incomplete
 > 
 > So, would you say that using le_to_cpu in the MMIO clocksource reading
 > functions makes good sense, or not?

There has to be a place for optional endianess conversion (le_to_cpu,
be_to_cpu, or none at all) somewhere in the MMIO access paths, and it
has to be controlled by platform code.  I don't know the new clocksource
code well enough to suggest exactly where that should be done.

[PATCH] arm: fix regression in ixp4xx clocksource

[Krzysztof Halasa]

Mikael Pettersson <mikpe@it.uu.se> writes:

> ARM cores often have software-controllable endianess (see the ARM ARM).
> Off the top of my head:
> - ixp4xx: natively BE core and peripherals, but some people like to
>   switch the core to LE for user-space SW compatibility reasons
>   (I run my ixp4xx in BE as intended though)


Ok I'm back to work.

IXP4xx have some problems working in LE mode, namely:
- the network drivers (at least Ethernet and HSS) have to swap buffers
  because the NPE (network coprocessors) are BE,
- the hw crypto stuff doesn't mostly work on LE - parts work, but the
  rest seems to be broken in firmware (NPE microcode) - or so it seems
  (swapping the data buffers would work).

There is a special hardware in IXP4xx which makes it possible to
effectively switch the NPEs to LE. Unfortunately the first CPU revision
(IXP42x rev. A0) doesn't support it. And while I have the code working,
it's not "yet" upstream.

In short:
- all IXP4xx can work BE,
- IXP42x rev. A0 can work LE with impaired network transfers and hw
  crypto,
- later IXP4xx can work "fully" LE (not upstream).
-- 
Krzysztof Halasa

[PATCH] arm: fix regression in ixp4xx clocksource

[Russell King - ARM Linux]

On Sat, Jun 11, 2011 at 07:15:10AM +0200, Richard Cochran wrote:
> On Wed, Jun 01, 2011 at 11:58:26PM +0100, Russell King - ARM Linux wrote:
> > On Mon, May 30, 2011 at 10:43:07AM +0200, Richard Cochran wrote:
> > > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> > > 
> > >    clocksource: convert ARM 32-bit up counting clocksources
> > > 
> > > broke the build for ixp4xx and made big endian operation impossible.
> > > This commit restores the original behaviour.
> > 
> > I'm really not happy about using the MMIO clocksource stuff with random
> > other read functions like this - it defeats the entire purpose of the
> > MMIO clocksource stuff.  Maybe we should just undo the change for IXP4xx
> > and treat it as "special" for the time being.
> 
> I took another look at the whole MMIO clocksource implementation and
> can't understand why all of the accessors use le_to_cpu. This seems to
> imply that all peripheral buses are little endian (except for ixp,
> which no one cares about, anyhow).

What would they use as an ioremap-compatible interface instead of
readl()/readw() ?

readl_be()/readw_be() doesn't work for the majority of platforms using
this stuff.  We also don't have native endian equivalents across the
kernel.  Not everyone provides the relaxed versions.

It's basically a problem, and its one of the unfortunate prices paid
by consolidating code - things become harder and less efficient to do.

In this case, I think we need a set of _be() clocksource accessors in
mmio.c, and leave it up to the platform to select the correct accessor.

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

On Sun, Jun 12, 2011 at 02:51:59PM +0100, Russell King - ARM Linux wrote:
>
> It's basically a problem, and its one of the unfortunate prices paid
> by consolidating code - things become harder and less efficient to do.
> 
> In this case, I think we need a set of _be() clocksource accessors in
> mmio.c, and leave it up to the platform to select the correct accessor.

So, is IXP the only machine that would need such accessors?

If so, then I think just using my patch as a single exception would be
fine (maybe adding a comment justifying the exception).

Thanks,
Richard

[PATCH] arm: fix regression in ixp4xx clocksource

[Krzysztof Halasa]

> Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
>
>    clocksource: convert ARM 32-bit up counting clocksources
>
> broke the build for ixp4xx and made big endian operation impossible.
> This commit restores the original behaviour.

So, what do we do with this?
It would be nice to have the fix in the tree before 3.0.
I guess I'll add this if nothing better shows up.

Richard Cochran <richardcochran@gmail.com> writes:

>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
>  arch/arm/mach-ixp4xx/common.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-ixp4xx/common.c b/arch/arm/mach-ixp4xx/common.c
> index 74ed81a..0777257 100644
> --- a/arch/arm/mach-ixp4xx/common.c
> +++ b/arch/arm/mach-ixp4xx/common.c
> @@ -419,14 +419,20 @@ static void notrace ixp4xx_update_sched_clock(void)
>  /*
>   * clocksource
>   */
> +
> +static cycle_t ixp4xx_clocksource_read(struct clocksource *c)
> +{
> +	return *IXP4XX_OSTS;
> +}
> +
>  unsigned long ixp4xx_timer_freq = IXP4XX_TIMER_FREQ;
>  EXPORT_SYMBOL(ixp4xx_timer_freq);
>  static void __init ixp4xx_clocksource_init(void)
>  {
>  	init_sched_clock(&cd, ixp4xx_update_sched_clock, 32, ixp4xx_timer_freq);
>  
> -	clocksource_mmio_init(&IXP4XX_OSTS, "OSTS", ixp4xx_timer_freq, 200, 32,
> -			clocksource_mmio_readl_up);
> +	clocksource_mmio_init(NULL, "OSTS", ixp4xx_timer_freq, 200, 32,
> +			ixp4xx_clocksource_read);
>  }
>  
>  /*

-- 
Krzysztof Halasa

[PATCH] arm: fix regression in ixp4xx clocksource

[Richard Cochran]

On Sun, Jun 26, 2011 at 03:38:05PM +0200, Krzysztof Halasa wrote:
> > Commit 234b6ceddb4fc2a4bc5b9a7670f070f6e69e0868
> >
> >    clocksource: convert ARM 32-bit up counting clocksources
> >
> > broke the build for ixp4xx and made big endian operation impossible.
> > This commit restores the original behaviour.
> 
> So, what do we do with this?
> It would be nice to have the fix in the tree before 3.0.
> I guess I'll add this if nothing better shows up.

I'll say it once again:

If IXP is the only machine that needs this exception, then I think my
patch is okay. Otherwise, we would want a set of big-endian-friendly
clock reading methods.

But I really don't know whether IXP is all alone in this...

Just my 2 cents,

Richard