RE: [PATCH] clocksource: Add BE/LE APIs support for clocksource counter reading.

2014-09-10 Thread li.xi...@freescale.com
> Cc: daniel.lezc...@linaro.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] clocksource: Add BE/LE APIs support for clocksource
> counter reading.
> 
> On Wed, 10 Sep 2014, Xiubo Li wrote:
> 
> So you add BE/LE APIs according to the subject line, but you fail
> again to tell WHY. If that would be the only issue 
> 
> > Signed-off-by: Xiubo Li 
> > ---
> >  drivers/clocksource/mmio.c | 44
> 
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> > index 1593ade..ddc5214 100644
> > --- a/drivers/clocksource/mmio.c
> > +++ b/drivers/clocksource/mmio.c
> 
> So you think that adding this to mmio.c is the solution to the
> problem?
> 

It's just one solution of this, and will be a little faster than the old one
When reading the counter value.


> Ever heard about function prototypes, header files and such?
> 

Yes, this will be fixed later if this is acceptable.

> > @@ -20,21 +20,65 @@ static inline struct clocksource_mmio
> *to_mmio_clksrc(struct clocksource *c)
> > return container_of(c, struct clocksource_mmio, clksrc);
> >  }
> >
> > +cycle_t clocksource_mmio_readl_up_be(struct clocksource *c)
> > +{
> > +   return (cycle_t)be32_to_cpu(readl_relaxed(to_mmio_clksrc(c)->reg));
> > +}
> > +
> > +cycle_t clocksource_mmio_readl_up_le(struct clocksource *c)
> > +{
> > +   return (cycle_t)le32_to_cpu(readl_relaxed(to_mmio_clksrc(c)->reg));
> > +}
> > +
> >  cycle_t clocksource_mmio_readl_up(struct clocksource *c)
> >  {
> > return (cycle_t)readl_relaxed(to_mmio_clksrc(c)->reg);
> >  }
> 
> So now we have three global functions which provide access to the mmio
> clocksource. And one of them is always redundant for a given compile
> time endianess. You can't be serious about this...
> 
 
Yes, any good suggestion about this ?

I found whatever the method it is about the endianness will always be many
Different voices from different people and found it hard to taste all of them. 

So firstly I just using the driver's local APIs.

Thanks,

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


Re: [PATCH] clocksource: Add BE/LE APIs support for clocksource counter reading.

2014-09-10 Thread Thomas Gleixner
On Wed, 10 Sep 2014, Xiubo Li wrote:

So you add BE/LE APIs according to the subject line, but you fail
again to tell WHY. If that would be the only issue 
 
> Signed-off-by: Xiubo Li 
> ---
>  drivers/clocksource/mmio.c | 44 
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> index 1593ade..ddc5214 100644
> --- a/drivers/clocksource/mmio.c
> +++ b/drivers/clocksource/mmio.c

So you think that adding this to mmio.c is the solution to the
problem?

Ever heard about function prototypes, header files and such? 

> @@ -20,21 +20,65 @@ static inline struct clocksource_mmio 
> *to_mmio_clksrc(struct clocksource *c)
>   return container_of(c, struct clocksource_mmio, clksrc);
>  }
>  
> +cycle_t clocksource_mmio_readl_up_be(struct clocksource *c)
> +{
> + return (cycle_t)be32_to_cpu(readl_relaxed(to_mmio_clksrc(c)->reg));
> +}
> +
> +cycle_t clocksource_mmio_readl_up_le(struct clocksource *c)
> +{
> + return (cycle_t)le32_to_cpu(readl_relaxed(to_mmio_clksrc(c)->reg));
> +}
> +
>  cycle_t clocksource_mmio_readl_up(struct clocksource *c)
>  {
>   return (cycle_t)readl_relaxed(to_mmio_clksrc(c)->reg);
>  }

So now we have three global functions which provide access to the mmio
clocksource. And one of them is always redundant for a given compile
time endianess. You can't be serious about this...

But ... wait, I've seen enough qualitee code from Fleascale in the
past 10 years, so you actually might be serious about it.

JFYI: You are trying to submit code to the public maintained codebase
of the Linux kernel, not to some random sh^H^Hgit repository inside of
Fleascale which accepts any random amount of crap. Can you see the
difference?

I can see it rather clearly.

Thanks,

tglx



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


Re: [PATCH] clocksource: Add BE/LE APIs support for clocksource counter reading.

