From: tglx@linutronix.de (Thomas Gleixner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] clockevents: introduce ->set_dev_mode() which can return error
Date: Sun, 25 Jan 2015 16:03:41 +0100 (CET) [thread overview]
Message-ID: <alpine.DEB.2.11.1501251537380.5526@nanos> (raw)
In-Reply-To: <CAKohpomZJAymPHVNR9LkHM8V2tkmM=H+btYEiAJ9hq0mAoOzYw@mail.gmail.com>
> > case CLOCK_EVT_MODE_RESUME:
> > return dev->setup_resume(dev);
>
> Because setting to all these existing modes isn't allowed
> to fail currently, shouldn't we make return type of all the new
> callbacks as 'void' and return 0 from this routine ? That way, these
> callbacks would stay consistent with the set_mode() callback..
If we go to individual callbacks then we analyze of a case by case
basis which ones need a return value and which ones to not.
> > That gives us a clear distinction of required and optional callbacks,
> > which will make error handling and recovery simpler as well.
>
> Now that we are looking forward to providing individual callbacks
> per feature, I wanted to propose few more solutions to it, though I am
> quite sure you would have already considered and rejected them.
>
> I remember from your earlier mail that you didn't wanted to add
> ONESHOT_STOPPED as a feature (as it isn't a feature really)
> and that restricts us from doing this:
>
> static int __clockevents_set_mode(dev, mode)
> {
> if ((mode == CLOCK_EVT_MODE_ONESHOT_STOPPED) &&
> !(dev->features & CLOCK_EVT_FEAT_ONESHOT_STOPPED))
> return -ENOSYS;
>
> dev->set_mode(mode, dev);
> return 0;
> }
>
> But what about providing a separate callback only for ONESHOT_STOPPED
> mode and use set_mode() for everything else ?
And then fixup all drivers which do not have a default clause in their
switch case and add MODE_ONESHOT_STOPPED? And half a year later we do
the same for the next mode. No way.
The current interface is suboptimal and we fix it proper. Period.
Thanks,
tglx
next prev parent reply other threads:[~2015-01-25 15:03 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-09 22:03 [PATCH 0/2] clockevents: introduce ->set_dev_mode() and convert a few drivers Kevin Hilman
2014-12-09 22:03 ` [PATCH 1/2] clockevents: introduce ->set_dev_mode() which can return error Kevin Hilman
2014-12-10 3:27 ` Preeti U Murthy
2015-01-23 11:57 ` Thomas Gleixner
2015-01-25 13:57 ` Viresh Kumar
2015-01-25 15:03 ` Thomas Gleixner [this message]
2014-12-09 22:03 ` [PATCH 2/2] clockevents: migrate some drivers to new ->set_dev_mode() Kevin Hilman
2015-01-15 18:41 ` [PATCH 0/2] clockevents: introduce ->set_dev_mode() and convert a few drivers Kevin Hilman
2015-01-15 22:25 ` Thomas Gleixner
-- strict thread matches above, loose matches on Subject: below --
2014-11-19 18:39 Kevin Hilman
2014-11-19 18:39 ` [PATCH 1/2] clockevents: introduce ->set_dev_mode() which can return error Kevin Hilman
2014-11-20 4:04 ` Preeti U Murthy
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=alpine.DEB.2.11.1501251537380.5526@nanos \
--to=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox