All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Lukasz Majczak <lma@chromium.org>,
	Gwendal Grignou <gwendal@chromium.org>,
	Tzung-Bi Shih <tzungbi@kernel.org>,
	Radoslaw Biernacki <biernacki@google.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Lee Jones <lee@kernel.org>, Benson Leung <bleung@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org,
	chrome-platform@lists.linux.dev
Subject: Re: [PATCH v3 2/3] watchdog: Add ChromeOS EC-based watchdog driver
Date: Mon, 22 Jan 2024 11:31:47 +0100	[thread overview]
Message-ID: <1e7b8590-7dc7-47a8-8105-6d01db627f85@kernel.org> (raw)
In-Reply-To: <20240119084328.3135503-3-lma@chromium.org>

On 19/01/2024 09:43, Lukasz Majczak wrote:
> Embedded Controller (EC) present on Chromebook devices
> can be used as a watchdog.
> Implement a driver to support it.
> 


...

> +static int cros_ec_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> +	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
> +	struct watchdog_device *wdd;
> +	union cros_ec_wdt_data arg;
> +	int ret = 0;
> +
> +	wdd = devm_kzalloc(&pdev->dev, sizeof(struct watchdog_device), GFP_KERNEL);

sizeof(*)

> +	if (!wdd)
> +		return -ENOMEM;
> +
> +	arg.req.command = EC_HANG_DETECT_CMD_GET_STATUS;
> +	ret = cros_ec_wdt_send_cmd(cros_ec, &arg);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get watchdog bootstatus");
> +
> +	wdd->parent = &pdev->dev;
> +	wdd->info = &cros_ec_wdt_ident;
> +	wdd->ops = &cros_ec_wdt_ops;
> +	wdd->timeout = CROS_EC_WATCHDOG_DEFAULT_TIME;
> +	wdd->min_timeout = EC_HANG_DETECT_MIN_TIMEOUT;
> +	wdd->max_timeout = EC_HANG_DETECT_MAX_TIMEOUT;
> +	if (arg.resp.status == EC_HANG_DETECT_AP_BOOT_EC_WDT)
> +		wdd->bootstatus = WDIOF_CARDRESET;
> +
> +	arg.req.command = EC_HANG_DETECT_CMD_CLEAR_STATUS;
> +	ret = cros_ec_wdt_send_cmd(cros_ec, &arg);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to clear watchdog bootstatus");
> +
> +	watchdog_stop_on_reboot(wdd);
> +	watchdog_stop_on_unregister(wdd);
> +	watchdog_set_drvdata(wdd, cros_ec);
> +	platform_set_drvdata(pdev, wdd);
> +
> +	return devm_watchdog_register_device(dev, wdd);
> +}
> +
> +static int __maybe_unused cros_ec_wdt_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
> +	int ret = 0;
> +
> +	if (watchdog_active(wdd))
> +		ret = cros_ec_wdt_stop(wdd);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused cros_ec_wdt_resume(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdd = platform_get_drvdata(pdev);
> +	int ret = 0;
> +
> +	if (watchdog_active(wdd))
> +		ret = cros_ec_wdt_start(wdd);
> +
> +	return ret;
> +}
> +
> +static struct platform_driver cros_ec_wdt_driver = {
> +	.probe		= cros_ec_wdt_probe,
> +	.suspend	= pm_ptr(cros_ec_wdt_suspend),
> +	.resume		= pm_ptr(cros_ec_wdt_resume),
> +	.driver		= {
> +		.name	= DRV_NAME,
> +	},
> +};
> +
> +module_platform_driver(cros_ec_wdt_driver);
> +
> +static const struct platform_device_id cros_ec_wdt_id[] = {
> +	{ DRV_NAME, 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ec_wdt_id);

device_id is not placed here, please open existing drivers for
reference. Why this isn't referenced in driver structure?

> +MODULE_DESCRIPTION("Cros EC Watchdog Device Driver");
> +MODULE_LICENSE("GPL");

Best regards,
Krzysztof


  reply	other threads:[~2024-01-22 10:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-19  8:43 [PATCH v3 0/3] Introduce EC-based watchdog Lukasz Majczak
2024-01-19  8:43 ` [PATCH v3 1/3] platform/chrome: Update binary interface for " Lukasz Majczak
2024-01-19  8:43 ` [PATCH v3 2/3] watchdog: Add ChromeOS EC-based watchdog driver Lukasz Majczak
2024-01-22 10:31   ` Krzysztof Kozlowski [this message]
2024-01-22 15:09     ` Łukasz Majczak
2024-01-19  8:43 ` [PATCH v3 3/3] mfd: cros_ec: Register EC-based watchdog subdevice Lukasz Majczak
2024-01-25 13:37   ` (subset) " Lee Jones
2024-01-25 13:55     ` Łukasz Majczak
2024-01-26  9:07       ` Lee Jones
2024-01-19 12:50 ` [PATCH v3 0/3] Introduce EC-based watchdog Guenter Roeck
2024-01-19 14:10   ` Łukasz Majczak
2024-01-19 14:42     ` Guenter Roeck
2024-01-19 14:50       ` Łukasz Majczak
2024-01-19 15:10         ` Łukasz Majczak
2024-03-25  1:54 ` patchwork-bot+chrome-platform
2024-03-25  2:13 ` patchwork-bot+chrome-platform

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=1e7b8590-7dc7-47a8-8105-6d01db627f85@kernel.org \
    --to=krzk@kernel.org \
    --cc=biernacki@google.com \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@chromium.org \
    --cc=gwendal@chromium.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=lma@chromium.org \
    --cc=tzungbi@kernel.org \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

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

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