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

Groups > linux.kernel > #1250858 > unrolled thread

Re: [PATCH] PM / OPP: fix debugfs files for 64-bit

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

  Re: [PATCH] PM / OPP: fix debugfs files for 64-bit Michael Turquette <mturquette@baylibre.com> - 2015-10-19 17:50 +0200
    Re: [PATCH] PM / OPP: fix debugfs files for 64-bit Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-10-19 18:00 +0200

#1250858 — Re: [PATCH] PM / OPP: fix debugfs files for 64-bit

Quoting Viresh Kumar (2015-10-08 00:48:28)
> On 07-10-15, 21:12, Arnd Bergmann wrote:
> > I think it clearly makes sense to have a fixed length for each of these
> > members:
> >
> > either 32 bit is enough to represent all possible values, then
> > there is no need to make them 'long' on 64-bit architectures, or 32 bit
> > is not enough and then the code is broken on 32-bit architectures today
> > and should be fixed.
> 
> I agree.
> 
> But I am not 100% sure why it was done this way to start with.
> Probably this is the logic behind that:
> - Max clock rate supported by a u32 is ~ 4.295 GHz
> - People expected that, we will not reach this rate for 32 bit systems
>   but for 64 bit ones.
> - If above is true, then making it u64 for all will generate not very
>   optimized code for 32bit systems, as we need to fetch two 32bit
>   values everytime then.
> - And making it u32 for 64 bit systems wouldn't be great as well, as
>   we need to mask out half of the read value.
> 
> Ofcourse, Mike and Stephen can correct me here :)

I chose unsigned long for the common clock framework _implemenation_,
because the long-standing clk.h _api_ returns this type for clk_get_rate
and passes this type in for clk_set_rate and clk_round_rate.

> 
> > In my patch, I assumed that if 32-bit architectures work fine today, then
> > we don't need more range on 64-bit architectures either.
> 
> The problem here is that we haven't fixed it properly.
> - clock framework expects it to be unsigned long
> - DT is sending a 64 bit value in Hz
> - But we are storing and exposing it in u32

Don't forget cpufreq is using unsigned int for KHz.

> 
> That's weird, isn't it?

Yes it is.

> 
> So, either we update clock API and other similar APIs to u64 or u32
> (which may not be the right thing to do), Or we keep it unsigned long
> here as well and add debugfs_create_ulong().

This comes up every now and then. I'm still trying to figure out if
sub-Hertz quantities should be considered (e.g. representing freqs in
milliHertz).

Regards,
Mike

> 
> -- 
> viresh
--
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]

#1250864

On Mon, Oct 19, 2015 at 08:40:09AM -0700, Michael Turquette wrote:
> Quoting Viresh Kumar (2015-10-08 00:48:28)
> > On 07-10-15, 21:12, Arnd Bergmann wrote:
> > > I think it clearly makes sense to have a fixed length for each of these
> > > members:
> > >
> > > either 32 bit is enough to represent all possible values, then
> > > there is no need to make them 'long' on 64-bit architectures, or 32 bit
> > > is not enough and then the code is broken on 32-bit architectures today
> > > and should be fixed.
> > 
> > I agree.
> > 
> > But I am not 100% sure why it was done this way to start with.
> > Probably this is the logic behind that:
> > - Max clock rate supported by a u32 is ~ 4.295 GHz

The limit is actually half that when you consider clk_round_rate()
returns a long, because clk_round_rate() needs to be able to return
errors.

The use of long and unsigned long was done in the knowledge that it'd
restrict to 2GHz the upper limit, partially because 64-bit math on
32-bit systems is inefficient and expensive, and I initially wanted
people to use the thing, so it had to be lean and fast.  (If I'd
started out with something as complex as the CCF, it would've been
laughed out as being "far too complex" and "we don't need this").

It also started out as a way to convey clock information to drivers
for peripherals, which commonly don't need clocks up in the GHz range,
indeed, it was rare at that time the API was designed for peripheral
clocks to be much above 100MHz.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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