2014-09-10 Thread Thomas Gleixner
On Wed, 10 Sep 2014, Xiubo Li wrote:

So you add BE/LE APIs according to the subject line, but you fail
again to tell WHY. If that would be the only issue 
 
 Signed-off-by: Xiubo Li li.xi...@freescale.com
 ---
  drivers/clocksource/mmio.c | 44 
  1 file changed, 44 insertions(+)
 
 diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
 index 1593ade..ddc5214 100644
 --- a/drivers/clocksource/mmio.c
 +++ b/drivers/clocksource/mmio.c

So you think that adding this to mmio.c is the solution to the
problem?

Ever heard about function prototypes, header files and such? 

 @@ -20,21 +20,65 @@ static inline struct clocksource_mmio 
 *to_mmio_clksrc(struct clocksource *c)
   return container_of(c, struct clocksource_mmio, clksrc);
  }
  
 +cycle_t clocksource_mmio_readl_up_be(struct clocksource *c)
 +{
 + return (cycle_t)be32_to_cpu(readl_relaxed(to_mmio_clksrc(c)-reg));
 +}
 +
 +cycle_t clocksource_mmio_readl_up_le(struct clocksource *c)
 +{
 + return (cycle_t)le32_to_cpu(readl_relaxed(to_mmio_clksrc(c)-reg));
 +}
 +
  cycle_t clocksource_mmio_readl_up(struct clocksource *c)
  {
   return (cycle_t)readl_relaxed(to_mmio_clksrc(c)-reg);
  }

So now we have three global functions which provide access to the mmio
clocksource. And one of them is always redundant for a given compile
time endianess. You can't be serious about this...

But ... wait, I've seen enough qualitee code from Fleascale in the
past 10 years, so you actually might be serious about it.

JFYI: You are trying to submit code to the public maintained codebase
of the Linux kernel, not to some random sh^H^Hgit repository inside of
Fleascale which accepts any random amount of crap. Can you see the
difference?

I can see it rather clearly.

Thanks,

tglx



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


RE: [PATCH] clocksource: Add BE/LE APIs support for clocksource counter reading.

2014-09-10 Thread li.xi...@freescale.com
 Cc: daniel.lezc...@linaro.org; linux-kernel@vger.kernel.org
 Subject: Re: [PATCH] clocksource: Add BE/LE APIs support for clocksource
 counter reading.
 
 On Wed, 10 Sep 2014, Xiubo Li wrote:
 
 So you add BE/LE APIs according to the subject line, but you fail
 again to tell WHY. If that would be the only issue 
 
  Signed-off-by: Xiubo Li li.xi...@freescale.com
  ---
   drivers/clocksource/mmio.c | 44
 
   1 file changed, 44 insertions(+)
 
  diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
  index 1593ade..ddc5214 100644
  --- a/drivers/clocksource/mmio.c
  +++ b/drivers/clocksource/mmio.c
 
 So you think that adding this to mmio.c is the solution to the
 problem?
 

It's just one solution of this, and will be a little faster than the old one
When reading the counter value.


 Ever heard about function prototypes, header files and such?
 

Yes, this will be fixed later if this is acceptable.

  @@ -20,21 +20,65 @@ static inline struct clocksource_mmio
 *to_mmio_clksrc(struct clocksource *c)
  return container_of(c, struct clocksource_mmio, clksrc);
   }
 
  +cycle_t clocksource_mmio_readl_up_be(struct clocksource *c)
  +{
  +   return (cycle_t)be32_to_cpu(readl_relaxed(to_mmio_clksrc(c)-reg));
  +}
  +
  +cycle_t clocksource_mmio_readl_up_le(struct clocksource *c)
  +{
  +   return (cycle_t)le32_to_cpu(readl_relaxed(to_mmio_clksrc(c)-reg));
  +}
  +
   cycle_t clocksource_mmio_readl_up(struct clocksource *c)
   {
  return (cycle_t)readl_relaxed(to_mmio_clksrc(c)-reg);
   }
 
 So now we have three global functions which provide access to the mmio
 clocksource. And one of them is always redundant for a given compile
 time endianess. You can't be serious about this...
 
 
Yes, any good suggestion about this ?

I found whatever the method it is about the endianness will always be many
Different voices from different people and found it hard to taste all of them. 

So firstly I just using the driver's local APIs.

Thanks,

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