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

Groups > linux.kernel > #1278187 > unrolled thread

[char-misc-next 0/6] mei: create proper iAMT watchdog driver

Back to article view | Back to linux.kernel

Contents

  [char-misc-next 0/6] mei: create proper iAMT watchdog driver Tomas Winkler <tomas.winkler@intel.com> - 2015-11-26 13:40 +0100
    [char-misc-next 1/6] mei: drop nfc leftovers from the mei driver Tomas Winkler <tomas.winkler@intel.com> - 2015-11-26 13:40 +0100
    [char-misc-next 6/6] mei: wd: re-register device on event Tomas Winkler <tomas.winkler@intel.com> - 2015-11-26 13:40 +0100
      Re: [char-misc-next 6/6] mei: wd: re-register device on event Guenter Roeck <linux@roeck-us.net> - 2015-11-30 18:10 +0100
        Re: [char-misc-next 6/6] mei: wd: re-register device on event "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-12-01 13:10 +0100
    [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver Tomas Winkler <tomas.winkler@intel.com> - 2015-11-26 13:40 +0100
      Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver Guenter Roeck <linux@roeck-us.net> - 2015-11-30 18:00 +0100
        Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-12-01 13:00 +0100
          Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver Guenter Roeck <linux@roeck-us.net> - 2015-12-01 17:10 +0100
            RE: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver "Winkler, Tomas" <tomas.winkler@intel.com> - 2015-12-02 08:50 +0100

#1278187 — [char-misc-next 0/6] mei: create proper iAMT watchdog driver

Instead of integrating the iAMT watchdog within the mei core driver
we will create a watchdog device on the mei client bus and
create a proper watchdog driver for it.

The patches should apply and compile on both watchdog-next and char-misc-next
trees/branches.
I would prefer this will go via char-misc-next as this is the tree
we work against and conflicts should be trivial if at any.

Alexander Usyskin (4):
  mei: wd: drop the watchdog code from the core mei driver
  mei: wd: implement MEI iAMT watchdog driver
  mei: wd: register wd device only if required
  mei: wd: re-register device on event

Tomas Winkler (2):
  mei: drop nfc leftovers from the mei driver
  mei: bus: whitelist the watchdog client

 Documentation/misc-devices/mei/mei.txt |  12 +-
 MAINTAINERS                            |   1 +
 drivers/misc/mei/Kconfig               |   6 +-
 drivers/misc/mei/Makefile              |   1 -
 drivers/misc/mei/bus-fixup.c           |  29 ++
 drivers/misc/mei/client.c              |  12 +-
 drivers/misc/mei/client.h              |   4 -
 drivers/misc/mei/init.c                |  10 +-
 drivers/misc/mei/interrupt.c           |  15 -
 drivers/misc/mei/mei_dev.h             |  72 +----
 drivers/misc/mei/wd.c                  | 391 ----------------------
 drivers/watchdog/Kconfig               |  15 +
 drivers/watchdog/Makefile              |   1 +
 drivers/watchdog/mei_wdt.c             | 571 +++++++++++++++++++++++++++++++++
 14 files changed, 632 insertions(+), 508 deletions(-)
 delete mode 100644 drivers/misc/mei/wd.c
 create mode 100644 drivers/watchdog/mei_wdt.c

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278188 — [char-misc-next 1/6] mei: drop nfc leftovers from the mei driver

We left few function prototypes in the header file after
moving nfc logic to bus.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/misc/mei/mei_dev.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
index 4250555d5e72..b54d9d9cacea 100644
--- a/drivers/misc/mei/mei_dev.h
+++ b/drivers/misc/mei/mei_dev.h
@@ -649,17 +649,6 @@ int mei_amthif_irq_read_msg(struct mei_cl *cl,
 			    struct mei_cl_cb *complete_list);
 int mei_amthif_irq_read(struct mei_device *dev, s32 *slots);
 
-/*
- * NFC functions
- */
-int mei_nfc_host_init(struct mei_device *dev, struct mei_me_client *me_cl);
-void mei_nfc_host_exit(struct mei_device *dev);
-
-/*
- * NFC Client UUID
- */
-extern const uuid_le mei_nfc_guid;
-
 int mei_wd_send(struct mei_device *dev);
 int mei_wd_stop(struct mei_device *dev);
 int mei_wd_host_init(struct mei_device *dev, struct mei_me_client *me_cl);
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278189 — [char-misc-next 6/6] mei: wd: re-register device on event

From: Alexander Usyskin <alexander.usyskin@intel.com>

For Intel SKL platform the ME device can inform the host via
asynchronous notification that the watchdog feature was activated on the
device. The activation doesn't require reboot.
In that case the driver register the watchdog device with the kernel.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/watchdog/mei_wdt.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
index 47f0dc2e822a..ee3b5d12b1b2 100644
--- a/drivers/watchdog/mei_wdt.c
+++ b/drivers/watchdog/mei_wdt.c
@@ -279,6 +279,21 @@ out:
 		complete(&wdt->response);
 }
 
+/*
+ * mei_wdt_event_notif - callback for notification
+ *
+ * @cldev: bus device
+ */
+static void mei_wdt_event_notif(struct mei_cl_device *cldev)
+{
+	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
+
+	if (wdt->state == MEI_WDT_NOT_REQUIRED) {
+		mei_wdt_register(wdt);
+		wdt->state = MEI_WDT_IDLE;
+	}
+}
+
 /**
  * mei_wdt_event - callback for event receive
  *
@@ -291,6 +306,9 @@ static void mei_wdt_event(struct mei_cl_device *cldev,
 {
 	if (events & BIT(MEI_CL_EVENT_RX))
 		mei_wdt_event_rx(cldev);
+
+	if (events & BIT(MEI_CL_EVENT_NOTIF))
+		mei_wdt_event_notif(cldev);
 }
 
 /**
@@ -466,9 +484,13 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
 
 	wd_info.firmware_version = mei_cldev_ver(cldev);
 
-	ret = mei_cldev_register_event_cb(wdt->cldev, BIT(MEI_CL_EVENT_RX),
+	ret = mei_cldev_register_event_cb(wdt->cldev,
+					  BIT(MEI_CL_EVENT_RX) |
+					  BIT(MEI_CL_EVENT_NOTIF),
 					  mei_wdt_event, NULL);
-	if (ret) {
+
+	/* on legacy devices notification is not supported */
+	if (ret && ret != -EOPNOTSUPP) {
 		dev_err(&cldev->dev, "Could not register event ret=%d\n", ret);
 		goto err_disable;
 	}
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280113 — Re: [char-misc-next 6/6] mei: wd: re-register device on event

On 11/26/2015 04:31 AM, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> For Intel SKL platform the ME device can inform the host via
> asynchronous notification that the watchdog feature was activated on the
> device. The activation doesn't require reboot.
> In that case the driver register the watchdog device with the kernel.
>

Can the watchdog device also be disabled on the fly ?

> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>   drivers/watchdog/mei_wdt.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index 47f0dc2e822a..ee3b5d12b1b2 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
> @@ -279,6 +279,21 @@ out:
>   		complete(&wdt->response);
>   }
>
> +/*
> + * mei_wdt_event_notif - callback for notification
> + *
> + * @cldev: bus device
> + */
> +static void mei_wdt_event_notif(struct mei_cl_device *cldev)
> +{
> +	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
> +
> +	if (wdt->state == MEI_WDT_NOT_REQUIRED) {
> +		mei_wdt_register(wdt);
> +		wdt->state = MEI_WDT_IDLE;
> +	}
> +}
> +
>   /**
>    * mei_wdt_event - callback for event receive
>    *
> @@ -291,6 +306,9 @@ static void mei_wdt_event(struct mei_cl_device *cldev,
>   {
>   	if (events & BIT(MEI_CL_EVENT_RX))
>   		mei_wdt_event_rx(cldev);
> +
> +	if (events & BIT(MEI_CL_EVENT_NOTIF))
> +		mei_wdt_event_notif(cldev);
>   }
>
>   /**
> @@ -466,9 +484,13 @@ static int mei_wdt_probe(struct mei_cl_device *cldev,
>
>   	wd_info.firmware_version = mei_cldev_ver(cldev);
>
> -	ret = mei_cldev_register_event_cb(wdt->cldev, BIT(MEI_CL_EVENT_RX),
> +	ret = mei_cldev_register_event_cb(wdt->cldev,
> +					  BIT(MEI_CL_EVENT_RX) |
> +					  BIT(MEI_CL_EVENT_NOTIF),
>   					  mei_wdt_event, NULL);
> -	if (ret) {
> +
> +	/* on legacy devices notification is not supported */
> +	if (ret && ret != -EOPNOTSUPP) {
>   		dev_err(&cldev->dev, "Could not register event ret=%d\n", ret);
>   		goto err_disable;
>   	}
>
Doesn't that mean, though, that the RX event is not registered either,
even if it is supported ? If so, should you retry and register only
MEI_CL_EVENT_RX ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280812 — Re: [char-misc-next 6/6] mei: wd: re-register device on event

T24gTW9uLCAyMDE1LTExLTMwIGF0IDA5OjA4IC0wODAwLCBHdWVudGVyIFJvZWNrIHdyb3RlOg0K
PiBPbiAxMS8yNi8yMDE1IDA0OjMxIEFNLCBUb21hcyBXaW5rbGVyIHdyb3RlOg0KPiA+IEZyb206
IEFsZXhhbmRlciBVc3lza2luIDxhbGV4YW5kZXIudXN5c2tpbkBpbnRlbC5jb20+DQo+ID4gDQo+
ID4gRm9yIEludGVsIFNLTCBwbGF0Zm9ybSB0aGUgTUUgZGV2aWNlIGNhbiBpbmZvcm0gdGhlIGhv
c3QgdmlhDQo+ID4gYXN5bmNocm9ub3VzIG5vdGlmaWNhdGlvbiB0aGF0IHRoZSB3YXRjaGRvZyBm
ZWF0dXJlIHdhcyBhY3RpdmF0ZWQNCj4gPiBvbiB0aGUNCj4gPiBkZXZpY2UuIFRoZSBhY3RpdmF0
aW9uIGRvZXNuJ3QgcmVxdWlyZSByZWJvb3QuDQo+ID4gSW4gdGhhdCBjYXNlIHRoZSBkcml2ZXIg
cmVnaXN0ZXIgdGhlIHdhdGNoZG9nIGRldmljZSB3aXRoIHRoZQ0KPiA+IGtlcm5lbC4NCj4gPiAN
Cj4gDQo+IENhbiB0aGUgd2F0Y2hkb2cgZGV2aWNlIGFsc28gYmUgZGlzYWJsZWQgb24gdGhlIGZs
eSA/DQpZZXMsIGl0IGNhbiBiZSBkaXNhYmxlZCBhbHNvIG9uIHRoZSBmbHksIGJ1dCB0aGlzIGlz
IG5vdCBkZWxpdmVyZWQgdmlhDQphbiBldmVudCwgdGhpcyBpcyBoYW5kbGVkIHZpYSBwaW5nIHJl
dHVybiB2YWx1ZS4gDQoNCj4gDQo+ID4gU2lnbmVkLW9mZi1ieTogQWxleGFuZGVyIFVzeXNraW4g
PGFsZXhhbmRlci51c3lza2luQGludGVsLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBUb21hcyBX
aW5rbGVyIDx0b21hcy53aW5rbGVyQGludGVsLmNvbT4NCj4gPiAtLS0NCj4gPiAgIGRyaXZlcnMv
d2F0Y2hkb2cvbWVpX3dkdC5jIHwgMjYgKysrKysrKysrKysrKysrKysrKysrKysrLS0NCj4gPiAg
IDEgZmlsZSBjaGFuZ2VkLCAyNCBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPiA+IA0K
PiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3dhdGNoZG9nL21laV93ZHQuYw0KPiA+IGIvZHJpdmVy
cy93YXRjaGRvZy9tZWlfd2R0LmMNCj4gPiBpbmRleCA0N2YwZGMyZTgyMmEuLmVlM2I1ZDEyYjFi
MiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL3dhdGNoZG9nL21laV93ZHQuYw0KPiA+ICsrKyBi
L2RyaXZlcnMvd2F0Y2hkb2cvbWVpX3dkdC5jDQo+ID4gQEAgLTI3OSw2ICsyNzksMjEgQEAgb3V0
Og0KPiA+ICAgCQljb21wbGV0ZSgmd2R0LT5yZXNwb25zZSk7DQo+ID4gICB9DQo+ID4gDQo+ID4g
Ky8qDQo+ID4gKyAqIG1laV93ZHRfZXZlbnRfbm90aWYgLSBjYWxsYmFjayBmb3Igbm90aWZpY2F0
aW9uDQo+ID4gKyAqDQo+ID4gKyAqIEBjbGRldjogYnVzIGRldmljZQ0KPiA+ICsgKi8NCj4gPiAr
c3RhdGljIHZvaWQgbWVpX3dkdF9ldmVudF9ub3RpZihzdHJ1Y3QgbWVpX2NsX2RldmljZSAqY2xk
ZXYpDQo+ID4gK3sNCj4gPiArCXN0cnVjdCBtZWlfd2R0ICp3ZHQgPSBtZWlfY2xkZXZfZ2V0X2Ry
dmRhdGEoY2xkZXYpOw0KPiA+ICsNCj4gPiArCWlmICh3ZHQtPnN0YXRlID09IE1FSV9XRFRfTk9U
X1JFUVVJUkVEKSB7DQo+ID4gKwkJbWVpX3dkdF9yZWdpc3Rlcih3ZHQpOw0KPiA+ICsJCXdkdC0+
c3RhdGUgPSBNRUlfV0RUX0lETEU7DQo+ID4gKwl9DQo+ID4gK30NCj4gPiArDQo+ID4gICAvKioN
Cj4gPiAgICAqIG1laV93ZHRfZXZlbnQgLSBjYWxsYmFjayBmb3IgZXZlbnQgcmVjZWl2ZQ0KPiA+
ICAgICoNCj4gPiBAQCAtMjkxLDYgKzMwNiw5IEBAIHN0YXRpYyB2b2lkIG1laV93ZHRfZXZlbnQo
c3RydWN0IG1laV9jbF9kZXZpY2UNCj4gPiAqY2xkZXYsDQo+ID4gICB7DQo+ID4gICAJaWYgKGV2
ZW50cyAmIEJJVChNRUlfQ0xfRVZFTlRfUlgpKQ0KPiA+ICAgCQltZWlfd2R0X2V2ZW50X3J4KGNs
ZGV2KTsNCj4gPiArDQo+ID4gKwlpZiAoZXZlbnRzICYgQklUKE1FSV9DTF9FVkVOVF9OT1RJRikp
DQo+ID4gKwkJbWVpX3dkdF9ldmVudF9ub3RpZihjbGRldik7DQo+ID4gICB9DQo+ID4gDQo+ID4g
ICAvKioNCj4gPiBAQCAtNDY2LDkgKzQ4NCwxMyBAQCBzdGF0aWMgaW50IG1laV93ZHRfcHJvYmUo
c3RydWN0IG1laV9jbF9kZXZpY2UNCj4gPiAqY2xkZXYsDQo+ID4gDQo+ID4gICAJd2RfaW5mby5m
aXJtd2FyZV92ZXJzaW9uID0gbWVpX2NsZGV2X3ZlcihjbGRldik7DQo+ID4gDQo+ID4gLQlyZXQg
PSBtZWlfY2xkZXZfcmVnaXN0ZXJfZXZlbnRfY2Iod2R0LT5jbGRldiwNCj4gPiBCSVQoTUVJX0NM
X0VWRU5UX1JYKSwNCj4gPiArCXJldCA9IG1laV9jbGRldl9yZWdpc3Rlcl9ldmVudF9jYih3ZHQt
PmNsZGV2LA0KPiA+ICsJCQkJCSAgQklUKE1FSV9DTF9FVkVOVF9SWCkgfA0KPiA+ICsJCQkJCSAg
QklUKE1FSV9DTF9FVkVOVF9OT1RJRiksDQo+ID4gICAJCQkJCSAgbWVpX3dkdF9ldmVudCwgTlVM
TCk7DQo+ID4gLQlpZiAocmV0KSB7DQo+ID4gKw0KPiA+ICsJLyogb24gbGVnYWN5IGRldmljZXMg
bm90aWZpY2F0aW9uIGlzIG5vdCBzdXBwb3J0ZWQgKi8NCj4gPiArCWlmIChyZXQgJiYgcmV0ICE9
IC1FT1BOT1RTVVBQKSB7DQo+ID4gICAJCWRldl9lcnIoJmNsZGV2LT5kZXYsICJDb3VsZCBub3Qg
cmVnaXN0ZXIgZXZlbnQNCj4gPiByZXQ9JWRcbiIsIHJldCk7DQo+ID4gICAJCWdvdG8gZXJyX2Rp
c2FibGU7DQo+ID4gICAJfQ0KPiA+IA0KPiBEb2Vzbid0IHRoYXQgbWVhbiwgdGhvdWdoLCB0aGF0
IHRoZSBSWCBldmVudCBpcyBub3QgcmVnaXN0ZXJlZA0KPiBlaXRoZXIsDQo+IGV2ZW4gaWYgaXQg
aXMgc3VwcG9ydGVkID8gSWYgc28sIHNob3VsZCB5b3UgcmV0cnkgYW5kIHJlZ2lzdGVyIG9ubHkN
Cj4gTUVJX0NMX0VWRU5UX1JYID8NCg0KVGhlIGNvZGUgaXMgY29ycmVjdCBidXQgd2UgZGVmaW5p
dGVseSBuZWVkIHRvIHRoaW5rIG9mIGJldHRlciBBUEkgDQpUb21hcw0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1278190 — [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver

From: Alexander Usyskin <alexander.usyskin@intel.com>

Create a driver with the generic watchdog interface
for the MEI iAMT watchdog device.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 Documentation/misc-devices/mei/mei.txt |  12 +-
 MAINTAINERS                            |   1 +
 drivers/watchdog/Kconfig               |  15 ++
 drivers/watchdog/Makefile              |   1 +
 drivers/watchdog/mei_wdt.c             | 432 +++++++++++++++++++++++++++++++++
 5 files changed, 455 insertions(+), 6 deletions(-)
 create mode 100644 drivers/watchdog/mei_wdt.c

diff --git a/Documentation/misc-devices/mei/mei.txt b/Documentation/misc-devices/mei/mei.txt
index 91c1fa34f48b..2b80a0cd621f 100644
--- a/Documentation/misc-devices/mei/mei.txt
+++ b/Documentation/misc-devices/mei/mei.txt
@@ -231,15 +231,15 @@ IT knows when a platform crashes even when there is a hard failure on the host.
 The Intel AMT Watchdog is composed of two parts:
 	1) Firmware feature - receives the heartbeats
 	   and sends an event when the heartbeats stop.
