Re: [PATCH v2 07/11] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2025-08-29 11:11:29
Also in:
asahi, linux-arm-kernel, linux-devicetree, linux-hwmon, linux-rtc, lkml
Hi James, On Wed, Aug 27, 2025 at 09:22:41PM +1000, James Calligeros wrote:
+static void macsmc_hid_event_button(struct macsmc_hid *smchid, unsigned long event)
+{
+ u8 button = (event >> 8) & 0xff;
+ u8 state = !!(event & 0xff);
+
+ switch (button) {
+ case BTN_POWER:
+ case BTN_TOUCHID:
+ if (smchid->wakeup_mode) {
+ if (state)
+ pm_wakeup_hard_event(smchid->dev);
+ } else {
+ input_report_key(smchid->input, KEY_POWER, state);
+ input_sync(smchid->input);
+ }I believe you should be using pm_wakeup_event() in all cases so that pressing power would interrupt suspend even if resume() handler has not been run yet. Also I do not think suppressing KEY_POWER is needed. Userspace should be smart and decide whether to shutdown the system or not when receiving KEY_POWER depending on the overall system state. ...
+
+static int macsmc_hid_probe(struct platform_device *pdev)
+{
+ struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+ struct macsmc_hid *smchid;
+ bool have_lid, have_power;
+ int ret;int error;
+
+ /* Bail early if this SMC neither supports power button nor lid events */
+ have_lid = apple_smc_key_exists(smc, SMC_KEY(MSLD));
+ have_power = apple_smc_key_exists(smc, SMC_KEY(bHLD));
+ if (!have_lid && !have_power)
+ return -ENODEV;
+
+ smchid = devm_kzalloc(&pdev->dev, sizeof(*smchid), GFP_KERNEL);
+ if (!smchid)
+ return -ENOMEM;
+
+ smchid->dev = &pdev->dev;
+ smchid->smc = smc;
+ platform_set_drvdata(pdev, smchid);
+
+ smchid->input = devm_input_allocate_device(&pdev->dev);
+ if (!smchid->input)
+ return -ENOMEM;
+
+ smchid->input->phys = "macsmc-hid (0)";
+ smchid->input->name = "Apple SMC power/lid events";
+
+ if (have_lid)
+ input_set_capability(smchid->input, EV_SW, SW_LID);
+ if (have_power)
+ input_set_capability(smchid->input, EV_KEY, KEY_POWER);
+
+ ret = input_register_device(smchid->input);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register input device: %d\n", ret);
+ return ret;
+ }
+
+ if (have_lid) {
+ u8 val;
+
+ ret = apple_smc_read_u8(smc, SMC_KEY(MSLD), &val);
+ if (ret < 0)
+ dev_warn(&pdev->dev, "Failed to read initial lid state\n");
+ else
+ input_report_switch(smchid->input, SW_LID, val);
+ }
+
+ if (have_power) {
+ u32 val;
+
+ ret = apple_smc_read_u32(smc, SMC_KEY(bHLD), &val);
+ if (ret < 0)
+ dev_warn(&pdev->dev, "Failed to read initial power button state\n");
+ else
+ input_report_key(smchid->input, KEY_POWER, val & 1);
+ }Since you are doing this to seed initial switch/button state I would do this before registering input device (this is safe to do so). Thanks. -- Dmitry