All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Nishanth Menon <nm@ti.com>, Dmitry Torokhov <dtor@chromium.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] PM / OPP: fix debugfs files for 64-bit
Date: Wed, 07 Oct 2015 21:12:55 +0200	[thread overview]
Message-ID: <5308760.2G5KJFF9Pi@wuerfel> (raw)
In-Reply-To: <20151007130324.GA4557@linux>

On Wednesday 07 October 2015 18:33:24 Viresh Kumar wrote:
> Cc'ing Mike and Stephen..
> 
> On 07-10-15, 13:57, Greg Kroah-Hartman wrote:
> > On Wed, Oct 07, 2015 at 04:51:49PM +0530, Viresh Kumar wrote:
> > > On 07-10-15, 12:07, Greg Kroah-Hartman wrote:
> > > > Why would you be wanting to create a "unsigned long" as an api anyway?
> > > > Just force it to be u64 all the time, can't you do that?
> > > 
> > > Okay, so the variable in question (lets say frequency) is an 'unsigned
> > > long' and that's how all the APIs of clock framework expect/define
> > > it.
> > > 
> > > And you are probably saying that we do this:
> > > 
> > > unsigned long freq;
> > > 
> > > debugfs_create_u64((u64 *)&freq);
> > > 
> > > Right? Or are you asking to update clock APIs to be converted to u64?
> > 
> > Yes, they should be u64 as I doubt you want to debug problems that you
> > have in the driver where it works on a 64bit system but doesn't on a
> > 32bit one.
> 
> Firstly changing the clock API (and other similar APIs) to make
> frequency u64 instead of 'unsigned long', looks like a giant effort.
> There are too many users of those API, etc..
> 
> Over that, it might be good performance wise to use u32 for 32 bit
> systems and u64 for 64 bit one, to represent clock frequency and maybe
> that's why we chose unsigned long there.

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.

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.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PM / OPP: fix debugfs files for 64-bit
Date: Wed, 07 Oct 2015 21:12:55 +0200	[thread overview]
Message-ID: <5308760.2G5KJFF9Pi@wuerfel> (raw)
In-Reply-To: <20151007130324.GA4557@linux>

On Wednesday 07 October 2015 18:33:24 Viresh Kumar wrote:
> Cc'ing Mike and Stephen..
> 
> On 07-10-15, 13:57, Greg Kroah-Hartman wrote:
> > On Wed, Oct 07, 2015 at 04:51:49PM +0530, Viresh Kumar wrote:
> > > On 07-10-15, 12:07, Greg Kroah-Hartman wrote:
> > > > Why would you be wanting to create a "unsigned long" as an api anyway?
> > > > Just force it to be u64 all the time, can't you do that?
> > > 
> > > Okay, so the variable in question (lets say frequency) is an 'unsigned
> > > long' and that's how all the APIs of clock framework expect/define
> > > it.
> > > 
> > > And you are probably saying that we do this:
> > > 
> > > unsigned long freq;
> > > 
> > > debugfs_create_u64((u64 *)&freq);
> > > 
> > > Right? Or are you asking to update clock APIs to be converted to u64?
> > 
> > Yes, they should be u64 as I doubt you want to debug problems that you
> > have in the driver where it works on a 64bit system but doesn't on a
> > 32bit one.
> 
> Firstly changing the clock API (and other similar APIs) to make
> frequency u64 instead of 'unsigned long', looks like a giant effort.
> There are too many users of those API, etc..
> 
> Over that, it might be good performance wise to use u32 for 32 bit
> systems and u64 for 64 bit one, to represent clock frequency and maybe
> that's why we chose unsigned long there.

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.

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.

	Arnd

  parent reply	other threads:[~2015-10-07 19:13 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-07  7:35 [PATCH] PM / OPP: fix debugfs files for 64-bit Arnd Bergmann
2015-10-07  7:35 ` Arnd Bergmann
2015-10-07 10:59 ` Viresh Kumar
2015-10-07 10:59   ` Viresh Kumar
2015-10-07 11:03   ` Viresh Kumar
2015-10-07 11:03     ` Viresh Kumar
2015-10-07 11:07     ` Greg Kroah-Hartman
2015-10-07 11:07       ` Greg Kroah-Hartman
2015-10-07 11:21       ` Viresh Kumar
2015-10-07 11:21         ` Viresh Kumar
2015-10-07 12:57         ` Greg Kroah-Hartman
2015-10-07 12:57           ` Greg Kroah-Hartman
2015-10-07 13:03           ` Viresh Kumar
2015-10-07 13:03             ` Viresh Kumar
2015-10-07 13:03             ` Viresh Kumar
2015-10-07 17:19             ` Greg Kroah-Hartman
2015-10-07 17:19               ` Greg Kroah-Hartman
2015-10-07 17:27               ` Viresh Kumar
2015-10-07 17:27                 ` Viresh Kumar
2015-10-07 17:39                 ` Greg Kroah-Hartman
2015-10-07 17:39                   ` Greg Kroah-Hartman
2015-10-07 18:00                   ` Viresh Kumar
2015-10-07 18:00                     ` Viresh Kumar
2015-10-07 19:12             ` Arnd Bergmann [this message]
2015-10-07 19:12               ` Arnd Bergmann
2015-10-08  7:48               ` Viresh Kumar
2015-10-08  7:48                 ` Viresh Kumar
2015-10-08 14:25                 ` Alan Stern
2015-10-08 14:25                   ` Alan Stern
2015-10-08 14:25                   ` Alan Stern
2015-10-19 15:40                 ` Michael Turquette
2015-10-19 15:40                   ` Michael Turquette
2015-10-19 15:53                   ` Russell King - ARM Linux
2015-10-19 15:53                     ` Russell King - ARM Linux
2015-10-20 10:22                     ` Michael Turquette
2015-10-20 10:22                       ` Michael Turquette
2015-10-07 16:33         ` Russell King - ARM Linux
2015-10-07 16:33           ` Russell King - ARM Linux
2015-10-07 16:36           ` Viresh Kumar
2015-10-07 16:36             ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5308760.2G5KJFF9Pi@wuerfel \
    --to=arnd@arndb.de \
    --cc=b.zolnierkie@samsung.com \
    --cc=dtor@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=len.brown@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=nm@ti.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@codeaurora.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.