-	2) Intel MEI driver - connects to the watchdog feature, configures the
-	   watchdog and sends the heartbeats.
+	2) Intel MEI iAMT watchdog driver - connects to the watchdog feature,
+	   configures the watchdog and sends the heartbeats.
 
-The Intel MEI driver uses the kernel watchdog API to configure the Intel AMT
-Watchdog and to send heartbeats to it. The default timeout of the
+The Intel iAMT watchdog MEI driver uses the kernel watchdog API to configure
+the Intel AMT Watchdog and to send heartbeats to it. The default timeout of the
 watchdog is 120 seconds.
 
-If the Intel AMT Watchdog feature does not exist (i.e. the connection failed),
-the Intel MEI driver will disable the sending of heartbeats.
+If the Intel AMT is not enabled in the firmware then the watchdog client won't enumerate
+on the me client bus and watchdog devices won't be exposed.
 
 
 Supported Chipsets
diff --git a/MAINTAINERS b/MAINTAINERS
index 050d0e77a2cf..cf0a51518f4a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5664,6 +5664,7 @@ S:	Supported
 F:	include/uapi/linux/mei.h
 F:	include/linux/mei_cl_bus.h
 F:	drivers/misc/mei/*
+F:	drivers/watchdog/mei_wdt.c
 F:	Documentation/misc-devices/mei/*
 
 INTEL MIC DRIVERS (mic)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7a8a6c6952e9..ec584714829d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1154,6 +1154,21 @@ config SBC_EPX_C3_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called sbc_epx_c3.
 
+config INTEL_MEI_WDT
+	tristate "Intel MEI iAMT Watchdog"
+	depends on INTEL_MEI && X86
+	select WATCHDOG_CORE
+	---help---
+	  A device driver for the Intel MEI iAMT watchdog.
+
+	  The Intel AMT Watchdog is an OS Health (Hang/Crash) watchdog.
+	  Whenever the OS hangs or crashes, iAMT will send an event
+	  to any subscriber to this event. The watchdog doesn't reset the
+	  the platform.
+
+	  To compile this driver as a module, choose M here:
+	  the module will be called mei_wdt.
+
 # M32R Architecture
 
 # M68K Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 53d4827ddfe1..9069c9dd8aa8 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
 obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
+obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
new file mode 100644
index 000000000000..149b29f341cf
--- /dev/null
+++ b/drivers/watchdog/mei_wdt.c
@@ -0,0 +1,432 @@
+/*
+ * Intel Management Engine Interface (Intel MEI) Linux driver
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/watchdog.h>
+
+#include <linux/uuid.h>
+#include <linux/mei_cl_bus.h>
+
+/*
+ * iAMT Watchdog Device
+ */
+#define INTEL_AMT_WATCHDOG_ID "iamt_wdt"
+
+#define MEI_WDT_DEFAULT_TIMEOUT   120  /* seconds */
+#define MEI_WDT_MIN_TIMEOUT       120  /* seconds */
+#define MEI_WDT_MAX_TIMEOUT     65535  /* seconds */
+
+/* Commands */
+#define MEI_MANAGEMENT_CONTROL 0x02
+
+/* MEI Management Control version number */
+#define MEI_MC_VERSION_NUMBER  0x10
+
+/* Sub Commands */
+#define MEI_MC_START_WD_TIMER_REQ  0x13
+#define MEI_MC_STOP_WD_TIMER_REQ   0x14
+
+/**
+ * enum mei_wdt_state - internal watchdog state
+ *
+ * @MEI_WDT_IDLE: wd is idle and not opened
+ * @MEI_WDT_START: wd was opened, start was called
+ * @MEI_WDT_RUNNING: wd is expecting keep alive pings
+ * @MEI_WDT_STOPPING: wd is stopping and will move to IDLE
+ */
+enum mei_wdt_state {
+	MEI_WDT_IDLE,
+	MEI_WDT_START,
+	MEI_WDT_RUNNING,
+	MEI_WDT_STOPPING,
+};
+
+struct mei_wdt;
+
+/**
+ * struct mei_wdt_dev - watchdog device wrapper
+ *
+ * @wdd: watchdog device
+ * @wdt: back pointer to mei_wdt driver
+ * @refcnt: reference counter
+ */
+struct mei_wdt_dev {
+	struct watchdog_device wdd;
+	struct mei_wdt *wdt;
+	struct kref refcnt;
+};
+
+/**
+ * struct mei_wdt - mei watchdog driver
+ *
+ * @cldev: mei watchdog client device
+ * @mwd: watchdog device wrapper
+ * @state: watchdog internal state
+ * @timeout: watchdog current timeout
+ */
+struct mei_wdt {
+	struct mei_cl_device *cldev;
+	struct mei_wdt_dev *mwd;
+	enum mei_wdt_state state;
+	u16 timeout;
+};
+
+struct mei_wdt_hdr {
+	u8 command;
+	u8 bytecount;
+	u8 subcommand;
+	u8 versionnumber;
+};
+
+struct mei_wdt_start_request {
+	struct mei_wdt_hdr hdr;
+	u16 timeout;
+	u8 reserved[17];
+} __packed;
+
+struct mei_wdt_stop_request {
+	struct mei_wdt_hdr hdr;
+} __packed;
+
+/**
+ * mei_wdt_ping - send wd start command
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: number of bytes sent on success,
+ *         negative errno code on failure
+ */
+static int mei_wdt_ping(struct mei_wdt *wdt)
+{
+	struct mei_wdt_start_request req;
+	const size_t req_len = sizeof(req);
+
+	memset(&req, 0, req_len);
+	req.hdr.command = MEI_MANAGEMENT_CONTROL;
+	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr, subcommand);
+	req.hdr.subcommand = MEI_MC_START_WD_TIMER_REQ;
+	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
+	req.timeout = wdt->timeout;
+
+	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
+}
+
+/**
+ * mei_wdt_stop - send wd stop command
+ *
+ * @wdt: mei watchdog device
+ *
+ * Return: number of bytes sent on success,
+ *         negative errno code on failure
+ */
+static int mei_wdt_stop(struct mei_wdt *wdt)
+{
+	struct mei_wdt_stop_request req;
+	const size_t req_len = sizeof(req);
+
+	memset(&req, 0, req_len);
+	req.hdr.command = MEI_MANAGEMENT_CONTROL;
+	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr, subcommand);
+	req.hdr.subcommand = MEI_MC_STOP_WD_TIMER_REQ;
+	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
+
+	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
+}
+
+/**
+ * mei_wdt_ops_start - wd start command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 on success or -ENODEV;
+ */
+static int mei_wdt_ops_start(struct watchdog_device *wdd)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+
+	if (!mwd)
+		return -ENODEV;
+
+	mwd->wdt->state = MEI_WDT_START;
+	wdd->timeout = mwd->wdt->timeout;
+	return 0;
+}
+
+/**
+ * mei_wdt_ops_stop - wd stop command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 if success, negative errno code for failure
+ */
+static int mei_wdt_ops_stop(struct watchdog_device *wdd)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+	struct mei_wdt *wdt;
+	int ret;
+
+	if (!mwd)
+		return -ENODEV;
+
+	wdt = mwd->wdt;
+
+	if (wdt->state != MEI_WDT_RUNNING)
+		return 0;
+
+	wdt->state = MEI_WDT_STOPPING;
+
+	ret = mei_wdt_stop(wdt);
+	if (ret < 0)
+		return ret;
+
+	wdt->state = MEI_WDT_IDLE;
+
+	return 0;
+}
+
+/**
+ * mei_wdt_ops_ping - wd ping command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ *
+ * Return: 0 if success, negative errno code on failure
+ */
+static int mei_wdt_ops_ping(struct watchdog_device *wdd)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+	struct mei_wdt *wdt;
+	int ret;
+
+	if (!mwd)
+		return -ENODEV;
+
+	wdt = mwd->wdt;
+
+	if (wdt->state != MEI_WDT_START &&
+	    wdt->state != MEI_WDT_RUNNING)
+		return 0;
+
+	ret = mei_wdt_ping(wdt);
+	if (ret < 0)
+		return ret;
+
+	wdt->state = MEI_WDT_RUNNING;
+
+	return 0;
+}
+
+/**
+ * mei_wdt_ops_set_timeout - wd set timeout command from the watchdog core.
+ *
+ * @wdd: watchdog device
+ * @timeout: timeout value to set
+ *
+ * Return: 0 if success, negative errno code for failure
+ */
+static int mei_wdt_ops_set_timeout(struct watchdog_device *wdd,
+				   unsigned int timeout)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+	struct mei_wdt *wdt;
+
+	if (!mwd)
+		return -ENODEV;
+
+	wdt = mwd->wdt;
+
+	/* valid value is already checked by the caller */
+	wdt->timeout = timeout;
+	wdd->timeout = timeout;
+
+	return 0;
+}
+
+static void mei_wdt_release(struct kref *ref)
+{
+	struct mei_wdt_dev *mwd = container_of(ref, struct mei_wdt_dev, refcnt);
+
+	kfree(mwd);
+}
+
+static void mei_wdt_ops_ref(struct watchdog_device *wdd)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+
+	kref_get(&mwd->refcnt);
+}
+
+static void mei_wdt_ops_unref(struct watchdog_device *wdd)
+{
+	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
+
+	kref_put(&mwd->refcnt, mei_wdt_release);
+}
+
+static const struct watchdog_ops wd_ops = {
+	.owner       = THIS_MODULE,
+	.start       = mei_wdt_ops_start,
+	.stop        = mei_wdt_ops_stop,
+	.ping        = mei_wdt_ops_ping,
+	.set_timeout = mei_wdt_ops_set_timeout,
+	.ref         = mei_wdt_ops_ref,
+	.unref       = mei_wdt_ops_unref,
+};
+
+static struct watchdog_info wd_info = {
+	.identity = INTEL_AMT_WATCHDOG_ID,
+	.options  = WDIOF_KEEPALIVEPING |
+		    WDIOF_SETTIMEOUT |
+		    WDIOF_ALARMONLY,
+};
+
+static int mei_wdt_register(struct mei_wdt *wdt)
+{
+	struct mei_wdt_dev *mwd;
+	struct device *dev;
+	int ret;
+
+	if (!wdt || !wdt->cldev)
+		return -EINVAL;
+
+	dev = &wdt->cldev->dev;
+
+	mwd = kzalloc(sizeof(struct mei_wdt_dev), GFP_KERNEL);
+	if (!mwd)
+		return -ENOMEM;
+
+	mwd->wdt = wdt;
+	mwd->wdd.info = &wd_info;
+	mwd->wdd.ops = &wd_ops;
+	mwd->wdd.parent = dev;
+	mwd->wdd.timeout = MEI_WDT_DEFAULT_TIMEOUT;
+	mwd->wdd.min_timeout = MEI_WDT_MIN_TIMEOUT;
+	mwd->wdd.max_timeout = MEI_WDT_MAX_TIMEOUT;
+	kref_init(&mwd->refcnt);
+
+	ret = watchdog_register_device(&mwd->wdd);
+	if (ret) {
+		dev_err(dev, "unable to register watchdog device = %d.\n", ret);
+		kref_put(&mwd->refcnt, mei_wdt_release);
+		return ret;
+	}
+
+	wdt->mwd = mwd;
+	watchdog_set_drvdata(&mwd->wdd, mwd);
+	return 0;
+}
+
+static void mei_wdt_unregister(struct mei_wdt *wdt)
+{
+	if (!wdt->mwd)
+		return;
+
+	watchdog_unregister_device(&wdt->mwd->wdd);
+	kref_put(&wdt->mwd->refcnt, mei_wdt_release);
+	wdt->mwd = NULL;
+}
+
+static int mei_wdt_probe(struct mei_cl_device *cldev,
+			 const struct mei_cl_device_id *id)
+{
+	struct mei_wdt *wdt;
+	int ret;
+
+	wdt = kzalloc(sizeof(struct mei_wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	wdt->timeout = MEI_WDT_DEFAULT_TIMEOUT;
+	wdt->state = MEI_WDT_IDLE;
+	wdt->cldev = cldev;
+	mei_cldev_set_drvdata(cldev, wdt);
+
+	ret = mei_cldev_enable(cldev);
+	if (ret < 0) {
+		dev_err(&cldev->dev, "Could not enable cl device\n");
+		goto err_out;
+	}
+
+	wd_info.firmware_version = mei_cldev_ver(cldev);
+
+	ret = mei_wdt_register(wdt);
+	if (ret)
+		goto err_disable;
+
+	return 0;
+
+err_disable:
+	mei_cldev_disable(cldev);
+
+err_out:
+	kfree(wdt);
+
+	return ret;
+}
+
+static int mei_wdt_remove(struct mei_cl_device *cldev)
+{
+	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
+
+	mei_cldev_disable(cldev);
+
+	mei_wdt_unregister(wdt);
+
+	kfree(wdt);
+
+	return 0;
+}
+
+#define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
+			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
+
+static struct mei_cl_device_id mei_wdt_tbl[] = {
+	{ .uuid = MEI_UUID_WD, .version = MEI_CL_VERSION_ANY},
+	/* required last entry */
+	{ }
+};
+MODULE_DEVICE_TABLE(mei, mei_wdt_tbl);
+
+static struct mei_cl_driver mei_wdt_driver = {
+	.id_table = mei_wdt_tbl,
+	.name = KBUILD_MODNAME,
+
+	.probe = mei_wdt_probe,
+	.remove = mei_wdt_remove,
+};
+
+static int __init mei_wdt_init(void)
+{
+	int ret;
+
+	ret = mei_cldev_driver_register(&mei_wdt_driver);
+	if (ret) {
+		pr_err(KBUILD_MODNAME ": module registration failed\n");
+		return ret;
+	}
+	return 0;
+}
+
+static void __exit mei_wdt_exit(void)
+{
+	mei_cldev_driver_unregister(&mei_wdt_driver);
+}
+
+module_init(mei_wdt_init);
+module_exit(mei_wdt_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Device driver for Intel MEI iAMT watchdog");
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280107 — Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver

On 11/26/2015 04:31 AM, Tomas Winkler wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
>
> Create a driver with the generic watchdog interface
> for the MEI iAMT watchdog device.
>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>   Documentation/misc-devices/mei/mei.txt |  12 +-
>   MAINTAINERS                            |   1 +
>   drivers/watchdog/Kconfig               |  15 ++
>   drivers/watchdog/Makefile              |   1 +
>   drivers/watchdog/mei_wdt.c             | 432 +++++++++++++++++++++++++++++++++
>   5 files changed, 455 insertions(+), 6 deletions(-)
>   create mode 100644 drivers/watchdog/mei_wdt.c
>
> diff --git a/Documentation/misc-devices/mei/mei.txt b/Documentation/misc-devices/mei/mei.txt
> index 91c1fa34f48b..2b80a0cd621f 100644
> --- a/Documentation/misc-devices/mei/mei.txt
> +++ b/Documentation/misc-devices/mei/mei.txt
> @@ -231,15 +231,15 @@ IT knows when a platform crashes even when there is a hard failure on the host.
>   The Intel AMT Watchdog is composed of two parts:
>   	1) Firmware feature - receives the heartbeats
>   	   and sends an event when the heartbeats stop.
> -	2) Intel MEI driver - connects to the watchdog feature, configures the
> -	   watchdog and sends the heartbeats.
> +	2) Intel MEI iAMT watchdog driver - connects to the watchdog feature,
> +	   configures the watchdog and sends the heartbeats.
>
> -The Intel MEI driver uses the kernel watchdog API to configure the Intel AMT
> -Watchdog and to send heartbeats to it. The default timeout of the
> +The Intel iAMT watchdog MEI driver uses the kernel watchdog API to configure
> +the Intel AMT Watchdog and to send heartbeats to it. The default timeout of the
>   watchdog is 120 seconds.
>
> -If the Intel AMT Watchdog feature does not exist (i.e. the connection failed),
> -the Intel MEI driver will disable the sending of heartbeats.
> +If the Intel AMT is not enabled in the firmware then the watchdog client won't enumerate
> +on the me client bus and watchdog devices won't be exposed.
>
>
>   Supported Chipsets
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 050d0e77a2cf..cf0a51518f4a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5664,6 +5664,7 @@ S:	Supported
>   F:	include/uapi/linux/mei.h
>   F:	include/linux/mei_cl_bus.h
>   F:	drivers/misc/mei/*
> +F:	drivers/watchdog/mei_wdt.c
>   F:	Documentation/misc-devices/mei/*
>
>   INTEL MIC DRIVERS (mic)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 7a8a6c6952e9..ec584714829d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1154,6 +1154,21 @@ config SBC_EPX_C3_WATCHDOG
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called sbc_epx_c3.
>
> +config INTEL_MEI_WDT
> +	tristate "Intel MEI iAMT Watchdog"
> +	depends on INTEL_MEI && X86
> +	select WATCHDOG_CORE
> +	---help---
> +	  A device driver for the Intel MEI iAMT watchdog.
> +
> +	  The Intel AMT Watchdog is an OS Health (Hang/Crash) watchdog.
> +	  Whenever the OS hangs or crashes, iAMT will send an event
> +	  to any subscriber to this event. The watchdog doesn't reset the
> +	  the platform.
> +
> +	  To compile this driver as a module, choose M here:
> +	  the module will be called mei_wdt.
> +
>   # M32R Architecture
>
>   # M68K Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 53d4827ddfe1..9069c9dd8aa8 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -123,6 +123,7 @@ obj-$(CONFIG_MACHZ_WDT) += machzwd.o
>   obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
>   obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
>   obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
> +obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
>
>   # M32R Architecture
>
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> new file mode 100644
> index 000000000000..149b29f341cf
> --- /dev/null
> +++ b/drivers/watchdog/mei_wdt.c
> @@ -0,0 +1,432 @@
> +/*
> + * Intel Management Engine Interface (Intel MEI) Linux driver
> + * Copyright (c) 2015, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/watchdog.h>
> +
> +#include <linux/uuid.h>
> +#include <linux/mei_cl_bus.h>
> +
> +/*
> + * iAMT Watchdog Device
> + */
> +#define INTEL_AMT_WATCHDOG_ID "iamt_wdt"
> +
> +#define MEI_WDT_DEFAULT_TIMEOUT   120  /* seconds */
> +#define MEI_WDT_MIN_TIMEOUT       120  /* seconds */

Is the large minimum timeout on purpose ?
Just asking, since it is quite unusual.

> +#define MEI_WDT_MAX_TIMEOUT     65535  /* seconds */
> +
> +/* Commands */
> +#define MEI_MANAGEMENT_CONTROL 0x02
> +
> +/* MEI Management Control version number */
> +#define MEI_MC_VERSION_NUMBER  0x10
> +
> +/* Sub Commands */
> +#define MEI_MC_START_WD_TIMER_REQ  0x13
> +#define MEI_MC_STOP_WD_TIMER_REQ   0x14
> +
> +/**
> + * enum mei_wdt_state - internal watchdog state
> + *
> + * @MEI_WDT_IDLE: wd is idle and not opened
> + * @MEI_WDT_START: wd was opened, start was called
> + * @MEI_WDT_RUNNING: wd is expecting keep alive pings
> + * @MEI_WDT_STOPPING: wd is stopping and will move to IDLE
> + */
> +enum mei_wdt_state {
> +	MEI_WDT_IDLE,
> +	MEI_WDT_START,
> +	MEI_WDT_RUNNING,
> +	MEI_WDT_STOPPING,
> +};
> +
> +struct mei_wdt;
> +
> +/**
> + * struct mei_wdt_dev - watchdog device wrapper
> + *
> + * @wdd: watchdog device
> + * @wdt: back pointer to mei_wdt driver
> + * @refcnt: reference counter
> + */
> +struct mei_wdt_dev {
> +	struct watchdog_device wdd;
> +	struct mei_wdt *wdt;
> +	struct kref refcnt;
> +};
> +
> +/**
> + * struct mei_wdt - mei watchdog driver
> + *
> + * @cldev: mei watchdog client device
> + * @mwd: watchdog device wrapper
> + * @state: watchdog internal state
> + * @timeout: watchdog current timeout
> + */
> +struct mei_wdt {
> +	struct mei_cl_device *cldev;
> +	struct mei_wdt_dev *mwd;
> +	enum mei_wdt_state state;
> +	u16 timeout;
> +};

Any special reason for having two data structures instead of one ?
You could just move the variables from struct mei_wdt_dev into
struct mei_wdt, no ?

> +
> +struct mei_wdt_hdr {
> +	u8 command;
> +	u8 bytecount;
> +	u8 subcommand;
> +	u8 versionnumber;
> +};
> +
> +struct mei_wdt_start_request {
> +	struct mei_wdt_hdr hdr;
> +	u16 timeout;
> +	u8 reserved[17];
> +} __packed;
> +
> +struct mei_wdt_stop_request {
> +	struct mei_wdt_hdr hdr;
> +} __packed;
> +
> +/**
> + * mei_wdt_ping - send wd start command
> + *
> + * @wdt: mei watchdog device
> + *
> + * Return: number of bytes sent on success,
> + *         negative errno code on failure
> + */
> +static int mei_wdt_ping(struct mei_wdt *wdt)
> +{
> +	struct mei_wdt_start_request req;
> +	const size_t req_len = sizeof(req);
> +
> +	memset(&req, 0, req_len);
> +	req.hdr.command = MEI_MANAGEMENT_CONTROL;
> +	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr, subcommand);
> +	req.hdr.subcommand = MEI_MC_START_WD_TIMER_REQ;
> +	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
> +	req.timeout = wdt->timeout;
> +
> +	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
> +}
> +
> +/**
> + * mei_wdt_stop - send wd stop command
> + *
> + * @wdt: mei watchdog device
> + *
> + * Return: number of bytes sent on success,
> + *         negative errno code on failure
> + */
> +static int mei_wdt_stop(struct mei_wdt *wdt)
> +{
> +	struct mei_wdt_stop_request req;
> +	const size_t req_len = sizeof(req);
> +
> +	memset(&req, 0, req_len);
> +	req.hdr.command = MEI_MANAGEMENT_CONTROL;
> +	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr, subcommand);
> +	req.hdr.subcommand = MEI_MC_STOP_WD_TIMER_REQ;
> +	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
> +
> +	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
> +}
> +
> +/**
> + * mei_wdt_ops_start - wd start command from the watchdog core.
> + *
> + * @wdd: watchdog device
> + *
> + * Return: 0 on success or -ENODEV;
> + */
> +static int mei_wdt_ops_start(struct watchdog_device *wdd)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +
> +	if (!mwd)
> +		return -ENODEV;

This can only happen because you call watchdog_set_drvdata() after
watchdog device registration. If that happens, the system is in
really bad shape.

I would suggest to move the call to watchdog_set_drvdata() ahead
of watchdog_register_device() and drop those checks.

> +
> +	mwd->wdt->state = MEI_WDT_START;
> +	wdd->timeout = mwd->wdt->timeout;
> +	return 0;
> +}
> +
> +/**
> + * mei_wdt_ops_stop - wd stop command from the watchdog core.
> + *
> + * @wdd: watchdog device
> + *
> + * Return: 0 if success, negative errno code for failure
> + */
> +static int mei_wdt_ops_stop(struct watchdog_device *wdd)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +	struct mei_wdt *wdt;
> +	int ret;
> +
> +	if (!mwd)
> +		return -ENODEV;
> +
> +	wdt = mwd->wdt;
> +
> +	if (wdt->state != MEI_WDT_RUNNING)
> +		return 0;
> +
> +	wdt->state = MEI_WDT_STOPPING;
> +
> +	ret = mei_wdt_stop(wdt);
> +	if (ret < 0)
> +		return ret;
> +
> +	wdt->state = MEI_WDT_IDLE;
> +
> +	return 0;
> +}
> +
> +/**
> + * mei_wdt_ops_ping - wd ping command from the watchdog core.
> + *
> + * @wdd: watchdog device
> + *
> + * Return: 0 if success, negative errno code on failure
> + */
> +static int mei_wdt_ops_ping(struct watchdog_device *wdd)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +	struct mei_wdt *wdt;
> +	int ret;
> +
> +	if (!mwd)
> +		return -ENODEV;
> +
> +	wdt = mwd->wdt;
> +
> +	if (wdt->state != MEI_WDT_START &&
> +	    wdt->state != MEI_WDT_RUNNING)

Unnecessary continuation line ?

> +		return 0;
> +
> +	ret = mei_wdt_ping(wdt);
> +	if (ret < 0)
> +		return ret;
> +
> +	wdt->state = MEI_WDT_RUNNING;
> +
> +	return 0;
> +}
> +
> +/**
> + * mei_wdt_ops_set_timeout - wd set timeout command from the watchdog core.
> + *
> + * @wdd: watchdog device
> + * @timeout: timeout value to set
> + *
> + * Return: 0 if success, negative errno code for failure
> + */
> +static int mei_wdt_ops_set_timeout(struct watchdog_device *wdd,
> +				   unsigned int timeout)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +	struct mei_wdt *wdt;
> +
> +	if (!mwd)
> +		return -ENODEV;
> +
> +	wdt = mwd->wdt;
> +
> +	/* valid value is already checked by the caller */
> +	wdt->timeout = timeout;
> +	wdd->timeout = timeout;

One of those seems unnecessary. Why keep the timeout twice ?

> +
> +	return 0;
> +}
> +
> +static void mei_wdt_release(struct kref *ref)
> +{
> +	struct mei_wdt_dev *mwd = container_of(ref, struct mei_wdt_dev, refcnt);
> +
> +	kfree(mwd);
> +}
> +
> +static void mei_wdt_ops_ref(struct watchdog_device *wdd)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +
> +	kref_get(&mwd->refcnt);
> +}
> +
> +static void mei_wdt_ops_unref(struct watchdog_device *wdd)
> +{
> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
> +
> +	kref_put(&mwd->refcnt, mei_wdt_release);
> +}
> +
> +static const struct watchdog_ops wd_ops = {
> +	.owner       = THIS_MODULE,
> +	.start       = mei_wdt_ops_start,
> +	.stop        = mei_wdt_ops_stop,
> +	.ping        = mei_wdt_ops_ping,
> +	.set_timeout = mei_wdt_ops_set_timeout,
> +	.ref         = mei_wdt_ops_ref,
> +	.unref       = mei_wdt_ops_unref,
> +};
> +
> +static struct watchdog_info wd_info = {
> +	.identity = INTEL_AMT_WATCHDOG_ID,
> +	.options  = WDIOF_KEEPALIVEPING |
> +		    WDIOF_SETTIMEOUT |
> +		    WDIOF_ALARMONLY,
> +};
> +
> +static int mei_wdt_register(struct mei_wdt *wdt)
> +{
> +	struct mei_wdt_dev *mwd;
> +	struct device *dev;
> +	int ret;
> +
> +	if (!wdt || !wdt->cldev)
> +		return -EINVAL;
> +
> +	dev = &wdt->cldev->dev;
> +
> +	mwd = kzalloc(sizeof(struct mei_wdt_dev), GFP_KERNEL);
> +	if (!mwd)
> +		return -ENOMEM;
> +
> +	mwd->wdt = wdt;
> +	mwd->wdd.info = &wd_info;
> +	mwd->wdd.ops = &wd_ops;
> +	mwd->wdd.parent = dev;
> +	mwd->wdd.timeout = MEI_WDT_DEFAULT_TIMEOUT;
> +	mwd->wdd.min_timeout = MEI_WDT_MIN_TIMEOUT;
> +	mwd->wdd.max_timeout = MEI_WDT_MAX_TIMEOUT;
> +	kref_init(&mwd->refcnt);
> +
> +	ret = watchdog_register_device(&mwd->wdd);
> +	if (ret) {
> +		dev_err(dev, "unable to register watchdog device = %d.\n", ret);
> +		kref_put(&mwd->refcnt, mei_wdt_release);
> +		return ret;
> +	}
> +
> +	wdt->mwd = mwd;
> +	watchdog_set_drvdata(&mwd->wdd, mwd);
> +	return 0;
> +}
> +
> +static void mei_wdt_unregister(struct mei_wdt *wdt)
> +{
> +	if (!wdt->mwd)
> +		return;
> +
> +	watchdog_unregister_device(&wdt->mwd->wdd);
> +	kref_put(&wdt->mwd->refcnt, mei_wdt_release);
> +	wdt->mwd = NULL;
> +}

Seems to me that using two separate data structures instead of one
adds a lot of complexity.

> +
> +static int mei_wdt_probe(struct mei_cl_device *cldev,
> +			 const struct mei_cl_device_id *id)
> +{
> +	struct mei_wdt *wdt;
> +	int ret;
> +
> +	wdt = kzalloc(sizeof(struct mei_wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	wdt->timeout = MEI_WDT_DEFAULT_TIMEOUT;
> +	wdt->state = MEI_WDT_IDLE;
> +	wdt->cldev = cldev;
> +	mei_cldev_set_drvdata(cldev, wdt);
> +
> +	ret = mei_cldev_enable(cldev);
> +	if (ret < 0) {
> +		dev_err(&cldev->dev, "Could not enable cl device\n");
> +		goto err_out;
> +	}
> +
> +	wd_info.firmware_version = mei_cldev_ver(cldev);
> +
> +	ret = mei_wdt_register(wdt);
> +	if (ret)
> +		goto err_disable;
> +
> +	return 0;
> +
> +err_disable:
> +	mei_cldev_disable(cldev);
> +
> +err_out:
> +	kfree(wdt);
> +
> +	return ret;
> +}
> +
> +static int mei_wdt_remove(struct mei_cl_device *cldev)
> +{
> +	struct mei_wdt *wdt = mei_cldev_get_drvdata(cldev);
> +
> +	mei_cldev_disable(cldev);
> +
> +	mei_wdt_unregister(wdt);
> +
> +	kfree(wdt);
> +
> +	return 0;
> +}
> +
> +#define MEI_UUID_WD UUID_LE(0x05B79A6F, 0x4628, 0x4D7F, \
> +			    0x89, 0x9D, 0xA9, 0x15, 0x14, 0xCB, 0x32, 0xAB)
> +
> +static struct mei_cl_device_id mei_wdt_tbl[] = {
> +	{ .uuid = MEI_UUID_WD, .version = MEI_CL_VERSION_ANY},
> +	/* required last entry */
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(mei, mei_wdt_tbl);
> +
> +static struct mei_cl_driver mei_wdt_driver = {
> +	.id_table = mei_wdt_tbl,
> +	.name = KBUILD_MODNAME,
> +
> +	.probe = mei_wdt_probe,
> +	.remove = mei_wdt_remove,
> +};
> +
> +static int __init mei_wdt_init(void)
> +{
> +	int ret;
> +
> +	ret = mei_cldev_driver_register(&mei_wdt_driver);
> +	if (ret) {
> +		pr_err(KBUILD_MODNAME ": module registration failed\n");
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static void __exit mei_wdt_exit(void)
> +{
> +	mei_cldev_driver_unregister(&mei_wdt_driver);
> +}
> +
> +module_init(mei_wdt_init);
> +module_exit(mei_wdt_exit);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Device driver for Intel MEI iAMT watchdog");
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280810 — Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver

T24gTW9uLCAyMDE1LTExLTMwIGF0IDA4OjU1IC0wODAwLCBHdWVudGVyIFJvZWNrIHdyb3RlOg0K
PiBPbiAxMS8yNi8yMDE1IDA0OjMxIEFNLCBUb21hcyBXaW5rbGVyIHdyb3RlOg0KPiA+IEZyb206
IEFsZXhhbmRlciBVc3lza2luIDxhbGV4YW5kZXIudXN5c2tpbkBpbnRlbC5jb20+DQo+ID4gDQo+
ID4gQ3JlYXRlIGEgZHJpdmVyIHdpdGggdGhlIGdlbmVyaWMgd2F0Y2hkb2cgaW50ZXJmYWNlDQo+
ID4gZm9yIHRoZSBNRUkgaUFNVCB3YXRjaGRvZyBkZXZpY2UuDQo+ID4gDQo+ID4gU2lnbmVkLW9m
Zi1ieTogQWxleGFuZGVyIFVzeXNraW4gPGFsZXhhbmRlci51c3lza2luQGludGVsLmNvbT4NCj4g
PiBTaWduZWQtb2ZmLWJ5OiBUb21hcyBXaW5rbGVyIDx0b21hcy53aW5rbGVyQGludGVsLmNvbT4N
Cj4gPiAtLS0NCj4gPiAgIERvY3VtZW50YXRpb24vbWlzYy1kZXZpY2VzL21laS9tZWkudHh0IHwg
IDEyICstDQo+ID4gICBNQUlOVEFJTkVSUyAgICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAg
MSArDQo+ID4gICBkcml2ZXJzL3dhdGNoZG9nL0tjb25maWcgICAgICAgICAgICAgICB8ICAxNSAr
Kw0KPiA+ICAgZHJpdmVycy93YXRjaGRvZy9NYWtlZmlsZSAgICAgICAgICAgICAgfCAgIDEgKw0K
PiA+ICAgZHJpdmVycy93YXRjaGRvZy9tZWlfd2R0LmMgICAgICAgICAgICAgfCA0MzINCj4gPiAr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysNCj4gPiAgIDUgZmlsZXMgY2hhbmdlZCwg
NDU1IGluc2VydGlvbnMoKyksIDYgZGVsZXRpb25zKC0pDQo+ID4gICBjcmVhdGUgbW9kZSAxMDA2
NDQgZHJpdmVycy93YXRjaGRvZy9tZWlfd2R0LmMNCj4gPiANCj4gPiBkaWZmIC0tZ2l0IGEvRG9j
dW1lbnRhdGlvbi9taXNjLWRldmljZXMvbWVpL21laS50eHQNCj4gPiBiL0RvY3VtZW50YXRpb24v
bWlzYy1kZXZpY2VzL21laS9tZWkudHh0DQo+ID4gaW5kZXggOTFjMWZhMzRmNDhiLi4yYjgwYTBj
ZDYyMWYgMTAwNjQ0DQo+ID4gLS0tIGEvRG9jdW1lbnRhdGlvbi9taXNjLWRldmljZXMvbWVpL21l
aS50eHQNCj4gPiArKysgYi9Eb2N1bWVudGF0aW9uL21pc2MtZGV2aWNlcy9tZWkvbWVpLnR4dA0K
PiA+IEBAIC0yMzEsMTUgKzIzMSwxNSBAQCBJVCBrbm93cyB3aGVuIGEgcGxhdGZvcm0gY3Jhc2hl
cyBldmVuIHdoZW4NCj4gPiB0aGVyZSBpcyBhIGhhcmQgZmFpbHVyZSBvbiB0aGUgaG9zdC4NCj4g
PiAgIFRoZSBJbnRlbCBBTVQgV2F0Y2hkb2cgaXMgY29tcG9zZWQgb2YgdHdvIHBhcnRzOg0KPiA+
ICAgCTEpIEZpcm13YXJlIGZlYXR1cmUgLSByZWNlaXZlcyB0aGUgaGVhcnRiZWF0cw0KPiA+ICAg
CSAgIGFuZCBzZW5kcyBhbiBldmVudCB3aGVuIHRoZSBoZWFydGJlYXRzIHN0b3AuDQo+ID4gLQky
KSBJbnRlbCBNRUkgZHJpdmVyIC0gY29ubmVjdHMgdG8gdGhlIHdhdGNoZG9nIGZlYXR1cmUsDQo+
ID4gY29uZmlndXJlcyB0aGUNCj4gPiAtCSAgIHdhdGNoZG9nIGFuZCBzZW5kcyB0aGUgaGVhcnRi
ZWF0cy4NCj4gPiArCTIpIEludGVsIE1FSSBpQU1UIHdhdGNoZG9nIGRyaXZlciAtIGNvbm5lY3Rz
IHRvIHRoZQ0KPiA+IHdhdGNoZG9nIGZlYXR1cmUsDQo+ID4gKwkgICBjb25maWd1cmVzIHRoZSB3
YXRjaGRvZyBhbmQgc2VuZHMgdGhlIGhlYXJ0YmVhdHMuDQo+ID4gDQo+ID4gLVRoZSBJbnRlbCBN
RUkgZHJpdmVyIHVzZXMgdGhlIGtlcm5lbCB3YXRjaGRvZyBBUEkgdG8gY29uZmlndXJlIHRoZQ0K
PiA+IEludGVsIEFNVA0KPiA+IC1XYXRjaGRvZyBhbmQgdG8gc2VuZCBoZWFydGJlYXRzIHRvIGl0
LiBUaGUgZGVmYXVsdCB0aW1lb3V0IG9mIHRoZQ0KPiA+ICtUaGUgSW50ZWwgaUFNVCB3YXRjaGRv
ZyBNRUkgZHJpdmVyIHVzZXMgdGhlIGtlcm5lbCB3YXRjaGRvZyBBUEkgdG8NCj4gPiBjb25maWd1
cmUNCj4gPiArdGhlIEludGVsIEFNVCBXYXRjaGRvZyBhbmQgdG8gc2VuZCBoZWFydGJlYXRzIHRv
IGl0LiBUaGUgZGVmYXVsdA0KPiA+IHRpbWVvdXQgb2YgdGhlDQo+ID4gICB3YXRjaGRvZyBpcyAx
MjAgc2Vjb25kcy4NCj4gPiANCj4gPiAtSWYgdGhlIEludGVsIEFNVCBXYXRjaGRvZyBmZWF0dXJl
IGRvZXMgbm90IGV4aXN0IChpLmUuIHRoZQ0KPiA+IGNvbm5lY3Rpb24gZmFpbGVkKSwNCj4gPiAt
dGhlIEludGVsIE1FSSBkcml2ZXIgd2lsbCBkaXNhYmxlIHRoZSBzZW5kaW5nIG9mIGhlYXJ0YmVh
dHMuDQo+ID4gK0lmIHRoZSBJbnRlbCBBTVQgaXMgbm90IGVuYWJsZWQgaW4gdGhlIGZpcm13YXJl
IHRoZW4gdGhlIHdhdGNoZG9nDQo+ID4gY2xpZW50IHdvbid0IGVudW1lcmF0ZQ0KPiA+ICtvbiB0
aGUgbWUgY2xpZW50IGJ1cyBhbmQgd2F0Y2hkb2cgZGV2aWNlcyB3b24ndCBiZSBleHBvc2VkLg0K
PiA+IA0KPiA+IA0KPiA+ICAgU3VwcG9ydGVkIENoaXBzZXRzDQo+ID4gZGlmZiAtLWdpdCBhL01B
SU5UQUlORVJTIGIvTUFJTlRBSU5FUlMNCj4gPiBpbmRleCAwNTBkMGU3N2EyY2YuLmNmMGE1MTUx
OGY0YSAxMDA2NDQNCj4gPiAtLS0gYS9NQUlOVEFJTkVSUw0KPiA+ICsrKyBiL01BSU5UQUlORVJT
DQo+ID4gQEAgLTU2NjQsNiArNTY2NCw3IEBAIFM6CVN1cHBvcnRlZA0KPiA+ICAgRjoJaW5jbHVk
ZS91YXBpL2xpbnV4L21laS5oDQo+ID4gICBGOglpbmNsdWRlL2xpbnV4L21laV9jbF9idXMuaA0K
PiA+ICAgRjoJZHJpdmVycy9taXNjL21laS8qDQo+ID4gK0Y6CWRyaXZlcnMvd2F0Y2hkb2cvbWVp
X3dkdC5jDQo+ID4gICBGOglEb2N1bWVudGF0aW9uL21pc2MtZGV2aWNlcy9tZWkvKg0KPiA+IA0K
PiA+ICAgSU5URUwgTUlDIERSSVZFUlMgKG1pYykNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy93
YXRjaGRvZy9LY29uZmlnIGIvZHJpdmVycy93YXRjaGRvZy9LY29uZmlnDQo+ID4gaW5kZXggN2E4
YTZjNjk1MmU5Li5lYzU4NDcxNDgyOWQgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy93YXRjaGRv
Zy9LY29uZmlnDQo+ID4gKysrIGIvZHJpdmVycy93YXRjaGRvZy9LY29uZmlnDQo+ID4gQEAgLTEx
NTQsNiArMTE1NCwyMSBAQCBjb25maWcgU0JDX0VQWF9DM19XQVRDSERPRw0KPiA+ICAgCSAgVG8g
Y29tcGlsZSB0aGlzIGRyaXZlciBhcyBhIG1vZHVsZSwgY2hvb3NlIE0gaGVyZTogdGhlDQo+ID4g
ICAJICBtb2R1bGUgd2lsbCBiZSBjYWxsZWQgc2JjX2VweF9jMy4NCj4gPiANCj4gPiArY29uZmln
IElOVEVMX01FSV9XRFQNCj4gPiArCXRyaXN0YXRlICJJbnRlbCBNRUkgaUFNVCBXYXRjaGRvZyIN
Cj4gPiArCWRlcGVuZHMgb24gSU5URUxfTUVJICYmIFg4Ng0KPiA+ICsJc2VsZWN0IFdBVENIRE9H
X0NPUkUNCj4gPiArCS0tLWhlbHAtLS0NCj4gPiArCSAgQSBkZXZpY2UgZHJpdmVyIGZvciB0aGUg
SW50ZWwgTUVJIGlBTVQgd2F0Y2hkb2cuDQo+ID4gKw0KPiA+ICsJICBUaGUgSW50ZWwgQU1UIFdh
dGNoZG9nIGlzIGFuIE9TIEhlYWx0aCAoSGFuZy9DcmFzaCkNCj4gPiB3YXRjaGRvZy4NCj4gPiAr
CSAgV2hlbmV2ZXIgdGhlIE9TIGhhbmdzIG9yIGNyYXNoZXMsIGlBTVQgd2lsbCBzZW5kIGFuDQo+
ID4gZXZlbnQNCj4gPiArCSAgdG8gYW55IHN1YnNjcmliZXIgdG8gdGhpcyBldmVudC4gVGhlIHdh
dGNoZG9nIGRvZXNuJ3QNCj4gPiByZXNldCB0aGUNCj4gPiArCSAgdGhlIHBsYXRmb3JtLg0KPiA+
ICsNCj4gPiArCSAgVG8gY29tcGlsZSB0aGlzIGRyaXZlciBhcyBhIG1vZHVsZSwgY2hvb3NlIE0g
aGVyZToNCj4gPiArCSAgdGhlIG1vZHVsZSB3aWxsIGJlIGNhbGxlZCBtZWlfd2R0Lg0KPiA+ICsN
Cj4gPiAgICMgTTMyUiBBcmNoaXRlY3R1cmUNCj4gPiANCj4gPiAgICMgTTY4SyBBcmNoaXRlY3R1
cmUNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy93YXRjaGRvZy9NYWtlZmlsZSBiL2RyaXZlcnMv
d2F0Y2hkb2cvTWFrZWZpbGUNCj4gPiBpbmRleCA1M2Q0ODI3ZGRmZTEuLjkwNjljOWRkOGFhOCAx
MDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL3dhdGNoZG9nL01ha2VmaWxlDQo+ID4gKysrIGIvZHJp
dmVycy93YXRjaGRvZy9NYWtlZmlsZQ0KPiA+IEBAIC0xMjMsNiArMTIzLDcgQEAgb2JqLSQoQ09O
RklHX01BQ0haX1dEVCkgKz0gbWFjaHp3ZC5vDQo+ID4gICBvYmotJChDT05GSUdfU0JDX0VQWF9D
M19XQVRDSERPRykgKz0gc2JjX2VweF9jMy5vDQo+ID4gICBvYmotJChDT05GSUdfSU5URUxfU0NV
X1dBVENIRE9HKSArPSBpbnRlbF9zY3Vfd2F0Y2hkb2cubw0KPiA+ICAgb2JqLSQoQ09ORklHX0lO
VEVMX01JRF9XQVRDSERPRykgKz0gaW50ZWwtbWlkX3dkdC5vDQo+ID4gK29iai0kKENPTkZJR19J
TlRFTF9NRUlfV0RUKSArPSBtZWlfd2R0Lm8NCj4gPiANCj4gPiAgICMgTTMyUiBBcmNoaXRlY3R1
cmUNCj4gPiANCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy93YXRjaGRvZy9tZWlfd2R0LmMNCj4g
PiBiL2RyaXZlcnMvd2F0Y2hkb2cvbWVpX3dkdC5jDQo+ID4gbmV3IGZpbGUgbW9kZSAxMDA2NDQN
Cj4gPiBpbmRleCAwMDAwMDAwMDAwMDAuLjE0OWIyOWYzNDFjZg0KPiA+IC0tLSAvZGV2L251bGwN
Cj4gPiArKysgYi9kcml2ZXJzL3dhdGNoZG9nL21laV93ZHQuYw0KPiA+IEBAIC0wLDAgKzEsNDMy
IEBADQo+ID4gKy8qDQo+ID4gKyAqIEludGVsIE1hbmFnZW1lbnQgRW5naW5lIEludGVyZmFjZSAo
SW50ZWwgTUVJKSBMaW51eCBkcml2ZXINCj4gPiArICogQ29weXJpZ2h0IChjKSAyMDE1LCBJbnRl
bCBDb3Jwb3JhdGlvbi4NCj4gPiArICoNCj4gPiArICogVGhpcyBwcm9ncmFtIGlzIGZyZWUgc29m
dHdhcmU7IHlvdSBjYW4gcmVkaXN0cmlidXRlIGl0IGFuZC9vcg0KPiA+IG1vZGlmeSBpdA0KPiA+
ICsgKiB1bmRlciB0aGUgdGVybXMgYW5kIGNvbmRpdGlvbnMgb2YgdGhlIEdOVSBHZW5lcmFsIFB1
YmxpYw0KPiA+IExpY2Vuc2UsDQo+ID4gKyAqIHZlcnNpb24gMiwgYXMgcHVibGlzaGVkIGJ5IHRo
ZSBGcmVlIFNvZnR3YXJlIEZvdW5kYXRpb24uDQo+ID4gKyAqDQo+ID4gKyAqIFRoaXMgcHJvZ3Jh
bSBpcyBkaXN0cmlidXRlZCBpbiB0aGUgaG9wZSBpdCB3aWxsIGJlIHVzZWZ1bCwgYnV0DQo+ID4g
V0lUSE9VVA0KPiA+ICsgKiBBTlkgV0FSUkFOVFk7IHdpdGhvdXQgZXZlbiB0aGUgaW1wbGllZCB3
YXJyYW50eSBvZg0KPiA+IE1FUkNIQU5UQUJJTElUWSBvcg0KPiA+ICsgKiBGSVRORVNTIEZPUiBB
IFBBUlRJQ1VMQVIgUFVSUE9TRS4gU2VlIHRoZSBHTlUgR2VuZXJhbCBQdWJsaWMNCj4gPiBMaWNl
bnNlIGZvcg0KPiA+ICsgKiBtb3JlIGRldGFpbHMuDQo+ID4gKyAqLw0KPiA+ICsNCj4gPiArI2lu
Y2x1ZGUgPGxpbnV4L21vZHVsZS5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvc2xhYi5oPg0KPiA+
ICsjaW5jbHVkZSA8bGludXgvaW50ZXJydXB0Lmg+DQo+ID4gKyNpbmNsdWRlIDxsaW51eC93YXRj
aGRvZy5oPg0KPiA+ICsNCj4gPiArI2luY2x1ZGUgPGxpbnV4L3V1aWQuaD4NCj4gPiArI2luY2x1
ZGUgPGxpbnV4L21laV9jbF9idXMuaD4NCj4gPiArDQo+ID4gKy8qDQo+ID4gKyAqIGlBTVQgV2F0
Y2hkb2cgRGV2aWNlDQo+ID4gKyAqLw0KPiA+ICsjZGVmaW5lIElOVEVMX0FNVF9XQVRDSERPR19J
RCAiaWFtdF93ZHQiDQo+ID4gKw0KPiA+ICsjZGVmaW5lIE1FSV9XRFRfREVGQVVMVF9USU1FT1VU
ICAgMTIwICAvKiBzZWNvbmRzICovDQo+ID4gKyNkZWZpbmUgTUVJX1dEVF9NSU5fVElNRU9VVCAg
ICAgICAxMjAgIC8qIHNlY29uZHMgKi8NCj4gDQo+IElzIHRoZSBsYXJnZSBtaW5pbXVtIHRpbWVv
dXQgb24gcHVycG9zZSA/DQo+IEp1c3QgYXNraW5nLCBzaW5jZSBpdCBpcyBxdWl0ZSB1bnVzdWFs
Lg0KDQpUaGUgcmVjb21tZW5kYXRpb24gaXMgdG8gcGluZyBlYWNoIDkwIHNlYyBhbmQgc2V0IHRo
ZSAgdmFsdWUgdG8gMTIwLiANCkkgdGhpbmsgdGhlcmUgaXMgbm8gSFcgbGltaXRhdGlvbiwgYnV0
IHNpbmNlIHRoaXMgaXMgb25seSBhIG1vbml0b3JpbmcNCndhdGNoZG9nIHRoaXMgdmFsdWUgaXMg
c3VmZmljaWVudCBmb3IgdGhlIHVzZSBjYXNlLg0KDQo+IA0KPiA+ICsjZGVmaW5lIE1FSV9XRFRf
TUFYX1RJTUVPVVQgICAgIDY1NTM1ICAvKiBzZWNvbmRzICovDQo+ID4gKw0KPiA+ICsvKiBDb21t
YW5kcyAqLw0KPiA+ICsjZGVmaW5lIE1FSV9NQU5BR0VNRU5UX0NPTlRST0wgMHgwMg0KPiA+ICsN
Cj4gPiArLyogTUVJIE1hbmFnZW1lbnQgQ29udHJvbCB2ZXJzaW9uIG51bWJlciAqLw0KPiA+ICsj
ZGVmaW5lIE1FSV9NQ19WRVJTSU9OX05VTUJFUiAgMHgxMA0KPiA+ICsNCj4gPiArLyogU3ViIENv
bW1hbmRzICovDQo+ID4gKyNkZWZpbmUgTUVJX01DX1NUQVJUX1dEX1RJTUVSX1JFUSAgMHgxMw0K
PiA+ICsjZGVmaW5lIE1FSV9NQ19TVE9QX1dEX1RJTUVSX1JFUSAgIDB4MTQNCj4gPiArDQo+ID4g
Ky8qKg0KPiA+ICsgKiBlbnVtIG1laV93ZHRfc3RhdGUgLSBpbnRlcm5hbCB3YXRjaGRvZyBzdGF0
ZQ0KPiA+ICsgKg0KPiA+ICsgKiBATUVJX1dEVF9JRExFOiB3ZCBpcyBpZGxlIGFuZCBub3Qgb3Bl
bmVkDQo+ID4gKyAqIEBNRUlfV0RUX1NUQVJUOiB3ZCB3YXMgb3BlbmVkLCBzdGFydCB3YXMgY2Fs
bGVkDQo+ID4gKyAqIEBNRUlfV0RUX1JVTk5JTkc6IHdkIGlzIGV4cGVjdGluZyBrZWVwIGFsaXZl
IHBpbmdzDQo+ID4gKyAqIEBNRUlfV0RUX1NUT1BQSU5HOiB3ZCBpcyBzdG9wcGluZyBhbmQgd2ls
bCBtb3ZlIHRvIElETEUNCj4gPiArICovDQo+ID4gK2VudW0gbWVpX3dkdF9zdGF0ZSB7DQo+ID4g
KwlNRUlfV0RUX0lETEUsDQo+ID4gKwlNRUlfV0RUX1NUQVJULA0KPiA+ICsJTUVJX1dEVF9SVU5O
SU5HLA0KPiA+ICsJTUVJX1dEVF9TVE9QUElORywNCj4gPiArfTsNCj4gPiArDQo+ID4gK3N0cnVj
dCBtZWlfd2R0Ow0KPiA+ICsNCj4gPiArLyoqDQo+ID4gKyAqIHN0cnVjdCBtZWlfd2R0X2RldiAt
IHdhdGNoZG9nIGRldmljZSB3cmFwcGVyDQo+ID4gKyAqDQo+ID4gKyAqIEB3ZGQ6IHdhdGNoZG9n
IGRldmljZQ0KPiA+ICsgKiBAd2R0OiBiYWNrIHBvaW50ZXIgdG8gbWVpX3dkdCBkcml2ZXINCj4g
PiArICogQHJlZmNudDogcmVmZXJlbmNlIGNvdW50ZXINCj4gPiArICovDQo+ID4gK3N0cnVjdCBt
ZWlfd2R0X2RldiB7DQo+ID4gKwlzdHJ1Y3Qgd2F0Y2hkb2dfZGV2aWNlIHdkZDsNCj4gPiArCXN0
cnVjdCBtZWlfd2R0ICp3ZHQ7DQo+ID4gKwlzdHJ1Y3Qga3JlZiByZWZjbnQ7DQo+ID4gK307DQo+
ID4gKw0KPiA+ICsvKioNCj4gPiArICogc3RydWN0IG1laV93ZHQgLSBtZWkgd2F0Y2hkb2cgZHJp
dmVyDQo+ID4gKyAqDQo+ID4gKyAqIEBjbGRldjogbWVpIHdhdGNoZG9nIGNsaWVudCBkZXZpY2UN
Cj4gPiArICogQG13ZDogd2F0Y2hkb2cgZGV2aWNlIHdyYXBwZXINCj4gPiArICogQHN0YXRlOiB3
YXRjaGRvZyBpbnRlcm5hbCBzdGF0ZQ0KPiA+ICsgKiBAdGltZW91dDogd2F0Y2hkb2cgY3VycmVu
dCB0aW1lb3V0DQo+ID4gKyAqLw0KPiA+ICtzdHJ1Y3QgbWVpX3dkdCB7DQo+ID4gKwlzdHJ1Y3Qg
bWVpX2NsX2RldmljZSAqY2xkZXY7DQo+ID4gKwlzdHJ1Y3QgbWVpX3dkdF9kZXYgKm13ZDsNCj4g
PiArCWVudW0gbWVpX3dkdF9zdGF0ZSBzdGF0ZTsNCj4gPiArCXUxNiB0aW1lb3V0Ow0KPiA+ICt9
Ow0KPiANCj4gQW55IHNwZWNpYWwgcmVhc29uIGZvciBoYXZpbmcgdHdvIGRhdGEgc3RydWN0dXJl
cyBpbnN0ZWFkIG9mIG9uZSA/DQo+IFlvdSBjb3VsZCBqdXN0IG1vdmUgdGhlIHZhcmlhYmxlcyBm
cm9tIHN0cnVjdCBtZWlfd2R0X2RldiBpbnRvDQo+IHN0cnVjdCBtZWlfd2R0LCBubyA/DQoNClll
cywgb24gbmV3ZXIgcGxhdGZvcm0gbWVpX3dkdF9kZXYgbWlnaHQgYmUgbm90IHByZXNlbnQgaW4g
Y2FzZSB0aGUgdGhlDQpkZXZpY2UgaXMgbm90IHByb3Zpc2lvbmVkLiBUaGlzIGNhbWUgdG8gYWN0
aW9uIGluIHRoZSBmb2xsb3dpbmcNCnBhdGNoZXMuDQoNCj4gPiArDQo+ID4gK3N0cnVjdCBtZWlf
d2R0X2hkciB7DQo+ID4gKwl1OCBjb21tYW5kOw0KPiA+ICsJdTggYnl0ZWNvdW50Ow0KPiA+ICsJ
dTggc3ViY29tbWFuZDsNCj4gPiArCXU4IHZlcnNpb25udW1iZXI7DQo+ID4gK307DQo+ID4gKw0K
PiA+ICtzdHJ1Y3QgbWVpX3dkdF9zdGFydF9yZXF1ZXN0IHsNCj4gPiArCXN0cnVjdCBtZWlfd2R0
X2hkciBoZHI7DQo+ID4gKwl1MTYgdGltZW91dDsNCj4gPiArCXU4IHJlc2VydmVkWzE3XTsNCj4g
PiArfSBfX3BhY2tlZDsNCj4gPiArDQo+ID4gK3N0cnVjdCBtZWlfd2R0X3N0b3BfcmVxdWVzdCB7
DQo+ID4gKwlzdHJ1Y3QgbWVpX3dkdF9oZHIgaGRyOw0KPiA+ICt9IF9fcGFja2VkOw0KPiA+ICsN
Cj4gPiArLyoqDQo+ID4gKyAqIG1laV93ZHRfcGluZyAtIHNlbmQgd2Qgc3RhcnQgY29tbWFuZA0K
PiA+ICsgKg0KPiA+ICsgKiBAd2R0OiBtZWkgd2F0Y2hkb2cgZGV2aWNlDQo+ID4gKyAqDQo+ID4g
KyAqIFJldHVybjogbnVtYmVyIG9mIGJ5dGVzIHNlbnQgb24gc3VjY2VzcywNCj4gPiArICogICAg
ICAgICBuZWdhdGl2ZSBlcnJubyBjb2RlIG9uIGZhaWx1cmUNCj4gPiArICovDQo+ID4gK3N0YXRp
YyBpbnQgbWVpX3dkdF9waW5nKHN0cnVjdCBtZWlfd2R0ICp3ZHQpDQo+ID4gK3sNCj4gPiArCXN0
cnVjdCBtZWlfd2R0X3N0YXJ0X3JlcXVlc3QgcmVxOw0KPiA+ICsJY29uc3Qgc2l6ZV90IHJlcV9s
ZW4gPSBzaXplb2YocmVxKTsNCj4gPiArDQo+ID4gKwltZW1zZXQoJnJlcSwgMCwgcmVxX2xlbik7
DQo+ID4gKwlyZXEuaGRyLmNvbW1hbmQgPSBNRUlfTUFOQUdFTUVOVF9DT05UUk9MOw0KPiA+ICsJ
cmVxLmhkci5ieXRlY291bnQgPSByZXFfbGVuIC0gb2Zmc2V0b2Yoc3RydWN0IG1laV93ZHRfaGRy
LA0KPiA+IHN1YmNvbW1hbmQpOw0KPiA+ICsJcmVxLmhkci5zdWJjb21tYW5kID0gTUVJX01DX1NU
QVJUX1dEX1RJTUVSX1JFUTsNCj4gPiArCXJlcS5oZHIudmVyc2lvbm51bWJlciA9IE1FSV9NQ19W
RVJTSU9OX05VTUJFUjsNCj4gPiArCXJlcS50aW1lb3V0ID0gd2R0LT50aW1lb3V0Ow0KPiA+ICsN
Cj4gPiArCXJldHVybiBtZWlfY2xkZXZfc2VuZCh3ZHQtPmNsZGV2LCAodTggKikmcmVxLCByZXFf
bGVuKTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArLyoqDQo+ID4gKyAqIG1laV93ZHRfc3RvcCAtIHNl
bmQgd2Qgc3RvcCBjb21tYW5kDQo+ID4gKyAqDQo+ID4gKyAqIEB3ZHQ6IG1laSB3YXRjaGRvZyBk
ZXZpY2UNCj4gPiArICoNCj4gPiArICogUmV0dXJuOiBudW1iZXIgb2YgYnl0ZXMgc2VudCBvbiBz
dWNjZXNzLA0KPiA+ICsgKiAgICAgICAgIG5lZ2F0aXZlIGVycm5vIGNvZGUgb24gZmFpbHVyZQ0K
PiA+ICsgKi8NCj4gPiArc3RhdGljIGludCBtZWlfd2R0X3N0b3Aoc3RydWN0IG1laV93ZHQgKndk
dCkNCj4gPiArew0KPiA+ICsJc3RydWN0IG1laV93ZHRfc3RvcF9yZXF1ZXN0IHJlcTsNCj4gPiAr
CWNvbnN0IHNpemVfdCByZXFfbGVuID0gc2l6ZW9mKHJlcSk7DQo+ID4gKw0KPiA+ICsJbWVtc2V0
KCZyZXEsIDAsIHJlcV9sZW4pOw0KPiA+ICsJcmVxLmhkci5jb21tYW5kID0gTUVJX01BTkFHRU1F
TlRfQ09OVFJPTDsNCj4gPiArCXJlcS5oZHIuYnl0ZWNvdW50ID0gcmVxX2xlbiAtIG9mZnNldG9m
KHN0cnVjdCBtZWlfd2R0X2hkciwNCj4gPiBzdWJjb21tYW5kKTsNCj4gPiArCXJlcS5oZHIuc3Vi
Y29tbWFuZCA9IE1FSV9NQ19TVE9QX1dEX1RJTUVSX1JFUTsNCj4gPiArCXJlcS5oZHIudmVyc2lv
bm51bWJlciA9IE1FSV9NQ19WRVJTSU9OX05VTUJFUjsNCj4gPiArDQo+ID4gKwlyZXR1cm4gbWVp
X2NsZGV2X3NlbmQod2R0LT5jbGRldiwgKHU4ICopJnJlcSwgcmVxX2xlbik7DQo+ID4gK30NCj4g
PiArDQo+ID4gKy8qKg0KPiA+ICsgKiBtZWlfd2R0X29wc19zdGFydCAtIHdkIHN0YXJ0IGNvbW1h
bmQgZnJvbSB0aGUgd2F0Y2hkb2cgY29yZS4NCj4gPiArICoNCj4gPiArICogQHdkZDogd2F0Y2hk
b2cgZGV2aWNlDQo+ID4gKyAqDQo+ID4gKyAqIFJldHVybjogMCBvbiBzdWNjZXNzIG9yIC1FTk9E
RVY7DQo+ID4gKyAqLw0KPiA+ICtzdGF0aWMgaW50IG1laV93ZHRfb3BzX3N0YXJ0KHN0cnVjdCB3
YXRjaGRvZ19kZXZpY2UgKndkZCkNCj4gPiArew0KPiA+ICsJc3RydWN0IG1laV93ZHRfZGV2ICpt
d2QgPSB3YXRjaGRvZ19nZXRfZHJ2ZGF0YSh3ZGQpOw0KPiA+ICsNCj4gPiArCWlmICghbXdkKQ0K
PiA+ICsJCXJldHVybiAtRU5PREVWOw0KPiANCj4gVGhpcyBjYW4gb25seSBoYXBwZW4gYmVjYXVz
ZSB5b3UgY2FsbCB3YXRjaGRvZ19zZXRfZHJ2ZGF0YSgpIGFmdGVyDQo+IHdhdGNoZG9nIGRldmlj
ZSByZWdpc3RyYXRpb24uIElmIHRoYXQgaGFwcGVucywgdGhlIHN5c3RlbSBpcyBpbg0KPiByZWFs
bHkgYmFkIHNoYXBlLg0KDQptZWlfd2R0X2RldiBjYW4gZGVzdHJveWVkIGR1cmluZyANCmRyaXZl
ciBvcGVyYXRpb24gaWYgdGhlIGRldmljZSBpcyB1bnByb3Zpc2lvbmVkLCBidXQgc3RpbGwgeW91
IHRoZQ0KY29uZGl0aW9uIHNob3VsZCBub3QgaGFwcGVuIHVubGVzcyB3ZSBoYXZlIGEgYnVnLiBX
ZSBjYW4gcHV0IFdBUk5fT04oKQ0KdGhlcmUuIA0KDQo+IA0KPiBJIHdvdWxkIHN1Z2dlc3QgdG8g
bW92ZSB0aGUgY2FsbCB0byB3YXRjaGRvZ19zZXRfZHJ2ZGF0YSgpIGFoZWFkDQo+IG9mIHdhdGNo
ZG9nX3JlZ2lzdGVyX2RldmljZSgpIGFuZCBkcm9wIHRob3NlIGNoZWNrcy4NCj4gDQo+ID4gKw0K
PiA+ICsJbXdkLT53ZHQtPnN0YXRlID0gTUVJX1dEVF9TVEFSVDsNCj4gPiArCXdkZC0+dGltZW91
dCA9IG13ZC0+d2R0LT50aW1lb3V0Ow0KPiA+ICsJcmV0dXJuIDA7DQo+ID4gK30NCj4gPiArDQo+
ID4gKy8qKg0KPiA+ICsgKiBtZWlfd2R0X29wc19zdG9wIC0gd2Qgc3RvcCBjb21tYW5kIGZyb20g
dGhlIHdhdGNoZG9nIGNvcmUuDQo+ID4gKyAqDQo+ID4gKyAqIEB3ZGQ6IHdhdGNoZG9nIGRldmlj
ZQ0KPiA+ICsgKg0KPiA+ICsgKiBSZXR1cm46IDAgaWYgc3VjY2VzcywgbmVnYXRpdmUgZXJybm8g
Y29kZSBmb3IgZmFpbHVyZQ0KPiA+ICsgKi8NCj4gPiArc3RhdGljIGludCBtZWlfd2R0X29wc19z
dG9wKHN0cnVjdCB3YXRjaGRvZ19kZXZpY2UgKndkZCkNCj4gPiArew0KPiA+ICsJc3RydWN0IG1l
aV93ZHRfZGV2ICptd2QgPSB3YXRjaGRvZ19nZXRfZHJ2ZGF0YSh3ZGQpOw0KPiA+ICsJc3RydWN0
IG1laV93ZHQgKndkdDsNCj4gPiArCWludCByZXQ7DQo+ID4gKw0KPiA+ICsJaWYgKCFtd2QpDQo+
ID4gKwkJcmV0dXJuIC1FTk9ERVY7DQo+ID4gKw0KPiA+ICsJd2R0ID0gbXdkLT53ZHQ7DQo+ID4g
Kw0KPiA+ICsJaWYgKHdkdC0+c3RhdGUgIT0gTUVJX1dEVF9SVU5OSU5HKQ0KPiA+ICsJCXJldHVy
biAwOw0KPiA+ICsNCj4gPiArCXdkdC0+c3RhdGUgPSBNRUlfV0RUX1NUT1BQSU5HOw0KPiA+ICsN
Cj4gPiArCXJldCA9IG1laV93ZHRfc3RvcCh3ZHQpOw0KPiA+ICsJaWYgKHJldCA8IDApDQo+ID4g
KwkJcmV0dXJuIHJldDsNCj4gPiArDQo+ID4gKwl3ZHQtPnN0YXRlID0gTUVJX1dEVF9JRExFOw0K
PiA+ICsNCj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICsvKioNCj4gPiArICog
bWVpX3dkdF9vcHNfcGluZyAtIHdkIHBpbmcgY29tbWFuZCBmcm9tIHRoZSB3YXRjaGRvZyBjb3Jl
Lg0KPiA+ICsgKg0KPiA+ICsgKiBAd2RkOiB3YXRjaGRvZyBkZXZpY2UNCj4gPiArICoNCj4gPiAr
ICogUmV0dXJuOiAwIGlmIHN1Y2Nlc3MsIG5lZ2F0aXZlIGVycm5vIGNvZGUgb24gZmFpbHVyZQ0K
PiA+ICsgKi8NCj4gPiArc3RhdGljIGludCBtZWlfd2R0X29wc19waW5nKHN0cnVjdCB3YXRjaGRv
Z19kZXZpY2UgKndkZCkNCj4gPiArew0KPiA+ICsJc3RydWN0IG1laV93ZHRfZGV2ICptd2QgPSB3
YXRjaGRvZ19nZXRfZHJ2ZGF0YSh3ZGQpOw0KPiA+ICsJc3RydWN0IG1laV93ZHQgKndkdDsNCj4g
PiArCWludCByZXQ7DQo+ID4gKw0KPiA+ICsJaWYgKCFtd2QpDQo+ID4gKwkJcmV0dXJuIC1FTk9E
RVY7DQo+ID4gKw0KPiA+ICsJd2R0ID0gbXdkLT53ZHQ7DQo+ID4gKw0KPiA+ICsJaWYgKHdkdC0+
c3RhdGUgIT0gTUVJX1dEVF9TVEFSVCAmJg0KPiA+ICsJICAgIHdkdC0+c3RhdGUgIT0gTUVJX1dE
VF9SVU5OSU5HKQ0KPiANCj4gVW5uZWNlc3NhcnkgY29udGludWF0aW9uIGxpbmUgPw0KTG9va3Mg
bW9yZSByZWFkYWJsZSB0byBtZS4gYnV0IHdlIGNhbiBhbHNvIHN0cmFpZ2h0IHRoZSBjb25kaXRp
b24uIA0KPiANCj4gPiArCQlyZXR1cm4gMDsNCj4gPiArDQo+ID4gKwlyZXQgPSBtZWlfd2R0X3Bp
bmcod2R0KTsNCj4gPiArCWlmIChyZXQgPCAwKQ0KPiA+ICsJCXJldHVybiByZXQ7DQo+ID4gKw0K
PiA+ICsJd2R0LT5zdGF0ZSA9IE1FSV9XRFRfUlVOTklORzsNCj4gPiArDQo+ID4gKwlyZXR1cm4g
MDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArLyoqDQo+ID4gKyAqIG1laV93ZHRfb3BzX3NldF90aW1l
b3V0IC0gd2Qgc2V0IHRpbWVvdXQgY29tbWFuZCBmcm9tIHRoZQ0KPiA+IHdhdGNoZG9nIGNvcmUu
DQo+ID4gKyAqDQo+ID4gKyAqIEB3ZGQ6IHdhdGNoZG9nIGRldmljZQ0KPiA+ICsgKiBAdGltZW91
dDogdGltZW91dCB2YWx1ZSB0byBzZXQNCj4gPiArICoNCj4gPiArICogUmV0dXJuOiAwIGlmIHN1
Y2Nlc3MsIG5lZ2F0aXZlIGVycm5vIGNvZGUgZm9yIGZhaWx1cmUNCj4gPiArICovDQo+ID4gK3N0
YXRpYyBpbnQgbWVpX3dkdF9vcHNfc2V0X3RpbWVvdXQoc3RydWN0IHdhdGNoZG9nX2RldmljZSAq
d2RkLA0KPiA+ICsJCQkJICAgdW5zaWduZWQgaW50IHRpbWVvdXQpDQo+ID4gK3sNCj4gPiArCXN0
cnVjdCBtZWlfd2R0X2RldiAqbXdkID0gd2F0Y2hkb2dfZ2V0X2RydmRhdGEod2RkKTsNCj4gPiAr
CXN0cnVjdCBtZWlfd2R0ICp3ZHQ7DQo+ID4gKw0KPiA+ICsJaWYgKCFtd2QpDQo+ID4gKwkJcmV0
dXJuIC1FTk9ERVY7DQo+ID4gKw0KPiA+ICsJd2R0ID0gbXdkLT53ZHQ7DQo+ID4gKw0KPiA+ICsJ
LyogdmFsaWQgdmFsdWUgaXMgYWxyZWFkeSBjaGVja2VkIGJ5IHRoZSBjYWxsZXIgKi8NCj4gPiAr
CXdkdC0+dGltZW91dCA9IHRpbWVvdXQ7DQo+ID4gKwl3ZGQtPnRpbWVvdXQgPSB0aW1lb3V0Ow0K
PiANCj4gT25lIG9mIHRob3NlIHNlZW1zIHVubmVjZXNzYXJ5LiBXaHkga2VlcCB0aGUgdGltZW91
dCB0d2ljZSA/DQoNCldlIG5lZWQgdHdvIGFzIHdkZCBtYXkgbm90IGV4aXN0cyBhbmQgd2Ugc3Rp
bGwgbmVlZCB0byBzZW5kIHBpbmcgdG8NCmRldGVjdCBpZiB0aGUgZGV2aWNlIGlzIHByb3Zpc2lv
bmVkLiANCg0KPiA+ICsNCj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0
aWMgdm9pZCBtZWlfd2R0X3JlbGVhc2Uoc3RydWN0IGtyZWYgKnJlZikNCj4gPiArew0KPiA+ICsJ
c3RydWN0IG1laV93ZHRfZGV2ICptd2QgPSBjb250YWluZXJfb2YocmVmLCBzdHJ1Y3QNCj4gPiBt
ZWlfd2R0X2RldiwgcmVmY250KTsNCj4gPiArDQo+ID4gKwlrZnJlZShtd2QpOw0KPiA+ICt9DQo+
ID4gKw0KPiA+ICtzdGF0aWMgdm9pZCBtZWlfd2R0X29wc19yZWYoc3RydWN0IHdhdGNoZG9nX2Rl
dmljZSAqd2RkKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgbWVpX3dkdF9kZXYgKm13ZCA9IHdhdGNo
ZG9nX2dldF9kcnZkYXRhKHdkZCk7DQo+ID4gKw0KPiA+ICsJa3JlZl9nZXQoJm13ZC0+cmVmY250
KTsNCj4gPiArfQ0KPiA+ICsNCj4gPiArc3RhdGljIHZvaWQgbWVpX3dkdF9vcHNfdW5yZWYoc3Ry
dWN0IHdhdGNoZG9nX2RldmljZSAqd2RkKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgbWVpX3dkdF9k
ZXYgKm13ZCA9IHdhdGNoZG9nX2dldF9kcnZkYXRhKHdkZCk7DQo+ID4gKw0KPiA+ICsJa3JlZl9w
dXQoJm13ZC0+cmVmY250LCBtZWlfd2R0X3JlbGVhc2UpOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtz
dGF0aWMgY29uc3Qgc3RydWN0IHdhdGNoZG9nX29wcyB3ZF9vcHMgPSB7DQo+ID4gKwkub3duZXIg
ICAgICAgPSBUSElTX01PRFVMRSwNCj4gPiArCS5zdGFydCAgICAgICA9IG1laV93ZHRfb3BzX3N0
YXJ0LA0KPiA+ICsJLnN0b3AgICAgICAgID0gbWVpX3dkdF9vcHNfc3RvcCwNCj4gPiArCS5waW5n
ICAgICAgICA9IG1laV93ZHRfb3BzX3BpbmcsDQo+ID4gKwkuc2V0X3RpbWVvdXQgPSBtZWlfd2R0
X29wc19zZXRfdGltZW91dCwNCj4gPiArCS5yZWYgICAgICAgICA9IG1laV93ZHRfb3BzX3JlZiwN
Cj4gPiArCS51bnJlZiAgICAgICA9IG1laV93ZHRfb3BzX3VucmVmLA0KPiA+ICt9Ow0KPiA+ICsN
Cj4gPiArc3RhdGljIHN0cnVjdCB3YXRjaGRvZ19pbmZvIHdkX2luZm8gPSB7DQo+ID4gKwkuaWRl
bnRpdHkgPSBJTlRFTF9BTVRfV0FUQ0hET0dfSUQsDQo+ID4gKwkub3B0aW9ucyAgPSBXRElPRl9L
RUVQQUxJVkVQSU5HIHwNCj4gPiArCQkgICAgV0RJT0ZfU0VUVElNRU9VVCB8DQo+ID4gKwkJICAg
IFdESU9GX0FMQVJNT05MWSwNCj4gPiArfTsNCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgbWVpX3dk
dF9yZWdpc3RlcihzdHJ1Y3QgbWVpX3dkdCAqd2R0KQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgbWVp
X3dkdF9kZXYgKm13ZDsNCj4gPiArCXN0cnVjdCBkZXZpY2UgKmRldjsNCj4gPiArCWludCByZXQ7
DQo+ID4gKw0KPiA+ICsJaWYgKCF3ZHQgfHwgIXdkdC0+Y2xkZXYpDQo+ID4gKwkJcmV0dXJuIC1F
SU5WQUw7DQo+ID4gKw0KPiA+ICsJZGV2ID0gJndkdC0+Y2xkZXYtPmRldjsNCj4gPiArDQo+ID4g
Kwltd2QgPSBremFsbG9jKHNpemVvZihzdHJ1Y3QgbWVpX3dkdF9kZXYpLCBHRlBfS0VSTkVMKTsN
Cj4gPiArCWlmICghbXdkKQ0KPiA+ICsJCXJldHVybiAtRU5PTUVNOw0KPiA+ICsNCj4gPiArCW13
ZC0+d2R0ID0gd2R0Ow0KPiA+ICsJbXdkLT53ZGQuaW5mbyA9ICZ3ZF9pbmZvOw0KPiA+ICsJbXdk
LT53ZGQub3BzID0gJndkX29wczsNCj4gPiArCW13ZC0+d2RkLnBhcmVudCA9IGRldjsNCj4gPiAr
CW13ZC0+d2RkLnRpbWVvdXQgPSBNRUlfV0RUX0RFRkFVTFRfVElNRU9VVDsNCj4gPiArCW13ZC0+
d2RkLm1pbl90aW1lb3V0ID0gTUVJX1dEVF9NSU5fVElNRU9VVDsNCj4gPiArCW13ZC0+d2RkLm1h
eF90aW1lb3V0ID0gTUVJX1dEVF9NQVhfVElNRU9VVDsNCj4gPiArCWtyZWZfaW5pdCgmbXdkLT5y
ZWZjbnQpOw0KPiA+ICsNCj4gPiArCXJldCA9IHdhdGNoZG9nX3JlZ2lzdGVyX2RldmljZSgmbXdk
LT53ZGQpOw0KPiA+ICsJaWYgKHJldCkgew0KPiA+ICsJCWRldl9lcnIoZGV2LCAidW5hYmxlIHRv
IHJlZ2lzdGVyIHdhdGNoZG9nIGRldmljZSA9DQo+ID4gJWQuXG4iLCByZXQpOw0KPiA+ICsJCWty
ZWZfcHV0KCZtd2QtPnJlZmNudCwgbWVpX3dkdF9yZWxlYXNlKTsNCj4gPiArCQlyZXR1cm4gcmV0
Ow0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCXdkdC0+bXdkID0gbXdkOw0KPiA+ICsJd2F0Y2hkb2df
c2V0X2RydmRhdGEoJm13ZC0+d2RkLCBtd2QpOw0KPiA+ICsJcmV0dXJuIDA7DQo+ID4gK30NCj4g
PiArDQo+ID4gK3N0YXRpYyB2b2lkIG1laV93ZHRfdW5yZWdpc3RlcihzdHJ1Y3QgbWVpX3dkdCAq
d2R0KQ0KPiA+ICt7DQo+ID4gKwlpZiAoIXdkdC0+bXdkKQ0KPiA+ICsJCXJldHVybjsNCj4gPiAr
DQo+ID4gKwl3YXRjaGRvZ191bnJlZ2lzdGVyX2RldmljZSgmd2R0LT5td2QtPndkZCk7DQo+ID4g
KwlrcmVmX3B1dCgmd2R0LT5td2QtPnJlZmNudCwgbWVpX3dkdF9yZWxlYXNlKTsNCj4gPiArCXdk
dC0+bXdkID0gTlVMTDsNCj4gPiArfQ0KPiANCj4gU2VlbXMgdG8gbWUgdGhhdCB1c2luZyB0d28g
c2VwYXJhdGUgZGF0YSBzdHJ1Y3R1cmVzIGluc3RlYWQgb2Ygb25lDQo+IGFkZHMgYSBsb3Qgb2Yg
Y29tcGxleGl0eS4NCg0KSXQgbWlnaHQgYmUgcmVkdWNlZCBidXQgSSdtIG5vdCBzdXJlIGl0IGNh
biBiZSBzaWduaWZpY2FudGx5IHNpbXBsZXIuIA0KSXQgdGhlIHJlZmVyZW5jZSBjb3VudGVyIHdp
bGwgYmUgcGFydCBvZiB3YXRjaGRvZ19kZXZpY2UgaXQgd291bGQgYmUNCnNpbXBsZXIuICANCg0K
PiA+ICsNCj4gPiArc3RhdGljIGludCBtZWlfd2R0X3Byb2JlKHN0cnVjdCBtZWlfY2xfZGV2aWNl
ICpjbGRldiwNCj4gPiArCQkJIGNvbnN0IHN0cnVjdCBtZWlfY2xfZGV2aWNlX2lkICppZCkNCj4g
PiArew0KPiA+ICsJc3RydWN0IG1laV93ZHQgKndkdDsNCj4gPiArCWludCByZXQ7DQo+ID4gKw0K
PiA+ICsJd2R0ID0ga3phbGxvYyhzaXplb2Yoc3RydWN0IG1laV93ZHQpLCBHRlBfS0VSTkVMKTsN
Cj4gPiArCWlmICghd2R0KQ0KPiA+ICsJCXJldHVybiAtRU5PTUVNOw0KPiA+ICsNCj4gPiArCXdk
dC0+dGltZW91dCA9IE1FSV9XRFRfREVGQVVMVF9USU1FT1VUOw0KPiA+ICsJd2R0LT5zdGF0ZSA9
IE1FSV9XRFRfSURMRTsNCj4gPiArCXdkdC0+Y2xkZXYgPSBjbGRldjsNCj4gPiArCW1laV9jbGRl
dl9zZXRfZHJ2ZGF0YShjbGRldiwgd2R0KTsNCj4gPiArDQo+ID4gKwlyZXQgPSBtZWlfY2xkZXZf
ZW5hYmxlKGNsZGV2KTsNCj4gPiArCWlmIChyZXQgPCAwKSB7DQo+ID4gKwkJZGV2X2VycigmY2xk
ZXYtPmRldiwgIkNvdWxkIG5vdCBlbmFibGUgY2wNCj4gPiBkZXZpY2VcbiIpOw0KPiA+ICsJCWdv
dG8gZXJyX291dDsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwl3ZF9pbmZvLmZpcm13YXJlX3ZlcnNp
b24gPSBtZWlfY2xkZXZfdmVyKGNsZGV2KTsNCj4gPiArDQo+ID4gKwlyZXQgPSBtZWlfd2R0X3Jl
Z2lzdGVyKHdkdCk7DQo+ID4gKwlpZiAocmV0KQ0KPiA+ICsJCWdvdG8gZXJyX2Rpc2FibGU7DQo+
ID4gKw0KPiA+ICsJcmV0dXJuIDA7DQo+ID4gKw0KPiA+ICtlcnJfZGlzYWJsZToNCj4gPiArCW1l
aV9jbGRldl9kaXNhYmxlKGNsZGV2KTsNCj4gPiArDQo+ID4gK2Vycl9vdXQ6DQo+ID4gKwlrZnJl
ZSh3ZHQpOw0KPiA+ICsNCj4gPiArCXJldHVybiByZXQ7DQo+ID4gK30NCj4gPiArDQo+ID4gK3N0
YXRpYyBpbnQgbWVpX3dkdF9yZW1vdmUoc3RydWN0IG1laV9jbF9kZXZpY2UgKmNsZGV2KQ0KPiA+
ICt7DQo+ID4gKwlzdHJ1Y3QgbWVpX3dkdCAqd2R0ID0gbWVpX2NsZGV2X2dldF9kcnZkYXRhKGNs
ZGV2KTsNCj4gPiArDQo+ID4gKwltZWlfY2xkZXZfZGlzYWJsZShjbGRldik7DQo+ID4gKw0KPiA+
ICsJbWVpX3dkdF91bnJlZ2lzdGVyKHdkdCk7DQo+ID4gKw0KPiA+ICsJa2ZyZWUod2R0KTsNCj4g
PiArDQo+ID4gKwlyZXR1cm4gMDsNCj4gPiArfQ0KPiA+ICsNCj4gPiArI2RlZmluZSBNRUlfVVVJ
RF9XRCBVVUlEX0xFKDB4MDVCNzlBNkYsIDB4NDYyOCwgMHg0RDdGLCBcDQo+ID4gKwkJCSAgICAw
eDg5LCAweDlELCAweEE5LCAweDE1LCAweDE0LCAweENCLA0KPiA+IDB4MzIsIDB4QUIpDQo+ID4g
Kw0KPiA+ICtzdGF0aWMgc3RydWN0IG1laV9jbF9kZXZpY2VfaWQgbWVpX3dkdF90YmxbXSA9IHsN
Cj4gPiArCXsgLnV1aWQgPSBNRUlfVVVJRF9XRCwgLnZlcnNpb24gPSBNRUlfQ0xfVkVSU0lPTl9B
Tll9LA0KPiA+ICsJLyogcmVxdWlyZWQgbGFzdCBlbnRyeSAqLw0KPiA+ICsJeyB9DQo+ID4gK307
DQo+ID4gK01PRFVMRV9ERVZJQ0VfVEFCTEUobWVpLCBtZWlfd2R0X3RibCk7DQo+ID4gKw0KPiA+
ICtzdGF0aWMgc3RydWN0IG1laV9jbF9kcml2ZXIgbWVpX3dkdF9kcml2ZXIgPSB7DQo+ID4gKwku
aWRfdGFibGUgPSBtZWlfd2R0X3RibCwNCj4gPiArCS5uYW1lID0gS0JVSUxEX01PRE5BTUUsDQo+
ID4gKw0KPiA+ICsJLnByb2JlID0gbWVpX3dkdF9wcm9iZSwNCj4gPiArCS5yZW1vdmUgPSBtZWlf
d2R0X3JlbW92ZSwNCj4gPiArfTsNCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgX19pbml0IG1laV93
ZHRfaW5pdCh2b2lkKQ0KPiA+ICt7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCXJldCA9
IG1laV9jbGRldl9kcml2ZXJfcmVnaXN0ZXIoJm1laV93ZHRfZHJpdmVyKTsNCj4gPiArCWlmIChy
ZXQpIHsNCj4gPiArCQlwcl9lcnIoS0JVSUxEX01PRE5BTUUgIjogbW9kdWxlIHJlZ2lzdHJhdGlv
bg0KPiA+IGZhaWxlZFxuIik7DQo+ID4gKwkJcmV0dXJuIHJldDsNCj4gPiArCX0NCj4gPiArCXJl
dHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgdm9pZCBfX2V4aXQgbWVpX3dkdF9l
eGl0KHZvaWQpDQo+ID4gK3sNCj4gPiArCW1laV9jbGRldl9kcml2ZXJfdW5yZWdpc3RlcigmbWVp
X3dkdF9kcml2ZXIpOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICttb2R1bGVfaW5pdChtZWlfd2R0X2lu
aXQpOw0KPiA+ICttb2R1bGVfZXhpdChtZWlfd2R0X2V4aXQpOw0KPiA+ICsNCj4gPiArTU9EVUxF
X0FVVEhPUigiSW50ZWwgQ29ycG9yYXRpb24iKTsNCj4gPiArTU9EVUxFX0xJQ0VOU0UoIkdQTCIp
Ow0KPiA+ICtNT0RVTEVfREVTQ1JJUFRJT04oIkRldmljZSBkcml2ZXIgZm9yIEludGVsIE1FSSBp
QU1UIHdhdGNoZG9nIik7DQo+ID4gDQo+IA==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1280989 — Re: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver

On 12/01/2015 03:55 AM, Winkler, Tomas wrote:
[ ... ]

>>> +/**
>>> + * struct mei_wdt_dev - watchdog device wrapper
>>> + *
>>> + * @wdd: watchdog device
>>> + * @wdt: back pointer to mei_wdt driver
>>> + * @refcnt: reference counter
>>> + */
>>> +struct mei_wdt_dev {
>>> +	struct watchdog_device wdd;
>>> +	struct mei_wdt *wdt;
>>> +	struct kref refcnt;
>>> +};
>>> +
>>> +/**
>>> + * struct mei_wdt - mei watchdog driver
>>> + *
>>> + * @cldev: mei watchdog client device
>>> + * @mwd: watchdog device wrapper
>>> + * @state: watchdog internal state
>>> + * @timeout: watchdog current timeout
>>> + */
>>> +struct mei_wdt {
>>> +	struct mei_cl_device *cldev;
>>> +	struct mei_wdt_dev *mwd;
>>> +	enum mei_wdt_state state;
>>> +	u16 timeout;
>>> +};
>>
>> Any special reason for having two data structures instead of one ?
>> You could just move the variables from struct mei_wdt_dev into
>> struct mei_wdt, no ?
>
> Yes, on newer platform mei_wdt_dev might be not present in case the the
> device is not provisioned. This came to action in the following
> patches.
>

It is still an implementation choice to keep the data structures separate.
That mei_wdt_dev can be unregistered doesn't mean that the data structure
has to be destroyed or allocated on registration.

>>> +
>>> +struct mei_wdt_hdr {
>>> +	u8 command;
>>> +	u8 bytecount;
>>> +	u8 subcommand;
>>> +	u8 versionnumber;
>>> +};
>>> +
>>> +struct mei_wdt_start_request {
>>> +	struct mei_wdt_hdr hdr;
>>> +	u16 timeout;
>>> +	u8 reserved[17];
>>> +} __packed;
>>> +
>>> +struct mei_wdt_stop_request {
>>> +	struct mei_wdt_hdr hdr;
>>> +} __packed;
>>> +
>>> +/**
>>> + * mei_wdt_ping - send wd start command
>>> + *
>>> + * @wdt: mei watchdog device
>>> + *
>>> + * Return: number of bytes sent on success,
>>> + *         negative errno code on failure
>>> + */
>>> +static int mei_wdt_ping(struct mei_wdt *wdt)
>>> +{
>>> +	struct mei_wdt_start_request req;
>>> +	const size_t req_len = sizeof(req);
>>> +
>>> +	memset(&req, 0, req_len);
>>> +	req.hdr.command = MEI_MANAGEMENT_CONTROL;
>>> +	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr,
>>> subcommand);
>>> +	req.hdr.subcommand = MEI_MC_START_WD_TIMER_REQ;
>>> +	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
>>> +	req.timeout = wdt->timeout;
>>> +
>>> +	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
>>> +}
>>> +
>>> +/**
>>> + * mei_wdt_stop - send wd stop command
>>> + *
>>> + * @wdt: mei watchdog device
>>> + *
>>> + * Return: number of bytes sent on success,
>>> + *         negative errno code on failure
>>> + */
>>> +static int mei_wdt_stop(struct mei_wdt *wdt)
>>> +{
>>> +	struct mei_wdt_stop_request req;
>>> +	const size_t req_len = sizeof(req);
>>> +
>>> +	memset(&req, 0, req_len);
>>> +	req.hdr.command = MEI_MANAGEMENT_CONTROL;
>>> +	req.hdr.bytecount = req_len - offsetof(struct mei_wdt_hdr,
>>> subcommand);
>>> +	req.hdr.subcommand = MEI_MC_STOP_WD_TIMER_REQ;
>>> +	req.hdr.versionnumber = MEI_MC_VERSION_NUMBER;
>>> +
>>> +	return mei_cldev_send(wdt->cldev, (u8 *)&req, req_len);
>>> +}
>>> +
>>> +/**
>>> + * mei_wdt_ops_start - wd start command from the watchdog core.
>>> + *
>>> + * @wdd: watchdog device
>>> + *
>>> + * Return: 0 on success or -ENODEV;
>>> + */
>>> +static int mei_wdt_ops_start(struct watchdog_device *wdd)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +
>>> +	if (!mwd)
>>> +		return -ENODEV;
>>
>> This can only happen because you call watchdog_set_drvdata() after
>> watchdog device registration. If that happens, the system is in
>> really bad shape.
>
> mei_wdt_dev can destroyed during
> driver operation if the device is unprovisioned, but still you the
> condition should not happen unless we have a bug. We can put WARN_ON()
> there.
>

The calling code should take care of that and not call those functions
after unregistration. More on that below.

>>
>> I would suggest to move the call to watchdog_set_drvdata() ahead
>> of watchdog_register_device() and drop those checks.
>>
>>> +
>>> +	mwd->wdt->state = MEI_WDT_START;
>>> +	wdd->timeout = mwd->wdt->timeout;
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * mei_wdt_ops_stop - wd stop command from the watchdog core.
>>> + *
>>> + * @wdd: watchdog device
>>> + *
>>> + * Return: 0 if success, negative errno code for failure
>>> + */
>>> +static int mei_wdt_ops_stop(struct watchdog_device *wdd)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +	struct mei_wdt *wdt;
>>> +	int ret;
>>> +
>>> +	if (!mwd)
>>> +		return -ENODEV;
>>> +
>>> +	wdt = mwd->wdt;
>>> +
>>> +	if (wdt->state != MEI_WDT_RUNNING)
>>> +		return 0;
>>> +
>>> +	wdt->state = MEI_WDT_STOPPING;
>>> +
>>> +	ret = mei_wdt_stop(wdt);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	wdt->state = MEI_WDT_IDLE;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * mei_wdt_ops_ping - wd ping command from the watchdog core.
>>> + *
>>> + * @wdd: watchdog device
>>> + *
>>> + * Return: 0 if success, negative errno code on failure
>>> + */
>>> +static int mei_wdt_ops_ping(struct watchdog_device *wdd)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +	struct mei_wdt *wdt;
>>> +	int ret;
>>> +
>>> +	if (!mwd)
>>> +		return -ENODEV;
>>> +
>>> +	wdt = mwd->wdt;
>>> +
>>> +	if (wdt->state != MEI_WDT_START &&
>>> +	    wdt->state != MEI_WDT_RUNNING)
>>
>> Unnecessary continuation line ?
> Looks more readable to me. but we can also straight the condition.
>>
>>> +		return 0;
>>> +
>>> +	ret = mei_wdt_ping(wdt);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	wdt->state = MEI_WDT_RUNNING;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * mei_wdt_ops_set_timeout - wd set timeout command from the
>>> watchdog core.
>>> + *
>>> + * @wdd: watchdog device
>>> + * @timeout: timeout value to set
>>> + *
>>> + * Return: 0 if success, negative errno code for failure
>>> + */
>>> +static int mei_wdt_ops_set_timeout(struct watchdog_device *wdd,
>>> +				   unsigned int timeout)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +	struct mei_wdt *wdt;
>>> +
>>> +	if (!mwd)
>>> +		return -ENODEV;
>>> +
>>> +	wdt = mwd->wdt;
>>> +
>>> +	/* valid value is already checked by the caller */
>>> +	wdt->timeout = timeout;
>>> +	wdd->timeout = timeout;
>>
>> One of those seems unnecessary. Why keep the timeout twice ?
>
> We need two as wdd may not exists and we still need to send ping to
> detect if the device is provisioned.
>

Ok, makes sense.

>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void mei_wdt_release(struct kref *ref)
>>> +{
>>> +	struct mei_wdt_dev *mwd = container_of(ref, struct
>>> mei_wdt_dev, refcnt);
>>> +
>>> +	kfree(mwd);
>>> +}
>>> +
>>> +static void mei_wdt_ops_ref(struct watchdog_device *wdd)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +
>>> +	kref_get(&mwd->refcnt);
>>> +}
>>> +
>>> +static void mei_wdt_ops_unref(struct watchdog_device *wdd)
>>> +{
>>> +	struct mei_wdt_dev *mwd = watchdog_get_drvdata(wdd);
>>> +
>>> +	kref_put(&mwd->refcnt, mei_wdt_release);
>>> +}
>>> +
>>> +static const struct watchdog_ops wd_ops = {
>>> +	.owner       = THIS_MODULE,
>>> +	.start       = mei_wdt_ops_start,
>>> +	.stop        = mei_wdt_ops_stop,
>>> +	.ping        = mei_wdt_ops_ping,
>>> +	.set_timeout = mei_wdt_ops_set_timeout,
>>> +	.ref         = mei_wdt_ops_ref,
>>> +	.unref       = mei_wdt_ops_unref,
>>> +};
>>> +
>>> +static struct watchdog_info wd_info = {
>>> +	.identity = INTEL_AMT_WATCHDOG_ID,
>>> +	.options  = WDIOF_KEEPALIVEPING |
>>> +		    WDIOF_SETTIMEOUT |
>>> +		    WDIOF_ALARMONLY,
>>> +};
>>> +
>>> +static int mei_wdt_register(struct mei_wdt *wdt)
>>> +{
>>> +	struct mei_wdt_dev *mwd;
>>> +	struct device *dev;
>>> +	int ret;
>>> +
>>> +	if (!wdt || !wdt->cldev)
>>> +		return -EINVAL;
>>> +
>>> +	dev = &wdt->cldev->dev;
>>> +
>>> +	mwd = kzalloc(sizeof(struct mei_wdt_dev), GFP_KERNEL);
>>> +	if (!mwd)
>>> +		return -ENOMEM;
>>> +
>>> +	mwd->wdt = wdt;
>>> +	mwd->wdd.info = &wd_info;
>>> +	mwd->wdd.ops = &wd_ops;
>>> +	mwd->wdd.parent = dev;
>>> +	mwd->wdd.timeout = MEI_WDT_DEFAULT_TIMEOUT;
>>> +	mwd->wdd.min_timeout = MEI_WDT_MIN_TIMEOUT;
>>> +	mwd->wdd.max_timeout = MEI_WDT_MAX_TIMEOUT;
>>> +	kref_init(&mwd->refcnt);
>>> +
>>> +	ret = watchdog_register_device(&mwd->wdd);
>>> +	if (ret) {
>>> +		dev_err(dev, "unable to register watchdog device =
>>> %d.\n", ret);
>>> +		kref_put(&mwd->refcnt, mei_wdt_release);
>>> +		return ret;
>>> +	}
>>> +
>>> +	wdt->mwd = mwd;
>>> +	watchdog_set_drvdata(&mwd->wdd, mwd);
>>> +	return 0;
>>> +}
>>> +
>>> +static void mei_wdt_unregister(struct mei_wdt *wdt)
>>> +{
>>> +	if (!wdt->mwd)
>>> +		return;
>>> +
>>> +	watchdog_unregister_device(&wdt->mwd->wdd);
>>> +	kref_put(&wdt->mwd->refcnt, mei_wdt_release);
>>> +	wdt->mwd = NULL;

If setting this to NULL was really needed, you would have a race condition here:
wdt->mwd is set to NULL only after the pointer it points to was freed, leaving
a small window where the code above could still access it.

>>> +}
>>
>> Seems to me that using two separate data structures instead of one
>> adds a lot of complexity.
>
> It might be reduced but I'm not sure it can be significantly simpler.
> It the reference counter will be part of watchdog_device it would be
> simpler.
>

It would be there if struct watchdog_device was allocated by the
watchdog core, which is not the case. I am still trying to create
a situation where the local data structure (struct mei_wdt in this case)
can be released while the watchdog device is still open
(ie how to unprovision the watchdog device while in use).

Once I figure that out, I hope I can find a way to protect it
differently and drop the ref/unref callbacks. I suspect it may
be necessary to separate struct watchdog_device into two parts:
one used by drivers and one used by the watchdog core.
The watchdog core would then manage its own data structure, and
no longer depend on struct watchdog_device (owned by the driver)
to actually be there. Different story, though.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281506 — RE: [char-misc-next 3/6] mei: wd: implement MEI iAMT watchdog driver

DQo+ID4+DQo+ID4+IEFueSBzcGVjaWFsIHJlYXNvbiBmb3IgaGF2aW5nIHR3byBkYXRhIHN0cnVj
dHVyZXMgaW5zdGVhZCBvZiBvbmUgPw0KPiA+PiBZb3UgY291bGQganVzdCBtb3ZlIHRoZSB2YXJp
YWJsZXMgZnJvbSBzdHJ1Y3QgbWVpX3dkdF9kZXYgaW50bw0KPiA+PiBzdHJ1Y3QgbWVpX3dkdCwg
bm8gPw0KPiA+DQo+ID4gWWVzLCBvbiBuZXdlciBwbGF0Zm9ybSBtZWlfd2R0X2RldiBtaWdodCBi
ZSBub3QgcHJlc2VudCBpbiBjYXNlIHRoZSB0aGUNCj4gPiBkZXZpY2UgaXMgbm90IHByb3Zpc2lv
bmVkLiBUaGlzIGNhbWUgdG8gYWN0aW9uIGluIHRoZSBmb2xsb3dpbmcNCj4gPiBwYXRjaGVzLg0K
PiA+DQo+IA0KPiBJdCBpcyBzdGlsbCBhbiBpbXBsZW1lbnRhdGlvbiBjaG9pY2UgdG8ga2VlcCB0
aGUgZGF0YSBzdHJ1Y3R1cmVzIHNlcGFyYXRlLg0KPiBUaGF0IG1laV93ZHRfZGV2IGNhbiBiZSB1
bnJlZ2lzdGVyZWQgZG9lc24ndCBtZWFuIHRoYXQgdGhlIGRhdGEgc3RydWN0dXJlDQo+IGhhcyB0
byBiZSBkZXN0cm95ZWQgb3IgYWxsb2NhdGVkIG9uIHJlZ2lzdHJhdGlvbi4NCg0KVGhhdCdzIGNv
cnJlY3QsIHRoZSBpc3N1ZSBpcyB0aGF0IGlmIHRoZSBNRUkgZGV2aWNlIHJlc2V0cyBvciBnbyB0
aHJvdWdoIHN1c3BlbmQvcmVzdW1lIHRoZSBtZWlfd2R0IHdpbGwgYmUgcmVtb3ZlZCBkZXN0cm95
ZWQNCldoeSB3YXRjaGRvZyBkYWVtb24gd2lsbCBzdGlsbCBob2xkcyBvcGVuIGhhbmRsZXIgdG8g
d2F0Y2hkb2dfZGV2aWNlIGFuZCB3aWxsIHJlbGVhc2VzIGl0IHVwb24gcGluZyBmYWlsdXJlLiBU
aGlzIGlzIHdoZXJlIHJlZmVyZW5jZSBjb3VudGVyIHdpbGwgY29tZSBpbi4NCkN1cnJlbnRseSB3
ZSBkbyBub3Qgc3VwcG9ydCBzZWFtbGVzcyByZXNldC4gDQoNCj4gPj4+ICsNCj4gPj4+ICsJLyog
dmFsaWQgdmFsdWUgaXMgYWxyZWFkeSBjaGVja2VkIGJ5IHRoZSBjYWxsZXIgKi8NCj4gPj4+ICsJ
d2R0LT50aW1lb3V0ID0gdGltZW91dDsNCj4gPj4+ICsJd2RkLT50aW1lb3V0ID0gdGltZW91dDsN
Cj4gPj4NCj4gPj4gT25lIG9mIHRob3NlIHNlZW1zIHVubmVjZXNzYXJ5LiBXaHkga2VlcCB0aGUg
dGltZW91dCB0d2ljZSA/DQo+ID4NCj4gPiBXZSBuZWVkIHR3byBhcyB3ZGQgbWF5IG5vdCBleGlz
dHMgYW5kIHdlIHN0aWxsIG5lZWQgdG8gc2VuZCBwaW5nIHRvDQo+ID4gZGV0ZWN0IGlmIHRoZSBk
ZXZpY2UgaXMgcHJvdmlzaW9uZWQuDQo+ID4NCj4gDQo+IE9rLCBtYWtlcyBzZW5zZS4NCj4gDQo+
ID4+PiArDQo+ID4+PiArc3RhdGljIHZvaWQgbWVpX3dkdF91bnJlZ2lzdGVyKHN0cnVjdCBtZWlf
d2R0ICp3ZHQpDQo+ID4+PiArew0KPiA+Pj4gKwlpZiAoIXdkdC0+bXdkKQ0KPiA+Pj4gKwkJcmV0
dXJuOw0KPiA+Pj4gKw0KPiA+Pj4gKwl3YXRjaGRvZ191bnJlZ2lzdGVyX2RldmljZSgmd2R0LT5t
d2QtPndkZCk7DQo+ID4+PiArCWtyZWZfcHV0KCZ3ZHQtPm13ZC0+cmVmY250LCBtZWlfd2R0X3Jl
bGVhc2UpOw0KPiA+Pj4gKwl3ZHQtPm13ZCA9IE5VTEw7DQo+IA0KPiBJZiBzZXR0aW5nIHRoaXMg
dG8gTlVMTCB3YXMgcmVhbGx5IG5lZWRlZCwgeW91IHdvdWxkIGhhdmUgYSByYWNlIGNvbmRpdGlv
biBoZXJlOg0KPiB3ZHQtPm13ZCBpcyBzZXQgdG8gTlVMTCBvbmx5IGFmdGVyIHRoZSBwb2ludGVy
IGl0IHBvaW50cyB0byB3YXMgZnJlZWQsIGxlYXZpbmcNCj4gYSBzbWFsbCB3aW5kb3cgd2hlcmUg
dGhlIGNvZGUgYWJvdmUgY291bGQgc3RpbGwgYWNjZXNzIGl0Lg0KDQoNClllcywgZ29vZCBjYXRj
aCB3aWxsIGZpeCB0aGF0Lg0KDQo+IA0KPiA+Pj4gK30NCj4gPj4NCj4gPj4gU2VlbXMgdG8gbWUg
dGhhdCB1c2luZyB0d28gc2VwYXJhdGUgZGF0YSBzdHJ1Y3R1cmVzIGluc3RlYWQgb2Ygb25lDQo+
ID4+IGFkZHMgYSBsb3Qgb2YgY29tcGxleGl0eS4NCj4gPg0KPiA+IEl0IG1pZ2h0IGJlIHJlZHVj
ZWQgYnV0IEknbSBub3Qgc3VyZSBpdCBjYW4gYmUgc2lnbmlmaWNhbnRseSBzaW1wbGVyLg0KPiA+
IEl0IHRoZSByZWZlcmVuY2UgY291bnRlciB3aWxsIGJlIHBhcnQgb2Ygd2F0Y2hkb2dfZGV2aWNl
IGl0IHdvdWxkIGJlDQo+ID4gc2ltcGxlci4NCj4gPg0KPiANCj4gSXQgd291bGQgYmUgdGhlcmUg
aWYgc3RydWN0IHdhdGNoZG9nX2RldmljZSB3YXMgYWxsb2NhdGVkIGJ5IHRoZQ0KPiB3YXRjaGRv
ZyBjb3JlLCB3aGljaCBpcyBub3QgdGhlIGNhc2UuIEkgYW0gc3RpbGwgdHJ5aW5nIHRvIGNyZWF0
ZQ0KPiBhIHNpdHVhdGlvbiB3aGVyZSB0aGUgbG9jYWwgZGF0YSBzdHJ1Y3R1cmUgKHN0cnVjdCBt
ZWlfd2R0IGluIHRoaXMgY2FzZSkNCj4gY2FuIGJlIHJlbGVhc2VkIHdoaWxlIHRoZSB3YXRjaGRv
ZyBkZXZpY2UgaXMgc3RpbGwgb3Blbg0KPiAoaWUgaG93IHRvIHVucHJvdmlzaW9uIHRoZSB3YXRj
aGRvZyBkZXZpY2Ugd2hpbGUgaW4gdXNlKS4NCg0KVGhpcyBoYXBwZW4gb24gZGV2aWNlIHN1c3Bl
bmQvcmVzdW1lIG9yIGRldmljZSByZXNldCwgdGhpcyBpcyBub3QgdGhlIGNhc2Ugb2YgcHJvdmlz
aW9uaW5nLg0KQ3VycmVudGx5IHRoZSBvdXIgYnVzIGxheWVyIGRvZXNuJ3Qgc3VwcG9ydCBzZWFt
bGVzcyByZWNvbm5lY3Rpb24uIA0KDQo+IE9uY2UgSSBmaWd1cmUgdGhhdCBvdXQsIEkgaG9wZSBJ
IGNhbiBmaW5kIGEgd2F5IHRvIHByb3RlY3QgaXQNCj4gZGlmZmVyZW50bHkgYW5kIGRyb3AgdGhl
IHJlZi91bnJlZiBjYWxsYmFja3MuIEkgc3VzcGVjdCBpdCBtYXkNCj4gYmUgbmVjZXNzYXJ5IHRv
IHNlcGFyYXRlIHN0cnVjdCB3YXRjaGRvZ19kZXZpY2UgaW50byB0d28gcGFydHM6DQo+IG9uZSB1
c2VkIGJ5IGRyaXZlcnMgYW5kIG9uZSB1c2VkIGJ5IHRoZSB3YXRjaGRvZyBjb3JlLg0KPiBUaGUg
d2F0Y2hkb2cgY29yZSB3b3VsZCB0aGVuIG1hbmFnZSBpdHMgb3duIGRhdGEgc3RydWN0dXJlLCBh
bmQNCj4gbm8gbG9uZ2VyIGRlcGVuZCBvbiBzdHJ1Y3Qgd2F0Y2hkb2dfZGV2aWNlIChvd25lZCBi
eSB0aGUgZHJpdmVyKQ0KPiB0byBhY3R1YWxseSBiZSB0aGVyZS4gRGlmZmVyZW50IHN0b3J5LCB0
aG91Z2guDQoNCg0KSSB0aGluayB0aGF0IHdvdWxkIHNvbWVob3cgZml0IGFsc28gb3VyIGRyaXZl
ci4gDQoNClRoYW5rcw0KVG9tYXMgDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web