* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 11:01 UTC (permalink / raw)
To: Gabriel Beddingfield
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, hharte
In-Reply-To: <CAOdF7ntda5TuCDBUTsuPZ0TCofo-GVfBmrGVyPk-hqGgHEkBQw@mail.gmail.com>
On Wed, 4 Oct 2017, Gabriel Beddingfield wrote:
> On Wed, Oct 4, 2017 at 11:22 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Long story short: you can't always have your low-power clock be your
> monotonic/sched clock.
sched_clock and the clocksource for timekeeping, which feeds monotonic are
not required to be the same thing.
> The SoC we use backs the monotonic clock (sched_clock_register()) with a
Again. monotonic clock != sched clock. The clocksource which feeds the
monotonic timekeeper clock is registered via clocksource_register() & al.
> counter that is high frequency (>10 MHz) in their reference
> implementation. But it does not count when the system is in low-power
> mode. However, it can be configured to use a 32kHz clock that *does*
> count when the system is in low-power mode. So, we started by using this
> clock and setting the CLOCK_SOURCE_SUSPEND_NONSTOP flag. It worked
> great... at first.
>
> Then we found that devices would randomly experience a 36-hour time jump.
> While we don't have a definitive root cause, the current theory is that
> we are getting non-atomic reads because the clock source isn't
> synchronized with the the high frequency clock (which is used for most of
> the digital logic on the SoC).
Groan. Engineering based on theories is doomed to begin with.
Your 36 hour time jump is probably exactly 36.4089 hours as that's
((1 << 32) / 32768) / 3600
i.e. the 32bit rollover of the clocksource. So, if the clocksource->read()
function returns a full 64bit counter value, then it must have protection
against observing the rollover independent of the clock which feeds that
counter. Of course the frequency changes the probablity of observing it,
but still the read function must be protected against observing the
rollover unconditionally.
Which SoC/clocksource driver are you talking about?
> Therefore, we moved the monotonic/sched clock back to the high-frequency source.
Please stop confusing timekeeping clock source and sched clock. They might
be the same physical device but conceptually they are different.
> Meanwhile, we can directly read the RTC clock on this system, and it will
> give us 32kHz resolution and also runs non-stop. Since reads are
> non-atomic, we have to read the registers in a loop. We used this
> register to implement read_persistent_clock64(). Because we have to read
> the registers in a loop, it seemed unfit for use as the monotonic/sched
> clock.
I can understand that. Though, using that value for injecting accurate
sleep time should just work with the existing code no matter how long the
actual sleep time was. The timekeeping core takes the nsec part of the
timespec value retrieved via read_persistent_clock64() into account.
I still have a hard time to figure out what you are trying to achieve.
Thanks,
tglx
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 11:05 UTC (permalink / raw)
To: John Stultz
Cc: Gabriel Beddingfield, LKML, Stephen Boyd, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, hharte, Miroslav Lichvar
In-Reply-To: <CALAqxLUO-62B0umVSUK979A+dApOLN3xt5nnZA5HKHsWnRz3KQ@mail.gmail.com>
On Wed, 4 Oct 2017, John Stultz wrote:
> On Wed, Oct 4, 2017 at 9:11 AM, Gabriel Beddingfield <gabe@nestlabs.com> wrote:
> > TL;DR: the "delta_delta" hack[1 and 2] in kernel/time/timekeeping.c
> > and drivers/rtc/class.c undermines the NTP system. It's not
> > appropriate to use if sub-second precision is available. I've attached
> > a patch to resolve this... please let me know the ways you hate it.
> > :-)
> >
> > Hello Kernel Timekeeping Maintainers,
> >
> > We have been developing a device that has very a very aggressive power
> > policy, doing suspend/resume cycles a few times a minute ("echo mem >
> > /sys/power/state"). In doing so, we found that the system time
> > experiences a lot of jitter (compared to, say, an NTP server). It was
> > not uncommon for us to see time corrections of 1s to 4s on a regular
> > basis. This didn't happen when the device stayed awake, only when it
> > was allowed to do suspend/resume.
> >
> > We found that the problem is an interaction between the NTP code and
> > what I call the "delta_delta hack." (see [1] and [2]) This code
> > allocates a static variable in a function that contains an offset from
> > the system time to the persistent/rtc clock. It uses that time to
> > fudge the suspend timestamp so that on resume the sleep time will be
> > compensated. It's kind of a statistical hack that assumes things will
> > average out. It seems to have two main assumptions:
> >
> > 1. The persistent/rtc clock has only single-second precision
> > 2. The system does not frequently suspend/resume.
> > 3. If delta_delta is less than 2 seconds, these assumptions are "true"
> >
> > Because the delta_delta hack is trying to maintain an offset from the
> > system time to the persistent/rtc clock, any minor NTP corrections
> > that have occurred since the last suspend will be discarded. However,
> > the NTP subsystem isn't notified that this is happening -- and so it
> > causes some level of instability in its PLL logic.
>
> So, on resume when we call __timekeeping_inject_sleeptime(), that uses
> the TK_CLEAR_NTP which clears the NTP state (sets STA_UNSYNC, etc) .
> I'm not sure how else we can notify userspace. It may be that ntpd
> doesn't expect the kernel to set things as unsynced and doesn't
> recover well, but the proper fix for that probably is in userspace.
Errm. No, __timekeeping_inject_sleeptime() only updates the timekeeper.
We have two call sites:
timekeeping_resume()
{
.....
if (sleeptime_injected)
__timekeeping_inject_sleeptime(tk, &ts_delta);
...
timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
...
}
and
timekeeping_inject_sleeptime64()
{
__timekeeping_inject_sleeptime(tk, &delta);
...
timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
...
}
But Gabriel talks about the effects from injecting sleep time in
timekeeping_resume() because that's where we use
read_persistent_clock64(). And there we don't clear NTP, unless there is
some magic I'm missing completely.
Thanks,
tglx
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Miroslav Lichvar @ 2017-10-05 11:08 UTC (permalink / raw)
To: John Stultz
Cc: Gabriel Beddingfield, LKML, Stephen Boyd, Thomas Gleixner,
Alessandro Zummo, Alexandre Belloni, linux-rtc, Guy Erb, hharte
In-Reply-To: <CALAqxLUO-62B0umVSUK979A+dApOLN3xt5nnZA5HKHsWnRz3KQ@mail.gmail.com>
On Wed, Oct 04, 2017 at 05:16:31PM -0700, John Stultz wrote:
> On Wed, Oct 4, 2017 at 9:11 AM, Gabriel Beddingfield <gabe@nestlabs.com> wrote:
> > We found that the problem is an interaction between the NTP code and
> > what I call the "delta_delta hack." (see [1] and [2]) This code
> > allocates a static variable in a function that contains an offset from
> > the system time to the persistent/rtc clock. It uses that time to
> > fudge the suspend timestamp so that on resume the sleep time will be
> > compensated. It's kind of a statistical hack that assumes things will
> > average out. It seems to have two main assumptions:
> >
> > 1. The persistent/rtc clock has only single-second precision
> > 2. The system does not frequently suspend/resume.
> > 3. If delta_delta is less than 2 seconds, these assumptions are "true"
> >
> > Because the delta_delta hack is trying to maintain an offset from the
> > system time to the persistent/rtc clock, any minor NTP corrections
> > that have occurred since the last suspend will be discarded. However,
> > the NTP subsystem isn't notified that this is happening -- and so it
> > causes some level of instability in its PLL logic.
This is interesting. What polling interval was ntpd using? If I
understand it correctly, with a high-resolution persistent clock the
delta-delta compensation should be very small and shouldn't disrupt
ntpd. Does this instability disappear when ntpd is not controlling the
clock (i.e. "disable ntp" in ntp.conf)?
> We should also figure out how to best handle ntpd in userspace dealing
> with frequent suspend/resume cycles. This is problematic, as the
> closest analogy is trying driving on the road while frequently falling
> asleep. This is not something I think ntpd handles well. I suspect
> our options are that any ntp adjustments being made might be made for
> far too long (causing potentially massive over-correction) or not at
> all, and not at all seems slightly better in my mind.
Yeah, controlling the clock in such conditions will be difficult. The
kernel/ntp PLL requires periodic updates. There is some code in
ntp_update_offset() that reduces the frequency adjustment when PLL
updates are missing, but I'm not actually sure if it works correctly
with suspend.
--
Miroslav Lichvar
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 14:11 UTC (permalink / raw)
To: John Stultz
Cc: Gabriel Beddingfield, LKML, Stephen Boyd, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, hharte, Miroslav Lichvar
In-Reply-To: <alpine.DEB.2.20.1710051301250.2083@nanos>
On Thu, 5 Oct 2017, Thomas Gleixner wrote:
> On Wed, 4 Oct 2017, John Stultz wrote:
> > So, on resume when we call __timekeeping_inject_sleeptime(), that uses
> > the TK_CLEAR_NTP which clears the NTP state (sets STA_UNSYNC, etc) .
> > I'm not sure how else we can notify userspace. It may be that ntpd
> > doesn't expect the kernel to set things as unsynced and doesn't
> > recover well, but the proper fix for that probably is in userspace.
>
> Errm. No, __timekeeping_inject_sleeptime() only updates the timekeeper.
That should read:
updates the timekeeper data fields, but does not call
timekeeping_update().
>
> We have two call sites:
>
> timekeeping_resume()
> {
> .....
> if (sleeptime_injected)
> __timekeeping_inject_sleeptime(tk, &ts_delta);
> ...
> timekeeping_update(tk, TK_MIRROR | TK_CLOCK_WAS_SET);
> ...
> }
>
> and
>
> timekeeping_inject_sleeptime64()
> {
> __timekeeping_inject_sleeptime(tk, &delta);
> ...
> timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
> ...
> }
>
> But Gabriel talks about the effects from injecting sleep time in
> timekeeping_resume() because that's where we use
> read_persistent_clock64(). And there we don't clear NTP, unless there is
> some magic I'm missing completely.
>
> Thanks,
>
> tglx
>
>
>
^ permalink raw reply
* DryIce , RTC not working on imx53.
From: Vellemans, Noel @ 2017-10-05 14:18 UTC (permalink / raw)
To: linux-arm-kernel@lists.infradead.org; +Cc: linux-rtc@vger.kernel.org
SGVsbG8sDQoNCkRyeUljZSAsIFNSVEMgbm90IHdvcmtpbmcgb24gaW14NTMuICgga2VybmVsIDQu
eCkgwqAoIHNhbWUgaGFyZHdhcmUgcnVubmluZyBvbGRlciBrZXJuZWwgdmVyc2lvbnMuLiBtZWFu
cyAsIHJ0YyBpcyB3b3JraW5nKQ0KDQpEdXJpbmcgYm9vdCBhbGwgc2VlbXMgdG8gYmUgZmluZSBi
dXQgb25jZSB5b3UgdHJ5IHRvIHJlYWQgb3Igd3JpdGUgdGhlIGhhcmR3YXJlIGNsb2NrIGxhdGVy
IG9uIOKApiBpdCBiYWlscyBvdXQgd2l0aCB0aGlzIGVycm9yIG9uIHRoZSBjb25zb2xlLiANCg0K
aHdjbG9jaw0KDQpbICAgOTcuMTg2NTc3XSBpbXhkaV9ydGMgNTNmYTQwMDAucnRjOiBXcml0ZS13
YWl0IHRpbWVvdXQgdmFsID0gMHg1YTJmZjhkMyByZWcgPSAweDAwMDAwMDA4DQoNCkh3Y2xvY2sg
OiBzZWxlY3QoKSB0byAvZGV2L3J0YzAgdG8gd2FpdCBmb3IgY2xvY2sgdGljayB0aW1lZCBvdXQ6
IE5vIHN1Y2ggZmlsZSBvciBkaXJlY3RvcnkNCg0KDQoNCkkndmUgQWRkZWQgc29tZSBkcml2ZXIg
4oCTIHByaW50a+KAmXPigKYuDQoNCiMgaHdjbG9jaw0KWyAgIDczLjM2MjU1OV0gZHJ5aWNlX3J0
Y19yZWFkX3RpbWUgLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tDQpbICAgNzMuMzk1MDc3XSBkcnlpY2VfcnRjX3JlYWRfdGltZSAtLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NClsgICA3My40MTQxNTZdIGRyeWljZV9y
dGNfcmVhZF90aW1lIC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLQ0KWyAgIDczLjQyMTcwMF0gZGlfd3JpdGVfd2FpdCAtLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NClsgICA3My40NzI2MjRdIGRpX2ludF9lbmFibGUg
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQpbICAgNzMu
NTE0NjA5XSBpbXhkaV9ydGMgNTNmYTQwMDAuc3J0YzogV3JpdGUtd2FpdCB0aW1lb3V0IHZhbCA9
IDB4NWEzMDAwYzggcmVnID0gMHgwMDAwMDAwOA0KWyAgIDczLjUyMzAxOV0gZGlfaW50X2VuYWJs
ZSAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCg0KPDwg
U1RBTExTIGZvciA1IHNlY29uZHMgaGVyZSA+Pg0KPDwgU1RBTExTIGZvciA1IHNlY29uZHMgaGVy
ZSA+Pg0KPDwgU1RBTExTIGZvciA1IHNlY29uZHMgaGVyZSA+Pg0KPDwgU1RBTExTIGZvciA1IHNl
Y29uZHMgaGVyZSA+Pg0KPDwgU1RBTExTIGZvciA1IHNlY29uZHMgaGVyZSA+Pg0KDQpod2Nsb2Nr
WyAgIDc4LjU4NDkwOV0gZHJ5aWNlX3J0Y19hbGFybV9pcnFfZW5hYmxlIC0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQ0KOiBzZWxlY3QoKSB0byAvZGV2L3J0
YzAgdG8gd2FpdCBmWyAgIDc4LjU5MzQ1Nl0gZGlfaW50X2Rpc2FibGUgLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQpvciBjbG9jayB0aWNrIHRpbWVkIG91
dDogTm8gc3VjaCBmaWxlIG9yIGRpcmVjdG9yeQ0KDQoNCg0KDQoNCg0KU3RyYWNlIC4uIGxvZ2dp
bmcgPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCg0KDQpzdGF0KCIvbGliL2xkLXVD
bGliYy5zby4wIiwge3N0X21vZGU9U19JRlJFR3wwNzc3LCBzdF9zaXplPTI1MzAwLCAuLi59KSA9
IDANCm1tYXAyKE5VTEwsIDQwOTYsIFBST1RfUkVBRHxQUk9UX1dSSVRFLCBNQVBfUFJJVkFURXxN
QVBfQU5PTllNT1VTfDB4NDAwMDAwMCwgLTEsIDApID0gMHhiNmYwNDAwMA0Kc2V0X3RscygweGI2
ZjA0NDkwLCAweGI2ZjA0YjM4LCAweGI2ZjA3MDg4LCAweGI2ZjA0NDkwLCAweGI2ZjA2Zjc0KSA9
IDANCm1wcm90ZWN0KDB4YjZlZDIwMDAsIDQwOTYsIFBST1RfUkVBRCkgICA9IDANCm1wcm90ZWN0
KDB4YjZmMDYwMDAsIDQwOTYsIFBST1RfUkVBRCkgICA9IDANCmlvY3RsKDAsIFNORENUTF9UTVJf
VElNRUJBU0Ugb3IgVENHRVRTLCB7QjExNTIwMCBvcG9zdCBpc2lnIGljYW5vbiBlY2hvIC4uLn0p
ID0gMA0KaW9jdGwoMSwgU05EQ1RMX1RNUl9USU1FQkFTRSBvciBUQ0dFVFMsIHtCMTE1MjAwIG9w
b3N0IGlzaWcgaWNhbm9uIGVjaG8gLi4ufSkgPSAwDQpnZXR0aW1lb2ZkYXkoezE1MTMwOTU0MzAs
IDcwODA5N30sIE5VTEwpID0gMA0KZ2V0dWlkMzIoKSAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgID0gMA0Kb3BlbigiL2Rldi9ydGMiLCBPX1JET05MWXxPX0xBUkdFRklMRSkgID0gLTEgRU5P
RU5UIChObyBzdWNoIGZpbGUgb3IgZGlyZWN0b3J5KQ0Kb3BlbigiL2Rldi9ydGMwIiwgT19SRE9O
TFl8T19MQVJHRUZJTEUpID0gMw0KYnJrKDApICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgID0gMHgxODAwMA0KYnJrKDB4MTkwMDApICAgICAgICAgICAgICAgICAgICAgICAgICAgID0g
MHgxOTAwMA0Kc3RhdDY0KCIvZXRjL2FkanRpbWUiLCAweGJlZWMyNmE4KSAgICAgID0gLTEgRU5P
RU5UIChObyBzdWNoIGZpbGUgb3IgZGlyZWN0b3J5KQ0KaW9jdGwoMywgUEhOX1NFVF9SRUdTIG9y
IFJUQ19VSUVfT04sIDApID0gMA0Kc2VsZWN0KDQsIFszXSwgTlVMTCwgTlVMTCwgezUsIDB9DQoN
Cg0KPDwgU1RBTExTIGZvciA1IHNlY29uZHMgaGVyZSAgLS0gIHNlbGVjdCBpcyBub3QgcmV0dXJu
aW5nICEhISB0aW1lb3V0IGlzIDUgc2Vjb25kc+KApi4gPj4NCg0KKSAgICAgID0gMCAoVGltZW91
dClbICAxNDEuNzY2MTYyXSBkcnlpY2VfcnRjX2FsYXJtX2lycV9lbmFibGUgLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQoNCndyaXRlKDIsICJod2Nsb2Nr
IiwgN2h3Y2xvY2spICAgICAgICAgICAgICAgICAgPSA3DQp3cml0ZSgyLCAiOiAiLCAyOiApICAg
ICAgICAgICAgICAgICAgICAgICA9WyAgMTQxLjc4MjE5NV0gZGlfaW50X2Rpc2FibGUgLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQoyDQp3cml0ZSgyLCAi
c2VsZWN0KCkgdG8gIiwgMTJzZWxlY3QoKSB0byApICAgICAgICAgICAgPSAxMg0Kd3JpdGUoMiwg
Ii9kZXYvcnRjMCIsIDkvZGV2L3J0YzApICAgICAgICAgICAgICAgID0gOQ0Kd3JpdGUoMiwgIiB0
byB3YWl0IGZvciBjbG9jayB0aWNrIHRpbWVkIG91Ii4uLiwgMzMgdG8gd2FpdCBmb3IgY2xvY2sg
dGljayB0aW1lZCBvdXQpID0gMzMNCndyaXRlKDIsICI6ICIsIDI6ICkgICAgICAgICAgICAgICAg
ICAgICAgID0gMg0Kd3JpdGUoMiwgIk5vIHN1Y2ggZmlsZSBvciBkaXJlY3RvcnkiLCAyNU5vIHN1
Y2ggZmlsZSBvciBkaXJlY3RvcnkpID0gMjUNCndyaXRlKDIsICJcbiIsIDENCikgICAgICAgICAg
ICAgICAgICAgICAgID0gMQ0KaW9jdGwoMywgUEhOX05PVF9PSCBvciBSVENfVUlFX09GRiwgMCkg
ID0gMA0KY2xvc2UoMykgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgID0gMA0KZXhpdF9n
cm91cCg3NCkgICAgICAgICAgICAgICAgICAgICAgICAgID0gPw0KDQoNCg0KDQoNCg0KDQoNClFV
SUNLIGFuYWx5c2VzICAoIGNvdWxkIGJlIHdyb25nKSA/IA0KSXQgc2VlbXMgdGhhdCBod2Nsb2Nr
IGlzIHJlYWRpbmcgdGhlIGN1cnJlbnQtdGltZXN0YW1wIDMgdGltZXMgYW5kIGlmIG5vdCBjaGFu
Z2VkIGluIHRob3NlIDMgcmVhZCBjeWNsZXPigKYgaXQgc2V0cyB1cCBhbiByZWFkLWludGVycnVw
dC1hYm9ydCBhYmxlIHRpbWUgcmVhZGVyIHRoYXQgc2hvdWxkIHJldHVybiBhcyBzb29uIGFzIHRo
ZSBpcnEgZmlyZXPigKYgYnV0IHRoaXMgc2VlbXMgdG8gYmUgbWlzc2luZyAhDQoNCkZZSTogIEni
gJl2ZSBiZWVuIHVzaW5nIGZvbGxvd2luZyBjb21taW50IHRvIGVuYWJsZSBzcnRjLg0KY29tbWl0
IDViNzI1MDU0MTQ3ZGVhZjk2NmIzOTE5ZTEwYTg2YzZiZmU5NDZhMTgNCkF1dGhvcjogUGF0cmlj
ayBCcnVlbm4gPHAuYnJ1ZW5uQGJlY2tob2ZmLmNvbT4NCkRhdGU6wqAgwqBXZWQgSnVsIDI2IDE0
OjA1OjMyIDIwMTcgKzAyMDANCg0KwqAgwqAgQVJNOiBkdHM6IGlteDUzOiBhZGQgc3J0YyBub2Rl
DQrCoCDCoMKgDQrCoCDCoCBUaGUgaS5NWDUzIGhhcyBhbiBpbnRlZ3JhdGVkIHNlY3VyZSByZWFs
IHRpbWUgY2xvY2suIEFkZCBpdCB0byB0aGUgZHRzaS4NCsKgIMKgwqANCsKgIMKgIFNpZ25lZC1v
ZmYtYnk6IFBhdHJpY2sgQnJ1ZW5uIDxwLmJydWVubkBiZWNraG9mZi5jb20+DQrCoCDCoCBTaWdu
ZWQtb2ZmLWJ5OiBTaGF3biBHdW8gPHNoYXduZ3VvQGtlcm5lbC5vcmc+DQoNCmRpZmYgLS1naXQg
YS9hcmNoL2FybS9ib290L2R0cy9pbXg1My5kdHNpIGIvYXJjaC9hcm0vYm9vdC9kdHMvaW14NTMu
ZHRzaQ0KaW5kZXggMmU1MTZmNC4uOGJmMGQ4OSAxMDA2NDQNCi0tLSBhL2FyY2gvYXJtL2Jvb3Qv
ZHRzL2lteDUzLmR0c2kNCisrKyBiL2FyY2gvYXJtL2Jvb3QvZHRzL2lteDUzLmR0c2kNCkBAIC00
MzMsNiArNDMzLDE1IEBADQrCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCBjbG9jay1uYW1lcyA9ICJpcGciLCAicGVyIjsNCsKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIH07DQrCoA0KK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgc3J0Yzogc3J0Y0A1M2ZhNDAwMCB7DQorwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqBjb21wYXRpYmxlID0gImZzbCxpbXg1My1ydGMiLCAiZnNsLGlteDI1
LXJ0YyI7DQorwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBy
ZWcgPSA8MHg1M2ZhNDAwMCAweDQwMDA+Ow0KK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgaW50ZXJydXB0cyA9IDwyND47DQorwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBpbnRlcnJ1cHQtcGFyZW50ID0gPCZ0emljPjsN
CivCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGNsb2NrcyA9
IDwmY2xrcyBJTVg1X0NMS19TUlRDX0dBVEU+Ow0KK8KgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgY2xvY2stbmFtZXMgPSAiaXBnIjsNCivCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoH07DQorDQoNCkJlc3QgUmVnYXJkcw0KTm9lbA0KDQoNCg==
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Gabriel Beddingfield @ 2017-10-05 16:46 UTC (permalink / raw)
To: John Stultz
Cc: Thomas Gleixner, LKML, Stephen Boyd, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <CALAqxLUnpNQrduA09V4uAe=3-57UU9g9Qyaqg7PE9RnNjGAVeA@mail.gmail.com>
Hi John,
First, my apologies for calling it a "hack." I just went back and looked at the
commit history and this is first-class stuff... and you explained it very well
(including the NTP interaction) in the commit message. I'm pretty sure I
read this before, but I reckon most of it went over my head and I garbled it.
On Wed, Oct 4, 2017 at 5:20 PM, John Stultz <john.stultz@linaro.org> wrote:
> Yea. I thought arm devices often had read_persistent_clock64() backed
> by the 32k timer (which is poor for time initialization but works well
> for suspend timing).
>
> Maybe I misunderstood on the first read. Is it then that the
> relatively fine-grained read_persistent_clock64() is colliding with
> the delta_delta logic that assumes we get coarse 1sec resolution? In
> that case the huristic above seems sane.
Yes, exactly.
-gabe
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Gabriel Beddingfield @ 2017-10-05 16:47 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <alpine.DEB.2.20.1710051216050.2083@nanos>
Hi Thomas,
On Thu, Oct 5, 2017 at 4:01 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > The SoC we use backs the monotonic clock (sched_clock_register()) with a
>
> Again. monotonic clock != sched clock. The clocksource which feeds the
> monotonic timekeeper clock is registered via clocksource_register() & al.
Sorry. I was hedging since I wasn't sure *which* term to use.
> Groan. Engineering based on theories is doomed to begin with.
I knew you'd say that. :-p
> Your 36 hour time jump is probably exactly 36.4089 hours as that's
>
> ((1 << 32) / 32768) / 3600
>
> i.e. the 32bit rollover of the clocksource. So, if the clocksource->read()
> function returns a full 64bit counter value, then it must have protection
> against observing the rollover independent of the clock which feeds that
> counter. Of course the frequency changes the probablity of observing it,
> but still the read function must be protected against observing the
> rollover unconditionally.
Right, but isn't this what clocksource->mask is supposed to do? When we change
the back-end frequency, we're still using the same front-end 32-bit register and
we don't see the same jumps.
> Which SoC/clocksource driver are you talking about?
NXP i.MX 6SoloX
drivers/clocksource/timer-imx-gpt.c
drivers/rtc/rtc-snvs.c
arch/arm/boot/dts/imx6sx.dtsi (included from imx6sx-sdb.dts)
(node: soc/aips1/gpt)
We patched the RTC driver to call register_persistent_clock()
(arch/arm/kernel/time.c)
The upstream driver doesn't support using the 32kHz clock, but the silicone
does... so we modified the driver to accept it and set the "NONSTOP" flag.
> I can understand that. Though, using that value for injecting accurate
> sleep time should just work with the existing code no matter how long the
> actual sleep time was. The timekeeping core takes the nsec part of the
> timespec value retrieved via read_persistent_clock64() into account.
>
> I still have a hard time to figure out what you are trying to achieve.
I'm trying to disable this block:
int timekeeping_suspend(void)
{
struct timekeeper *tk = &tk_core.timekeeper;
unsigned long flags;
#ifdef CONFIG_PERSISTENT_CLOCK_IS_LOW_PRECISION
struct timespec64 delta, delta_delta;
static struct timespec64 old_delta;
#endif
read_persistent_clock64(&timekeeping_suspend_time);
//...
#ifdef CONFIG_PERSISTENT_CLOCK_IS_LOW_PRECISION
if (persistent_clock_exists) {
/*
* To avoid drift caused by repeated suspend/resumes,
* which each can add ~1 second drift error,
* try to compensate so the difference in system time
* and persistent_clock time stays close to constant.
*/
delta = timespec64_sub(tk_xtime(tk), timekeeping_suspend_time);
delta_delta = timespec64_sub(delta, old_delta);
if (abs(delta_delta.tv_sec) >= 2) {
/*
* if delta_delta is too large, assume time correction
* has occurred and set old_delta to the current delta.
*/
old_delta = delta;
} else {
/* Otherwise try to adjust old_system to compensate */
timekeeping_suspend_time =
timespec64_add(timekeeping_suspend_time, delta_delta);
}
}
#endif
In the typical case, it is trying to maintain the assumption that
(ideally) 'delta'
is a constant. It tries to maintain this assumption by fiddling with
the "suspend
time" -- adding or subtracting a little to it.
However, if NTP has adjusted the system time but *not* adjusted the persistent
clock's time... then the assumption is violated, 'delta' is *not*
constant, and this
block cancels out the NTP corrections.
-gabe
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 18:01 UTC (permalink / raw)
To: Gabriel Beddingfield
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <CAOdF7nu9JpG0C=d_2vMka00eEX_wWWv-UUaiUo-YsDcP9-7VdQ@mail.gmail.com>
Gabriel,
On Thu, 5 Oct 2017, Gabriel Beddingfield wrote:
> On Thu, Oct 5, 2017 at 4:01 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > i.e. the 32bit rollover of the clocksource. So, if the clocksource->read()
> > function returns a full 64bit counter value, then it must have protection
> > against observing the rollover independent of the clock which feeds that
> > counter. Of course the frequency changes the probablity of observing it,
> > but still the read function must be protected against observing the
> > rollover unconditionally.
>
> Right, but isn't this what clocksource->mask is supposed to do? When we change
> the back-end frequency, we're still using the same front-end 32-bit register and
> we don't see the same jumps.
Right. That's what the mask should protect. I was assuming that this is one
of the fancy clocksources which expose two 32bit registers of a 64bit
counter and the rollover protection was missing. So that's not the
case. Good, or not so good :)
> > Which SoC/clocksource driver are you talking about?
>
> NXP i.MX 6SoloX
> drivers/clocksource/timer-imx-gpt.c
So that clocksource driver looks correct. Do you have an idea in which
context this time jump happens? Does it happen when you exercise your high
frequency suspend/resume dance or is that happening just when you let the
machine run forever as well?
The timekeeping_resume() path definitely has an issue:
cycle_now = tk_clock_read(&tk->tkr_mono);
if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) &&
cycle_now > tk->tkr_mono.cycle_last) {
This works nice for clocksources which wont wrap across suspend/resume but
not for those which can. That cycle_now -> cycle_last check should take
cs-mask into account ...
Of course for clocksources which can wrap within realistic suspend times,
which 36 hours might be accounted for, this would need an extra sanity
check against a RTC whether wrap time has been exceeded.
I haven't thought it through whether that buggered check fully explains
what you are observing, but it's wrong nevertheless. John?
Thanks,
tglx
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Gabriel Beddingfield @ 2017-10-05 20:14 UTC (permalink / raw)
To: John Stultz
Cc: LKML, Stephen Boyd, Thomas Gleixner, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte,
Miroslav Lichvar
In-Reply-To: <CALAqxLUO-62B0umVSUK979A+dApOLN3xt5nnZA5HKHsWnRz3KQ@mail.gmail.com>
Hi John,
On Wed, Oct 4, 2017 at 5:16 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Please let me know what you think -- and what the right approach for
>> solving this would be.
>
> So I suspect the best solution for you here is: provide some
> infrastructure so clocksources that set CLOCK_SOURCE_SUSPEND_NONSTOP
> which are not the current clocksource can be used for suspend timing.
Let me see if I understand how this might work in my situation...
1. I register a clocksource and set the NONSTOP flag.
2. Give it a "low" rating so that it's not selected as the timekeeping
clocksource.
3. Create functions clocksource_select_persistent() and
timekeeping_notify_persistent()
4. Add `struct tk_read_base tk_persistent' to `struct timekeeper'
5. Possibly add a change_persistent_clocksource() function to timekeeping.c
> We should also figure out how to best handle ntpd in userspace dealing
> with frequent suspend/resume cycles. This is problematic, as the
> closest analogy is trying driving on the road while frequently falling
> asleep. This is not something I think ntpd handles well. I suspect
> our options are that any ntp adjustments being made might be made for
> far too long (causing potentially massive over-correction) or not at
> all, and not at all seems slightly better in my mind.
Indeed. Because of this, we've actually disabled NTP time slewing for now. We
always "jump" the time whenever we sync with the server.
However, I set up a test with the chrony NTP daemon (and the "delta_delta"
compensation disabled). I modified chrony to do the following:
1. hold a "wake lock" with the power management daemon whenever
doing anything (e.g. sync with time server)
2. use an alarmtimer (timerfd backed by CLOCK_BOOTTIME_ALARM) to
back up the select(2) timeout.
In this configuration, the NTP corrections were very stable, the drift
converged, the
jitter was negligible (less than 0.010 sec), and the time synchronized
very slowly
and methodically (as it should).
In a similar test with connman's NTP implementation (which is much less
sophisticated and does not estimate drift), we got "good" results.
Thanks,
Gabe
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Gabriel Beddingfield @ 2017-10-05 20:51 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <alpine.DEB.2.20.1710051943570.2398@nanos>
Hi Thomas,
On Thu, Oct 5, 2017 at 11:01 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > Which SoC/clocksource driver are you talking about?
>>
>> NXP i.MX 6SoloX
>> drivers/clocksource/timer-imx-gpt.c
>
> So that clocksource driver looks correct. Do you have an idea in which
> context this time jump happens? Does it happen when you exercise your high
> frequency suspend/resume dance or is that happening just when you let the
> machine run forever as well?
We couldn't devise any reproduction steps. We observed it happening at
unexpected
times in a fleet of devices -- and we couldn't find any patterns to clue us in.
>
> The timekeeping_resume() path definitely has an issue:
>
> cycle_now = tk_clock_read(&tk->tkr_mono);
> if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) &&
> cycle_now > tk->tkr_mono.cycle_last) {
>
> This works nice for clocksources which wont wrap across suspend/resume but
> not for those which can. That cycle_now -> cycle_last check should take
> cs-mask into account ...
>
> Of course for clocksources which can wrap within realistic suspend times,
> which 36 hours might be accounted for, this would need an extra sanity
> check against a RTC whether wrap time has been exceeded.
>
> I haven't thought it through whether that buggered check fully explains
> what you are observing, but it's wrong nevertheless. John?
Nah. It looks like the consequence is that you'll either fail to
inject the sleep time
or you'll fall back to having the RTC inject the sleep time. In our
case, we never
sleep for more than a couple of minutes so the error would be seconds rather
than hours.
-gabe
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 21:04 UTC (permalink / raw)
To: Gabriel Beddingfield
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <CAOdF7nssJZZG_VKO+G_K6+aukOApmXp823yA9k3sUsSYpSB71g@mail.gmail.com>
On Thu, 5 Oct 2017, Gabriel Beddingfield wrote:
> Hi Thomas,
>
> On Thu, Oct 5, 2017 at 11:01 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > Which SoC/clocksource driver are you talking about?
> >>
> >> NXP i.MX 6SoloX
> >> drivers/clocksource/timer-imx-gpt.c
> >
> > So that clocksource driver looks correct. Do you have an idea in which
> > context this time jump happens? Does it happen when you exercise your high
> > frequency suspend/resume dance or is that happening just when you let the
> > machine run forever as well?
>
> We couldn't devise any reproduction steps. We observed it happening at
> unexpected times in a fleet of devices -- and we couldn't find any
> patterns to clue us in.
Ok. Did you talk to NXP about that? Or did you try to exercise reads in a
loop to detect the wreckage and maybe a pattern in there?
> > The timekeeping_resume() path definitely has an issue:
> >
> > cycle_now = tk_clock_read(&tk->tkr_mono);
> > if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) &&
> > cycle_now > tk->tkr_mono.cycle_last) {
> >
> > This works nice for clocksources which wont wrap across suspend/resume but
> > not for those which can. That cycle_now -> cycle_last check should take
> > cs-mask into account ...
> >
> > Of course for clocksources which can wrap within realistic suspend times,
> > which 36 hours might be accounted for, this would need an extra sanity
> > check against a RTC whether wrap time has been exceeded.
> >
> > I haven't thought it through whether that buggered check fully explains
> > what you are observing, but it's wrong nevertheless. John?
>
> Nah. It looks like the consequence is that you'll either fail to inject
> the sleep time or you'll fall back to having the RTC inject the sleep
> time. In our case, we never sleep for more than a couple of minutes so
> the error would be seconds rather than hours.
Fair enough. It's still wrong though and wants to be fixed.
Thanks,
tglx
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Gabriel Beddingfield @ 2017-10-05 21:12 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <alpine.DEB.2.20.1710052302440.2398@nanos>
On Thu, Oct 5, 2017 at 2:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > So that clocksource driver looks correct. Do you have an idea in which
>> > context this time jump happens? Does it happen when you exercise your high
>> > frequency suspend/resume dance or is that happening just when you let the
>> > machine run forever as well?
>>
>> We couldn't devise any reproduction steps. We observed it happening at
>> unexpected times in a fleet of devices -- and we couldn't find any
>> patterns to clue us in.
>
> Ok. Did you talk to NXP about that? Or did you try to exercise reads in a
> loop to detect the wreckage and maybe a pattern in there?
Yes, we talked to NXP about it. They don't have a conclusion on what happened.
While they've been very helpful... we were off the path from their reference
implementation and so it's not a high priority for them.
No, we didn't try that because we prioritized the "persistent clock" approach.
I have a little more time now and can try the loop-reading strategy.
-gabe
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 21:31 UTC (permalink / raw)
To: Gabriel Beddingfield
Cc: John Stultz, LKML, Stephen Boyd, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte,
Miroslav Lichvar
In-Reply-To: <CAOdF7nv5re6Hvv5e3Mj=tiQKBw2CvEEdE1Nt96gqZmhr5+xZ+g@mail.gmail.com>
Gabriel,
On Thu, 5 Oct 2017, Gabriel Beddingfield wrote:
> Hi John,
>
> On Wed, Oct 4, 2017 at 5:16 PM, John Stultz <john.stultz@linaro.org> wrote:
> >> Please let me know what you think -- and what the right approach for
> >> solving this would be.
> >
> > So I suspect the best solution for you here is: provide some
> > infrastructure so clocksources that set CLOCK_SOURCE_SUSPEND_NONSTOP
> > which are not the current clocksource can be used for suspend timing.
>
> Let me see if I understand how this might work in my situation...
>
> 1. I register a clocksource and set the NONSTOP flag.
> 2. Give it a "low" rating so that it's not selected as the timekeeping
> clocksource.
> 3. Create functions clocksource_select_persistent() and
> timekeeping_notify_persistent()
> 4. Add `struct tk_read_base tk_persistent' to `struct timekeeper'
> 5. Possibly add a change_persistent_clocksource() function to timekeeping.c
That might work, but that looks a tad too complex. Let me give it a try:
1) Create a new flag CLOCK_SOURCE_SUSPEND_BACKUP
This makes sense because such a clocksource is likely to be something
which you don't want ever to use for timekeeping even if its the only
thing aside of jiffies.
2) Set this flag when registering a clocksource, which excludes it from the
normal selection process.
3) Make the registration code select such a clocksource as the backup for
suspend resume to brige the gap when the timekeeper clocksource does not
have the NONSTOP flag set.
You don't need the extra timekeeping_notify_persistent() because in that
case the maybe current backup clocksource is definitely not in use and
can be replaced without the stompmachine muck which we need for
replacing the current timekeeper clocksource. The system cannot be in
suspend at this point obviously. so all it needs is to switch a pointer.
You neither need this extra stuff in struct timekeeper, it's big enough
anyway. A simple static struct tk_read_base should be sufficient.
On suspend you do
if (tk_backup->clock)
sync_data(timekeeper, tk_backup);
You still want to record the RTC based time if possible, in case that
the backup timekeeper can wrap so you have a sanity check for that
similar to the one we need for NONSTOP clocksources. If there is no RTC
then we need a sensible cutoff for that wraparound time which makes sure
that we don't trip over our own feet.
On resume you check tk_backup->clock again, do the RTC sanity check, if
available and valid (either wraps above cutoff or is checked via RTC). If
that's ok, then you update the timekeeper and proceed. If not, use the
fallback or do nothing in the worst case.
Thoughts?
Thanks,
tglx
^ permalink raw reply
* Re: Extreme time jitter with suspend/resume cycles
From: Thomas Gleixner @ 2017-10-05 21:33 UTC (permalink / raw)
To: Gabriel Beddingfield
Cc: LKML, Stephen Boyd, John Stultz, Alessandro Zummo,
Alexandre Belloni, linux-rtc, Guy Erb, Howard Harte
In-Reply-To: <alpine.DEB.2.20.1710052302440.2398@nanos>
On Thu, 5 Oct 2017, Thomas Gleixner wrote:
> On Thu, 5 Oct 2017, Gabriel Beddingfield wrote:
>
> > Hi Thomas,
> >
> > On Thu, Oct 5, 2017 at 11:01 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > >> > Which SoC/clocksource driver are you talking about?
> > >>
> > >> NXP i.MX 6SoloX
> > >> drivers/clocksource/timer-imx-gpt.c
> > >
> > > So that clocksource driver looks correct. Do you have an idea in which
> > > context this time jump happens? Does it happen when you exercise your high
> > > frequency suspend/resume dance or is that happening just when you let the
> > > machine run forever as well?
> >
> > We couldn't devise any reproduction steps. We observed it happening at
> > unexpected times in a fleet of devices -- and we couldn't find any
> > patterns to clue us in.
>
> Ok. Did you talk to NXP about that? Or did you try to exercise reads in a
> loop to detect the wreckage and maybe a pattern in there?
The reason I'm asking is to exclude any weird issue in the timekeeping
code, which is still a possibility, despite the fact that I went through it
with a fine comb after stumbling over that check in the resume path.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v5 2/6] dt-bindings: input: Add common keyboard document bindings
From: Rob Herring @ 2017-10-05 22:22 UTC (permalink / raw)
To: Chen Zhong
Cc: Dmitry Torokhov, Lee Jones, Alexandre Belloni, Mark Rutland,
Matthias Brugger, Eddie Huang, Alessandro Zummo, Linus Walleij,
Chanwoo Choi, Jaechul Lee, Andi Shyti, linux-input, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-rtc
In-Reply-To: <1506509048-19032-3-git-send-email-chen.zhong@mediatek.com>
On Wed, Sep 27, 2017 at 06:44:04PM +0800, Chen Zhong wrote:
> This patch adds the device tree binding documentation for common
> keyboard.
>
> Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
> ---
> Documentation/devicetree/bindings/input/keys.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/keys.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 3/6] dt-bindings: input: Add document bindings for mtk-pmic-keys
From: Rob Herring @ 2017-10-05 22:23 UTC (permalink / raw)
To: Chen Zhong
Cc: Dmitry Torokhov, Lee Jones, Alexandre Belloni, Mark Rutland,
Matthias Brugger, Eddie Huang, Alessandro Zummo, Linus Walleij,
Chanwoo Choi, Jaechul Lee, Andi Shyti, linux-input, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-rtc
In-Reply-To: <1506509048-19032-4-git-send-email-chen.zhong@mediatek.com>
On Wed, Sep 27, 2017 at 06:44:05PM +0800, Chen Zhong wrote:
> This patch adds the device tree binding documentation for the MediaTek
> pmic keys found on PMIC MT6397/MT6323.
>
> Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
> ---
> .../devicetree/bindings/input/mtk-pmic-keys.txt | 43 ++++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 0/6] dt-bindings: rtc: document existing bindings
From: Rob Herring @ 2017-10-05 22:27 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel, devicetree
In-Reply-To: <20170927140345.5537-1-alexandre.belloni@free-electrons.com>
On Wed, Sep 27, 2017 at 04:03:39PM +0200, Alexandre Belloni wrote:
> Document currently undocumented bindings.
>
> Rob, as discussed, all of this can probably go through your tree.
>
> Changes in v2:
> - made a proper series
> - fixed sirf2 example
> - listed the ds1307 compatible that support the trickle charging properties
>
> Alexandre Belloni (6):
> dt-bindings: trivial: Add RTCs
> dt-bindings: rtc: add stericsson,coh901331 bindings
> dt-bindings: rtc: Add sirf,prima2-sysrtc bindings
> dt-bindings: rtc: DS1307 and compatibles are not trivial
> dt-bindings: rtc: Add bindings for m41t80 and compatibles
> dt-bindings: rtc: merge ds1339 in ds1307 documentation
Series applied.
Rob
^ permalink raw reply
* [RFC v4 1/8] platform/x86: intel_pmc_ipc: Use MFD framework to create dependent devices
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Currently, we have lot of repetitive code in dependent device resource
allocation and device creation handling code. This logic can be improved if
we use MFD framework for dependent device creation. This patch adds this
support.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/intel_pmc_ipc.c | 394 ++++++++++++-----------------------
1 file changed, 137 insertions(+), 257 deletions(-)
Changes since v3:
* Changed PLATFORM_DEVID_AUTO to PLATFORM_DEVID_NONE in mfd device creation.
* Fixed error in resource initalization logic in ipc_create_punit_device.
* Removed mfd cell id initialization.
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 751b121..c85351e 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -33,6 +33,7 @@
#include <linux/suspend.h>
#include <linux/acpi.h>
#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/mfd/core.h>
#include <asm/intel_pmc_ipc.h>
@@ -87,6 +88,7 @@
#define PLAT_RESOURCE_ISP_IFACE_INDEX 5
#define PLAT_RESOURCE_GTD_DATA_INDEX 6
#define PLAT_RESOURCE_GTD_IFACE_INDEX 7
+#define PLAT_RESOURCE_MEM_MAX_INDEX 8
#define PLAT_RESOURCE_ACPI_IO_INDEX 0
/*
@@ -105,8 +107,6 @@
#define TELEM_SSRAM_SIZE 240
#define TELEM_PMC_SSRAM_OFFSET 0x1B00
#define TELEM_PUNIT_SSRAM_OFFSET 0x1A00
-#define TCO_PMC_OFFSET 0x8
-#define TCO_PMC_SIZE 0x4
/* PMC register bit definitions */
@@ -123,25 +123,9 @@ static struct intel_pmc_ipc_dev {
int cmd;
struct completion cmd_complete;
- /* The following PMC BARs share the same ACPI device with the IPC */
- resource_size_t acpi_io_base;
- int acpi_io_size;
- struct platform_device *tco_dev;
-
/* gcr */
void __iomem *gcr_mem_base;
bool has_gcr_regs;
-
- /* punit */
- struct platform_device *punit_dev;
-
- /* Telemetry */
- resource_size_t telem_pmc_ssram_base;
- resource_size_t telem_punit_ssram_base;
- int telem_pmc_ssram_size;
- int telem_punit_ssram_size;
- u8 telem_res_inval;
- struct platform_device *telemetry_dev;
} ipcdev;
static char *ipc_err_sources[] = {
@@ -589,44 +573,6 @@ static const struct attribute_group intel_ipc_group = {
.attrs = intel_ipc_attrs,
};
-static struct resource punit_res_array[] = {
- /* Punit BIOS */
- {
- .flags = IORESOURCE_MEM,
- },
- {
- .flags = IORESOURCE_MEM,
- },
- /* Punit ISP */
- {
- .flags = IORESOURCE_MEM,
- },
- {
- .flags = IORESOURCE_MEM,
- },
- /* Punit GTD */
- {
- .flags = IORESOURCE_MEM,
- },
- {
- .flags = IORESOURCE_MEM,
- },
-};
-
-#define TCO_RESOURCE_ACPI_IO 0
-#define TCO_RESOURCE_SMI_EN_IO 1
-#define TCO_RESOURCE_GCR_MEM 2
-static struct resource tco_res[] = {
- /* ACPI - TCO */
- {
- .flags = IORESOURCE_IO,
- },
- /* ACPI - SMI */
- {
- .flags = IORESOURCE_IO,
- },
-};
-
static struct itco_wdt_platform_data tco_info = {
.name = "Apollo Lake SoC",
.version = 5,
@@ -634,234 +580,179 @@ static struct itco_wdt_platform_data tco_info = {
.update_no_reboot_bit = update_no_reboot_bit,
};
-#define TELEMETRY_RESOURCE_PUNIT_SSRAM 0
-#define TELEMETRY_RESOURCE_PMC_SSRAM 1
-static struct resource telemetry_res[] = {
- /*Telemetry*/
- {
- .flags = IORESOURCE_MEM,
- },
- {
- .flags = IORESOURCE_MEM,
- },
-};
-
-static int ipc_create_punit_device(void)
+static int ipc_create_punit_device(struct platform_device *pdev)
{
- struct platform_device *pdev;
- const struct platform_device_info pdevinfo = {
- .parent = ipcdev.dev,
- .name = PUNIT_DEVICE_NAME,
- .id = -1,
- .res = punit_res_array,
- .num_res = ARRAY_SIZE(punit_res_array),
+ struct resource *res;
+ static struct resource punit_res[PLAT_RESOURCE_MEM_MAX_INDEX];
+ static struct mfd_cell punit_cell;
+ int mindex, pindex = 0;
+
+ for (mindex = 0; mindex <= PLAT_RESOURCE_MEM_MAX_INDEX; mindex++) {
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, mindex);
+
+ switch (mindex) {
+ /* Get PUNIT resources */
+ case PLAT_RESOURCE_BIOS_DATA_INDEX:
+ case PLAT_RESOURCE_BIOS_IFACE_INDEX:
+ /* BIOS resources are required, so return error if not
+ * available */
+ if (!res) {
+ dev_err(&pdev->dev,
+ "Failed to get punit mem resource %d\n",
+ pindex);
+ return -ENXIO;
+ }
+ case PLAT_RESOURCE_ISP_DATA_INDEX:
+ case PLAT_RESOURCE_ISP_IFACE_INDEX:
+ case PLAT_RESOURCE_GTD_DATA_INDEX:
+ case PLAT_RESOURCE_GTD_IFACE_INDEX:
+ /* if valid resource found, copy the resource to PUNIT
+ * resource */
+ if (res)
+ memcpy(&punit_res[pindex], res, sizeof(*res));
+ punit_res[pindex].flags = IORESOURCE_MEM;
+ dev_dbg(&pdev->dev, "PUNIT memory res: %pR\n",
+ &punit_res[pindex]);
+ pindex++;
+ break;
};
+ }
- pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev))
- return PTR_ERR(pdev);
-
- ipcdev.punit_dev = pdev;
+ /* Create PUNIT IPC MFD cell */
+ punit_cell.name = PUNIT_DEVICE_NAME;
+ punit_cell.num_resources = ARRAY_SIZE(punit_res);
+ punit_cell.resources = punit_res;
+ punit_cell.ignore_resource_conflicts = 1;
- return 0;
+ return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+ &punit_cell, 1, NULL, 0, NULL);
}
-static int ipc_create_tco_device(void)
+static int ipc_create_wdt_device(struct platform_device *pdev)
{
- struct platform_device *pdev;
+ static struct resource wdt_ipc_res[2];
struct resource *res;
- const struct platform_device_info pdevinfo = {
- .parent = ipcdev.dev,
- .name = TCO_DEVICE_NAME,
- .id = -1,
- .res = tco_res,
- .num_res = ARRAY_SIZE(tco_res),
- .data = &tco_info,
- .size_data = sizeof(tco_info),
- };
+ static struct mfd_cell wdt_cell;
- res = tco_res + TCO_RESOURCE_ACPI_IO;
- res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET;
- res->end = res->start + TCO_REGS_SIZE - 1;
+ /* If we have ACPI based watchdog use that instead, othewise create
+ * a MFD cell for iTCO watchdog*/
+ if (acpi_has_watchdog())
+ return 0;
- res = tco_res + TCO_RESOURCE_SMI_EN_IO;
- res->start = ipcdev.acpi_io_base + SMI_EN_OFFSET;
- res->end = res->start + SMI_EN_SIZE - 1;
-
- pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev))
- return PTR_ERR(pdev);
-
- ipcdev.tco_dev = pdev;
+ /* Get iTCO watchdog resources */
+ res = platform_get_resource(pdev, IORESOURCE_IO,
+ PLAT_RESOURCE_ACPI_IO_INDEX);
+ if (!res) {
+ dev_err(&pdev->dev, "Failed to get wdt resource\n");
+ return -ENXIO;
+ }
- return 0;
+ wdt_ipc_res[0].start = res->start + TCO_BASE_OFFSET;
+ wdt_ipc_res[0].end = res->start + TCO_BASE_OFFSET +
+ TCO_REGS_SIZE - 1;
+ wdt_ipc_res[0].flags = IORESOURCE_IO;
+ wdt_ipc_res[1].start = res->start + SMI_EN_OFFSET;
+ wdt_ipc_res[1].end = res->start +
+ SMI_EN_OFFSET + SMI_EN_SIZE - 1;
+ wdt_ipc_res[1].flags = IORESOURCE_IO;
+
+ dev_dbg(&pdev->dev, "watchdog res 0: %pR\n",
+ &wdt_ipc_res[0]);
+ dev_dbg(&pdev->dev, "watchdog res 1: %pR\n",
+ &wdt_ipc_res[1]);
+
+ wdt_cell.name = TCO_DEVICE_NAME;
+ wdt_cell.platform_data = &tco_info;
+ wdt_cell.pdata_size = sizeof(tco_info);
+ wdt_cell.num_resources = ARRAY_SIZE(wdt_ipc_res);
+ wdt_cell.resources = wdt_ipc_res;
+ wdt_cell.ignore_resource_conflicts = 1;
+
+ return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+ &wdt_cell, 1, NULL, 0, NULL);
}
-static int ipc_create_telemetry_device(void)
+static int ipc_create_telemetry_device(struct platform_device *pdev)
{
- struct platform_device *pdev;
+ static struct resource telemetry_ipc_res[2];
struct resource *res;
- const struct platform_device_info pdevinfo = {
- .parent = ipcdev.dev,
- .name = TELEMETRY_DEVICE_NAME,
- .id = -1,
- .res = telemetry_res,
- .num_res = ARRAY_SIZE(telemetry_res),
- };
-
- res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
- res->start = ipcdev.telem_punit_ssram_base;
- res->end = res->start + ipcdev.telem_punit_ssram_size - 1;
-
- res = telemetry_res + TELEMETRY_RESOURCE_PMC_SSRAM;
- res->start = ipcdev.telem_pmc_ssram_base;
- res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
-
- pdev = platform_device_register_full(&pdevinfo);
- if (IS_ERR(pdev))
- return PTR_ERR(pdev);
+ static struct mfd_cell telemetry_cell;
- ipcdev.telemetry_dev = pdev;
+ /* Get telemetry resources */
+ res = platform_get_resource(pdev, IORESOURCE_MEM,
+ PLAT_RESOURCE_TELEM_SSRAM_INDEX);
+ if (!res) {
+ dev_err(&pdev->dev, "Failed to get telemetry resource\n");
+ return -ENXIO;
+ }
- return 0;
+ telemetry_ipc_res[0].start = res->start +
+ TELEM_PUNIT_SSRAM_OFFSET;
+ telemetry_ipc_res[0].end = res->start +
+ TELEM_PUNIT_SSRAM_OFFSET + TELEM_SSRAM_SIZE - 1;
+ telemetry_ipc_res[0].flags = IORESOURCE_MEM;
+ telemetry_ipc_res[1].start = res->start + TELEM_PMC_SSRAM_OFFSET;
+ telemetry_ipc_res[1].end = res->start +
+ TELEM_PMC_SSRAM_OFFSET + TELEM_SSRAM_SIZE - 1;
+ telemetry_ipc_res[1].flags = IORESOURCE_MEM;
+
+ dev_dbg(&pdev->dev, "Telemetry res 0: %pR\n",
+ &telemetry_ipc_res[0]);
+ dev_dbg(&pdev->dev, "Telemetry res 1: %pR\n",
+ &telemetry_ipc_res[1]);
+
+ telemetry_cell.name = TELEMETRY_DEVICE_NAME;
+ telemetry_cell.num_resources = ARRAY_SIZE(telemetry_ipc_res);
+ telemetry_cell.resources = telemetry_ipc_res;
+ telemetry_cell.ignore_resource_conflicts = 1;
+
+ return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+ &telemetry_cell, 1, NULL, 0, NULL);
}
-static int ipc_create_pmc_devices(void)
+static int ipc_create_pmc_devices(struct platform_device *pdev)
{
int ret;
- /* If we have ACPI based watchdog use that instead */
- if (!acpi_has_watchdog()) {
- ret = ipc_create_tco_device();
- if (ret) {
- dev_err(ipcdev.dev, "Failed to add tco platform device\n");
- return ret;
- }
- }
+ ret = ipc_create_punit_device(pdev);
+ if (ret < 0)
+ return ret;
- ret = ipc_create_punit_device();
- if (ret) {
- dev_err(ipcdev.dev, "Failed to add punit platform device\n");
- platform_device_unregister(ipcdev.tco_dev);
- }
+ ret = ipc_create_wdt_device(pdev);
+ if (ret < 0)
+ return ret;
- if (!ipcdev.telem_res_inval) {
- ret = ipc_create_telemetry_device();
- if (ret)
- dev_warn(ipcdev.dev,
- "Failed to add telemetry platform device\n");
- }
+ ret = ipc_create_telemetry_device(pdev);
+ if (ret < 0)
+ return ret;
- return ret;
+ return 0;
}
static int ipc_plat_get_res(struct platform_device *pdev)
{
- struct resource *res, *punit_res;
+ struct resource *res;
void __iomem *addr;
- int size;
-
- res = platform_get_resource(pdev, IORESOURCE_IO,
- PLAT_RESOURCE_ACPI_IO_INDEX);
- if (!res) {
- dev_err(&pdev->dev, "Failed to get io resource\n");
- return -ENXIO;
- }
- size = resource_size(res);
- ipcdev.acpi_io_base = res->start;
- ipcdev.acpi_io_size = size;
- dev_info(&pdev->dev, "io res: %pR\n", res);
-
- punit_res = punit_res_array;
- /* This is index 0 to cover BIOS data register */
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_BIOS_DATA_INDEX);
- if (!res) {
- dev_err(&pdev->dev, "Failed to get res of punit BIOS data\n");
- return -ENXIO;
- }
- *punit_res = *res;
- dev_info(&pdev->dev, "punit BIOS data res: %pR\n", res);
- /* This is index 1 to cover BIOS interface register */
+ /* Get IPC resources */
res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_BIOS_IFACE_INDEX);
+ PLAT_RESOURCE_IPC_INDEX);
if (!res) {
- dev_err(&pdev->dev, "Failed to get res of punit BIOS iface\n");
+ dev_err(&pdev->dev, "Failed to get IPC resources\n");
return -ENXIO;
}
- *++punit_res = *res;
- dev_info(&pdev->dev, "punit BIOS interface res: %pR\n", res);
-
- /* This is index 2 to cover ISP data register, optional */
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_ISP_DATA_INDEX);
- ++punit_res;
- if (res) {
- *punit_res = *res;
- dev_info(&pdev->dev, "punit ISP data res: %pR\n", res);
- }
- /* This is index 3 to cover ISP interface register, optional */
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_ISP_IFACE_INDEX);
- ++punit_res;
- if (res) {
- *punit_res = *res;
- dev_info(&pdev->dev, "punit ISP interface res: %pR\n", res);
- }
-
- /* This is index 4 to cover GTD data register, optional */
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_GTD_DATA_INDEX);
- ++punit_res;
- if (res) {
- *punit_res = *res;
- dev_info(&pdev->dev, "punit GTD data res: %pR\n", res);
- }
-
- /* This is index 5 to cover GTD interface register, optional */
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_GTD_IFACE_INDEX);
- ++punit_res;
- if (res) {
- *punit_res = *res;
- dev_info(&pdev->dev, "punit GTD interface res: %pR\n", res);
- }
-
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_IPC_INDEX);
- if (!res) {
- dev_err(&pdev->dev, "Failed to get ipc resource\n");
- return -ENXIO;
- }
- size = PLAT_RESOURCE_IPC_SIZE + PLAT_RESOURCE_GCR_SIZE;
- res->end = res->start + size - 1;
+ res->end = (res->start + PLAT_RESOURCE_IPC_SIZE +
+ PLAT_RESOURCE_GCR_SIZE - 1);
addr = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(addr))
return PTR_ERR(addr);
ipcdev.ipc_base = addr;
-
ipcdev.gcr_mem_base = addr + PLAT_RESOURCE_GCR_OFFSET;
- dev_info(&pdev->dev, "ipc res: %pR\n", res);
-
- ipcdev.telem_res_inval = 0;
- res = platform_get_resource(pdev, IORESOURCE_MEM,
- PLAT_RESOURCE_TELEM_SSRAM_INDEX);
- if (!res) {
- dev_err(&pdev->dev, "Failed to get telemetry ssram resource\n");
- ipcdev.telem_res_inval = 1;
- } else {
- ipcdev.telem_punit_ssram_base = res->start +
- TELEM_PUNIT_SSRAM_OFFSET;
- ipcdev.telem_punit_ssram_size = TELEM_SSRAM_SIZE;
- ipcdev.telem_pmc_ssram_base = res->start +
- TELEM_PMC_SSRAM_OFFSET;
- ipcdev.telem_pmc_ssram_size = TELEM_SSRAM_SIZE;
- dev_info(&pdev->dev, "telemetry ssram res: %pR\n", res);
- }
+ dev_dbg(&pdev->dev, "PMC IPC resource %pR\n", res);
return 0;
}
@@ -916,7 +807,7 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
- ret = ipc_create_pmc_devices();
+ ret = ipc_create_pmc_devices(pdev);
if (ret) {
dev_err(&pdev->dev, "Failed to create pmc devices\n");
return ret;
@@ -925,38 +816,27 @@ static int ipc_plat_probe(struct platform_device *pdev)
if (devm_request_irq(&pdev->dev, ipcdev.irq, ioc, IRQF_NO_SUSPEND,
"intel_pmc_ipc", &ipcdev)) {
dev_err(&pdev->dev, "Failed to request irq\n");
- ret = -EBUSY;
- goto err_irq;
+ return -EBUSY;
}
ret = sysfs_create_group(&pdev->dev.kobj, &intel_ipc_group);
if (ret) {
dev_err(&pdev->dev, "Failed to create sysfs group %d\n",
ret);
- goto err_sys;
+ devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev);
+ return ret;
}
ipcdev.has_gcr_regs = true;
return 0;
-err_sys:
- devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev);
-err_irq:
- platform_device_unregister(ipcdev.tco_dev);
- platform_device_unregister(ipcdev.punit_dev);
- platform_device_unregister(ipcdev.telemetry_dev);
-
- return ret;
}
static int ipc_plat_remove(struct platform_device *pdev)
{
sysfs_remove_group(&pdev->dev.kobj, &intel_ipc_group);
- devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev);
- platform_device_unregister(ipcdev.tco_dev);
- platform_device_unregister(ipcdev.punit_dev);
- platform_device_unregister(ipcdev.telemetry_dev);
ipcdev.dev = NULL;
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* [RFC v4 0/8] PMC/PUNIT IPC driver cleanup
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Hi All,
Currently intel_pmc_ipc.c, intel_punit_ipc.c, intel_scu_ipc.c drivers implements the same IPC features.
This code duplication could be avoided if we implement the IPC driver as a generic library and let custom
device drivers use API provided by generic driver. This patchset mainly addresses this issue.
Along with above code duplication issue, This patchset also addresses following issues in intel_pmc_ipc and
intel_punit_ipc drivers.
1. Intel_pmc_ipc.c driver does not use any resource managed (devm_*) calls.
2. In Intel_pmc_ipc.c driver, dependent devices like PUNIT, Telemetry and iTCO are created manually and uses lot of redundant buffer code.
3. Global variable is used to store the IPC device structure and it is used across all functions in intel_pmc_ipc.c and intel_punit_ipc.c.
More info on Intel IPC device library:
-------------------------------------
A generic Intel IPC class driver has been implemented and all common IPC helper functions has been moved to this driver. It exposes APIs to create IPC device channel, send raw IPC command and simple IPC commands. It also creates device attribute to send IPC command from user space.
API for creating a new IPC channel device is,
struct intel_ipc_dev *devm_intel_ipc_dev_create(struct device *dev, const char *devname, struct intel_ipc_dev_cfg *cfg, struct intel_ipc_dev_ops *ops)
The IPC channel drivers (PUNIT/PMC/SCU) when creating a new device can configure their device params like register mapping, irq, irq-mode, channel type,etc using intel_ipc_dev_cfg and intel_ipc_dev_ops arguments. After a new IPC channel device is created, IPC users can use the generic APIs to make IPC calls.
For example, after using this new model, IPC call to PMC device will look like,
pmc_ipc_dev = intel_ipc_dev_get(INTEL_PMC_IPC_DEV);
ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, (u32 *)ipc_in, 1, NULL, 0, 0, 0);
intel_ipc_dev_put(pmc_ipc_dev);
I am still testing the driver in different products. But posted it to get some early comments. I also welcome any PMC/PUNIT driver users to check these patches in their product.
Changes since v3:
* Rebased on top of Andy's review branch.
* Fixed resource ioremap warning in intel_punit_ipc.c.
* Divided "platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates" patch
into two patches. One fixing the existing issue and another to add regmap support.
* Fixed error in resource initalization logic in ipc_create_punit_device.
* Changed PLATFORM_DEVID_AUTO to PLATFORM_DEVID_NONE in mfd device creation.
* Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
* Fixed NULL pointer exception in intel_ipc_dev_get().
* In intel_ipc_dev.c,
* Fixed error in check for duplicate intel_ipc_dev.
* Added custom interrupt handler support.
* Used char array for error string conversion.
* Added put dev support.
Changes since v2:
* Refactored intel_scu_ipc.c to use generic IPC device APIs.
* Fixed intel_pmc_ipc.c to use pcim_* device managed functions.
Changes since v1:
* Merged devm_* changes in pmc_plat_probe and pmc_pci_probe functions into a
single patch.
* Addressed Andy's comment about keeping the library generic by not implementing
the low level reg access calls in intel_ipc_dev.c. This version will start using
the regmap pointer provided by channel drivers instead of fixed memory map.
* Removed custom IPC APIs in intel_pmc_ipc.c and intel_punit_ipc.c.
* Cleaned up IPC driver users to use APIs provided by generic library (intel_ipc_dev.c).
Kuppuswamy Sathyanarayanan (8):
platform/x86: intel_pmc_ipc: Use MFD framework to create dependent
devices
platform/x86: intel_pmc_ipc: Use spin_lock to protect GCR updates
platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates
platform: x86: Add generic Intel IPC driver
platform/x86: intel_punit_ipc: Fix resource ioremap warning
platform/x86: intel_punit_ipc: Use generic intel ipc device calls
platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls
platform/x86: intel_scu_ipc: Use generic Intel IPC device calls
arch/x86/include/asm/intel_pmc_ipc.h | 37 +-
arch/x86/include/asm/intel_punit_ipc.h | 125 ++--
arch/x86/include/asm/intel_scu_ipc.h | 23 +-
arch/x86/platform/intel-mid/intel-mid.c | 15 +-
drivers/mfd/intel_soc_pmic_bxtwc.c | 21 +-
drivers/platform/x86/Kconfig | 11 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_ipc_dev.c | 576 +++++++++++++++
drivers/platform/x86/intel_pmc_ipc.c | 886 +++++++++---------------
drivers/platform/x86/intel_punit_ipc.c | 311 +++------
drivers/platform/x86/intel_scu_ipc.c | 483 ++++++-------
drivers/platform/x86/intel_telemetry_pltdrv.c | 212 +++---
drivers/rtc/rtc-mrst.c | 16 +-
drivers/watchdog/intel-mid_wdt.c | 12 +-
drivers/watchdog/intel_scu_watchdog.c | 18 +-
include/linux/mfd/intel_soc_pmic.h | 2 +
include/linux/platform_data/x86/intel_ipc_dev.h | 206 ++++++
17 files changed, 1683 insertions(+), 1272 deletions(-)
create mode 100644 drivers/platform/x86/intel_ipc_dev.c
create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
--
2.7.4
^ permalink raw reply
* [RFC v4 5/8] platform/x86: intel_punit_ipc: Fix resource ioremap warning
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
For PUNIT device, ISPDRIVER_IPC and GTDDRIVER_IPC resources are
not mandatory. So when PMC IPC driver creates a PUNIT device, if
these resources are not available then it creates dummy resource
entries for these missing resources. But during PUNIT device probe
, doing ioremap on these dummy resources generates following warning
messages.
intel_punit_ipc intel_punit_ipc: can't request region for resource [mem
0x00000000]
intel_punit_ipc intel_punit_ipc: can't request region for resource [mem
0x00000000]
intel_punit_ipc intel_punit_ipc: can't request region for resource [mem
0x00000000]
intel_punit_ipc intel_punit_ipc: can't request region for resource [mem
0x00000000]
This patch fixes this issue by adding extra check for resource size
before performing ioremap operation.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/intel_punit_ipc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/intel_punit_ipc.c b/drivers/platform/x86/intel_punit_ipc.c
index a47a41f..b5b8901 100644
--- a/drivers/platform/x86/intel_punit_ipc.c
+++ b/drivers/platform/x86/intel_punit_ipc.c
@@ -252,28 +252,28 @@ static int intel_punit_get_bars(struct platform_device *pdev)
* - GTDRIVER_IPC BASE_IFACE
*/
res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
- if (res) {
+ if (res && resource_size(res) > 1) {
addr = devm_ioremap_resource(&pdev->dev, res);
if (!IS_ERR(addr))
punit_ipcdev->base[ISPDRIVER_IPC][BASE_DATA] = addr;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
- if (res) {
+ if (res && resource_size(res) > 1) {
addr = devm_ioremap_resource(&pdev->dev, res);
if (!IS_ERR(addr))
punit_ipcdev->base[ISPDRIVER_IPC][BASE_IFACE] = addr;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
- if (res) {
+ if (res && resource_size(res) > 1) {
addr = devm_ioremap_resource(&pdev->dev, res);
if (!IS_ERR(addr))
punit_ipcdev->base[GTDRIVER_IPC][BASE_DATA] = addr;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 5);
- if (res) {
+ if (res && resource_size(res) > 1) {
addr = devm_ioremap_resource(&pdev->dev, res);
if (!IS_ERR(addr))
punit_ipcdev->base[GTDRIVER_IPC][BASE_IFACE] = addr;
--
2.7.4
^ permalink raw reply related
* [RFC v4 2/8] platform/x86: intel_pmc_ipc: Use spin_lock to protect GCR updates
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Currently, update_no_reboot_bit() function implemented in this driver
uses mutex_lock() to protect its register updates. But this function is
called with in atomic context in iTCO_wdt_start() and iTCO_wdt_stop()
functions in iTCO_wdt.c driver, which in turn causes "sleeping into
atomic context" issue. This patch fixes this issue by replacing the
mutex_lock() with spin_lock() to protect the GCR read/write/update APIs.
Fixes: 9d855d4 ("platform/x86: intel_pmc_ipc: Fix iTCO_wdt GCS memory
mapping failure")
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/intel_pmc_ipc.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index c85351e..c68f6a4 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -34,6 +34,7 @@
#include <linux/acpi.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/mfd/core.h>
+#include <linux/spinlock.h>
#include <asm/intel_pmc_ipc.h>
@@ -126,6 +127,7 @@ static struct intel_pmc_ipc_dev {
/* gcr */
void __iomem *gcr_mem_base;
bool has_gcr_regs;
+ spinlock_t gcr_lock;
} ipcdev;
static char *ipc_err_sources[] = {
@@ -209,17 +211,17 @@ int intel_pmc_gcr_read(u32 offset, u32 *data)
{
int ret;
- mutex_lock(&ipclock);
+ spin_lock(&ipcdev.gcr_lock);
ret = is_gcr_valid(offset);
if (ret < 0) {
- mutex_unlock(&ipclock);
+ spin_unlock(&ipcdev.gcr_lock);
return ret;
}
*data = readl(ipcdev.gcr_mem_base + offset);
- mutex_unlock(&ipclock);
+ spin_unlock(&ipcdev.gcr_lock);
return 0;
}
@@ -239,17 +241,17 @@ int intel_pmc_gcr_write(u32 offset, u32 data)
{
int ret;
- mutex_lock(&ipclock);
+ spin_lock(&ipcdev.gcr_lock);
ret = is_gcr_valid(offset);
if (ret < 0) {
- mutex_unlock(&ipclock);
+ spin_unlock(&ipcdev.gcr_lock);
return ret;
}
writel(data, ipcdev.gcr_mem_base + offset);
- mutex_unlock(&ipclock);
+ spin_unlock(&ipcdev.gcr_lock);
return 0;
}
@@ -271,7 +273,7 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
u32 new_val;
int ret = 0;
- mutex_lock(&ipclock);
+ spin_lock(&ipcdev.gcr_lock);
ret = is_gcr_valid(offset);
if (ret < 0)
@@ -293,7 +295,7 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
}
gcr_ipc_unlock:
- mutex_unlock(&ipclock);
+ spin_unlock(&ipcdev.gcr_lock);
return ret;
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_update);
@@ -471,6 +473,8 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (pmc->dev)
return -EBUSY;
+ spin_lock_init(&ipcdev.gcr_lock);
+
pmc->irq_mode = IPC_TRIGGER_MODE_IRQ;
ret = pcim_enable_device(pdev);
@@ -794,6 +798,7 @@ static int ipc_plat_probe(struct platform_device *pdev)
ipcdev.dev = &pdev->dev;
ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
init_completion(&ipcdev.cmd_complete);
+ spin_lock_init(&ipcdev.gcr_lock);
ipcdev.irq = platform_get_irq(pdev, 0);
if (ipcdev.irq < 0) {
--
2.7.4
^ permalink raw reply related
* [RFC v4 3/8] platform/x86: intel_pmc_ipc: Use regmap calls for GCR updates
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
This patch adds support for regmap based implementation for GCR
read/write/update APIs.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_pmc_ipc.c | 120 +++++++++++++----------------------
2 files changed, 44 insertions(+), 77 deletions(-)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1f7959f..da2d9ba 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1067,6 +1067,7 @@ config PVPANIC
config INTEL_PMC_IPC
tristate "Intel PMC IPC Driver"
depends on ACPI
+ select REGMAP_MMIO
---help---
This driver provides support for PMC control on some Intel platforms.
The PMC is an ARC processor which defines IPC commands for communication
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index c68f6a4..55611d2 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -35,6 +35,7 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/mfd/core.h>
#include <linux/spinlock.h>
+#include <linux/regmap.h>
#include <asm/intel_pmc_ipc.h>
@@ -126,8 +127,7 @@ static struct intel_pmc_ipc_dev {
/* gcr */
void __iomem *gcr_mem_base;
- bool has_gcr_regs;
- spinlock_t gcr_lock;
+ struct regmap *gcr_regs;
} ipcdev;
static char *ipc_err_sources[] = {
@@ -149,6 +149,15 @@ static char *ipc_err_sources[] = {
"Unsigned kernel",
};
+static struct regmap_config gcr_regmap_config = {
+ .name = "intel_pmc_gcr",
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
+ .max_register = PLAT_RESOURCE_GCR_SIZE,
+};
+
/* Prevent concurrent calls to the PMC */
static DEFINE_MUTEX(ipclock);
@@ -182,21 +191,6 @@ static inline u32 ipc_data_readl(u32 offset)
return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
}
-static inline u64 gcr_data_readq(u32 offset)
-{
- return readq(ipcdev.gcr_mem_base + offset);
-}
-
-static inline int is_gcr_valid(u32 offset)
-{
- if (!ipcdev.has_gcr_regs)
- return -EACCES;
-
- if (offset > PLAT_RESOURCE_GCR_SIZE)
- return -EINVAL;
-
- return 0;
-}
/**
* intel_pmc_gcr_read() - Read PMC GCR register
@@ -209,21 +203,12 @@ static inline int is_gcr_valid(u32 offset)
*/
int intel_pmc_gcr_read(u32 offset, u32 *data)
{
- int ret;
-
- spin_lock(&ipcdev.gcr_lock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0) {
- spin_unlock(&ipcdev.gcr_lock);
- return ret;
- }
-
- *data = readl(ipcdev.gcr_mem_base + offset);
+ struct intel_pmc_ipc_dev *pmc = &ipcdev;
- spin_unlock(&ipcdev.gcr_lock);
+ if (!pmc->gcr_regs)
+ return -EACCES;
- return 0;
+ return regmap_read(pmc->gcr_regs, offset, data);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_read);
@@ -239,21 +224,12 @@ EXPORT_SYMBOL_GPL(intel_pmc_gcr_read);
*/
int intel_pmc_gcr_write(u32 offset, u32 data)
{
- int ret;
-
- spin_lock(&ipcdev.gcr_lock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0) {
- spin_unlock(&ipcdev.gcr_lock);
- return ret;
- }
-
- writel(data, ipcdev.gcr_mem_base + offset);
+ struct intel_pmc_ipc_dev *pmc = &ipcdev;
- spin_unlock(&ipcdev.gcr_lock);
+ if (!pmc->gcr_regs)
+ return -EACCES;
- return 0;
+ return regmap_write(pmc->gcr_regs, offset, data);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_write);
@@ -270,33 +246,12 @@ EXPORT_SYMBOL_GPL(intel_pmc_gcr_write);
*/
int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
{
- u32 new_val;
- int ret = 0;
-
- spin_lock(&ipcdev.gcr_lock);
-
- ret = is_gcr_valid(offset);
- if (ret < 0)
- goto gcr_ipc_unlock;
-
- new_val = readl(ipcdev.gcr_mem_base + offset);
-
- new_val &= ~mask;
- new_val |= val & mask;
-
- writel(new_val, ipcdev.gcr_mem_base + offset);
-
- new_val = readl(ipcdev.gcr_mem_base + offset);
+ struct intel_pmc_ipc_dev *pmc = &ipcdev;
- /* check whether the bit update is successful */
- if ((new_val & mask) != (val & mask)) {
- ret = -EIO;
- goto gcr_ipc_unlock;
- }
+ if (!pmc->gcr_regs)
+ return -EACCES;
-gcr_ipc_unlock:
- spin_unlock(&ipcdev.gcr_lock);
- return ret;
+ return regmap_update_bits(pmc->gcr_regs, offset, mask, val);
}
EXPORT_SYMBOL_GPL(intel_pmc_gcr_update);
@@ -473,8 +428,6 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (pmc->dev)
return -EBUSY;
- spin_lock_init(&ipcdev.gcr_lock);
-
pmc->irq_mode = IPC_TRIGGER_MODE_IRQ;
ret = pcim_enable_device(pdev);
@@ -769,17 +722,26 @@ static int ipc_plat_get_res(struct platform_device *pdev)
*/
int intel_pmc_s0ix_counter_read(u64 *data)
{
+ struct intel_pmc_ipc_dev *pmc = &ipcdev;
u64 deep, shlw;
+ int ret;
- if (!ipcdev.has_gcr_regs)
+ if (!pmc->gcr_regs)
return -EACCES;
- deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG);
- shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG);
+ ret = regmap_bulk_read(pmc->gcr_regs, PMC_GCR_TELEM_DEEP_S0IX_REG,
+ &deep, 2);
+ if (ret)
+ return ret;
+
+ ret = regmap_bulk_read(pmc->gcr_regs, PMC_GCR_TELEM_SHLW_S0IX_REG,
+ &shlw, 2);
+ if (ret)
+ return ret;
*data = S0IX_RESIDENCY_IN_USECS(deep, shlw);
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(intel_pmc_s0ix_counter_read);
@@ -798,7 +760,6 @@ static int ipc_plat_probe(struct platform_device *pdev)
ipcdev.dev = &pdev->dev;
ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
init_completion(&ipcdev.cmd_complete);
- spin_lock_init(&ipcdev.gcr_lock);
ipcdev.irq = platform_get_irq(pdev, 0);
if (ipcdev.irq < 0) {
@@ -812,6 +773,13 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
+ ipcdev.gcr_regs = devm_regmap_init_mmio_clk(ipcdev.dev, NULL,
+ ipcdev.gcr_mem_base, &gcr_regmap_config);
+ if (IS_ERR(ipcdev.gcr_regs)) {
+ dev_err(ipcdev.dev, "gcr_regs regmap init failed\n");
+ return PTR_ERR(ipcdev.gcr_regs);;
+ }
+
ret = ipc_create_pmc_devices(pdev);
if (ret) {
dev_err(&pdev->dev, "Failed to create pmc devices\n");
@@ -832,8 +800,6 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
- ipcdev.has_gcr_regs = true;
-
return 0;
}
--
2.7.4
^ permalink raw reply related
* [RFC v4 6/8] platform/x86: intel_punit_ipc: Use generic intel ipc device calls
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:34 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Removed redundant IPC helper functions and refactored the driver to use
APIs provided by generic IPC driver. This patch also cleans-up PUNIT IPC
user drivers(intel_telemetry_pltdrv.c) to use APIs provided by generic IPC
driver.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/include/asm/intel_punit_ipc.h | 125 +++++------
drivers/platform/x86/Kconfig | 1 +
drivers/platform/x86/intel_punit_ipc.c | 303 ++++++++++----------------
drivers/platform/x86/intel_telemetry_pltdrv.c | 97 +++++----
4 files changed, 223 insertions(+), 303 deletions(-)
Changes since v2:
* Added unique name to PUNIT BIOS, GTD, & ISP regmaps.
* Added intel_ipc_dev_put() support.
Changes since v1:
* Removed custom APIs.
* Cleaned up PUNIT IPC user drivers to use APIs provided by generic
IPC driver.
diff --git a/arch/x86/include/asm/intel_punit_ipc.h b/arch/x86/include/asm/intel_punit_ipc.h
index 201eb9d..cf1630c 100644
--- a/arch/x86/include/asm/intel_punit_ipc.h
+++ b/arch/x86/include/asm/intel_punit_ipc.h
@@ -1,10 +1,8 @@
#ifndef _ASM_X86_INTEL_PUNIT_IPC_H_
#define _ASM_X86_INTEL_PUNIT_IPC_H_
-/*
- * Three types of 8bit P-Unit IPC commands are supported,
- * bit[7:6]: [00]: BIOS; [01]: GTD; [10]: ISPD.
- */
+#include <linux/platform_data/x86/intel_ipc_dev.h>
+
typedef enum {
BIOS_IPC = 0,
GTDRIVER_IPC,
@@ -12,61 +10,60 @@ typedef enum {
RESERVED_IPC,
} IPC_TYPE;
-#define IPC_TYPE_OFFSET 6
-#define IPC_PUNIT_BIOS_CMD_BASE (BIOS_IPC << IPC_TYPE_OFFSET)
-#define IPC_PUNIT_GTD_CMD_BASE (GTDDRIVER_IPC << IPC_TYPE_OFFSET)
-#define IPC_PUNIT_ISPD_CMD_BASE (ISPDRIVER_IPC << IPC_TYPE_OFFSET)
-#define IPC_PUNIT_CMD_TYPE_MASK (RESERVED_IPC << IPC_TYPE_OFFSET)
+#define PUNIT_BIOS_IPC_DEV "punit_bios_ipc"
+#define PUNIT_GTD_IPC_DEV "punit_gtd_ipc"
+#define PUNIT_ISP_IPC_DEV "punit_isp_ipc"
+#define PUNIT_PARAM_LEN 3
/* BIOS => Pcode commands */
-#define IPC_PUNIT_BIOS_ZERO (IPC_PUNIT_BIOS_CMD_BASE | 0x00)
-#define IPC_PUNIT_BIOS_VR_INTERFACE (IPC_PUNIT_BIOS_CMD_BASE | 0x01)
-#define IPC_PUNIT_BIOS_READ_PCS (IPC_PUNIT_BIOS_CMD_BASE | 0x02)
-#define IPC_PUNIT_BIOS_WRITE_PCS (IPC_PUNIT_BIOS_CMD_BASE | 0x03)
-#define IPC_PUNIT_BIOS_READ_PCU_CONFIG (IPC_PUNIT_BIOS_CMD_BASE | 0x04)
-#define IPC_PUNIT_BIOS_WRITE_PCU_CONFIG (IPC_PUNIT_BIOS_CMD_BASE | 0x05)
-#define IPC_PUNIT_BIOS_READ_PL1_SETTING (IPC_PUNIT_BIOS_CMD_BASE | 0x06)
-#define IPC_PUNIT_BIOS_WRITE_PL1_SETTING (IPC_PUNIT_BIOS_CMD_BASE | 0x07)
-#define IPC_PUNIT_BIOS_TRIGGER_VDD_RAM (IPC_PUNIT_BIOS_CMD_BASE | 0x08)
-#define IPC_PUNIT_BIOS_READ_TELE_INFO (IPC_PUNIT_BIOS_CMD_BASE | 0x09)
-#define IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x0a)
-#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x0b)
-#define IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x0c)
-#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x0d)
-#define IPC_PUNIT_BIOS_READ_TELE_TRACE (IPC_PUNIT_BIOS_CMD_BASE | 0x0e)
-#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE (IPC_PUNIT_BIOS_CMD_BASE | 0x0f)
-#define IPC_PUNIT_BIOS_READ_TELE_EVENT (IPC_PUNIT_BIOS_CMD_BASE | 0x10)
-#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT (IPC_PUNIT_BIOS_CMD_BASE | 0x11)
-#define IPC_PUNIT_BIOS_READ_MODULE_TEMP (IPC_PUNIT_BIOS_CMD_BASE | 0x12)
-#define IPC_PUNIT_BIOS_RESERVED (IPC_PUNIT_BIOS_CMD_BASE | 0x13)
-#define IPC_PUNIT_BIOS_READ_VOLTAGE_OVER (IPC_PUNIT_BIOS_CMD_BASE | 0x14)
-#define IPC_PUNIT_BIOS_WRITE_VOLTAGE_OVER (IPC_PUNIT_BIOS_CMD_BASE | 0x15)
-#define IPC_PUNIT_BIOS_READ_RATIO_OVER (IPC_PUNIT_BIOS_CMD_BASE | 0x16)
-#define IPC_PUNIT_BIOS_WRITE_RATIO_OVER (IPC_PUNIT_BIOS_CMD_BASE | 0x17)
-#define IPC_PUNIT_BIOS_READ_VF_GL_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x18)
-#define IPC_PUNIT_BIOS_WRITE_VF_GL_CTRL (IPC_PUNIT_BIOS_CMD_BASE | 0x19)
-#define IPC_PUNIT_BIOS_READ_FM_SOC_TEMP_THRESH (IPC_PUNIT_BIOS_CMD_BASE | 0x1a)
-#define IPC_PUNIT_BIOS_WRITE_FM_SOC_TEMP_THRESH (IPC_PUNIT_BIOS_CMD_BASE | 0x1b)
+#define IPC_PUNIT_BIOS_ZERO (0x00)
+#define IPC_PUNIT_BIOS_VR_INTERFACE (0x01)
+#define IPC_PUNIT_BIOS_READ_PCS (0x02)
+#define IPC_PUNIT_BIOS_WRITE_PCS (0x03)
+#define IPC_PUNIT_BIOS_READ_PCU_CONFIG (0x04)
+#define IPC_PUNIT_BIOS_WRITE_PCU_CONFIG (0x05)
+#define IPC_PUNIT_BIOS_READ_PL1_SETTING (0x06)
+#define IPC_PUNIT_BIOS_WRITE_PL1_SETTING (0x07)
+#define IPC_PUNIT_BIOS_TRIGGER_VDD_RAM (0x08)
+#define IPC_PUNIT_BIOS_READ_TELE_INFO (0x09)
+#define IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL (0x0a)
+#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE_CTRL (0x0b)
+#define IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL (0x0c)
+#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL (0x0d)
+#define IPC_PUNIT_BIOS_READ_TELE_TRACE (0x0e)
+#define IPC_PUNIT_BIOS_WRITE_TELE_TRACE (0x0f)
+#define IPC_PUNIT_BIOS_READ_TELE_EVENT (0x10)
+#define IPC_PUNIT_BIOS_WRITE_TELE_EVENT (0x11)
+#define IPC_PUNIT_BIOS_READ_MODULE_TEMP (0x12)
+#define IPC_PUNIT_BIOS_RESERVED (0x13)
+#define IPC_PUNIT_BIOS_READ_VOLTAGE_OVER (0x14)
+#define IPC_PUNIT_BIOS_WRITE_VOLTAGE_OVER (0x15)
+#define IPC_PUNIT_BIOS_READ_RATIO_OVER (0x16)
+#define IPC_PUNIT_BIOS_WRITE_RATIO_OVER (0x17)
+#define IPC_PUNIT_BIOS_READ_VF_GL_CTRL (0x18)
+#define IPC_PUNIT_BIOS_WRITE_VF_GL_CTRL (0x19)
+#define IPC_PUNIT_BIOS_READ_FM_SOC_TEMP_THRESH (0x1a)
+#define IPC_PUNIT_BIOS_WRITE_FM_SOC_TEMP_THRESH (0x1b)
/* GT Driver => Pcode commands */
-#define IPC_PUNIT_GTD_ZERO (IPC_PUNIT_GTD_CMD_BASE | 0x00)
-#define IPC_PUNIT_GTD_CONFIG (IPC_PUNIT_GTD_CMD_BASE | 0x01)
-#define IPC_PUNIT_GTD_READ_ICCP_LIC_CDYN_SCAL (IPC_PUNIT_GTD_CMD_BASE | 0x02)
-#define IPC_PUNIT_GTD_WRITE_ICCP_LIC_CDYN_SCAL (IPC_PUNIT_GTD_CMD_BASE | 0x03)
-#define IPC_PUNIT_GTD_GET_WM_VAL (IPC_PUNIT_GTD_CMD_BASE | 0x06)
-#define IPC_PUNIT_GTD_WRITE_CONFIG_WISHREQ (IPC_PUNIT_GTD_CMD_BASE | 0x07)
-#define IPC_PUNIT_GTD_READ_REQ_DUTY_CYCLE (IPC_PUNIT_GTD_CMD_BASE | 0x16)
-#define IPC_PUNIT_GTD_DIS_VOL_FREQ_CHG_REQUEST (IPC_PUNIT_GTD_CMD_BASE | 0x17)
-#define IPC_PUNIT_GTD_DYNA_DUTY_CYCLE_CTRL (IPC_PUNIT_GTD_CMD_BASE | 0x1a)
-#define IPC_PUNIT_GTD_DYNA_DUTY_CYCLE_TUNING (IPC_PUNIT_GTD_CMD_BASE | 0x1c)
+#define IPC_PUNIT_GTD_ZERO (0x00)
+#define IPC_PUNIT_GTD_CONFIG (0x01)
+#define IPC_PUNIT_GTD_READ_ICCP_LIC_CDYN_SCAL (0x02)
+#define IPC_PUNIT_GTD_WRITE_ICCP_LIC_CDYN_SCAL (0x03)
+#define IPC_PUNIT_GTD_GET_WM_VAL (0x06)
+#define IPC_PUNIT_GTD_WRITE_CONFIG_WISHREQ (0x07)
+#define IPC_PUNIT_GTD_READ_REQ_DUTY_CYCLE (0x16)
+#define IPC_PUNIT_GTD_DIS_VOL_FREQ_CHG_REQUEST (0x17)
+#define IPC_PUNIT_GTD_DYNA_DUTY_CYCLE_CTRL (0x1a)
+#define IPC_PUNIT_GTD_DYNA_DUTY_CYCLE_TUNING (0x1c)
/* ISP Driver => Pcode commands */
-#define IPC_PUNIT_ISPD_ZERO (IPC_PUNIT_ISPD_CMD_BASE | 0x00)
-#define IPC_PUNIT_ISPD_CONFIG (IPC_PUNIT_ISPD_CMD_BASE | 0x01)
-#define IPC_PUNIT_ISPD_GET_ISP_LTR_VAL (IPC_PUNIT_ISPD_CMD_BASE | 0x02)
-#define IPC_PUNIT_ISPD_ACCESS_IU_FREQ_BOUNDS (IPC_PUNIT_ISPD_CMD_BASE | 0x03)
-#define IPC_PUNIT_ISPD_READ_CDYN_LEVEL (IPC_PUNIT_ISPD_CMD_BASE | 0x04)
-#define IPC_PUNIT_ISPD_WRITE_CDYN_LEVEL (IPC_PUNIT_ISPD_CMD_BASE | 0x05)
+#define IPC_PUNIT_ISPD_ZERO (0x00)
+#define IPC_PUNIT_ISPD_CONFIG (0x01)
+#define IPC_PUNIT_ISPD_GET_ISP_LTR_VAL (0x02)
+#define IPC_PUNIT_ISPD_ACCESS_IU_FREQ_BOUNDS (0x03)
+#define IPC_PUNIT_ISPD_READ_CDYN_LEVEL (0x04)
+#define IPC_PUNIT_ISPD_WRITE_CDYN_LEVEL (0x05)
/* Error codes */
#define IPC_PUNIT_ERR_SUCCESS 0
@@ -77,25 +74,11 @@ typedef enum {
#define IPC_PUNIT_ERR_INVALID_VR_ID 5
#define IPC_PUNIT_ERR_VR_ERR 6
-#if IS_ENABLED(CONFIG_INTEL_PUNIT_IPC)
-
-int intel_punit_ipc_simple_command(int cmd, int para1, int para2);
-int intel_punit_ipc_command(u32 cmd, u32 para1, u32 para2, u32 *in, u32 *out);
-
-#else
-
-static inline int intel_punit_ipc_simple_command(int cmd,
- int para1, int para2)
+static inline void punit_cmd_init(u32 *cmd, u32 param1, u32 param2, u32 param3)
{
- return -ENODEV;
+ cmd[0] = param1;
+ cmd[1] = param2;
+ cmd[2] = param3;
}
-static inline int intel_punit_ipc_command(u32 cmd, u32 para1, u32 para2,
- u32 *in, u32 *out)
-{
- return -ENODEV;
-}
-
-#endif /* CONFIG_INTEL_PUNIT_IPC */
-
#endif
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 724ee696..9442c23 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1096,6 +1096,7 @@ config SURFACE_3_BUTTON
config INTEL_PUNIT_IPC
tristate "Intel P-Unit IPC Driver"
+ select REGMAP_MMIO
---help---
This driver provides support for Intel P-Unit Mailbox IPC mechanism,
which is used to bridge the communications between kernel and P-Unit.
diff --git a/drivers/platform/x86/intel_punit_ipc.c b/drivers/platform/x86/intel_punit_ipc.c
index b5b8901..f310a05 100644
--- a/drivers/platform/x86/intel_punit_ipc.c
+++ b/drivers/platform/x86/intel_punit_ipc.c
@@ -18,18 +18,18 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
+#include <linux/platform_data/x86/intel_ipc_dev.h>
+#include <linux/regmap.h>
#include <asm/intel_punit_ipc.h>
-/* IPC Mailbox registers */
-#define OFFSET_DATA_LOW 0x0
-#define OFFSET_DATA_HIGH 0x4
/* bit field of interface register */
#define CMD_RUN BIT(31)
-#define CMD_ERRCODE_MASK GENMASK(7, 0)
+#define CMD_ERRCODE_MASK GENMASK(7, 0)
#define CMD_PARA1_SHIFT 8
#define CMD_PARA2_SHIFT 16
-#define CMD_TIMEOUT_SECONDS 1
+/* IPC PUNIT commands */
+#define IPC_DEV_PUNIT_CMD_STATUS_ERR_MASK GENMASK(7, 0)
enum {
BASE_DATA = 0,
@@ -39,187 +39,42 @@ enum {
typedef struct {
struct device *dev;
- struct mutex lock;
- int irq;
- struct completion cmd_complete;
/* base of interface and data registers */
void __iomem *base[RESERVED_IPC][BASE_MAX];
+ struct intel_ipc_dev *ipc_dev[RESERVED_IPC];
IPC_TYPE type;
} IPC_DEV;
static IPC_DEV *punit_ipcdev;
-static inline u32 ipc_read_status(IPC_DEV *ipcdev, IPC_TYPE type)
-{
- return readl(ipcdev->base[type][BASE_IFACE]);
-}
-
-static inline void ipc_write_cmd(IPC_DEV *ipcdev, IPC_TYPE type, u32 cmd)
-{
- writel(cmd, ipcdev->base[type][BASE_IFACE]);
-}
-
-static inline u32 ipc_read_data_low(IPC_DEV *ipcdev, IPC_TYPE type)
-{
- return readl(ipcdev->base[type][BASE_DATA] + OFFSET_DATA_LOW);
-}
-
-static inline u32 ipc_read_data_high(IPC_DEV *ipcdev, IPC_TYPE type)
-{
- return readl(ipcdev->base[type][BASE_DATA] + OFFSET_DATA_HIGH);
-}
-
-static inline void ipc_write_data_low(IPC_DEV *ipcdev, IPC_TYPE type, u32 data)
-{
- writel(data, ipcdev->base[type][BASE_DATA] + OFFSET_DATA_LOW);
-}
-
-static inline void ipc_write_data_high(IPC_DEV *ipcdev, IPC_TYPE type, u32 data)
-{
- writel(data, ipcdev->base[type][BASE_DATA] + OFFSET_DATA_HIGH);
-}
+const char *ipc_dev_name[RESERVED_IPC] = {
+ PUNIT_BIOS_IPC_DEV,
+ PUNIT_GTD_IPC_DEV,
+ PUNIT_ISP_IPC_DEV
+};
-static const char *ipc_err_string(int error)
-{
- if (error == IPC_PUNIT_ERR_SUCCESS)
- return "no error";
- else if (error == IPC_PUNIT_ERR_INVALID_CMD)
- return "invalid command";
- else if (error == IPC_PUNIT_ERR_INVALID_PARAMETER)
- return "invalid parameter";
- else if (error == IPC_PUNIT_ERR_CMD_TIMEOUT)
- return "command timeout";
- else if (error == IPC_PUNIT_ERR_CMD_LOCKED)
- return "command locked";
- else if (error == IPC_PUNIT_ERR_INVALID_VR_ID)
- return "invalid vr id";
- else if (error == IPC_PUNIT_ERR_VR_ERR)
- return "vr error";
- else
- return "unknown error";
-}
+static struct regmap_config punit_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+};
-static int intel_punit_ipc_check_status(IPC_DEV *ipcdev, IPC_TYPE type)
+int pre_simple_cmd_fn(u32 *cmd_list, u32 cmdlen)
{
- int loops = CMD_TIMEOUT_SECONDS * USEC_PER_SEC;
- int errcode;
- int status;
-
- if (ipcdev->irq) {
- if (!wait_for_completion_timeout(&ipcdev->cmd_complete,
- CMD_TIMEOUT_SECONDS * HZ)) {
- dev_err(ipcdev->dev, "IPC timed out\n");
- return -ETIMEDOUT;
- }
- } else {
- while ((ipc_read_status(ipcdev, type) & CMD_RUN) && --loops)
- udelay(1);
- if (!loops) {
- dev_err(ipcdev->dev, "IPC timed out\n");
- return -ETIMEDOUT;
- }
- }
+ if (!cmd_list || cmdlen != PUNIT_PARAM_LEN)
+ return -EINVAL;
- status = ipc_read_status(ipcdev, type);
- errcode = status & CMD_ERRCODE_MASK;
- if (errcode) {
- dev_err(ipcdev->dev, "IPC failed: %s, IPC_STS=0x%x\n",
- ipc_err_string(errcode), status);
- return -EIO;
- }
+ cmd_list[0] |= CMD_RUN | cmd_list[1] << CMD_PARA1_SHIFT |
+ cmd_list[2] << CMD_PARA1_SHIFT;
return 0;
}
-/**
- * intel_punit_ipc_simple_command() - Simple IPC command
- * @cmd: IPC command code.
- * @para1: First 8bit parameter, set 0 if not used.
- * @para2: Second 8bit parameter, set 0 if not used.
- *
- * Send a IPC command to P-Unit when there is no data transaction
- *
- * Return: IPC error code or 0 on success.
- */
-int intel_punit_ipc_simple_command(int cmd, int para1, int para2)
-{
- IPC_DEV *ipcdev = punit_ipcdev;
- IPC_TYPE type;
- u32 val;
- int ret;
-
- mutex_lock(&ipcdev->lock);
-
- reinit_completion(&ipcdev->cmd_complete);
- type = (cmd & IPC_PUNIT_CMD_TYPE_MASK) >> IPC_TYPE_OFFSET;
-
- val = cmd & ~IPC_PUNIT_CMD_TYPE_MASK;
- val |= CMD_RUN | para2 << CMD_PARA2_SHIFT | para1 << CMD_PARA1_SHIFT;
- ipc_write_cmd(ipcdev, type, val);
- ret = intel_punit_ipc_check_status(ipcdev, type);
-
- mutex_unlock(&ipcdev->lock);
-
- return ret;
-}
-EXPORT_SYMBOL(intel_punit_ipc_simple_command);
-
-/**
- * intel_punit_ipc_command() - IPC command with data and pointers
- * @cmd: IPC command code.
- * @para1: First 8bit parameter, set 0 if not used.
- * @para2: Second 8bit parameter, set 0 if not used.
- * @in: Input data, 32bit for BIOS cmd, two 32bit for GTD and ISPD.
- * @out: Output data.
- *
- * Send a IPC command to P-Unit with data transaction
- *
- * Return: IPC error code or 0 on success.
- */
-int intel_punit_ipc_command(u32 cmd, u32 para1, u32 para2, u32 *in, u32 *out)
-{
- IPC_DEV *ipcdev = punit_ipcdev;
- IPC_TYPE type;
- u32 val;
- int ret;
-
- mutex_lock(&ipcdev->lock);
-
- reinit_completion(&ipcdev->cmd_complete);
- type = (cmd & IPC_PUNIT_CMD_TYPE_MASK) >> IPC_TYPE_OFFSET;
-
- if (in) {
- ipc_write_data_low(ipcdev, type, *in);
- if (type == GTDRIVER_IPC || type == ISPDRIVER_IPC)
- ipc_write_data_high(ipcdev, type, *++in);
- }
-
- val = cmd & ~IPC_PUNIT_CMD_TYPE_MASK;
- val |= CMD_RUN | para2 << CMD_PARA2_SHIFT | para1 << CMD_PARA1_SHIFT;
- ipc_write_cmd(ipcdev, type, val);
-
- ret = intel_punit_ipc_check_status(ipcdev, type);
- if (ret)
- goto out;
-
- if (out) {
- *out = ipc_read_data_low(ipcdev, type);
- if (type == GTDRIVER_IPC || type == ISPDRIVER_IPC)
- *++out = ipc_read_data_high(ipcdev, type);
- }
-
-out:
- mutex_unlock(&ipcdev->lock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(intel_punit_ipc_command);
-
-static irqreturn_t intel_punit_ioc(int irq, void *dev_id)
+/* Input data, 32bit for BIOS cmd, two 32bit for GTD and ISPD. */
+int pre_raw_cmd_fn(u32 *cmd_list, u32 cmdlen, u8 *in, u32 inlen, u32 *out,
+ u32 outlen, u32 dptr, u32 sptr)
{
- IPC_DEV *ipcdev = dev_id;
-
- complete(&ipcdev->cmd_complete);
- return IRQ_HANDLED;
+ return pre_simple_cmd_fn(cmd_list, cmdlen);
}
static int intel_punit_get_bars(struct platform_device *pdev)
@@ -282,9 +137,77 @@ static int intel_punit_get_bars(struct platform_device *pdev)
return 0;
}
+static int punit_ipc_err_code(int status)
+{
+ return (status & CMD_ERRCODE_MASK);
+}
+
+static int punit_ipc_busy_check(int status)
+{
+ return status | CMD_RUN;
+}
+
+static struct intel_ipc_dev *intel_punit_ipc_dev_create(struct device *dev,
+ const char *devname,
+ int irq,
+ void __iomem *base,
+ void __iomem *data)
+{
+ struct intel_ipc_dev_ops *ops;
+ struct intel_ipc_dev_cfg *cfg;
+ struct regmap *cmd_regs, *data_regs;
+
+ cfg = devm_kzalloc(dev, sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return ERR_PTR(-ENOMEM);
+
+ ops = devm_kzalloc(dev, sizeof(*ops), GFP_KERNEL);
+ if (!ops)
+ return ERR_PTR(-ENOMEM);
+
+ punit_regmap_config.name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s",
+ devname, "base");
+
+ cmd_regs = devm_regmap_init_mmio_clk(dev, NULL, base,
+ &punit_regmap_config);
+ if (IS_ERR(cmd_regs)) {
+ dev_err(dev, "cmd_regs regmap init failed\n");
+ return ERR_CAST(cmd_regs);;
+ }
+
+ punit_regmap_config.name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s",
+ devname, "data");
+
+ data_regs = devm_regmap_init_mmio_clk(dev, NULL, data,
+ &punit_regmap_config);
+ if (IS_ERR(data_regs)) {
+ dev_err(dev, "data_regs regmap init failed\n");
+ return ERR_CAST(data_regs);;
+ }
+
+ /* set IPC dev ops */
+ ops->to_err_code = punit_ipc_err_code;
+ ops->busy_check = punit_ipc_busy_check;
+ ops->pre_simple_cmd_fn = pre_simple_cmd_fn;
+ ops->pre_raw_cmd_fn = pre_raw_cmd_fn;
+
+ if (irq > 0)
+ cfg->mode = IPC_DEV_MODE_IRQ;
+ else
+ cfg->mode = IPC_DEV_MODE_POLLING;
+
+ cfg->chan_type = IPC_CHANNEL_IA_PUNIT;
+ cfg->irq = irq;
+ cfg->irqflags = IRQF_NO_SUSPEND | IRQF_SHARED;
+ cfg->cmd_regs = cmd_regs;
+ cfg->data_regs = data_regs;
+
+ return devm_intel_ipc_dev_create(dev, devname, cfg, ops);
+}
+
static int intel_punit_ipc_probe(struct platform_device *pdev)
{
- int irq, ret;
+ int irq, ret, i;
punit_ipcdev = devm_kzalloc(&pdev->dev,
sizeof(*punit_ipcdev), GFP_KERNEL);
@@ -294,35 +217,30 @@ static int intel_punit_ipc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, punit_ipcdev);
irq = platform_get_irq(pdev, 0);
- if (irq < 0) {
- punit_ipcdev->irq = 0;
- dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n");
- } else {
- ret = devm_request_irq(&pdev->dev, irq, intel_punit_ioc,
- IRQF_NO_SUSPEND, "intel_punit_ipc",
- &punit_ipcdev);
- if (ret) {
- dev_err(&pdev->dev, "Failed to request irq: %d\n", irq);
- return ret;
- }
- punit_ipcdev->irq = irq;
- }
ret = intel_punit_get_bars(pdev);
if (ret)
- goto out;
+ return ret;
+
+ for (i = 0; i < RESERVED_IPC; i++) {
+ punit_ipcdev->ipc_dev[i] = intel_punit_ipc_dev_create(
+ &pdev->dev,
+ ipc_dev_name[i],
+ irq,
+ punit_ipcdev->base[i][BASE_IFACE],
+ punit_ipcdev->base[i][BASE_DATA]);
+
+ if (IS_ERR(punit_ipcdev->ipc_dev[i])) {
+ dev_err(&pdev->dev, "%s create failed\n",
+ ipc_dev_name[i]);
+ return PTR_ERR(punit_ipcdev->ipc_dev[i]);
+ }
+ }
punit_ipcdev->dev = &pdev->dev;
- mutex_init(&punit_ipcdev->lock);
- init_completion(&punit_ipcdev->cmd_complete);
-out:
return ret;
-}
-static int intel_punit_ipc_remove(struct platform_device *pdev)
-{
- return 0;
}
static const struct acpi_device_id punit_ipc_acpi_ids[] = {
@@ -332,7 +250,6 @@ static const struct acpi_device_id punit_ipc_acpi_ids[] = {
static struct platform_driver intel_punit_ipc_driver = {
.probe = intel_punit_ipc_probe,
- .remove = intel_punit_ipc_remove,
.driver = {
.name = "intel_punit_ipc",
.acpi_match_table = ACPI_PTR(punit_ipc_acpi_ids),
diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c b/drivers/platform/x86/intel_telemetry_pltdrv.c
index e0424d5..bf8284a 100644
--- a/drivers/platform/x86/intel_telemetry_pltdrv.c
+++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
@@ -98,6 +98,7 @@ struct telem_ssram_region {
};
static struct telemetry_plt_config *telm_conf;
+static struct intel_ipc_dev *punit_bios_ipc_dev;
/*
* The following counters are programmed by default during setup.
@@ -127,7 +128,6 @@ static struct telemetry_evtmap
{"PMC_S0IX_BLOCK_IPS_CLOCKS", 0x600B},
};
-
static struct telemetry_evtmap
telemetry_apl_pss_default_events[TELEM_MAX_OS_ALLOCATED_EVENTS] = {
{"IA_CORE0_C6_RES", 0x0400},
@@ -283,13 +283,12 @@ static inline int telemetry_plt_config_ioss_event(u32 evt_id, int index)
static inline int telemetry_plt_config_pss_event(u32 evt_id, int index)
{
u32 write_buf;
- int ret;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
write_buf = evt_id | TELEM_EVENT_ENABLE;
- ret = intel_punit_ipc_command(IPC_PUNIT_BIOS_WRITE_TELE_EVENT,
- index, 0, &write_buf, NULL);
-
- return ret;
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT, index, 0);
+ return ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&write_buf, sizeof(write_buf), NULL, 0, 0, 0);
}
static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
@@ -435,6 +434,7 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
int ret, index, idx;
u32 *pss_evtmap;
u32 telem_ctrl;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
num_pss_evts = evtconfig.num_evts;
pss_period = evtconfig.period;
@@ -442,8 +442,9 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
/* PSS Config */
/* Get telemetry EVENT CTL */
- ret = intel_punit_ipc_command(IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL,
- 0, 0, NULL, &telem_ctrl);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN, NULL,
+ 0, &telem_ctrl, 1, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Read Failed\n");
return ret;
@@ -451,8 +452,9 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
/* Disable Telemetry */
TELEM_DISABLE(telem_ctrl);
- ret = intel_punit_ipc_command(IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL, 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -463,9 +465,10 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
/* Clear All Events */
TELEM_CLEAR_EVENTS(telem_ctrl);
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -489,9 +492,10 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
/* Clear All Events */
TELEM_CLEAR_EVENTS(telem_ctrl);
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -540,8 +544,9 @@ static int telemetry_setup_pssevtconfig(struct telemetry_evtconfig evtconfig,
TELEM_ENABLE_PERIODIC(telem_ctrl);
telem_ctrl |= pss_period;
- ret = intel_punit_ipc_command(IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL, 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Enable Write Failed\n");
return ret;
@@ -601,6 +606,7 @@ static int telemetry_setup(struct platform_device *pdev)
{
struct telemetry_evtconfig pss_evtconfig, ioss_evtconfig;
u32 read_buf, events, event_regs;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
int ret;
ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_INFO_READ,
@@ -626,8 +632,9 @@ static int telemetry_setup(struct platform_device *pdev)
telm_conf->ioss_config.max_period = TELEM_MAX_PERIOD(read_buf);
/* PUNIT Mailbox Setup */
- ret = intel_punit_ipc_command(IPC_PUNIT_BIOS_READ_TELE_INFO, 0, 0,
- NULL, &read_buf);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_READ_TELE_INFO, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ NULL, 0, &read_buf, 1, 0, 0);
if (ret) {
dev_err(&pdev->dev, "PSS TELEM_INFO Read Failed\n");
return ret;
@@ -695,6 +702,7 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
{
u32 telem_ctrl = 0;
int ret = 0;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
mutex_lock(&(telm_conf->telem_lock));
if (ioss_period) {
@@ -752,9 +760,9 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
}
/* Get telemetry EVENT CTL */
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL,
- 0, 0, NULL, &telem_ctrl);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_READ_TELE_EVENT_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ NULL, 0, &telem_ctrl, 1, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Read Failed\n");
goto out;
@@ -762,9 +770,11 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
/* Disable Telemetry */
TELEM_DISABLE(telem_ctrl);
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0,
+ 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Disable Write Failed\n");
goto out;
@@ -776,9 +786,11 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
TELEM_ENABLE_PERIODIC(telem_ctrl);
telem_ctrl |= pss_period;
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
- 0, 0, &telem_ctrl, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL, 0,
+ 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("PSS TELEM_CTRL Event Enable Write Failed\n");
goto out;
@@ -1013,6 +1025,7 @@ static int telemetry_plt_get_trace_verbosity(enum telemetry_unit telem_unit,
{
u32 temp = 0;
int ret;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
if (verbosity == NULL)
return -EINVAL;
@@ -1020,9 +1033,9 @@ static int telemetry_plt_get_trace_verbosity(enum telemetry_unit telem_unit,
mutex_lock(&(telm_conf->telem_trace_lock));
switch (telem_unit) {
case TELEM_PSS:
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL,
- 0, 0, NULL, &temp);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ NULL, 0, &temp, 1, 0, 0);
if (ret) {
pr_err("PSS TRACE_CTRL Read Failed\n");
goto out;
@@ -1058,15 +1071,16 @@ static int telemetry_plt_set_trace_verbosity(enum telemetry_unit telem_unit,
{
u32 temp = 0;
int ret;
+ u32 cmd[PUNIT_PARAM_LEN] = {0};
verbosity &= TELEM_TRC_VERBOSITY_MASK;
mutex_lock(&(telm_conf->telem_trace_lock));
switch (telem_unit) {
case TELEM_PSS:
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL,
- 0, 0, NULL, &temp);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_READ_TELE_TRACE_CTRL, 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ NULL, 0, &temp, 1, 0, 0);
if (ret) {
pr_err("PSS TRACE_CTRL Read Failed\n");
goto out;
@@ -1074,10 +1088,10 @@ static int telemetry_plt_set_trace_verbosity(enum telemetry_unit telem_unit,
TELEM_CLEAR_VERBOSITY_BITS(temp);
TELEM_SET_VERBOSITY_BITS(temp, verbosity);
-
- ret = intel_punit_ipc_command(
- IPC_PUNIT_BIOS_WRITE_TELE_TRACE_CTRL,
- 0, 0, &temp, NULL);
+ punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT_CTRL,
+ 0, 0);
+ ret = ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
+ (u8 *)&temp, sizeof(temp), NULL, 0, 0, 0);
if (ret) {
pr_err("PSS TRACE_CTRL Verbosity Set Failed\n");
goto out;
@@ -1139,6 +1153,10 @@ static int telemetry_pltdrv_probe(struct platform_device *pdev)
if (!id)
return -ENODEV;
+ punit_bios_ipc_dev = intel_ipc_dev_get(PUNIT_BIOS_IPC_DEV);
+ if (IS_ERR_OR_NULL(punit_bios_ipc_dev))
+ return PTR_ERR(punit_bios_ipc_dev);
+
telm_conf = (struct telemetry_plt_config *)id->driver_data;
res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1218,6 +1236,7 @@ static int telemetry_pltdrv_probe(struct platform_device *pdev)
static int telemetry_pltdrv_remove(struct platform_device *pdev)
{
telemetry_clear_pltdata();
+ intel_ipc_dev_put(punit_bios_ipc_dev);
iounmap(telm_conf->pss_config.regmap);
iounmap(telm_conf->ioss_config.regmap);
--
2.7.4
^ permalink raw reply related
* [RFC v4 7/8] platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:34 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Removed redundant IPC helper functions and refactored the driver to use
generic IPC device driver APIs. Also, cleaned up the driver to minimize
the usage of global variable ipcdev by propogating the struct
intel_pmc_ipc_dev pointer or by getting it from device private data.
This patch also cleans-up PMC IPC user drivers(intel_telemetry_pltdrv.c,
intel_soc_pmic_bxtwc.c) to use APIs provided by generic IPC driver.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
arch/x86/include/asm/intel_pmc_ipc.h | 37 +--
drivers/mfd/intel_soc_pmic_bxtwc.c | 21 +-
drivers/platform/x86/intel_pmc_ipc.c | 393 ++++++++++----------------
drivers/platform/x86/intel_telemetry_pltdrv.c | 119 ++++----
include/linux/mfd/intel_soc_pmic.h | 2 +
5 files changed, 238 insertions(+), 334 deletions(-)
Changes since v3:
* Added unique name to PMC regmaps.
* Added support to clear interrupt bit.
* Added intel_ipc_dev_put() support.
Changes since v1:
* Removed custom APIs.
* Cleaned up PMC IPC user drivers to use APIs provided by generic
IPC driver.
diff --git a/arch/x86/include/asm/intel_pmc_ipc.h b/arch/x86/include/asm/intel_pmc_ipc.h
index fac89eb..9fc7c3c 100644
--- a/arch/x86/include/asm/intel_pmc_ipc.h
+++ b/arch/x86/include/asm/intel_pmc_ipc.h
@@ -1,10 +1,15 @@
#ifndef _ASM_X86_INTEL_PMC_IPC_H_
#define _ASM_X86_INTEL_PMC_IPC_H_
+#include <linux/platform_data/x86/intel_ipc_dev.h>
+
+#define INTEL_PMC_IPC_DEV "intel_pmc_ipc"
+#define PMC_PARAM_LEN 2
+
/* Commands */
#define PMC_IPC_PMIC_ACCESS 0xFF
-#define PMC_IPC_PMIC_ACCESS_READ 0x0
-#define PMC_IPC_PMIC_ACCESS_WRITE 0x1
+#define PMC_IPC_PMIC_ACCESS_READ 0x0
+#define PMC_IPC_PMIC_ACCESS_WRITE 0x1
#define PMC_IPC_USB_PWR_CTRL 0xF0
#define PMC_IPC_PMIC_BLACKLIST_SEL 0xEF
#define PMC_IPC_PHY_CONFIG 0xEE
@@ -28,13 +33,14 @@
#define PMC_GCR_TELEM_DEEP_S0IX_REG 0x78
#define PMC_GCR_TELEM_SHLW_S0IX_REG 0x80
+static inline void pmc_cmd_init(u32 *cmd, u32 param1, u32 param2)
+{
+ cmd[0] = param1;
+ cmd[1] = param2;
+}
+
#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
-int intel_pmc_ipc_simple_command(int cmd, int sub);
-int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
- u32 *out, u32 outlen, u32 dptr, u32 sptr);
-int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
- u32 *out, u32 outlen);
int intel_pmc_s0ix_counter_read(u64 *data);
int intel_pmc_gcr_read(u32 offset, u32 *data);
int intel_pmc_gcr_write(u32 offset, u32 data);
@@ -42,23 +48,6 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val);
#else
-static inline int intel_pmc_ipc_simple_command(int cmd, int sub)
-{
- return -EINVAL;
-}
-
-static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
- u32 *out, u32 outlen, u32 dptr, u32 sptr)
-{
- return -EINVAL;
-}
-
-static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
- u32 *out, u32 outlen)
-{
- return -EINVAL;
-}
-
static inline int intel_pmc_s0ix_counter_read(u64 *data)
{
return -EINVAL;
diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index 15bc052..f9df9e7 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -271,6 +271,8 @@ static int regmap_ipc_byte_reg_read(void *context, unsigned int reg,
u8 ipc_in[2];
u8 ipc_out[4];
struct intel_soc_pmic *pmic = context;
+ u32 cmd[PMC_PARAM_LEN] = {PMC_IPC_PMIC_ACCESS,
+ PMC_IPC_PMIC_ACCESS_READ};
if (!pmic)
return -EINVAL;
@@ -284,9 +286,8 @@ static int regmap_ipc_byte_reg_read(void *context, unsigned int reg,
ipc_in[0] = reg;
ipc_in[1] = i2c_addr;
- ret = intel_pmc_ipc_command(PMC_IPC_PMIC_ACCESS,
- PMC_IPC_PMIC_ACCESS_READ,
- ipc_in, sizeof(ipc_in), (u32 *)ipc_out, 1);
+ ret = ipc_dev_raw_cmd(pmic->ipc_dev, cmd, PMC_PARAM_LEN, ipc_in,
+ sizeof(ipc_in), (u32 *)ipc_out, 1, 0, 0);
if (ret) {
dev_err(pmic->dev, "Failed to read from PMIC\n");
return ret;
@@ -303,6 +304,8 @@ static int regmap_ipc_byte_reg_write(void *context, unsigned int reg,
int i2c_addr;
u8 ipc_in[3];
struct intel_soc_pmic *pmic = context;
+ u32 cmd[PMC_PARAM_LEN] = {PMC_IPC_PMIC_ACCESS,
+ PMC_IPC_PMIC_ACCESS_WRITE};
if (!pmic)
return -EINVAL;
@@ -317,9 +320,8 @@ static int regmap_ipc_byte_reg_write(void *context, unsigned int reg,
ipc_in[0] = reg;
ipc_in[1] = i2c_addr;
ipc_in[2] = val;
- ret = intel_pmc_ipc_command(PMC_IPC_PMIC_ACCESS,
- PMC_IPC_PMIC_ACCESS_WRITE,
- ipc_in, sizeof(ipc_in), NULL, 0);
+ ret = ipc_dev_raw_cmd(pmic->ipc_dev, cmd, PMC_PARAM_LEN, ipc_in,
+ sizeof(ipc_in), NULL, 0, 0, 0);
if (ret) {
dev_err(pmic->dev, "Failed to write to PMIC\n");
return ret;
@@ -445,6 +447,10 @@ static int bxtwc_probe(struct platform_device *pdev)
if (!pmic)
return -ENOMEM;
+ pmic->ipc_dev = intel_ipc_dev_get(INTEL_PMC_IPC_DEV);
+ if (IS_ERR_OR_NULL(pmic->ipc_dev))
+ return PTR_ERR(pmic->ipc_dev);
+
ret = platform_get_irq(pdev, 0);
if (ret < 0) {
dev_err(&pdev->dev, "Invalid IRQ\n");
@@ -562,7 +568,10 @@ static int bxtwc_probe(struct platform_device *pdev)
static int bxtwc_remove(struct platform_device *pdev)
{
+ struct intel_soc_pmic *pmic = dev_get_drvdata(&pdev->dev);
+
sysfs_remove_group(&pdev->dev.kobj, &bxtwc_group);
+ intel_ipc_dev_put(pmic->ipc_dev);
return 0;
}
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index 55611d2..e93bdf8 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -47,18 +47,8 @@
* The ARC handles the interrupt and services it, writing optional data to
* the IPC1 registers, updates the IPC_STS response register with the status.
*/
-#define IPC_CMD 0x0
-#define IPC_CMD_MSI 0x100
#define IPC_CMD_SIZE 16
#define IPC_CMD_SUBCMD 12
-#define IPC_STATUS 0x04
-#define IPC_STATUS_IRQ 0x4
-#define IPC_STATUS_ERR 0x2
-#define IPC_STATUS_BUSY 0x1
-#define IPC_SPTR 0x08
-#define IPC_DPTR 0x0C
-#define IPC_WRITE_BUFFER 0x80
-#define IPC_READ_BUFFER 0x90
/* Residency with clock rate at 19.2MHz to usecs */
#define S0IX_RESIDENCY_IN_USECS(d, s) \
@@ -73,11 +63,6 @@
*/
#define IPC_DATA_BUFFER_SIZE 16
-#define IPC_LOOP_CNT 3000000
-#define IPC_MAX_SEC 3
-
-#define IPC_TRIGGER_MODE_IRQ true
-
/* exported resources from IFWI */
#define PLAT_RESOURCE_IPC_INDEX 0
#define PLAT_RESOURCE_IPC_SIZE 0x1000
@@ -117,36 +102,41 @@
#define PMC_CFG_NO_REBOOT_EN (1 << 4)
#define PMC_CFG_NO_REBOOT_DIS (0 << 4)
+/* IPC PMC commands */
+#define IPC_DEV_PMC_CMD_MSI BIT(8)
+#define IPC_DEV_PMC_CMD_SIZE 16
+#define IPC_DEV_PMC_CMD_SUBCMD 12
+#define IPC_DEV_PMC_CMD_STATUS BIT(2)
+#define IPC_DEV_PMC_CMD_STATUS_IRQ BIT(2)
+#define IPC_DEV_PMC_CMD_STATUS_ERR BIT(1)
+#define IPC_DEV_PMC_CMD_STATUS_ERR_MASK GENMASK(7, 0)
+#define IPC_DEV_PMC_CMD_STATUS_BUSY BIT(0)
+
+/*IPC PMC reg offsets */
+#define IPC_DEV_PMC_STATUS_OFFSET 0x04
+#define IPC_DEV_PMC_SPTR_OFFSET 0x08
+#define IPC_DEV_PMC_DPTR_OFFSET 0x0C
+#define IPC_DEV_PMC_WRBUF_OFFSET 0x80
+#define IPC_DEV_PMC_RBUF_OFFSET 0x90
+
static struct intel_pmc_ipc_dev {
struct device *dev;
+ struct intel_ipc_dev *pmc_ipc_dev;
+ struct intel_ipc_dev_ops ops;
+ struct intel_ipc_dev_cfg cfg;
void __iomem *ipc_base;
- bool irq_mode;
- int irq;
- int cmd;
- struct completion cmd_complete;
/* gcr */
void __iomem *gcr_mem_base;
struct regmap *gcr_regs;
} ipcdev;
-static char *ipc_err_sources[] = {
- [IPC_ERR_NONE] =
- "no error",
- [IPC_ERR_CMD_NOT_SUPPORTED] =
- "command not supported",
- [IPC_ERR_CMD_NOT_SERVICED] =
- "command not serviced",
- [IPC_ERR_UNABLE_TO_SERVICE] =
- "unable to service",
- [IPC_ERR_CMD_INVALID] =
- "command invalid",
- [IPC_ERR_CMD_FAILED] =
- "command failed",
- [IPC_ERR_EMSECURITY] =
- "Invalid Battery",
- [IPC_ERR_UNSIGNEDKERNEL] =
- "Unsigned kernel",
+static struct regmap_config pmc_regmap_config = {
+ .name = "intel_pmc_regs",
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
};
static struct regmap_config gcr_regmap_config = {
@@ -158,40 +148,6 @@ static struct regmap_config gcr_regmap_config = {
.max_register = PLAT_RESOURCE_GCR_SIZE,
};
-/* Prevent concurrent calls to the PMC */
-static DEFINE_MUTEX(ipclock);
-
-static inline void ipc_send_command(u32 cmd)
-{
- ipcdev.cmd = cmd;
- if (ipcdev.irq_mode) {
- reinit_completion(&ipcdev.cmd_complete);
- cmd |= IPC_CMD_MSI;
- }
- writel(cmd, ipcdev.ipc_base + IPC_CMD);
-}
-
-static inline u32 ipc_read_status(void)
-{
- return readl(ipcdev.ipc_base + IPC_STATUS);
-}
-
-static inline void ipc_data_writel(u32 data, u32 offset)
-{
- writel(data, ipcdev.ipc_base + IPC_WRITE_BUFFER + offset);
-}
-
-static inline u8 __maybe_unused ipc_data_readb(u32 offset)
-{
- return readb(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
-}
-
-static inline u32 ipc_data_readl(u32 offset)
-{
- return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
-}
-
-
/**
* intel_pmc_gcr_read() - Read PMC GCR register
* @offset: offset of GCR register from GCR address base
@@ -263,160 +219,109 @@ static int update_no_reboot_bit(void *priv, bool set)
PMC_CFG_NO_REBOOT_MASK, value);
}
-static int intel_pmc_ipc_check_status(void)
+static int pre_simple_cmd_fn(u32 *cmd_list, u32 cmdlen)
{
- int status;
- int ret = 0;
-
- if (ipcdev.irq_mode) {
- if (0 == wait_for_completion_timeout(
- &ipcdev.cmd_complete, IPC_MAX_SEC * HZ))
- ret = -ETIMEDOUT;
- } else {
- int loop_count = IPC_LOOP_CNT;
-
- while ((ipc_read_status() & IPC_STATUS_BUSY) && --loop_count)
- udelay(1);
- if (loop_count == 0)
- ret = -ETIMEDOUT;
- }
-
- status = ipc_read_status();
- if (ret == -ETIMEDOUT) {
- dev_err(ipcdev.dev,
- "IPC timed out, TS=0x%x, CMD=0x%x\n",
- status, ipcdev.cmd);
- return ret;
- }
+ if (!cmd_list || cmdlen != PMC_PARAM_LEN)
+ return -EINVAL;
- if (status & IPC_STATUS_ERR) {
- int i;
-
- ret = -EIO;
- i = (status >> IPC_CMD_SIZE) & 0xFF;
- if (i < ARRAY_SIZE(ipc_err_sources))
- dev_err(ipcdev.dev,
- "IPC failed: %s, STS=0x%x, CMD=0x%x\n",
- ipc_err_sources[i], status, ipcdev.cmd);
- else
- dev_err(ipcdev.dev,
- "IPC failed: unknown, STS=0x%x, CMD=0x%x\n",
- status, ipcdev.cmd);
- if ((i == IPC_ERR_UNSIGNEDKERNEL) || (i == IPC_ERR_EMSECURITY))
- ret = -EACCES;
- }
+ cmd_list[0] |= (cmd_list[1] << IPC_CMD_SUBCMD);
- return ret;
+ return 0;
}
-/**
- * intel_pmc_ipc_simple_command() - Simple IPC command
- * @cmd: IPC command code.
- * @sub: IPC command sub type.
- *
- * Send a simple IPC command to PMC when don't need to specify
- * input/output data and source/dest pointers.
- *
- * Return: an IPC error code or 0 on success.
- */
-int intel_pmc_ipc_simple_command(int cmd, int sub)
+static int pre_raw_cmd_fn(u32 *cmd_list, u32 cmdlen, u8 *in, u32 inlen,
+ u32 *out, u32 outlen, u32 dptr, u32 sptr)
{
int ret;
- mutex_lock(&ipclock);
- if (ipcdev.dev == NULL) {
- mutex_unlock(&ipclock);
- return -ENODEV;
- }
- ipc_send_command(sub << IPC_CMD_SUBCMD | cmd);
- ret = intel_pmc_ipc_check_status();
- mutex_unlock(&ipclock);
+ if (inlen > IPC_DATA_BUFFER_SIZE || outlen > IPC_DATA_BUFFER_SIZE/4)
+ return -EINVAL;
- return ret;
-}
-EXPORT_SYMBOL_GPL(intel_pmc_ipc_simple_command);
+ ret = pre_simple_cmd_fn(cmd_list, cmdlen);
+ if (ret < 0)
+ return ret;
-/**
- * intel_pmc_ipc_raw_cmd() - IPC command with data and pointers
- * @cmd: IPC command code.
- * @sub: IPC command sub type.
- * @in: input data of this IPC command.
- * @inlen: input data length in bytes.
- * @out: output data of this IPC command.
- * @outlen: output data length in dwords.
- * @sptr: data writing to SPTR register.
- * @dptr: data writing to DPTR register.
- *
- * Send an IPC command to PMC with input/output data and source/dest pointers.
- *
- * Return: an IPC error code or 0 on success.
- */
-int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen, u32 *out,
- u32 outlen, u32 dptr, u32 sptr)
-{
- u32 wbuf[4] = { 0 };
- int ret;
- int i;
+ cmd_list[0] |= (inlen << IPC_CMD_SIZE);
- if (inlen > IPC_DATA_BUFFER_SIZE || outlen > IPC_DATA_BUFFER_SIZE / 4)
- return -EINVAL;
+ return 0;
+}
- mutex_lock(&ipclock);
- if (ipcdev.dev == NULL) {
- mutex_unlock(&ipclock);
- return -ENODEV;
- }
- memcpy(wbuf, in, inlen);
- writel(dptr, ipcdev.ipc_base + IPC_DPTR);
- writel(sptr, ipcdev.ipc_base + IPC_SPTR);
- /* The input data register is 32bit register and inlen is in Byte */
- for (i = 0; i < ((inlen + 3) / 4); i++)
- ipc_data_writel(wbuf[i], 4 * i);
- ipc_send_command((inlen << IPC_CMD_SIZE) |
- (sub << IPC_CMD_SUBCMD) | cmd);
- ret = intel_pmc_ipc_check_status();
- if (!ret) {
- /* out is read from 32bit register and outlen is in 32bit */
- for (i = 0; i < outlen; i++)
- *out++ = ipc_data_readl(4 * i);
- }
- mutex_unlock(&ipclock);
+static int pre_irq_handler_fn(struct intel_ipc_dev *ipc_dev, int irq)
+{
+ return regmap_write_bits(ipc_dev->cfg->cmd_regs,
+ ipc_dev->cfg->status_reg,
+ IPC_DEV_PMC_CMD_STATUS_IRQ,
+ IPC_DEV_PMC_CMD_STATUS_IRQ);
+}
- return ret;
+static int pmc_ipc_err_code(int status)
+{
+ return ((status >> IPC_DEV_PMC_CMD_SIZE) &
+ IPC_DEV_PMC_CMD_STATUS_ERR_MASK);
}
-EXPORT_SYMBOL_GPL(intel_pmc_ipc_raw_cmd);
-/**
- * intel_pmc_ipc_command() - IPC command with input/output data
- * @cmd: IPC command code.
- * @sub: IPC command sub type.
- * @in: input data of this IPC command.
- * @inlen: input data length in bytes.
- * @out: output data of this IPC command.
- * @outlen: output data length in dwords.
- *
- * Send an IPC command to PMC with input/output data.
- *
- * Return: an IPC error code or 0 on success.
- */
-int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
- u32 *out, u32 outlen)
+static int pmc_ipc_busy_check(int status)
{
- return intel_pmc_ipc_raw_cmd(cmd, sub, in, inlen, out, outlen, 0, 0);
+ return status | IPC_DEV_PMC_CMD_STATUS_BUSY;
}
-EXPORT_SYMBOL_GPL(intel_pmc_ipc_command);
-static irqreturn_t ioc(int irq, void *dev_id)
+static u32 pmc_ipc_enable_msi(u32 cmd)
{
- int status;
+ return cmd | IPC_DEV_PMC_CMD_MSI;
+}
- if (ipcdev.irq_mode) {
- status = ipc_read_status();
- writel(status | IPC_STATUS_IRQ, ipcdev.ipc_base + IPC_STATUS);
- }
- complete(&ipcdev.cmd_complete);
+static struct intel_ipc_dev *intel_pmc_ipc_dev_create(
+ struct device *pmc_dev,
+ void __iomem *base,
+ int irq)
+{
+ struct intel_ipc_dev_ops *ops;
+ struct intel_ipc_dev_cfg *cfg;
+ struct regmap *cmd_regs;
+
+ cfg = devm_kzalloc(pmc_dev, sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return ERR_PTR(-ENOMEM);
+
+ ops = devm_kzalloc(pmc_dev, sizeof(*ops), GFP_KERNEL);
+ if (!ops)
+ return ERR_PTR(-ENOMEM);
+
+ cmd_regs = devm_regmap_init_mmio_clk(pmc_dev, NULL, base,
+ &pmc_regmap_config);
+ if (IS_ERR(cmd_regs)) {
+ dev_err(pmc_dev, "cmd_regs regmap init failed\n");
+ return ERR_CAST(cmd_regs);;
+ }
- return IRQ_HANDLED;
+ /* set IPC dev ops */
+ ops->to_err_code = pmc_ipc_err_code;
+ ops->busy_check = pmc_ipc_busy_check;
+ ops->enable_msi = pmc_ipc_enable_msi;
+ ops->pre_raw_cmd_fn = pre_raw_cmd_fn;
+ ops->pre_simple_cmd_fn = pre_simple_cmd_fn;
+ ops->pre_irq_handler_fn = pre_irq_handler_fn;
+
+ /* set cfg options */
+ if (irq > 0)
+ cfg->mode = IPC_DEV_MODE_IRQ;
+ else
+ cfg->mode = IPC_DEV_MODE_POLLING;
+
+ cfg->chan_type = IPC_CHANNEL_IA_PMC;
+ cfg->irq = irq;
+ cfg->use_msi = true;
+ cfg->support_sptr = true;
+ cfg->support_dptr = true;
+ cfg->cmd_regs = cmd_regs;
+ cfg->data_regs = cmd_regs;
+ cfg->wrbuf_reg = IPC_DEV_PMC_WRBUF_OFFSET;
+ cfg->rbuf_reg = IPC_DEV_PMC_RBUF_OFFSET;
+ cfg->sptr_reg = IPC_DEV_PMC_SPTR_OFFSET;
+ cfg->dptr_reg = IPC_DEV_PMC_DPTR_OFFSET;
+ cfg->status_reg = IPC_DEV_PMC_STATUS_OFFSET;
+
+ return devm_intel_ipc_dev_create(pmc_dev, INTEL_PMC_IPC_DEV, cfg, ops);
}
static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -428,8 +333,6 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (pmc->dev)
return -EBUSY;
- pmc->irq_mode = IPC_TRIGGER_MODE_IRQ;
-
ret = pcim_enable_device(pdev);
if (ret)
return ret;
@@ -438,15 +341,14 @@ static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (ret)
return ret;
- init_completion(&pmc->cmd_complete);
-
pmc->ipc_base = pcim_iomap_table(pdev)[0];
- ret = devm_request_irq(&pdev->dev, pdev->irq, ioc, 0, "intel_pmc_ipc",
- pmc);
- if (ret) {
- dev_err(&pdev->dev, "Failed to request irq\n");
- return ret;
+ pmc->pmc_ipc_dev = intel_pmc_ipc_dev_create(&pdev->dev,
+ pmc->ipc_base, pdev->irq);
+ if (IS_ERR(pmc->pmc_ipc_dev)) {
+ dev_err(&pdev->dev,
+ "Failed to create PMC IPC device\n");
+ return PTR_ERR(pmc->pmc_ipc_dev);
}
pmc->dev = &pdev->dev;
@@ -474,19 +376,19 @@ static ssize_t intel_pmc_ipc_simple_cmd_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int subcmd;
- int cmd;
+ struct intel_pmc_ipc_dev *pmc = dev_get_drvdata(dev);
+ int cmd[2];
int ret;
- ret = sscanf(buf, "%d %d", &cmd, &subcmd);
+ ret = sscanf(buf, "%d %d", &cmd[0], &cmd[2]);
if (ret != 2) {
dev_err(dev, "Error args\n");
return -EINVAL;
}
- ret = intel_pmc_ipc_simple_command(cmd, subcmd);
+ ret = ipc_dev_simple_cmd(pmc->pmc_ipc_dev, cmd, 2);
if (ret) {
- dev_err(dev, "command %d error with %d\n", cmd, ret);
+ dev_err(dev, "command %d error with %d\n", cmd[0], ret);
return ret;
}
return (ssize_t)count;
@@ -496,22 +398,23 @@ static ssize_t intel_pmc_ipc_northpeak_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
+ struct intel_pmc_ipc_dev *pmc = dev_get_drvdata(dev);
unsigned long val;
- int subcmd;
+ int cmd[2] = {PMC_IPC_NORTHPEAK_CTRL, 0};
int ret;
if (kstrtoul(buf, 0, &val))
return -EINVAL;
if (val)
- subcmd = 1;
- else
- subcmd = 0;
- ret = intel_pmc_ipc_simple_command(PMC_IPC_NORTHPEAK_CTRL, subcmd);
+ cmd[1] = 1;
+
+ ret = ipc_dev_simple_cmd(pmc->pmc_ipc_dev, cmd, 2);
if (ret) {
- dev_err(dev, "command north %d error with %d\n", subcmd, ret);
+ dev_err(dev, "command north %d error with %d\n", cmd[1], ret);
return ret;
}
+
return (ssize_t)count;
}
@@ -689,6 +592,7 @@ static int ipc_create_pmc_devices(struct platform_device *pdev)
static int ipc_plat_get_res(struct platform_device *pdev)
{
+ struct intel_pmc_ipc_dev *pmc = dev_get_drvdata(&pdev->dev);
struct resource *res;
void __iomem *addr;
@@ -707,9 +611,9 @@ static int ipc_plat_get_res(struct platform_device *pdev)
if (IS_ERR(addr))
return PTR_ERR(addr);
- ipcdev.ipc_base = addr;
- ipcdev.gcr_mem_base = addr + PLAT_RESOURCE_GCR_OFFSET;
- dev_dbg(&pdev->dev, "PMC IPC resource %pR\n", res);
+ pmc->ipc_base = addr;
+ pmc->gcr_mem_base = addr + PLAT_RESOURCE_GCR_OFFSET;
+ dev_info(&pdev->dev, "PMC IPC resource %pR\n", res);
return 0;
}
@@ -755,14 +659,15 @@ MODULE_DEVICE_TABLE(acpi, ipc_acpi_ids);
static int ipc_plat_probe(struct platform_device *pdev)
{
- int ret;
+ int ret, irq;
+ struct intel_pmc_ipc_dev *pmc = &ipcdev;
+
+ pmc->dev = &pdev->dev;
- ipcdev.dev = &pdev->dev;
- ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
- init_completion(&ipcdev.cmd_complete);
+ dev_set_drvdata(&pdev->dev, pmc);
- ipcdev.irq = platform_get_irq(pdev, 0);
- if (ipcdev.irq < 0) {
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
dev_err(&pdev->dev, "Failed to get irq\n");
return -EINVAL;
}
@@ -773,11 +678,11 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
- ipcdev.gcr_regs = devm_regmap_init_mmio_clk(ipcdev.dev, NULL,
- ipcdev.gcr_mem_base, &gcr_regmap_config);
- if (IS_ERR(ipcdev.gcr_regs)) {
- dev_err(ipcdev.dev, "gcr_regs regmap init failed\n");
- return PTR_ERR(ipcdev.gcr_regs);;
+ pmc->gcr_regs = devm_regmap_init_mmio_clk(pmc->dev, NULL,
+ pmc->gcr_mem_base, &gcr_regmap_config);
+ if (IS_ERR(pmc->gcr_regs)) {
+ dev_err(&pdev->dev, "gcr_regs regmap init failed\n");
+ return PTR_ERR(pmc->gcr_regs);;
}
ret = ipc_create_pmc_devices(pdev);
@@ -786,20 +691,20 @@ static int ipc_plat_probe(struct platform_device *pdev)
return ret;
}
- if (devm_request_irq(&pdev->dev, ipcdev.irq, ioc, IRQF_NO_SUSPEND,
- "intel_pmc_ipc", &ipcdev)) {
- dev_err(&pdev->dev, "Failed to request irq\n");
- return -EBUSY;
- }
-
ret = sysfs_create_group(&pdev->dev.kobj, &intel_ipc_group);
if (ret) {
dev_err(&pdev->dev, "Failed to create sysfs group %d\n",
ret);
- devm_free_irq(&pdev->dev, ipcdev.irq, &ipcdev);
return ret;
}
+ ipcdev.pmc_ipc_dev = intel_pmc_ipc_dev_create(&pdev->dev,
+ pmc->ipc_base, irq);
+ if (IS_ERR(pmc->pmc_ipc_dev)) {
+ dev_err(&pdev->dev, "Failed to create PMC IPC device\n");
+ return PTR_ERR(pmc->pmc_ipc_dev);
+ }
+
return 0;
}
diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c b/drivers/platform/x86/intel_telemetry_pltdrv.c
index bf8284a..a08a620 100644
--- a/drivers/platform/x86/intel_telemetry_pltdrv.c
+++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
@@ -56,10 +56,6 @@
#define IOSS_TELEM_TRACE_CTL_WRITE 0x6
#define IOSS_TELEM_EVENT_CTL_READ 0x7
#define IOSS_TELEM_EVENT_CTL_WRITE 0x8
-#define IOSS_TELEM_EVT_CTRL_WRITE_SIZE 0x4
-#define IOSS_TELEM_READ_WORD 0x1
-#define IOSS_TELEM_WRITE_FOURBYTES 0x4
-#define IOSS_TELEM_EVT_WRITE_SIZE 0x3
#define TELEM_INFO_SRAMEVTS_MASK 0xFF00
#define TELEM_INFO_SRAMEVTS_SHIFT 0x8
@@ -98,7 +94,7 @@ struct telem_ssram_region {
};
static struct telemetry_plt_config *telm_conf;
-static struct intel_ipc_dev *punit_bios_ipc_dev;
+static struct intel_ipc_dev *punit_bios_ipc_dev, *pmc_ipc_dev;
/*
* The following counters are programmed by default during setup.
@@ -267,17 +263,16 @@ static int telemetry_check_evtid(enum telemetry_unit telem_unit,
static inline int telemetry_plt_config_ioss_event(u32 evt_id, int index)
{
u32 write_buf;
- int ret;
+ u32 cmd[PMC_PARAM_LEN] = {0};
write_buf = evt_id | TELEM_EVENT_ENABLE;
write_buf <<= BITS_PER_BYTE;
write_buf |= index;
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_WRITE, (u8 *)&write_buf,
- IOSS_TELEM_EVT_WRITE_SIZE, NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_EVENT_WRITE);
- return ret;
+ return ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&write_buf, sizeof(write_buf), NULL, 0, 0, 0);
}
static inline int telemetry_plt_config_pss_event(u32 evt_id, int index)
@@ -287,6 +282,7 @@ static inline int telemetry_plt_config_pss_event(u32 evt_id, int index)
write_buf = evt_id | TELEM_EVENT_ENABLE;
punit_cmd_init(cmd, IPC_PUNIT_BIOS_WRITE_TELE_EVENT, index, 0);
+
return ipc_dev_raw_cmd(punit_bios_ipc_dev, cmd, PUNIT_PARAM_LEN,
(u8 *)&write_buf, sizeof(write_buf), NULL, 0, 0, 0);
}
@@ -298,15 +294,16 @@ static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
int ret, index, idx;
u32 *ioss_evtmap;
u32 telem_ctrl;
+ u32 cmd[PMC_PARAM_LEN] = {0};
num_ioss_evts = evtconfig.num_evts;
ioss_period = evtconfig.period;
ioss_evtmap = evtconfig.evtmap;
/* Get telemetry EVENT CTL */
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_READ, NULL, 0,
- &telem_ctrl, IOSS_TELEM_READ_WORD);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_EVENT_CTL_READ);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, NULL, 0,
+ &telem_ctrl, 1, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Read Failed\n");
return ret;
@@ -314,12 +311,9 @@ static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
/* Disable Telemetry */
TELEM_DISABLE(telem_ctrl);
-
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE,
- NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL, 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -330,12 +324,11 @@ static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
if (action == TELEM_RESET) {
/* Clear All Events */
TELEM_CLEAR_EVENTS(telem_ctrl);
-
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE,
- NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl),
+ NULL, 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -360,11 +353,11 @@ static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
/* Clear All Events */
TELEM_CLEAR_EVENTS(telem_ctrl);
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE,
- NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Disable Write Failed\n");
return ret;
@@ -412,10 +405,9 @@ static int telemetry_setup_iossevtconfig(struct telemetry_evtconfig evtconfig,
TELEM_ENABLE_PERIODIC(telem_ctrl);
telem_ctrl |= ioss_period;
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE, NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL, 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Enable Write Failed\n");
return ret;
@@ -609,8 +601,9 @@ static int telemetry_setup(struct platform_device *pdev)
u32 cmd[PUNIT_PARAM_LEN] = {0};
int ret;
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_INFO_READ,
- NULL, 0, &read_buf, IOSS_TELEM_READ_WORD);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_INFO_READ);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, NULL, 0,
+ &read_buf, 1, 0, 0);
if (ret) {
dev_err(&pdev->dev, "IOSS TELEM_INFO Read Failed\n");
return ret;
@@ -713,9 +706,10 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
}
/* Get telemetry EVENT CTL */
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_READ, NULL, 0,
- &telem_ctrl, IOSS_TELEM_READ_WORD);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_EVENT_CTL_READ);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, NULL, 0,
+ &telem_ctrl, 1, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Read Failed\n");
goto out;
@@ -723,12 +717,11 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
/* Disable Telemetry */
TELEM_DISABLE(telem_ctrl);
-
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE,
- NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Disable Write Failed\n");
goto out;
@@ -740,11 +733,11 @@ static int telemetry_plt_set_sampling_period(u8 pss_period, u8 ioss_period)
TELEM_ENABLE_PERIODIC(telem_ctrl);
telem_ctrl |= ioss_period;
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_EVENT_CTL_WRITE,
- (u8 *)&telem_ctrl,
- IOSS_TELEM_EVT_CTRL_WRITE_SIZE,
- NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_EVENT_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&telem_ctrl, sizeof(telem_ctrl), NULL,
+ 0, 0, 0);
if (ret) {
pr_err("IOSS TELEM_CTRL Event Enable Write Failed\n");
goto out;
@@ -1044,9 +1037,10 @@ static int telemetry_plt_get_trace_verbosity(enum telemetry_unit telem_unit,
break;
case TELEM_IOSS:
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_TRACE_CTL_READ, NULL, 0, &temp,
- IOSS_TELEM_READ_WORD);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_TRACE_CTL_READ);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, NULL, 0,
+ &temp, 1, 0, 0);
if (ret) {
pr_err("IOSS TRACE_CTL Read Failed\n");
goto out;
@@ -1099,9 +1093,10 @@ static int telemetry_plt_set_trace_verbosity(enum telemetry_unit telem_unit,
break;
case TELEM_IOSS:
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_TRACE_CTL_READ, NULL, 0, &temp,
- IOSS_TELEM_READ_WORD);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY,
+ IOSS_TELEM_TRACE_CTL_READ);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN, NULL, 0,
+ &temp, 1, 0, 0);
if (ret) {
pr_err("IOSS TRACE_CTL Read Failed\n");
goto out;
@@ -1109,10 +1104,9 @@ static int telemetry_plt_set_trace_verbosity(enum telemetry_unit telem_unit,
TELEM_CLEAR_VERBOSITY_BITS(temp);
TELEM_SET_VERBOSITY_BITS(temp, verbosity);
-
- ret = intel_pmc_ipc_command(PMC_IPC_PMC_TELEMTRY,
- IOSS_TELEM_TRACE_CTL_WRITE, (u8 *)&temp,
- IOSS_TELEM_WRITE_FOURBYTES, NULL, 0);
+ pmc_cmd_init(cmd, PMC_IPC_PMC_TELEMTRY, IOSS_TELEM_TRACE_CTL_WRITE);
+ ret = ipc_dev_raw_cmd(pmc_ipc_dev, cmd, PMC_PARAM_LEN,
+ (u8 *)&temp, sizeof(temp), NULL, 0, 0, 0);
if (ret) {
pr_err("IOSS TRACE_CTL Verbosity Set Failed\n");
goto out;
@@ -1157,6 +1151,10 @@ static int telemetry_pltdrv_probe(struct platform_device *pdev)
if (IS_ERR_OR_NULL(punit_bios_ipc_dev))
return PTR_ERR(punit_bios_ipc_dev);
+ pmc_ipc_dev = intel_ipc_dev_get(INTEL_PMC_IPC_DEV);
+ if (IS_ERR_OR_NULL(pmc_ipc_dev))
+ return PTR_ERR(pmc_ipc_dev);
+
telm_conf = (struct telemetry_plt_config *)id->driver_data;
res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1236,6 +1234,7 @@ static int telemetry_pltdrv_probe(struct platform_device *pdev)
static int telemetry_pltdrv_remove(struct platform_device *pdev)
{
telemetry_clear_pltdata();
+ intel_ipc_dev_put(pmc_ipc_dev);
intel_ipc_dev_put(punit_bios_ipc_dev);
iounmap(telm_conf->pss_config.regmap);
iounmap(telm_conf->ioss_config.regmap);
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index 5aacdb0..7cc39b6 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -20,6 +20,7 @@
#define __INTEL_SOC_PMIC_H__
#include <linux/regmap.h>
+#include <linux/platform_data/x86/intel_ipc_dev.h>
struct intel_soc_pmic {
int irq;
@@ -31,6 +32,7 @@ struct intel_soc_pmic {
struct regmap_irq_chip_data *irq_chip_data_chgr;
struct regmap_irq_chip_data *irq_chip_data_crit;
struct device *dev;
+ struct intel_ipc_dev *ipc_dev;
};
#endif /* __INTEL_SOC_PMIC_H__ */
--
2.7.4
^ permalink raw reply related
* [RFC v4 4/8] platform: x86: Add generic Intel IPC driver
From: sathyanarayanan.kuppuswamy @ 2017-10-07 2:33 UTC (permalink / raw)
To: a.zummo, x86, wim, mingo, alexandre.belloni, qipeng.zha, hpa,
dvhart, tglx, lee.jones, andy, souvik.k.chakravarty
Cc: linux-rtc, linux-watchdog, linux-kernel, platform-driver-x86,
sathyaosid, Kuppuswamy Sathyanarayanan
In-Reply-To: <cover.1507340643.git.sathyanarayanan.kuppuswamy@linux.intel.com>
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Currently intel_scu_ipc.c, intel_pmc_ipc.c and intel_punit_ipc.c
redundantly implements the same IPC features and has lot of code
duplication between them. This driver addresses this issue by grouping
the common IPC functionalities under the same driver.
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
drivers/platform/x86/Kconfig | 8 +
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel_ipc_dev.c | 576 ++++++++++++++++++++++++
include/linux/platform_data/x86/intel_ipc_dev.h | 206 +++++++++
4 files changed, 791 insertions(+)
create mode 100644 drivers/platform/x86/intel_ipc_dev.c
create mode 100644 include/linux/platform_data/x86/intel_ipc_dev.h
Changes since v3:
* Fixed NULL pointer exception in intel_ipc_dev_get().
* Fixed error in check for duplicate intel_ipc_dev.
* Added custom interrupt handler support.
* Used char array for error string conversion.
* Added put dev support.
* Added devm_* variant of intel_ipc_dev_get().
Changes since v2:
* Added ipc_dev_cmd API support.
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index da2d9ba..724ee696 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1153,6 +1153,14 @@ config SILEAD_DMI
with the OS-image for the device. This option supplies the missing
information. Enable this for x86 tablets with Silead touchscreens.
+config INTEL_IPC_DEV
+ bool "Intel IPC Device Driver"
+ depends on X86_64
+ ---help---
+ This driver implements core features of Intel IPC device. Devices
+ like PMC, SCU, PUNIT, etc can use interfaces provided by this
+ driver to implement IPC protocol of their respective device.
+
endif # X86_PLATFORM_DEVICES
config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2b315d0..99a1af1 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_PMC_ATOM) += pmc_atom.o
obj-$(CONFIG_MLX_PLATFORM) += mlx-platform.o
obj-$(CONFIG_MLX_CPLD_PLATFORM) += mlxcpld-hotplug.o
obj-$(CONFIG_INTEL_TURBO_MAX_3) += intel_turbo_max_3.o
+obj-$(CONFIG_INTEL_IPC_DEV) += intel_ipc_dev.o
diff --git a/drivers/platform/x86/intel_ipc_dev.c b/drivers/platform/x86/intel_ipc_dev.c
new file mode 100644
index 0000000..f55ddec
--- /dev/null
+++ b/drivers/platform/x86/intel_ipc_dev.c
@@ -0,0 +1,576 @@
+/*
+ * intel_ipc_dev.c: Intel IPC device class driver
+ *
+ * (C) Copyright 2017 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/platform_data/x86/intel_ipc_dev.h>
+#include <linux/regmap.h>
+
+/* mutex to sync different ipc devices in same channel */
+static struct mutex channel_lock[IPC_CHANNEL_MAX];
+
+static char *ipc_err_sources[] = {
+ [IPC_DEV_ERR_NONE] =
+ "No error",
+ [IPC_DEV_ERR_CMD_NOT_SUPPORTED] =
+ "Command not-supported/Invalid",
+ [IPC_DEV_ERR_CMD_NOT_SERVICED] =
+ "Command not-serviced/Invalid param",
+ [IPC_DEV_ERR_UNABLE_TO_SERVICE] =
+ "Unable-to-service/Cmd-timeout",
+ [IPC_DEV_ERR_CMD_INVALID] =
+ "Command-invalid/Cmd-locked",
+ [IPC_DEV_ERR_CMD_FAILED] =
+ "Command-failed/Invalid-VR-id",
+ [IPC_DEV_ERR_EMSECURITY] =
+ "Invalid Battery/VR-Error",
+ [IPC_DEV_ERR_UNSIGNEDKERNEL] =
+ "Unsigned kernel",
+};
+
+static void ipc_channel_lock_init(void)
+{
+ int i;
+
+ for (i = 0; i < IPC_CHANNEL_MAX; i++)
+ mutex_init(&channel_lock[i]);
+}
+
+static struct class intel_ipc_class = {
+ .name = "intel_ipc",
+ .owner = THIS_MODULE,
+};
+
+static int ipc_dev_lock(struct intel_ipc_dev *ipc_dev)
+{
+ int chan_type;
+
+ if (!ipc_dev || !ipc_dev->cfg)
+ return -ENODEV;
+
+ chan_type = ipc_dev->cfg->chan_type;
+ if (chan_type > IPC_CHANNEL_MAX)
+ return -EINVAL;
+
+ /* acquire channel lock */
+ mutex_lock(&channel_lock[chan_type]);
+
+ /* acquire IPC device lock */
+ mutex_lock(&ipc_dev->lock);
+
+ return 0;
+}
+
+static int ipc_dev_unlock(struct intel_ipc_dev *ipc_dev)
+{
+ int chan_type;
+
+ if (!ipc_dev || !ipc_dev->cfg)
+ return -ENODEV;
+
+ chan_type = ipc_dev->cfg->chan_type;
+ if (chan_type > IPC_CHANNEL_MAX)
+ return -EINVAL;
+
+ /* release IPC device lock */
+ mutex_unlock(&ipc_dev->lock);
+
+ /* release channel lock */
+ mutex_unlock(&channel_lock[chan_type]);
+
+ return 0;
+}
+
+static const char *ipc_dev_err_string(struct intel_ipc_dev *ipc_dev,
+ int error)
+{
+ if (error < IPC_DEV_ERR_MAX)
+ return ipc_err_sources[error];
+
+ return "Unknown Command";
+}
+
+/* Helper function to send given command to IPC device */
+static inline void ipc_dev_send_cmd(struct intel_ipc_dev *ipc_dev, u32 cmd)
+{
+ ipc_dev->cmd = cmd;
+
+ if (ipc_dev->cfg->mode == IPC_DEV_MODE_IRQ)
+ reinit_completion(&ipc_dev->cmd_complete);
+
+ if (ipc_dev->ops->enable_msi)
+ cmd = ipc_dev->ops->enable_msi(cmd);
+
+ regmap_write(ipc_dev->cfg->cmd_regs, ipc_dev->cfg->cmd_reg, cmd);
+}
+
+static inline int ipc_dev_status_busy(struct intel_ipc_dev *ipc_dev)
+{
+ int status;
+
+ regmap_read(ipc_dev->cfg->cmd_regs, ipc_dev->cfg->status_reg, &status);
+
+ if (ipc_dev->ops->busy_check)
+ return ipc_dev->ops->busy_check(status);
+
+ return 0;
+}
+
+/* Check the status of IPC command and return err code if failed */
+static int ipc_dev_check_status(struct intel_ipc_dev *ipc_dev)
+{
+ int loop_count = IPC_DEV_CMD_LOOP_CNT;
+ int status;
+ int ret = 0;
+
+ if (ipc_dev->cfg->mode == IPC_DEV_MODE_IRQ) {
+ if (!wait_for_completion_timeout(&ipc_dev->cmd_complete,
+ IPC_DEV_CMD_TIMEOUT))
+ ret = -ETIMEDOUT;
+ } else {
+ while (ipc_dev_status_busy(ipc_dev) && --loop_count)
+ udelay(1);
+ if (!loop_count)
+ ret = -ETIMEDOUT;
+ }
+
+ if (ret < 0) {
+ dev_err(&ipc_dev->dev,
+ "IPC timed out, CMD=0x%x\n", ipc_dev->cmd);
+ return ret;
+ }
+
+ regmap_read(ipc_dev->cfg->cmd_regs, ipc_dev->cfg->status_reg, &status);
+
+ if (ipc_dev->ops->to_err_code)
+ ret = ipc_dev->ops->to_err_code(status);
+
+ if (ret) {
+ dev_err(&ipc_dev->dev,
+ "IPC failed: %s, STS=0x%x, CMD=0x%x\n",
+ ipc_dev_err_string(ipc_dev, ret),
+ status, ipc_dev->cmd);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * ipc_dev_simple_cmd() - Send simple IPC command
+ * @ipc_dev : Reference to ipc device.
+ * @cmd_list : IPC command list.
+ * @cmdlen : Number of cmd/sub-cmds.
+ *
+ * Send a simple IPC command to ipc device.
+ * Use this when don't need to specify input/output data
+ * and source/dest pointers.
+ *
+ * Return: an IPC error code or 0 on success.
+ */
+
+int ipc_dev_simple_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list,
+ u32 cmdlen)
+{
+ int ret;
+
+ if (!cmd_list)
+ return -EINVAL;
+
+ ret = ipc_dev_lock(ipc_dev);
+ if (ret)
+ return ret;
+
+ /* Call custom pre-processing handler */
+ if (ipc_dev->ops->pre_simple_cmd_fn) {
+ ret = ipc_dev->ops->pre_simple_cmd_fn(cmd_list, cmdlen);
+ if (ret)
+ goto unlock_device;
+ }
+
+ ipc_dev_send_cmd(ipc_dev, cmd_list[0]);
+
+ ret = ipc_dev_check_status(ipc_dev);
+
+unlock_device:
+ ipc_dev_unlock(ipc_dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ipc_dev_simple_cmd);
+
+/**
+ * ipc_dev_cmd() - Send IPC command with data.
+ * @ipc_dev : Reference to ipc_dev.
+ * @cmd_list : Array of commands/sub-commands.
+ * @cmdlen : Number of commands.
+ * @in : Input data of this IPC command.
+ * @inlen : Input data length in dwords.
+ * @out : Output data of this IPC command.
+ * @outlen : Length of output data in dwords.
+ *
+ * Send an IPC command to device with input/output data.
+ *
+ * Return: an IPC error code or 0 on success.
+ */
+int ipc_dev_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list, u32 cmdlen,
+ u32 *in, u32 inlen, u32 *out, u32 outlen)
+{
+ int ret;
+
+ if (!cmd_list || !in)
+ return -EINVAL;
+
+ ret = ipc_dev_lock(ipc_dev);
+ if (ret)
+ return ret;
+
+ /* Call custom pre-processing handler. */
+ if (ipc_dev->ops->pre_cmd_fn) {
+ ret = ipc_dev->ops->pre_cmd_fn(cmd_list, cmdlen, in, inlen,
+ out, outlen);
+ if (ret)
+ goto unlock_device;
+ }
+
+ /* Write inlen dwords of data to wrbuf_reg. */
+ if (inlen > 0)
+ regmap_bulk_write(ipc_dev->cfg->data_regs,
+ ipc_dev->cfg->wrbuf_reg, in, inlen);
+
+ ipc_dev_send_cmd(ipc_dev, cmd_list[0]);
+
+ ret = ipc_dev_check_status(ipc_dev);
+
+ /* Read outlen dwords of data from rbug_reg. */
+ if (!ret && outlen > 0)
+ regmap_bulk_read(ipc_dev->cfg->data_regs,
+ ipc_dev->cfg->rbuf_reg, out, outlen);
+unlock_device:
+ ipc_dev_unlock(ipc_dev);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ipc_dev_cmd);
+
+/**
+ * ipc_dev_raw_cmd() - Send IPC command with data and pointers.
+ * @ipc_dev : Reference to ipc_dev.
+ * @cmd_list : Array of commands/sub-commands.
+ * @cmdlen : Number of commands.
+ * @in : Input data of this IPC command.
+ * @inlen : Input data length in bytes.
+ * @out : Output data of this IPC command.
+ * @outlen : Length of output data in dwords.
+ * @dptr : IPC destination data address.
+ * @sptr : IPC source data address.
+ *
+ * Send an IPC command to device with input/output data and
+ * source/dest pointers.
+ *
+ * Return: an IPC error code or 0 on success.
+ */
+
+int ipc_dev_raw_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list, u32 cmdlen,
+ u8 *in, u32 inlen, u32 *out, u32 outlen, u32 dptr, u32 sptr)
+{
+ int ret;
+ int inbuflen = DIV_ROUND_UP(inlen, 4);
+ u32 *inbuf;
+
+ if (!cmd_list || !in)
+ return -EINVAL;
+
+ inbuf = kzalloc(inbuflen, GFP_KERNEL);
+ if (!inbuf)
+ return -ENOMEM;
+
+ ret = ipc_dev_lock(ipc_dev);
+ if (ret)
+ return ret;
+
+ /* Call custom pre-processing handler. */
+ if (ipc_dev->ops->pre_raw_cmd_fn) {
+ ret = ipc_dev->ops->pre_raw_cmd_fn(cmd_list, cmdlen, in, inlen,
+ out, outlen, dptr, sptr);
+ if (ret)
+ goto unlock_device;
+ }
+
+ /* If supported, write DPTR register.*/
+ if (ipc_dev->cfg->support_dptr)
+ regmap_write(ipc_dev->cfg->cmd_regs, ipc_dev->cfg->dptr_reg,
+ dptr);
+
+ /* If supported, write SPTR register. */
+ if (ipc_dev->cfg->support_sptr)
+ regmap_write(ipc_dev->cfg->cmd_regs, ipc_dev->cfg->sptr_reg,
+ sptr);
+
+ memcpy(inbuf, in, inlen);
+
+ /* Write inlen dwords of data to wrbuf_reg. */
+ if (inlen > 0)
+ regmap_bulk_write(ipc_dev->cfg->data_regs,
+ ipc_dev->cfg->wrbuf_reg, inbuf, inbuflen);
+
+ ipc_dev_send_cmd(ipc_dev, cmd_list[0]);
+
+ ret = ipc_dev_check_status(ipc_dev);
+
+ /* Read outlen dwords of data from rbug_reg. */
+ if (!ret && outlen > 0)
+ regmap_bulk_read(ipc_dev->cfg->data_regs,
+ ipc_dev->cfg->rbuf_reg, out, outlen);
+unlock_device:
+ ipc_dev_unlock(ipc_dev);
+ kfree(inbuf);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ipc_dev_raw_cmd);
+
+/* sysfs option to send simple IPC commands from userspace */
+static ssize_t ipc_dev_cmd_reg_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct intel_ipc_dev *ipc_dev = dev_get_drvdata(dev);
+ u32 cmd;
+ int ret;
+
+ ret = sscanf(buf, "%d", &cmd);
+ if (ret != 1) {
+ dev_err(dev, "Error args\n");
+ return -EINVAL;
+ }
+
+ ret = ipc_dev_simple_cmd(ipc_dev, &cmd, 1);
+ if (ret) {
+ dev_err(dev, "command 0x%x error with %d\n", cmd, ret);
+ return ret;
+ }
+ return (ssize_t)count;
+}
+
+static DEVICE_ATTR(send_cmd, S_IWUSR, NULL, ipc_dev_cmd_reg_store);
+
+static struct attribute *ipc_dev_attrs[] = {
+ &dev_attr_send_cmd.attr,
+ NULL
+};
+
+static const struct attribute_group ipc_dev_group = {
+ .attrs = ipc_dev_attrs,
+};
+
+static const struct attribute_group *ipc_dev_groups[] = {
+ &ipc_dev_group,
+ NULL,
+};
+
+/* IPC device IRQ handler */
+static irqreturn_t ipc_dev_irq_handler(int irq, void *dev_id)
+{
+ struct intel_ipc_dev *ipc_dev = (struct intel_ipc_dev *)dev_id;
+
+ if (ipc_dev->ops->pre_irq_handler_fn)
+ ipc_dev->ops->pre_irq_handler_fn(ipc_dev, irq);
+
+ complete(&ipc_dev->cmd_complete);
+
+ return IRQ_HANDLED;
+}
+
+static void devm_intel_ipc_dev_release(struct device *dev, void *res)
+{
+ struct intel_ipc_dev *ipc_dev = *(struct intel_ipc_dev **)res;
+
+ if (!ipc_dev)
+ return;
+
+ device_del(&ipc_dev->dev);
+
+ kfree(ipc_dev);
+}
+
+static int match_name(struct device *dev, const void *data)
+{
+ if (!dev_name(dev))
+ return 0;
+
+ return !strcmp(dev_name(dev), (char *)data);
+}
+
+/**
+ * intel_ipc_dev_get() - Get Intel IPC device from name.
+ * @dev_name : Name of the IPC device.
+ *
+ * Return : ERR_PTR/NULL or intel_ipc_dev pointer on success.
+ */
+struct intel_ipc_dev *intel_ipc_dev_get(const char *dev_name)
+{
+ struct device *dev;
+
+ if (!dev_name)
+ return ERR_PTR(-EINVAL);
+
+ dev = class_find_device(&intel_ipc_class, NULL, dev_name, match_name);
+
+ return dev ? dev_get_drvdata(dev) : NULL;
+}
+EXPORT_SYMBOL_GPL(intel_ipc_dev_get);
+
+static void devm_intel_ipc_dev_put(struct device *dev, void *res)
+{
+ intel_ipc_dev_put(*(struct intel_ipc_dev **)res);
+}
+
+/**
+ * devm_intel_ipc_dev_get() - Resource managed version of intel_ipc_dev_get().
+ * @dev : Device pointer.
+ * @dev_name : Name of the IPC device.
+ *
+ * Return : ERR_PTR/NULL or intel_ipc_dev pointer on success.
+ */
+struct intel_ipc_dev *devm_intel_ipc_dev_get(struct device *dev,
+ const char *dev_name)
+{
+ struct intel_ipc_dev **ptr, *ipc_dev;
+
+ ptr = devres_alloc(devm_intel_ipc_dev_put, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ ipc_dev = intel_ipc_dev_get(dev_name);
+ if (!IS_ERR_OR_NULL(ipc_dev)) {
+ *ptr = ipc_dev;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return ipc_dev;
+}
+EXPORT_SYMBOL_GPL(devm_intel_ipc_dev_get);
+
+/**
+ * devm_intel_ipc_dev_create() - Create IPC device
+ * @dev : IPC parent device.
+ * @devname : Name of the IPC device.
+ * @cfg : IPC device configuration.
+ * @ops : IPC device ops.
+ *
+ * Resource managed API to create IPC device with
+ * given configuration.
+ *
+ * Return : IPC device pointer or ERR_PTR(error code).
+ */
+struct intel_ipc_dev *devm_intel_ipc_dev_create(struct device *dev,
+ const char *devname,
+ struct intel_ipc_dev_cfg *cfg,
+ struct intel_ipc_dev_ops *ops)
+{
+ struct intel_ipc_dev **ptr, *ipc_dev;
+ int ret;
+
+ if (!dev && !devname && !cfg)
+ return ERR_PTR(-EINVAL);
+
+ if (intel_ipc_dev_get(devname)) {
+ dev_err(dev, "IPC device %s already exist\n", devname);
+ return ERR_PTR(-EINVAL);
+ }
+
+ ptr = devres_alloc(devm_intel_ipc_dev_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ ipc_dev = kzalloc(sizeof(*ipc_dev), GFP_KERNEL);
+ if (!ipc_dev) {
+ ret = -ENOMEM;
+ goto err_dev_create;
+ }
+
+ ipc_dev->dev.class = &intel_ipc_class;
+ ipc_dev->dev.parent = dev;
+ ipc_dev->dev.groups = ipc_dev_groups;
+ ipc_dev->cfg = cfg;
+ ipc_dev->ops = ops;
+
+ mutex_init(&ipc_dev->lock);
+ init_completion(&ipc_dev->cmd_complete);
+ dev_set_drvdata(&ipc_dev->dev, ipc_dev);
+ dev_set_name(&ipc_dev->dev, devname);
+ device_initialize(&ipc_dev->dev);
+
+ ret = device_add(&ipc_dev->dev);
+ if (ret < 0) {
+ dev_err(&ipc_dev->dev, "%s device create failed\n",
+ __func__);
+ ret = -ENODEV;
+ goto err_dev_add;
+ }
+
+ if (ipc_dev->cfg->mode == IPC_DEV_MODE_IRQ) {
+ if (devm_request_irq(&ipc_dev->dev,
+ ipc_dev->cfg->irq,
+ ipc_dev_irq_handler,
+ ipc_dev->cfg->irqflags,
+ dev_name(&ipc_dev->dev),
+ ipc_dev)) {
+ dev_err(&ipc_dev->dev,
+ "Failed to request irq\n");
+ goto err_irq_request;
+ }
+ }
+
+ *ptr = ipc_dev;
+
+ devres_add(dev, ptr);
+
+ return ipc_dev;
+
+err_irq_request:
+ device_del(&ipc_dev->dev);
+err_dev_add:
+ kfree(ipc_dev);
+err_dev_create:
+ devres_free(ptr);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(devm_intel_ipc_dev_create);
+
+static int __init intel_ipc_init(void)
+{
+ ipc_channel_lock_init();
+ return class_register(&intel_ipc_class);
+}
+
+static void __exit intel_ipc_exit(void)
+{
+ class_unregister(&intel_ipc_class);
+}
+subsys_initcall(intel_ipc_init);
+module_exit(intel_ipc_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Kuppuswamy Sathyanarayanan<sathyanarayanan.kuppuswamy@linux.intel.com>");
+MODULE_DESCRIPTION("Intel IPC device class driver");
diff --git a/include/linux/platform_data/x86/intel_ipc_dev.h b/include/linux/platform_data/x86/intel_ipc_dev.h
new file mode 100644
index 0000000..eaeedaf
--- /dev/null
+++ b/include/linux/platform_data/x86/intel_ipc_dev.h
@@ -0,0 +1,206 @@
+/*
+ * Intel IPC class device header file.
+ *
+ * (C) Copyright 2017 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ *
+ */
+
+#ifndef INTEL_IPC_DEV_H
+#define INTEL_IPC_DEV_H
+
+#include <linux/module.h>
+#include <linux/device.h>
+
+/* IPC channel type */
+#define IPC_CHANNEL_IA_PMC 0
+#define IPC_CHANNEL_IA_PUNIT 1
+#define IPC_CHANNEL_PMC_PUNIT 2
+#define IPC_CHANNEL_IA_SCU 3
+#define IPC_CHANNEL_MAX 4
+
+/* IPC return code */
+#define IPC_DEV_ERR_NONE 0
+#define IPC_DEV_ERR_CMD_NOT_SUPPORTED 1
+#define IPC_DEV_ERR_CMD_NOT_SERVICED 2
+#define IPC_DEV_ERR_UNABLE_TO_SERVICE 3
+#define IPC_DEV_ERR_CMD_INVALID 4
+#define IPC_DEV_ERR_CMD_FAILED 5
+#define IPC_DEV_ERR_EMSECURITY 6
+#define IPC_DEV_ERR_UNSIGNEDKERNEL 7
+#define IPC_DEV_ERR_MAX 8
+
+/* IPC mode */
+#define IPC_DEV_MODE_IRQ 0
+#define IPC_DEV_MODE_POLLING 1
+
+/* IPC dev constants */
+#define IPC_DEV_CMD_LOOP_CNT 3000000
+#define IPC_DEV_CMD_TIMEOUT 3 * HZ
+#define IPC_DEV_DATA_BUFFER_SIZE 16
+
+struct intel_ipc_dev;
+struct intel_ipc_raw_cmd;
+
+/**
+ * struct intel_ipc_dev_cfg - IPC device config structure.
+ *
+ * IPC device drivers uses the following config options to
+ * register new IPC device.
+ *
+ * @cmd_regs : IPC device command base regmap.
+ * @data_regs : IPC device data base regmap.
+ * @wrbuf_reg : IPC device data write register address.
+ * @rbuf_reg : IPC device data read register address.
+ * @sptr_reg : IPC device source data pointer register address.
+ * @dptr_reg : IPC device destination data pointer register
+ * address.
+ * @status_reg : IPC command status register address.
+ * @cmd_reg : IPC command register address.
+ * @mode : IRQ/POLLING mode.
+ * @irq : IPC device IRQ number.
+ * @irqflags : IPC device IRQ flags.
+ * @chan_type : IPC device channel type(PMC/PUNIT).
+ * @msi : Enable/Disable MSI for IPC commands.
+ * @support_dptr : Support DPTR update.
+ * @support_sptr : Support SPTR update.
+ *
+ */
+struct intel_ipc_dev_cfg {
+ struct regmap *cmd_regs;
+ struct regmap *data_regs;
+ unsigned int wrbuf_reg;
+ unsigned int rbuf_reg;
+ unsigned int sptr_reg;
+ unsigned int dptr_reg;
+ unsigned int status_reg;
+ unsigned int cmd_reg;
+ int mode;
+ int irq;
+ int irqflags;
+ int chan_type;
+ bool use_msi;
+ bool support_dptr;
+ bool support_sptr;
+};
+
+/**
+ * struct intel_ipc_dev_ops - IPC device ops structure.
+ *
+ * Call backs for IPC device specific operations.
+ *
+ * @to_err_code : Status to error code conversion function.
+ * @busy_check : Check for IPC busy status.
+ * @enable_msi : Enable MSI for IPC commands.
+ * @pre_simple_cmd_fn : Custom pre-processing function for
+ * ipc_dev_simple_cmd()
+ * @pre_cmd_fn : Custom pre-processing function for
+ * ipc_dev_cmd()
+ * @pre_raw_cmd_fn : Custom pre-processing function for
+ * ipc_dev_raw_cmd()
+ *
+ */
+struct intel_ipc_dev_ops {
+ int (*to_err_code)(int status);
+ int (*busy_check)(int status);
+ u32 (*enable_msi)(u32 cmd);
+ int (*pre_simple_cmd_fn)(u32 *cmd_list, u32 cmdlen);
+ int (*pre_cmd_fn)(u32 *cmd_list, u32 cmdlen, u32 *in, u32 inlen,
+ u32 *out, u32 outlen);
+ int (*pre_raw_cmd_fn)(u32 *cmd_list, u32 cmdlen, u8 *in, u32 inlen,
+ u32 *out, u32 outlen, u32 dptr, u32 sptr);
+ int (*pre_irq_handler_fn)(struct intel_ipc_dev *ipc_dev, int irq);
+};
+
+/**
+ * struct intel_ipc_dev - Intel IPC device structure.
+ *
+ * Used with devm_intel_ipc_dev_create() to create new IPC device.
+ *
+ * @dev : IPC device object.
+ * @cmd : Current IPC device command.
+ * @cmd_complete : Command completion object.
+ * @lock : Lock to protect IPC device structure.
+ * @ops : IPC device ops pointer.
+ * @cfg : IPC device cfg pointer.
+ *
+ */
+struct intel_ipc_dev {
+ struct device dev;
+ int cmd;
+ struct completion cmd_complete;
+ struct mutex lock;
+ struct intel_ipc_dev_ops *ops;
+ struct intel_ipc_dev_cfg *cfg;
+};
+
+#if IS_ENABLED(CONFIG_INTEL_IPC_DEV)
+
+/* API to create new IPC device */
+struct intel_ipc_dev *devm_intel_ipc_dev_create(struct device *dev,
+ const char *devname, struct intel_ipc_dev_cfg *cfg,
+ struct intel_ipc_dev_ops *ops);
+
+int ipc_dev_simple_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list,
+ u32 cmdlen);
+int ipc_dev_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list, u32 cmdlen,
+ u32 *in, u32 inlen, u32 *out, u32 outlen);
+int ipc_dev_raw_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list, u32 cmdlen,
+ u8 *in, u32 inlen, u32 *out, u32 outlen, u32 dptr, u32 sptr);
+struct intel_ipc_dev *intel_ipc_dev_get(const char *dev_name);
+struct intel_ipc_dev *devm_intel_ipc_dev_get(struct device *dev,
+ const char *dev_name);
+static inline void intel_ipc_dev_put(struct intel_ipc_dev *ipc_dev)
+{
+ put_device(&ipc_dev->dev);
+}
+#else
+
+static inline struct intel_ipc_dev *devm_intel_ipc_dev_create(
+ struct device *dev,
+ const char *devname, struct intel_ipc_dev_cfg *cfg,
+ struct intel_ipc_dev_ops *ops)
+{
+ return -EINVAL;
+}
+
+static inline int ipc_dev_simple_cmd(struct intel_ipc_dev *ipc_dev,
+ u32 *cmd_list, u32 cmdlen)
+{
+ return -EINVAL;
+}
+
+static int ipc_dev_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list,
+ u32 cmdlen, u32 *in, u32 inlen, u32 *out, u32 outlen)
+{
+ return -EINVAL;
+}
+
+static inline int ipc_dev_raw_cmd(struct intel_ipc_dev *ipc_dev, u32 *cmd_list,
+ u32 cmdlen, u8 *in, u32 inlen, u32 *out, u32 outlen,
+ u32 dptr, u32 sptr);
+{
+ return -EINVAL;
+}
+
+static inline struct intel_ipc_dev *intel_ipc_dev_get(const char *dev_name)
+{
+ return NULL;
+}
+
+static inline struct intel_ipc_dev *devm_intel_ipc_dev_get(struct device *dev,
+ const char *dev_name);
+{
+ return NULL;
+}
+
+static inline void intel_ipc_dev_put(struct intel_ipc_dev *ipc_dev)
+{
+ return NULL;
+}
+#endif /* CONFIG_INTEL_IPC_DEV */
+#endif /* INTEL_IPC_DEV_H */
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox