All of lore.kernel.org
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 1/4] clockevents: Add generic timer broadcast receiver
Date: Tue, 15 Jan 2013 12:10:43 +0530	[thread overview]
Message-ID: <50F4F9EB.3010405@ti.com> (raw)
In-Reply-To: <20130114153612.GF7990@e106331-lin.cambridge.arm.com>

On Monday 14 January 2013 09:06 PM, Mark Rutland wrote:
> On Mon, Jan 14, 2013 at 02:17:26PM +0000, Thomas Gleixner wrote:
>> On Mon, 14 Jan 2013, Mark Rutland wrote:
>>> On Mon, Jan 14, 2013 at 11:50:55AM +0000, Thomas Gleixner wrote:
>>>> On Mon, 14 Jan 2013, Mark Rutland wrote:
>>>>
>>>>> On Mon, Jan 14, 2013 at 11:06:31AM +0000, Thomas Gleixner wrote:
>>>>>> On Wed, 9 Jan 2013, Mark Rutland wrote:
>>>>>>> +#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
>>>>>>> +extern int tick_receive_broadcast(void);
>>>>>>> +#else
>>>>>>> +static inline int tick_receive_broadcast(void)
>>>>>>> +{
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>
>>>>>> What's the inline function for? If an arch does not have broadcasting
>>>>>> support it should not have a receive broadcast function call either.
>>>>>
>>>>> That was how this was originally structured [1], but Santosh suggested this
>>>>> would break the build for !GENERIC_CLOCKEVENTS_BROADCAST [1]. It means that the
>>>>> arch-specific receive path (i.e. IPI handler) doesn't have to be #ifdef'd,
>>>>> which makes it less ugly.
>>>>
>>>> Hmm. If you want to keep the IPI around unconditionally the inline
>>>> makes some sense, though the question is whether keeping an unused IPI
>>>> around makes sense in the first place. I'd rather see a warning that
>>>> an unexpected IPI happened than a silent inline function being called.
>>>
>>> How about I add a warning (e.g. "Impossible timer broadcast received.") and
>>> return -EOPNOTSUPP when !GENERIC_CLOCKEVENTS_BROADCAST?
>>
>> You still need to do something with the return value in the arch IPI
>> code, right?
>
> Good point. Having the stub when !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST is
> clearly problematic.
>
> I'll go with your original suggestion, removing the tick_receive_broadcast stub
> for !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST and I'll #idef the IPI_TIMER handler.
> That way it'll fall down to the standard warning for an unexpected/unknown IPI
> for arch/arm at least.
>
The alternative is fine by me.

Regards
santosh

WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"nico@linaro.org" <nico@linaro.org>,
	Marc Zyngier <Marc.Zyngier@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"john.stultz@linaro.org" <john.stultz@linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCHv2 1/4] clockevents: Add generic timer broadcast receiver
Date: Tue, 15 Jan 2013 12:10:43 +0530	[thread overview]
Message-ID: <50F4F9EB.3010405@ti.com> (raw)
In-Reply-To: <20130114153612.GF7990@e106331-lin.cambridge.arm.com>

On Monday 14 January 2013 09:06 PM, Mark Rutland wrote:
> On Mon, Jan 14, 2013 at 02:17:26PM +0000, Thomas Gleixner wrote:
>> On Mon, 14 Jan 2013, Mark Rutland wrote:
>>> On Mon, Jan 14, 2013 at 11:50:55AM +0000, Thomas Gleixner wrote:
>>>> On Mon, 14 Jan 2013, Mark Rutland wrote:
>>>>
>>>>> On Mon, Jan 14, 2013 at 11:06:31AM +0000, Thomas Gleixner wrote:
>>>>>> On Wed, 9 Jan 2013, Mark Rutland wrote:
>>>>>>> +#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
>>>>>>> +extern int tick_receive_broadcast(void);
>>>>>>> +#else
>>>>>>> +static inline int tick_receive_broadcast(void)
>>>>>>> +{
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>
>>>>>> What's the inline function for? If an arch does not have broadcasting
>>>>>> support it should not have a receive broadcast function call either.
>>>>>
>>>>> That was how this was originally structured [1], but Santosh suggested this
>>>>> would break the build for !GENERIC_CLOCKEVENTS_BROADCAST [1]. It means that the
>>>>> arch-specific receive path (i.e. IPI handler) doesn't have to be #ifdef'd,
>>>>> which makes it less ugly.
>>>>
>>>> Hmm. If you want to keep the IPI around unconditionally the inline
>>>> makes some sense, though the question is whether keeping an unused IPI
>>>> around makes sense in the first place. I'd rather see a warning that
>>>> an unexpected IPI happened than a silent inline function being called.
>>>
>>> How about I add a warning (e.g. "Impossible timer broadcast received.") and
>>> return -EOPNOTSUPP when !GENERIC_CLOCKEVENTS_BROADCAST?
>>
>> You still need to do something with the return value in the arch IPI
>> code, right?
>
> Good point. Having the stub when !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST is
> clearly problematic.
>
> I'll go with your original suggestion, removing the tick_receive_broadcast stub
> for !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST and I'll #idef the IPI_TIMER handler.
> That way it'll fall down to the standard warning for an unexpected/unknown IPI
> for arch/arm at least.
>
The alternative is fine by me.

Regards
santosh


  reply	other threads:[~2013-01-15  6:40 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-09 14:46 [PATCHv2 0/4] clockevents: decouple broadcast mechanism from drivers Mark Rutland
2013-01-09 14:46 ` Mark Rutland
2013-01-09 14:46 ` [PATCHv2 1/4] clockevents: Add generic timer broadcast receiver Mark Rutland
2013-01-09 14:46   ` Mark Rutland
2013-01-14 11:06   ` Thomas Gleixner
2013-01-14 11:06     ` Thomas Gleixner
2013-01-14 11:29     ` Mark Rutland
2013-01-14 11:29       ` Mark Rutland
2013-01-14 11:50       ` Thomas Gleixner
2013-01-14 11:50         ` Thomas Gleixner
2013-01-14 12:12         ` Mark Rutland
2013-01-14 12:12           ` Mark Rutland
2013-01-14 14:17           ` Thomas Gleixner
2013-01-14 14:17             ` Thomas Gleixner
2013-01-14 15:36             ` Mark Rutland
2013-01-14 15:36               ` Mark Rutland
2013-01-15  6:40               ` Santosh Shilimkar [this message]
2013-01-15  6:40                 ` Santosh Shilimkar
2013-01-15 11:24               ` Thomas Gleixner
2013-01-15 11:24                 ` Thomas Gleixner
2013-01-15 12:00                 ` Mark Rutland
2013-01-15 12:00                   ` Mark Rutland
2013-01-09 14:46 ` [PATCHv2 2/4] arm: Use " Mark Rutland
2013-01-09 14:46   ` Mark Rutland
2013-01-09 14:46 ` [PATCHv2 3/4] clockevents: Add generic timer broadcast function Mark Rutland
2013-01-09 14:46   ` Mark Rutland
2013-01-14 11:10   ` Thomas Gleixner
2013-01-14 11:10     ` Thomas Gleixner
2013-01-09 14:46 ` [PATCHv2 4/4] arm: Add generic timer broadcast support Mark Rutland
2013-01-09 14:46   ` Mark Rutland
2013-01-09 20:46 ` [PATCHv2 0/4] clockevents: decouple broadcast mechanism from drivers Stephen Boyd
2013-01-09 20:46   ` Stephen Boyd
2013-01-10  9:44   ` Mark Rutland
2013-01-10  9:44     ` Mark Rutland

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=50F4F9EB.3010405@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.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.