* [PATCH v3 7/7] Input: mc13783-pwrbutton: add OF support
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Add OF support for the mc13783-pwrbutton so that it can be used with
modern DT based systems.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 94 +++++++++++++++++++++++---
1 file changed, 86 insertions(+), 8 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index c9eea57ceedd..a20236b19103 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/mfd/mc13783.h>
+#include <linux/property.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -109,8 +110,82 @@ static irqreturn_t button3_irq(int irq, void *_priv)
return button_irq(MC13783_IRQ_ONOFD3, _priv);
}
-static int mc13783_pwrbutton_probe(struct platform_device *pdev)
+#ifdef CONFIG_OF
+static struct mc13xxx_buttons_platform_data __init *mc13xxx_pwrbutton_probe_dt(
+ struct platform_device *pdev)
{
+ struct mc13xxx_buttons_platform_data *pdata;
+ struct fwnode_handle *child;
+ struct device *dev = &pdev->dev;
+ struct mc13xxx_button_devtype *devtype =
+ (struct mc13xxx_button_devtype *)platform_get_device_id(pdev)->driver_data;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ struct fwnode_handle *parent __free(fwnode_handle) =
+ device_get_named_child_node(dev->parent, "buttons");
+ if (!parent)
+ return ERR_PTR(-ENODATA);
+
+ fwnode_for_each_named_child_node(parent, child, "onkey") {
+ u32 idx;
+ u8 dbnc = MC13783_BUTTON_DBNC_30MS;
+ u16 dbnc_ms;
+
+ if (fwnode_property_read_u32(child, "reg", &idx))
+ continue;
+
+ if (idx > devtype->button_id_max) {
+ dev_warn(dev, "reg out of range\n");
+ continue;
+ }
+
+ fwnode_property_read_u16(child, "debounce-delay-ms", &dbnc_ms);
+ switch (dbnc_ms) {
+ case 0:
+ dbnc = MC13783_BUTTON_DBNC_0MS;
+ break;
+ case 30:
+ dbnc = MC13783_BUTTON_DBNC_30MS;
+ break;
+ case 150:
+ dbnc = MC13783_BUTTON_DBNC_150MS;
+ break;
+ case 750:
+ dbnc = MC13783_BUTTON_DBNC_750MS;
+ break;
+ default:
+ dev_warn(dev, "invalid debounce-delay-ms value\n");
+ continue;
+ }
+
+ if (fwnode_property_read_u32(child, "linux,code", &pdata->b_on_key[idx]))
+ continue;
+
+ if (fwnode_property_read_bool(child, "active-low"))
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_POL_INVERT;
+
+ if (fwnode_property_read_bool(child, "fsl,enable-reset"))
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_RESET_EN;
+
+ pdata->b_on_flags[idx] |= MC13783_BUTTON_ENABLE | dbnc;
+ }
+
+ return pdata;
+}
+#else
+static inline struct mc13xxx_buttons_platform_data __init *mc13xxx_pwrbutton_probe_dt(
+ struct platform_device *pdev)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif
+
+static int __init mc13783_pwrbutton_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
const struct mc13xxx_buttons_platform_data *pdata;
struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
struct mc13xxx_button_devtype *devtype =
@@ -121,9 +196,13 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
int reg = 0;
pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "missing platform data\n");
- return -ENODEV;
+ if (dev->parent->of_node) {
+ pdata = mc13xxx_pwrbutton_probe_dt(pdev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ } else if (!pdata) {
+ dev_err(dev, "missing platform data\n");
+ return -ENODATA;
}
pwr = devm_input_allocate_device(&pdev->dev);
@@ -290,15 +369,14 @@ static const struct platform_device_id mc13xxx_pwrbutton_idtable[] = {
};
static struct platform_driver mc13783_pwrbutton_driver = {
- .id_table = mc13xxx_pwrbutton_idtable,
- .probe = mc13783_pwrbutton_probe,
- .remove = mc13783_pwrbutton_remove,
.driver = {
.name = "mc13783-pwrbutton",
},
+ .id_table = mc13xxx_pwrbutton_idtable,
+ .remove = mc13783_pwrbutton_remove,
};
-module_platform_driver(mc13783_pwrbutton_driver);
+module_platform_driver_probe(mc13783_pwrbutton_driver, mc13783_pwrbutton_probe);
MODULE_ALIAS("platform:mc13783-pwrbutton");
MODULE_DESCRIPTION("MC13783 Power Button");
--
2.39.5
^ permalink raw reply related
* [PATCH v3 0/7] Fix, extend and support OF to mc13xxx pwrbutton
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
Goal of this patch series is to make the mc13892 PWRON1 button usable,
found e.g. on amazon kindle D01100/D01200/EY21 readers.
A ten-year-old IRQ issue needed a fix, mc13783-pwrbutton had to be
extended to the other to mc13xxx PMIC as well (keeping the mc13892
PWRON3 key unsupported for simplicity) and adding OF support.
The implementation has been tested on amazon kindle D01100 and D01200
readers using PWRON1 of a mc13892.
Changes in v3:
- Link to v2: https://lore.kernel.org/linux-input/20250823144441.12654-1-akurz@blala.de/
- Undo all changes to led-control (rename to fsl,led-control), thanks Rob
- Restructured the new buttons node for unevaluatedProperties: false
- Various other remarks from Rob
- Rebase to current state
Changes in v2:
- Link to v1: https://lore.kernel.org/linux-input/20250817102751.29709-1-akurz@blala.de/
- Convert dt-bindings from txt to fsl,mc13xxx.yaml and add vendor prefix
to led-control property, causing changes in dts and driver.
- Change node name from pwrbuttons to buttons
- Change property debounce-delay-value to debounce-delay-ms
- Fixed a section mismatch error
- Fixed https://lore.kernel.org/r/202508210551.VzAtE5re-lkp@intel.com/
(wrong index used when converting to array access)
- Usage of generic device properties API in mc13783-pwrbutton.c
- Provide chip-specific max button id via platform_device_id, therefore
swap patches 3 and 4.
Alexander Kurz (7):
Input: mc13783-pwrbutton: fix irq mixup
Input: mc13783-pwrbutton: use managed resources
Input: mc13783-pwrbutton: convert pdata members to array
Input: mc13783-pwrbutton: enable other mc13xxx PMIC
dt-bindings: mfd: fsl,mc13xxx: convert txt to DT schema
dt-bindings: mfd: fsl,mc13xxx: add buttons node
Input: mc13783-pwrbutton: add OF support
.../devicetree/bindings/mfd/fsl,mc13xxx.yaml | 288 ++++++++++++++++++
.../devicetree/bindings/mfd/mc13xxx.txt | 156 ----------
drivers/input/misc/Kconfig | 4 +-
drivers/input/misc/mc13783-pwrbutton.c | 235 ++++++++++----
include/linux/mfd/mc13783.h | 4 +-
include/linux/mfd/mc13xxx.h | 10 +-
6 files changed, 472 insertions(+), 225 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/mc13xxx.txt
--
2.39.5
^ permalink raw reply
* [PATCH v3 2/7] Input: mc13783-pwrbutton: use managed resources
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Use devres functionality to simplify resource freeing, dev.parent will
be set by devm_input_allocate_device().
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 28 ++++++++------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 4765b25bc9f6..9fd84b8d163d 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -21,6 +21,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/device.h>
#include <linux/errno.h>
#include <linux/input.h>
#include <linux/interrupt.h>
@@ -118,18 +119,13 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
return -ENODEV;
}
- pwr = input_allocate_device();
- if (!pwr) {
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
+ pwr = devm_input_allocate_device(&pdev->dev);
+ if (!pwr)
return -ENOMEM;
- }
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- err = -ENOMEM;
- dev_dbg(&pdev->dev, "Can't allocate power button\n");
- goto free_input_dev;
- }
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
@@ -155,7 +151,7 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
button1_irq, "b1on", priv);
if (err) {
dev_dbg(&pdev->dev, "Can't request irq\n");
- goto free_priv;
+ goto free_mc13xxx_lock;
}
}
@@ -203,7 +199,6 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
pwr->name = "mc13783_pwrbutton";
pwr->phys = "mc13783_pwrbutton/input0";
- pwr->dev.parent = &pdev->dev;
pwr->keycode = priv->keymap;
pwr->keycodemax = ARRAY_SIZE(priv->keymap);
@@ -234,12 +229,8 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
-free_priv:
+free_mc13xxx_lock:
mc13xxx_unlock(mc13783);
- kfree(priv);
-
-free_input_dev:
- input_free_device(pwr);
return err;
}
@@ -261,9 +252,6 @@ static void mc13783_pwrbutton_remove(struct platform_device *pdev)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
mc13xxx_unlock(priv->mc13783);
-
- input_unregister_device(priv->pwr);
- kfree(priv);
}
static struct platform_driver mc13783_pwrbutton_driver = {
--
2.39.5
^ permalink raw reply related
* [PATCH v3 1/7] Input: mc13783-pwrbutton: fix irq mixup
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
The mfd mc13xxx interrupt handling was migrated to regmap with commit
10f9edaeaa30 ("mfd: mc13xxx: Use regmap irq framework for interrupts").
As a consequence, button_irq() will get called with virtual irq instead
of chip-internal irq now. Add wrappers for the three supported interrupts.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 1c7faa9b7afe..4765b25bc9f6 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -88,6 +88,21 @@ static irqreturn_t button_irq(int irq, void *_priv)
return IRQ_HANDLED;
}
+static irqreturn_t button1_irq(int irq, void *_priv)
+{
+ return button_irq(MC13783_IRQ_ONOFD1, _priv);
+}
+
+static irqreturn_t button2_irq(int irq, void *_priv)
+{
+ return button_irq(MC13783_IRQ_ONOFD2, _priv);
+}
+
+static irqreturn_t button3_irq(int irq, void *_priv)
+{
+ return button_irq(MC13783_IRQ_ONOFD3, _priv);
+}
+
static int mc13783_pwrbutton_probe(struct platform_device *pdev)
{
const struct mc13xxx_buttons_platform_data *pdata;
@@ -137,7 +152,7 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1,
- button_irq, "b1on", priv);
+ button1_irq, "b1on", priv);
if (err) {
dev_dbg(&pdev->dev, "Can't request irq\n");
goto free_priv;
@@ -156,7 +171,7 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2,
- button_irq, "b2on", priv);
+ button2_irq, "b2on", priv);
if (err) {
dev_dbg(&pdev->dev, "Can't request irq\n");
goto free_irq_b1;
@@ -175,7 +190,7 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3,
- button_irq, "b3on", priv);
+ button3_irq, "b3on", priv);
if (err) {
dev_dbg(&pdev->dev, "Can't request irq: %d\n", err);
goto free_irq_b2;
--
2.39.5
^ permalink raw reply related
* [PATCH v3 3/7] Input: mc13783-pwrbutton: convert pdata members to array
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
As preparation for mc13783-pwrbutton OF support, convert the members of
mc13xxx_buttons_platform_data to arrays to allow index access within
the next commit.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
drivers/input/misc/mc13783-pwrbutton.c | 54 +++++++++++++-------------
include/linux/mfd/mc13xxx.h | 8 +---
2 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
index 9fd84b8d163d..ace9f286fd24 100644
--- a/drivers/input/misc/mc13783-pwrbutton.c
+++ b/drivers/input/misc/mc13783-pwrbutton.c
@@ -127,24 +127,24 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
- reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
- reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
- reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
+ reg |= (pdata->b_on_flags[0] & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
+ reg |= (pdata->b_on_flags[1] & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
+ reg |= (pdata->b_on_flags[2] & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
priv->pwr = pwr;
priv->mc13783 = mc13783;
mc13xxx_lock(mc13783);
- if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) {
- priv->keymap[0] = pdata->b1on_key;
- if (pdata->b1on_key != KEY_RESERVED)
- __set_bit(pdata->b1on_key, pwr->keybit);
+ if (pdata->b_on_flags[0] & MC13783_BUTTON_ENABLE) {
+ priv->keymap[0] = pdata->b_on_key[0];
+ if (pdata->b_on_key[0] != KEY_RESERVED)
+ __set_bit(pdata->b_on_key[0], pwr->keybit);
- if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT)
+ if (pdata->b_on_flags[0] & MC13783_BUTTON_POL_INVERT)
priv->flags |= MC13783_PWRB_B1_POL_INVERT;
- if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN)
+ if (pdata->b_on_flags[0] & MC13783_BUTTON_RESET_EN)
reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1,
@@ -155,15 +155,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
}
}
- if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) {
- priv->keymap[1] = pdata->b2on_key;
- if (pdata->b2on_key != KEY_RESERVED)
- __set_bit(pdata->b2on_key, pwr->keybit);
+ if (pdata->b_on_flags[1] & MC13783_BUTTON_ENABLE) {
+ priv->keymap[1] = pdata->b_on_key[1];
+ if (pdata->b_on_key[1] != KEY_RESERVED)
+ __set_bit(pdata->b_on_key[1], pwr->keybit);
- if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT)
+ if (pdata->b_on_flags[1] & MC13783_BUTTON_POL_INVERT)
priv->flags |= MC13783_PWRB_B2_POL_INVERT;
- if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN)
+ if (pdata->b_on_flags[1] & MC13783_BUTTON_RESET_EN)
reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2,
@@ -174,15 +174,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
}
}
- if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) {
- priv->keymap[2] = pdata->b3on_key;
- if (pdata->b3on_key != KEY_RESERVED)
- __set_bit(pdata->b3on_key, pwr->keybit);
+ if (pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE) {
+ priv->keymap[2] = pdata->b_on_key[2];
+ if (pdata->b_on_key[2] != KEY_RESERVED)
+ __set_bit(pdata->b_on_key[2], pwr->keybit);
- if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT)
+ if (pdata->b_on_flags[2] & MC13783_BUTTON_POL_INVERT)
priv->flags |= MC13783_PWRB_B3_POL_INVERT;
- if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN)
+ if (pdata->b_on_flags[2] & MC13783_BUTTON_RESET_EN)
reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3,
@@ -218,15 +218,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
free_irq:
mc13xxx_lock(mc13783);
- if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv);
free_irq_b2:
- if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[1] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv);
free_irq_b1:
- if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[0] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv);
free_mc13xxx_lock:
@@ -244,11 +244,11 @@ static void mc13783_pwrbutton_remove(struct platform_device *pdev)
mc13xxx_lock(priv->mc13783);
- if (pdata->b3on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv);
- if (pdata->b2on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[1] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv);
- if (pdata->b1on_flags & MC13783_BUTTON_ENABLE)
+ if (pdata->b_on_flags[0] & MC13783_BUTTON_ENABLE)
mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv);
mc13xxx_unlock(priv->mc13783);
diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
index f372926d5894..0393083af28a 100644
--- a/include/linux/mfd/mc13xxx.h
+++ b/include/linux/mfd/mc13xxx.h
@@ -187,12 +187,8 @@ struct mc13xxx_leds_platform_data {
#define MC13783_BUTTON_RESET_EN (1 << 4)
struct mc13xxx_buttons_platform_data {
- int b1on_flags;
- unsigned short b1on_key;
- int b2on_flags;
- unsigned short b2on_key;
- int b3on_flags;
- unsigned short b3on_key;
+ int b_on_flags[3];
+ unsigned int b_on_key[3];
};
#define MC13783_TS_ATO_FIRST false
--
2.39.5
^ permalink raw reply related
* [PATCH v3 6/7] dt-bindings: mfd: fsl,mc13xxx: add buttons node
From: Alexander Kurz @ 2025-08-29 20:15 UTC (permalink / raw)
To: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Dmitry Torokhov, Dzmitry Sankouski, Dr. David Alan Gilbert,
Heiko Stuebner, Uwe Kleine-König, devicetree, linux-input
Cc: linux-kernel, Alexander Kurz
In-Reply-To: <20250829201517.15374-1-akurz@blala.de>
Add a buttons node and properties describing the "ONOFD" (MC13783) and
"PWRON" (MC13892/MC34708) buttons available in the fsl,mc13xxx PMIC ICs.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
.../devicetree/bindings/mfd/fsl,mc13xxx.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
index 007c2e3eee91..d2886f2686a8 100644
--- a/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
+++ b/Documentation/devicetree/bindings/mfd/fsl,mc13xxx.yaml
@@ -39,6 +39,58 @@ properties:
interrupts:
maxItems: 1
+ buttons:
+ type: object
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^onkey@[0-2]$":
+ $ref: /schemas/input/input.yaml#
+ unevaluatedProperties: false
+ type: object
+
+ properties:
+ reg:
+ description: |
+ One of
+ MC13783 BUTTON IDs:
+ 0: ONOFD1
+ 1: ONOFD2
+ 2: ONOFD3
+
+ MC13892 BUTTON IDs:
+ 0: PWRON1
+ 1: PWRON2
+ 2: PWRON3
+
+ MC34708 BUTTON IDs:
+ 0: PWRON1
+ 1: PWRON2
+ maximum: 2
+
+ debounce-delay-ms:
+ enum: [0, 30, 150, 750]
+ default: 30
+ description:
+ Sets the debouncing delay in milliseconds.
+
+ active-low:
+ description: Set active when pin is pulled low.
+
+ linux,code: true
+
+ fsl,enable-reset:
+ description:
+ Setting of the global reset option.
+ type: boolean
+
+ unevaluatedProperties: false
+
leds:
type: object
$ref: /schemas/leds/common.yaml#
@@ -159,6 +211,12 @@ allOf:
const: fsl,mc34708
then:
properties:
+ buttons:
+ patternProperties:
+ "^onkey@[0-2]$":
+ properties:
+ reg:
+ maximum: 1
leds:
properties:
led-control:
@@ -187,6 +245,18 @@ examples:
fsl,mc13xxx-uses-rtc;
fsl,mc13xxx-uses-adc;
+ buttons {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ onkey@0 {
+ reg = <0>;
+ debounce-delay-ms = <30>;
+ active-low;
+ fsl,enable-reset;
+ };
+ };
+
leds {
#address-cells = <1>;
#size-cells = <0>;
--
2.39.5
^ permalink raw reply related
* Adding Kernel xpad support for Flydigi Apex 5
From: brandon @ 2025-08-29 18:10 UTC (permalink / raw)
To: trivial, vi, linux-input
Hello,
This adds support for the Flydigi Apex 5 controller to the xpad kernel module. This has been tested as part of: https://github.com/paroj/xpad/pull/328
This patch is very small and simple, it simply white lists the controller so it can be used. The trigger and stick mappings are none standard, but SDL has the correct mappings already shipped so it's not a problem in Steam or games I've tried.
Here is the patch:
--- linux/drivers/input/joystick/xpad.c.orig 2025-08-29 18:20:56.157442704 +0100
+++ linux/drivers/input/joystick/xpad.c 2025-08-29 18:19:29.539174760 +0100
@@ -422,6 +422,7 @@ static const struct xpad_device {
{ 0x3537, 0x1010, "GameSir G7 SE", 0, XTYPE_XBOXONE },
{ 0x366c, 0x0005, "ByoWave Proteus Controller", MAP_SHARE_BUTTON, XTYPE_XBOXONE, FLAG_DELAY_INIT },
{ 0x3767, 0x0101, "Fanatec Speedster 3 Forceshock Wheel", 0, XTYPE_XBOX },
+ { 0x37d7, 0x2501, "Flydigi Apex 5", 0, XTYPE_XBOX },
{ 0x413d, 0x2104, "Black Shark Green Ghost Gamepad", 0, XTYPE_XBOX360 },
{ 0xffff, 0xffff, "Chinese-made Xbox Controller", 0, XTYPE_XBOX },
{ 0x0000, 0x0000, "Generic X-Box pad", 0, XTYPE_UNKNOWN }
@@ -578,6 +579,7 @@ static const struct usb_device_id xpad_t
XPAD_XBOX360_VENDOR(0x3537), /* GameSir Controllers */
XPAD_XBOXONE_VENDOR(0x3537), /* GameSir Controllers */
XPAD_XBOXONE_VENDOR(0x366c), /* ByoWave controllers */
+ XPAD_XBOXONE_VENDOR(0x37d7), /* Flydigi Controllers */
XPAD_XBOX360_VENDOR(0x413d), /* Black Shark Green Ghost Controller */
{ }
};
Thanks,
Brandon Lin
^ permalink raw reply
* Re: [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: Rob Herring @ 2025-08-29 16:40 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250827-macsmc-subdevs-v2-2-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:36PM +1000, James Calligeros wrote:
> Apple Silicon devices integrate a vast array of sensors, monitoring
> current, power, temperature, and voltage across almost every part of
> the system. The sensors themselves are all connected to the System
> Management Controller (SMC). The SMC firmware exposes the data
> reported by these sensors via its standard FourCC-based key-value
> API. The SMC is also responsible for monitoring and controlling any
> fans connected to the system, exposing them in the same way.
>
> For reasons known only to Apple, each device exposes its sensors with
> an almost totally unique set of keys. This is true even for devices
> which share an SoC. An M1 Mac mini, for example, will report its core
> temperatures on different keys to an M1 MacBook Pro. Worse still, the
> SMC does not provide a way to enumerate the available keys at runtime,
> nor do the keys follow any sort of reasonable or consistent naming
> rules that could be used to deduce their purpose. We must therefore
> know which keys are present on any given device, and which function
> they serve, ahead of time.
>
> Add a schema so that we can describe the available sensors for a given
> Apple Silicon device in the Devicetree.
>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> .../bindings/hwmon/apple,smc-hwmon.yaml | 132 +++++++++++++++++++++++++
> .../bindings/mfd/apple,smc.yaml | 36 +++++++
> MAINTAINERS | 1 +
> 3 files changed, 169 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..08cc4f55f3a41ca8b3b428088f96240266fa42e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SMC Hardware Monitoring
> +
> +description:
> + Apple's System Management Controller (SMC) exposes a vast array of
> + hardware monitoring sensors, including temperature probes, current and
> + voltage sense, power meters, and fan speeds. It also provides endpoints
> + to manually control the speed of each fan individually. Each Apple
> + Silicon device exposes a different set of endpoints via SMC keys. This
> + is true even when two machines share an SoC. The CPU core temperature
> + sensor keys on an M1 Mac mini are different to those on an M1 MacBook
> + Pro, for example.
> +
> +maintainers:
> + - James Calligeros <jcalligeros99@gmail.com>
> +
> +definitions:
$defs
definitions was convention. $defs is in json-schema spec now.
> + apple,key-id:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: The SMC FourCC key of the desired sensor.
> + Must match the node's suffix.
> +
> + label:
> + description: Human-readable name for the sensor
> +
> +properties:
> + compatible:
> + const: apple,smc-hwmon
> +
> +patternProperties:
> + "^current-[A-Za-z0-9]{4}$":
> + type: object
> + additionalProperties: false
> +
> + properties:
> + apple,key-id:
> + $ref: "#/definitions/apple,key-id"
> +
> + label:
> + $ref: "#/definitions/label"
> +
> + required:
> + - apple,key-id
> + - label
This should be something like this:
"^current-[A-Za-z0-9]{4}$":
$ref: "#/$defs/sensor"
unevaluatedProperties: false
With the $defs/sensor being:
$defs:
sensor:
type: object
properties:
apple,key-id:
$ref: /schemas/types.yaml#/definitions/string
pattern: "^[A-Za-z0-9]{4}$"
description:
The SMC FourCC key of the desired sensor. Must match the
node's suffix.
label:
description: Human-readable name for the sensor
required:
- apple,key-id
- label
Though in general, 'label' should never be required being just for human
convenience.
> +
> + "^fan-[A-Za-z0-9]{4}$":
> + type: object
> + additionalProperties: false
And this one the same as above, but with the additional fan properties
listed here.
> +
> + properties:
> + apple,key-id:
> + $ref: "#/definitions/apple,key-id"
> +
> + apple,fan-minimum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: SMC key containing the fan's minimum speed
> +
> + apple,fan-maximum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: SMC key containing the fan's maximum speed
> +
> + apple,fan-target:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: Writeable endpoint for setting desired fan speed
> +
> + apple,fan-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: Writeable key to enable/disable manual fan control
> +
> + label:
> + $ref: "#/definitions/label"
> +
> + required:
> + - apple,key-id
> + - label
> +
> + "^power-[A-Za-z0-9]{4}$":
> + type: object
> + additionalProperties: false
> +
> + properties:
> + apple,key-id:
> + $ref: "#/definitions/apple,key-id"
> +
> + label:
> + $ref: "#/definitions/label"
> +
> + required:
> + - apple,key-id
> + - label
> +
> + "^temperature-[A-Za-z0-9]{4}$":
> + type: object
> + additionalProperties: false
> +
> + properties:
> + apple,key-id:
> + $ref: "#/definitions/apple,key-id"
> +
> + label:
> + $ref: "#/definitions/label"
> +
> + required:
> + - apple,key-id
> + - label
> +
> + "^voltage-[A-Za-z0-9]{4}$":
> + type: object
> + additionalProperties: false
> +
> + properties:
> + apple,key-id:
> + $ref: "#/definitions/apple,key-id"
> +
> + label:
> + $ref: "#/definitions/label"
> +
> + required:
> + - apple,key-id
> + - label
> +
> +additionalProperties: false
^ permalink raw reply
* Re: Bug report - Sticky keys acting not sticky sometimes
From: Alerymin @ 2025-08-29 12:50 UTC (permalink / raw)
To: Thorsten Leemhuis; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <27863761-d747-459a-af85-18abe207c0ca@leemhuis.info>
[-- Attachment #1.1: Type: text/plain, Size: 1668 bytes --]
On Friday, August 29th, 2025 at 12:38, Thorsten Leemhuis <regressions@leemhuis.info> wrote:
>
>
> On 29.08.25 14:05, Alerymin wrote:
>
> > On Friday, August 29th, 2025 at 11:58, Thorsten Leemhuis
> > regressions@leemhuis.info wrote:
> >
> > > On 29.08.25 13:36, Alerymin wrote:
> >
> > Well, I read the reporting page a bit too quickly so I CCed the
> > regressions list while I shouldn't have, sorry. I've never seen it
> > work normally, it's not a regression.
>
>
> Happens, no worries. But in that case a hint to anyone that might reply
> to this thread: feel free to drop the regressions list and the stable
> list (it's off-topic there, too)
>
> > On a github issues on kbd-
> > project legionus had a few ideas about it: https://github.com/
> > legionus/kbd/issues/140
>
>
> FWIW, some developers do not follow links in cases like this, as they
> get a lot of reports every day and not much time at hand (and walking
> through other tickets often is cumbersome). You chances will thus be
> higher is you summarize the important bits in a case like this; then
> people might follow the link if they really care.
>
> Good luck! Ciao, Thorsten
From the github issue. The problem may come from `drivers/tty/vt/keyboard.c`
Legionus says maybe it misses `slockstate = 0` around line 1492 before the return.
To test it I have a keyboard layout using that: https://codeberg.org/Alerymin/kbd-qwerty-lafayette
Just make sure you change the CtrlL to SCtrlL in the fr-lafayette.map so it registers a Sticky key (I had to keep a working version).
Load it with `loadkeys fr-lafayette.map`.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 343 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] platform/chrome: Fix a race when probing drivers
From: Tzung-Bi Shih @ 2025-08-29 12:50 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Benson Leung, linux-input, chrome-platform
In-Reply-To: <sqgfgwmbpxvaszyxt4mymne6dvhzjvuifogsqjdu6j3tm436ph@x7chldp3dfpr>
On Fri, Aug 29, 2025 at 11:28:55AM +0000, Dmitry Torokhov wrote:
> On Thu, Aug 28, 2025 at 08:35:56AM +0000, Tzung-Bi Shih wrote:
> > A race is observed when cros_ec_lpc and cros-ec-keyb are all built as
> > modules. cros_ec_lpc is cros-ec-keyb's parent. However, they can be
> > probed at the same time.
> >
> > Example:
> >
> > + -----------------------------------------------------------------+
> > | Some init process (e.g. udevd) | deferred_probe_work_func worker |
> > + -----------------------------------------------------------------+
> > | Probe cros-ec-keyb. | |
> > | - Decide to defer[1]. | |
> > | | A device bound to a driver[2]. |
> > | Probe cros_ec_lpc. | |
> > | - Init the struct[3]. | |
> > | | Retry cros-ec-keyb from the |
> > | | deferred list[4]. |
> > | | - Won't defer again as [3]. |
> > | | - Access uninitialized data in |
> > | | the struct. |
> > | - Register the device. | |
> > + -----------------------------------------------------------------+
> >
> > [1] https://elixir.bootlin.com/linux/v6.16/source/drivers/input/keyboard/cros_ec_keyb.c#L707
> > [2] https://elixir.bootlin.com/linux/v6.16/source/drivers/base/dd.c#L405
> > [3] https://elixir.bootlin.com/linux/v6.16/source/drivers/platform/chrome/cros_ec_lpc.c#L644
> > [4] https://elixir.bootlin.com/linux/v6.16/source/drivers/base/dd.c#L418
> >
> > Note that the device link[5] can't help as in the observed environment,
> > the devices are already added via device_add()[6].
> >
> > [5] https://www.kernel.org/doc/html/latest/driver-api/device_link.html#usage
> > [6] https://elixir.bootlin.com/linux/v6.16/source/drivers/acpi/acpi_platform.c#L177
> >
> > The series fixes the issue by ensuring the struct is ready for accessing
> > before continuing to probe cros-ec-keyb.
>
> Why is the keyboard platform device instantiated before the transport
> (cros_ec_lpc) is done initializing? I think this is the root of the
> issue...
I may misunderstand but it seems to me:
- The ACPI bus enumerated and instantiated the platform devices[6] first.
- The keyboard platform device was probed when `cros_ec_keyb_driver`
registered. It deferred as its parent's drvdata was NULL[1].
- The transport platform device was probed when `cros_ec_lpc_driver`
registered. It set the drvdata[3].
- The keyboard platform device was probed again from retrying the deferred
list, by another thread `deferred_probe_work_func`. The parent's drvdata
wasn't NULL and cros_ec_register() for the transport device weren't
finished. The race happened.
^ permalink raw reply
* Re: Bug report - Sticky keys acting not sticky sometimes
From: Thorsten Leemhuis @ 2025-08-29 12:38 UTC (permalink / raw)
To: Alerymin; +Cc: stable, regressions, dmitry.torokhov, linux-input
In-Reply-To: <175646914218.7.12773379431621187280.877650584@simplelogin.com>
On 29.08.25 14:05, Alerymin wrote:
> On Friday, August 29th, 2025 at 11:58, Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>> On 29.08.25 13:36, Alerymin wrote:
>
> Well, I read the reporting page a bit too quickly so I CCed the
> regressions list while I shouldn't have, sorry. I've never seen it
> work normally, it's not a regression.
Happens, no worries. But in that case a hint to anyone that might reply
to this thread: feel free to drop the regressions list and the stable
list (it's off-topic there, too)
> On a github issues on kbd-
> project legionus had a few ideas about it: https://github.com/
> legionus/kbd/issues/140
FWIW, some developers do not follow links in cases like this, as they
get a lot of reports every day and not much time at hand (and walking
through other tickets often is cumbersome). You chances will thus be
higher is you summarize the important bits in a case like this; then
people might follow the link if they really care.
Good luck! Ciao, Thorsten
^ permalink raw reply
* Re: Bug report - Sticky keys acting not sticky sometimes
From: Alerymin @ 2025-08-29 12:05 UTC (permalink / raw)
To: Thorsten Leemhuis; +Cc: stable, regressions, dmitry.torokhov, linux-input
In-Reply-To: <dd24398b-0d10-45d4-b93d-4377c017f2e7@leemhuis.info>
[-- Attachment #1.1: Type: text/plain, Size: 1590 bytes --]
On Friday, August 29th, 2025 at 11:58, Thorsten Leemhuis <regressions@leemhuis.info> wrote:
>
>
> Lo!
>
> On 29.08.25 13:36, Alerymin wrote:
>
> > Note:
> > The issue looks like it's from tty directly but I don't see who is the maintainer, so I email the closest I can get
> >
> > Description:
> > In command line, sticky keys reset only when typing ASCII and ISO-8859-1 characters.
> > Tested with the QWERTY Lafayette layout: https://codeberg.org/Alerymin/kbd-qwerty-lafayette
> >
> > Observed Behaviour:
> > When the layout is loaded in ISO-8859-15, most characters typed don't reset the sticky key, unless it's basic ASCII characters or Unicode
> > When the layout is loaded in ISO-8859-1, the sticky key works fine.
> >
> > Expected behaviour:
> > Sticky key working in ISO-8859-1 and ISO-8859-15
> >
> > System used:
> > Arch Linux, kernel 6.16.3-arch1-1
>
>
> Thx for the report. You CCed the regressions list, so please allow me to
> ask: what's the last version where this worked? And FWIW in case nobody
> comes around with an idea what might cause this any time soon: could you
> bisect[1] what's causing the problem? Ciao, Thorsten
>
> [1]
> https://docs.kernel.org/admin-guide/verify-bugs-and-bisect-regressions.html
Well, I read the reporting page a bit too quickly so I CCed the regressions list while I shouldn't have, sorry. I've never seen it work normally, it's not a regression.
On a github issues on kbd-project legionus had a few ideas about it: https://github.com/legionus/kbd/issues/140
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 343 bytes --]
^ permalink raw reply
* Re: Bug report - Sticky keys acting not sticky sometimes
From: Thorsten Leemhuis @ 2025-08-29 11:57 UTC (permalink / raw)
To: Alerymin, stable; +Cc: regressions, dmitry.torokhov, linux-input
In-Reply-To: <175646738541.6.2676742517164037652.877606794@simplelogin.com>
Lo!
On 29.08.25 13:36, Alerymin wrote:
> Note:
> The issue looks like it's from tty directly but I don't see who is the maintainer, so I email the closest I can get
>
> Description:
> In command line, sticky keys reset only when typing ASCII and ISO-8859-1 characters.
> Tested with the QWERTY Lafayette layout: https://codeberg.org/Alerymin/kbd-qwerty-lafayette
>
> Observed Behaviour:
> When the layout is loaded in ISO-8859-15, most characters typed don't reset the sticky key, unless it's basic ASCII characters or Unicode
> When the layout is loaded in ISO-8859-1, the sticky key works fine.
>
> Expected behaviour:
> Sticky key working in ISO-8859-1 and ISO-8859-15
>
> System used:
> Arch Linux, kernel 6.16.3-arch1-1
Thx for the report. You CCed the regressions list, so please allow me to
ask: what's the last version where this worked? And FWIW in case nobody
comes around with an idea what might cause this any time soon: could you
bisect[1] what's causing the problem? Ciao, Thorsten
[1]
https://docs.kernel.org/admin-guide/verify-bugs-and-bisect-regressions.html
^ permalink raw reply
* Re: Bug report - Sticky keys acting not sticky sometimes
From: Mathieu @ 2025-08-29 11:36 UTC (permalink / raw)
To: stable; +Cc: regressions, dmitry.torokhov, linux-input
[-- Attachment #1.1: Type: text/plain, Size: 672 bytes --]
Note:
The issue looks like it's from tty directly but I don't see who is the maintainer, so I email the closest I can get
Description:
In command line, sticky keys reset only when typing ASCII and ISO-8859-1 characters.
Tested with the QWERTY Lafayette layout: https://codeberg.org/Alerymin/kbd-qwerty-lafayette
Observed Behaviour:
When the layout is loaded in ISO-8859-15, most characters typed don't reset the sticky key, unless it's basic ASCII characters or Unicode
When the layout is loaded in ISO-8859-1, the sticky key works fine.
Expected behaviour:
Sticky key working in ISO-8859-1 and ISO-8859-15
System used:
Arch Linux, kernel 6.16.3-arch1-1
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 343 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] platform/chrome: Fix a race when probing drivers
From: Dmitry Torokhov @ 2025-08-29 11:28 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, linux-input, chrome-platform
In-Reply-To: <20250828083601.856083-1-tzungbi@kernel.org>
Hi Tzung-Bi,
On Thu, Aug 28, 2025 at 08:35:56AM +0000, Tzung-Bi Shih wrote:
> A race is observed when cros_ec_lpc and cros-ec-keyb are all built as
> modules. cros_ec_lpc is cros-ec-keyb's parent. However, they can be
> probed at the same time.
>
> Example:
>
> + -----------------------------------------------------------------+
> | Some init process (e.g. udevd) | deferred_probe_work_func worker |
> + -----------------------------------------------------------------+
> | Probe cros-ec-keyb. | |
> | - Decide to defer[1]. | |
> | | A device bound to a driver[2]. |
> | Probe cros_ec_lpc. | |
> | - Init the struct[3]. | |
> | | Retry cros-ec-keyb from the |
> | | deferred list[4]. |
> | | - Won't defer again as [3]. |
> | | - Access uninitialized data in |
> | | the struct. |
> | - Register the device. | |
> + -----------------------------------------------------------------+
>
> [1] https://elixir.bootlin.com/linux/v6.16/source/drivers/input/keyboard/cros_ec_keyb.c#L707
> [2] https://elixir.bootlin.com/linux/v6.16/source/drivers/base/dd.c#L405
> [3] https://elixir.bootlin.com/linux/v6.16/source/drivers/platform/chrome/cros_ec_lpc.c#L644
> [4] https://elixir.bootlin.com/linux/v6.16/source/drivers/base/dd.c#L418
>
> Note that the device link[5] can't help as in the observed environment,
> the devices are already added via device_add()[6].
>
> [5] https://www.kernel.org/doc/html/latest/driver-api/device_link.html#usage
> [6] https://elixir.bootlin.com/linux/v6.16/source/drivers/acpi/acpi_platform.c#L177
>
> The series fixes the issue by ensuring the struct is ready for accessing
> before continuing to probe cros-ec-keyb.
Why is the keyboard platform device instantiated before the transport
(cros_ec_lpc) is done initializing? I think this is the root of the
issue...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 07/11] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Dmitry Torokhov @ 2025-08-29 11:11 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Belloni, Jean Delvare, Guenter Roeck, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, Hector Martin
In-Reply-To: <20250827-macsmc-subdevs-v2-7-ce5e99d54c28@gmail.com>
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
^ permalink raw reply
* Re: [PATCH V0 1/2] hyper-v: Add CONFIG_HYPERV_VMBUS option
From: Nuno Das Neves @ 2025-08-29 0:29 UTC (permalink / raw)
To: Mukesh Rathor, dri-devel, linux-kernel, linux-input, linux-hyperv,
netdev, linux-pci, linux-scsi, linux-fbdev, linux-arch,
virtualization
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, gregkh, deller, arnd, sgarzare,
horms
In-Reply-To: <20250828005952.884343-2-mrathor@linux.microsoft.com>
On 8/27/2025 5:59 PM, Mukesh Rathor wrote:
> Somehow vmbus driver is hinged on CONFIG_HYPERV. It appears this is initial
> code that did not get addressed when the scope of CONFIG_HYPERV went beyond
> vmbus. This commit creates a fine grained HYPERV_VMBUS option and updates
> drivers that depend on VMBUS.
>
The commit message can be improved. The docs are helpful here:
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
In particular, some clearer reasons for the change.
e.g.
- CONFIG_HYPERV encompasses too much right now. It's not always clear what
depends on builtin hyperv code and what depends on vmbus.
- Since there is so much builtin hyperv code, building CONFIG_HYPERV as a
module doesn't make intuitive sense. Building vmbus support as a module does.
- There are actually some real scenarios someone may want to compile with
CONFIG_HYPERV but without vmbus, like baremetal root partition.
FWIW I think it's a good idea, interested to hear what others think.
Nuno
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> drivers/gpu/drm/Kconfig | 2 +-
> drivers/hid/Kconfig | 2 +-
> drivers/hv/Kconfig | 12 +++++++++---
> drivers/hv/Makefile | 2 +-
> drivers/input/serio/Kconfig | 4 ++--
> drivers/net/hyperv/Kconfig | 2 +-
> drivers/pci/Kconfig | 2 +-
> drivers/scsi/Kconfig | 2 +-
> drivers/uio/Kconfig | 2 +-
> drivers/video/fbdev/Kconfig | 2 +-
> include/asm-generic/mshyperv.h | 8 +++++---
> net/vmw_vsock/Kconfig | 2 +-
> 12 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index f7ea8e895c0c..58f34da061c6 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -398,7 +398,7 @@ source "drivers/gpu/drm/imagination/Kconfig"
>
> config DRM_HYPERV
> tristate "DRM Support for Hyper-V synthetic video device"
> - depends on DRM && PCI && HYPERV
> + depends on DRM && PCI && HYPERV_VMBUS
> select DRM_CLIENT_SELECTION
> select DRM_KMS_HELPER
> select DRM_GEM_SHMEM_HELPER
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a57901203aeb..fe3dc8c0db99 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1162,7 +1162,7 @@ config GREENASIA_FF
>
> config HID_HYPERV_MOUSE
> tristate "Microsoft Hyper-V mouse driver"
> - depends on HYPERV
> + depends on HYPERV_VMBUS
> help
> Select this option to enable the Hyper-V mouse driver.
>
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 2e8df09db599..08c4ed005137 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -44,18 +44,24 @@ config HYPERV_TIMER
>
> config HYPERV_UTILS
> tristate "Microsoft Hyper-V Utilities driver"
> - depends on HYPERV && CONNECTOR && NLS
> + depends on HYPERV_VMBUS && CONNECTOR && NLS
> depends on PTP_1588_CLOCK_OPTIONAL
> help
> Select this option to enable the Hyper-V Utilities.
>
> config HYPERV_BALLOON
> tristate "Microsoft Hyper-V Balloon driver"
> - depends on HYPERV
> + depends on HYPERV_VMBUS
> select PAGE_REPORTING
> help
> Select this option to enable Hyper-V Balloon driver.
>
> +config HYPERV_VMBUS
> + tristate "Microsoft Hyper-V Vmbus driver"
> + depends on HYPERV
> + help
> + Select this option to enable Hyper-V Vmbus driver.
> +
> config MSHV_ROOT
> tristate "Microsoft Hyper-V root partition support"
> depends on HYPERV && (X86_64 || ARM64)
> @@ -75,7 +81,7 @@ config MSHV_ROOT
>
> config MSHV_VTL
> tristate "Microsoft Hyper-V VTL driver"
> - depends on X86_64 && HYPERV_VTL_MODE
> + depends on X86_64 && HYPERV_VTL_MODE && HYPERV_VMBUS
> # Mapping VTL0 memory to a userspace process in VTL2 is supported in OpenHCL.
> # VTL2 for OpenHCL makes use of Huge Pages to improve performance on VMs,
> # specially with large memory requirements.
> diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
> index c53a0df746b7..050517756a82 100644
> --- a/drivers/hv/Makefile
> +++ b/drivers/hv/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_HYPERV) += hv_vmbus.o
> +obj-$(CONFIG_HYPERV_VMBUS) += hv_vmbus.o
> obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
> obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
> obj-$(CONFIG_MSHV_ROOT) += mshv_root.o
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index 17edc1597446..c7ef347a4dff 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -276,8 +276,8 @@ config SERIO_OLPC_APSP
>
> config HYPERV_KEYBOARD
> tristate "Microsoft Synthetic Keyboard driver"
> - depends on HYPERV
> - default HYPERV
> + depends on HYPERV_VMBUS
> + default HYPERV_VMBUS
> help
> Select this option to enable the Hyper-V Keyboard driver.
>
> diff --git a/drivers/net/hyperv/Kconfig b/drivers/net/hyperv/Kconfig
> index c8cbd85adcf9..982964c1a9fb 100644
> --- a/drivers/net/hyperv/Kconfig
> +++ b/drivers/net/hyperv/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config HYPERV_NET
> tristate "Microsoft Hyper-V virtual network driver"
> - depends on HYPERV
> + depends on HYPERV_VMBUS
> select UCS2_STRING
> select NLS
> help
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 9a249c65aedc..7065a8e5f9b1 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -221,7 +221,7 @@ config PCI_LABEL
>
> config PCI_HYPERV
> tristate "Hyper-V PCI Frontend"
> - depends on ((X86 && X86_64) || ARM64) && HYPERV && PCI_MSI && SYSFS
> + depends on ((X86 && X86_64) || ARM64) && HYPERV_VMBUS && PCI_MSI && SYSFS
> select PCI_HYPERV_INTERFACE
> select IRQ_MSI_LIB
> help
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 5522310bab8d..19d0884479a2 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -589,7 +589,7 @@ config XEN_SCSI_FRONTEND
>
> config HYPERV_STORAGE
> tristate "Microsoft Hyper-V virtual storage driver"
> - depends on SCSI && HYPERV
> + depends on SCSI && HYPERV_VMBUS
> depends on m || SCSI_FC_ATTRS != m
> default HYPERV
> help
> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> index b060dcd7c635..6f86a61231e6 100644
> --- a/drivers/uio/Kconfig
> +++ b/drivers/uio/Kconfig
> @@ -140,7 +140,7 @@ config UIO_MF624
>
> config UIO_HV_GENERIC
> tristate "Generic driver for Hyper-V VMBus"
> - depends on HYPERV
> + depends on HYPERV_VMBUS
> help
> Generic driver that you can bind, dynamically, to any
> Hyper-V VMBus device. It is useful to provide direct access
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index c21484d15f0c..72c63eaeb983 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1774,7 +1774,7 @@ config FB_BROADSHEET
>
> config FB_HYPERV
> tristate "Microsoft Hyper-V Synthetic Video support"
> - depends on FB && HYPERV
> + depends on FB && HYPERV_VMBUS
> select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA
> select FB_IOMEM_HELPERS_DEFERRED
> help
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 1d2ad1304ad4..66c58c91b530 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -165,6 +165,7 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
>
> void __init hv_mark_resources(void);
>
> +#if IS_ENABLED(CONFIG_HYPERV_VMBUS)
> /* Free the message slot and signal end-of-message if required */
> static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> {
> @@ -200,6 +201,10 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> }
> }
>
> +extern int vmbus_interrupt;
> +extern int vmbus_irq;
> +#endif /* CONFIG_HYPERV_VMBUS */
> +
> int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
>
> void hv_setup_vmbus_handler(void (*handler)(void));
> @@ -213,9 +218,6 @@ void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
> void hv_remove_crash_handler(void);
> void hv_setup_mshv_handler(void (*handler)(void));
>
> -extern int vmbus_interrupt;
> -extern int vmbus_irq;
> -
> #if IS_ENABLED(CONFIG_HYPERV)
> /*
> * Hypervisor's notion of virtual processor ID is different from
> diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
> index 56356d2980c8..8e803c4828c4 100644
> --- a/net/vmw_vsock/Kconfig
> +++ b/net/vmw_vsock/Kconfig
> @@ -72,7 +72,7 @@ config VIRTIO_VSOCKETS_COMMON
>
> config HYPERV_VSOCKETS
> tristate "Hyper-V transport for Virtual Sockets"
> - depends on VSOCKETS && HYPERV
> + depends on VSOCKETS && HYPERV_VMBUS
> help
> This module implements a Hyper-V transport for Virtual Sockets.
>
^ permalink raw reply
* Re: [PATCH v2 05/11] hwmon: Add Apple Silicon SMC hwmon driver
From: Guenter Roeck @ 2025-08-28 22:19 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Belloni, Jean Delvare, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250827-macsmc-subdevs-v2-5-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:39PM +1000, James Calligeros wrote:
> The System Management Controller on Apple Silicon devices is responsible
> for integrating and exposing the data reported by the vast array of
> hardware monitoring sensors present on these devices. It is also
> responsible for fan control, and allows users to manually set fan
> speeds if they so desire. Add a hwmon driver to expose current,
> power, temperature, and voltage monitoring sensors, as well as
> fan speed monitoring and control via the SMC on Apple Silicon devices.
>
> The SMC firmware has no consistency between devices, even when they
> share an SoC. The FourCC keys used to access sensors are almost
> random. An M1 Mac mini will have different FourCCs for its CPU core
> temperature sensors to an M1 MacBook Pro, for example. For this
> reason, the valid sensors for a given device are specified in a
> child of the SMC Devicetree node. The driver uses this information
> to determine which sensors to make available at runtime.
>
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc_hwmon.c | 848 +++++++++++++++++++++++++
Documentation/hwmon/macsmc_hwmon missing
> 4 files changed, 862 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5d53d243dc9abdf1db5865f8e6bcddbac3eafebe..2eb23522654dd050262eb06e077587030cc335aa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2408,6 +2408,7 @@ F: drivers/clk/clk-apple-nco.c
> F: drivers/cpufreq/apple-soc-cpufreq.c
> F: drivers/dma/apple-admac.c
> F: drivers/gpio/gpio-macsmc.c
> +F: drivers/hwmon/macsmc_hwmon.c
> F: drivers/pmdomain/apple/
> F: drivers/i2c/busses/i2c-pasemi-core.c
> F: drivers/i2c/busses/i2c-pasemi-platform.c
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..1ca6db71e4d9da32717fd14487c10944433ada41 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1164,6 +1164,18 @@ config SENSORS_LTQ_CPUTEMP
> If you say yes here you get support for the temperature
> sensor inside your CPU.
>
> +config SENSORS_MACSMC_HWMON
> + tristate "Apple SMC (Apple Silicon)"
> + depends on MFD_MACSMC && OF
> + help
> + This driver enables hwmon support for current, power, temperature,
> + and voltage sensors, as well as fan speed reporting and control
> + on Apple Silicon devices. Say Y here if you have an Apple Silicon
> + device.
> +
> + This driver can also be built as a module. If so, the module will
> + be called macsmc_hwmon.
> +
> config SENSORS_MAX1111
> tristate "Maxim MAX1111 Serial 8-bit ADC chip and compatibles"
> depends on SPI_MASTER
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..80fc8447aff15b3b8e8583dc755c8accb3b6a93e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_LTC4260) += ltc4260.o
> obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o
> obj-$(CONFIG_SENSORS_LTC4282) += ltc4282.o
> obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
> +obj-$(CONFIG_SENSORS_MACSMC_HWMON) += macsmc_hwmon.o
> obj-$(CONFIG_SENSORS_MAX1111) += max1111.o
> obj-$(CONFIG_SENSORS_MAX127) += max127.o
> obj-$(CONFIG_SENSORS_MAX16065) += max16065.o
> diff --git a/drivers/hwmon/macsmc_hwmon.c b/drivers/hwmon/macsmc_hwmon.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..e44ea357870238aa0b38d7b674a6c456271fdf0c
> --- /dev/null
> +++ b/drivers/hwmon/macsmc_hwmon.c
> @@ -0,0 +1,848 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple SMC hwmon driver for Apple Silicon platforms
> + *
> + * The System Management Controller on Apple Silicon devices is responsible for
> + * measuring data from sensors across the SoC and machine. These include power,
> + * temperature, voltage and current sensors. Some "sensors" actually expose
> + * derived values. An example of this is the key PHPC, which is an estimate
> + * of the heat energy being dissipated by the SoC.
> + *
> + * While each SoC only has one SMC variant, each platform exposes a different
> + * set of sensors. For example, M1 MacBooks expose battery telemetry sensors
> + * which are not present on the M1 Mac mini. For this reason, the available
> + * sensors for a given platform are described in the device tree in a child
> + * node of the SMC device. We must walk this list of available sensors and
> + * populate the required hwmon data structures at runtime.
> + *
> + * Originally based on a concept by Jean-Francois Bortolotti <jeff@borto.fr>
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
Unnecessary include file
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define MAX_LABEL_LENGTH 32
> +#define NUM_SENSOR_TYPES 5 /* temp, volt, current, power, fan */
> +
> +#define FLT_EXP_BIAS 127
> +#define FLT_EXP_MASK GENMASK(30, 23)
> +#define FLT_MANT_BIAS 23
> +#define FLT_MANT_MASK GENMASK(22, 0)
> +#define FLT_SIGN_MASK BIT(31)
> +
> +static bool fan_control;
> +module_param_unsafe(fan_control, bool, 0644);
> +MODULE_PARM_DESC(fan_control,
> + "Override the SMC to set your own fan speeds on supported machines");
> +
> +struct macsmc_hwmon_sensor {
> + struct apple_smc_key_info info;
> + smc_key macsmc_key;
> + char label[MAX_LABEL_LENGTH];
> +};
> +
> +struct macsmc_hwmon_fan {
> + struct macsmc_hwmon_sensor now;
> + struct macsmc_hwmon_sensor min;
> + struct macsmc_hwmon_sensor max;
> + struct macsmc_hwmon_sensor set;
> + struct macsmc_hwmon_sensor mode;
> + char label[MAX_LABEL_LENGTH];
> + u32 attrs;
> + bool manual;
> +};
> +
> +struct macsmc_hwmon_sensors {
> + struct hwmon_channel_info channel_info;
> + struct macsmc_hwmon_sensor *sensors;
> + u32 count;
> +};
> +
> +struct macsmc_hwmon_fans {
> + struct hwmon_channel_info channel_info;
> + struct macsmc_hwmon_fan *fans;
> + u32 count;
> +};
> +
> +struct macsmc_hwmon {
> + struct device *dev;
> + struct apple_smc *smc;
> + struct device *hwmon_dev;
> + struct hwmon_chip_info chip_info;
> + /* Chip + sensor types + NULL */
> + const struct hwmon_channel_info *channel_infos[1 + NUM_SENSOR_TYPES + 1];
> + struct macsmc_hwmon_sensors temp;
> + struct macsmc_hwmon_sensors volt;
> + struct macsmc_hwmon_sensors curr;
> + struct macsmc_hwmon_sensors power;
> + struct macsmc_hwmon_fans fan;
> +};
> +
> +static int macsmc_hwmon_read_label(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_temp:
> + if (channel >= hwmon->temp.count)
> + return -EINVAL;
Wrong. Such channels should not be instantiated. It is completely
unacceptable for the "sensors" command to report such errors.
Also more below on the use of -EINVAL when readin values.
> + *str = hwmon->temp.sensors[channel].label;
> + break;
> + case hwmon_in:
> + if (channel >= hwmon->volt.count)
> + return -EINVAL;
> + *str = hwmon->volt.sensors[channel].label;
> + break;
> + case hwmon_curr:
> + if (channel >= hwmon->curr.count)
> + return -EINVAL;
> + *str = hwmon->curr.sensors[channel].label;
> + break;
> + case hwmon_power:
> + if (channel >= hwmon->power.count)
> + return -EINVAL;
> + *str = hwmon->power.sensors[channel].label;
> + break;
> + case hwmon_fan:
> + if (channel >= hwmon->fan.count)
> + return -EINVAL;
> + *str = hwmon->fan.fans[channel].label;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * A number of sensors report data in a 48.16 fixed-point decimal format that is
> + * not used by any other function of the SMC.
> + */
> +static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
> + u64 *p, int scale)
> +{
> + u64 val;
> + int ret;
> +
> + ret = apple_smc_read_u64(smc, key, &val);
> + if (ret < 0)
> + return ret;
> +
> + *p = mult_frac(val, scale, 65536);
> +
> + return 0;
> +}
> +
> +/*
> + * Many sensors report their data as IEEE-754 floats. No other SMC function uses
> + * them.
> + */
> +static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
> + int *p, int scale)
> +{
> + u32 fval;
> + u64 val;
> + int ret, exp;
> +
> + ret = apple_smc_read_u32(smc, key, &fval);
> + if (ret < 0)
> + return ret;
> +
> + val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
> + exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
> + if (scale < 0) {
> + val <<= 32;
> + exp -= 32;
> + val /= -scale;
> + } else {
> + val *= scale;
> + }
> +
> + if (exp > 63)
> + val = U64_MAX;
> + else if (exp < -63)
> + val = 0;
> + else if (exp < 0)
> + val >>= -exp;
> + else if (exp != 0 && (val & ~((1UL << (64 - exp)) - 1))) /* overflow */
> + val = U64_MAX;
> + else
> + val <<= exp;
> +
> + if (fval & FLT_SIGN_MASK) {
> + if (val > (-(s64)INT_MIN))
> + *p = INT_MIN;
> + else
> + *p = -val;
> + } else {
> + if (val > INT_MAX)
> + *p = INT_MAX;
> + else
> + *p = val;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * The SMC has keys of multiple types, denoted by a FourCC of the same format
> + * as the key ID. We don't know what data type a key encodes until we poke at it.
> + */
> +static int macsmc_hwmon_read_key(struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor, int scale,
> + long *val)
> +{
> + int ret;
> +
> + switch (sensor->info.type_code) {
> + /* 32-bit IEEE 754 float */
> + case _SMC_KEY("flt "): {
> + u32 flt_ = 0;
> +
> + ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
> + &flt_, scale);
if (ret)
return ret;
> + *val = flt_;
> + break;
> + }
> + /* 48.16 fixed point decimal */
> + case _SMC_KEY("ioft"): {
> + u64 ioft = 0;
> +
> + ret = macsmc_hwmon_read_ioft_scaled(smc, sensor->macsmc_key,
> + &ioft, scale);
if (ret)
return ret;
> + *val = (long)ioft;
> + break;
> + }
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + if (ret)
> + return -EINVAL;
Drop
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_write_f32_scaled(struct apple_smc *smc, smc_key key,
> + int value, int scale)
> +{
> + u64 val;
> + u32 fval = 0;
> + int exp, neg;
> +
> + val = abs(value);
> + neg = val != value;
> +
> + if (scale > 1) {
> + val <<= 32;
> + exp = 32;
> + val /= scale;
> + } else if (scale < 1) {
> + val *= -scale;
> + }
> +
> + if (val) {
> + int msb = __fls(val) - exp;
> +
> + if (msb > 23) {
> + val >>= msb - 23;
> + exp -= msb - 23;
> + } else if (msb < 23) {
> + val <<= 23 - msb;
> + exp += msb;
> + }
> +
> + fval = FIELD_PREP(FLT_SIGN_MASK, neg) |
> + FIELD_PREP(FLT_EXP_MASK, exp + FLT_EXP_BIAS) |
> + FIELD_PREP(FLT_MANT_MASK, val);
> + }
> +
> + return apple_smc_write_u32(smc, key, fval);
> +}
> +
> +static int macsmc_hwmon_write_key(struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor, long val,
> + int scale)
> +{
> + switch (sensor->info.type_code) {
> + /* 32-bit IEEE 754 float */
> + case _SMC_KEY("flt "):
> + return macsmc_hwmon_write_f32_scaled(smc, sensor->macsmc_key,
> + val, scale);
> + /* unsigned 8-bit integer */
> + case _SMC_KEY("ui8 "):
> + return apple_smc_write_u8(smc, sensor->macsmc_key, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int macsmc_hwmon_read_fan(struct macsmc_hwmon *hwmon, u32 attr, int chan,
> + long *val)
> +{
> + if (!(hwmon->fan.fans[chan].attrs & BIT(attr)))
> + return -EINVAL;
This is wrong. If the fan does not exist, the is_visible function should
have dropped the attribute. Also, EINVAL is "invalid argument" which is
just impossible when _reading_ a value from the kernel.
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].now, 1, val);
> + case hwmon_fan_min:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].min, 1, val);
> + case hwmon_fan_max:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].max, 1, val);
> + case hwmon_fan_target:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].set, 1, val);
> + default:
> + return -EINVAL;
As above.
> + }
> +}
> +
> +static int macsmc_hwmon_write_fan(struct device *dev, u32 attr, int channel,
> + long val)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> + long min, max;
> + int ret;
> +
> + if (!fan_control || hwmon->fan.fans[channel].mode.macsmc_key == 0)
> + return -EOPNOTSUPP;
> +
> + if (channel >= hwmon->fan.count ||
> + !(hwmon->fan.fans[channel].attrs & BIT(attr)) ||
> + attr != hwmon_fan_target)
> + return -EINVAL;
And again.
> +
> + /*
> + * The SMC does no sanity checks on requested fan speeds, so we need to.
> + */
> + ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].min,
> + 1, &min);
> + if (ret)
> + return ret;
> +
> + ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].max,
> + 1, &max);
> + if (ret)
> + return ret;
> +
> + if (val >= min && val <= max) {
> + if (!hwmon->fan.fans[channel].manual) {
> + /* Write 1 to mode key for manual control */
> + ret = macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].mode, 1,
> + 1);
> + if (ret < 0)
> + return ret;
> +
> + hwmon->fan.fans[channel].manual = true;
> + }
> + return macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].set, val, 1);
> + } else if (!val) {
> + if (hwmon->fan.fans[channel].manual) {
> + ret = macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].mode, 0, 1);
> + if (ret < 0)
> + return ret;
> +
> + hwmon->fan.fans[channel].manual = false;
> + }
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + switch (type) {
> + case hwmon_temp:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->temp.sensors[channel], 1000, val);
> + break;
> + case hwmon_in:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->volt.sensors[channel], 1000, val);
> + break;
> + case hwmon_curr:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->curr.sensors[channel], 1000, val);
> + break;
> + case hwmon_power:
> + /* SMC returns power in Watts with acceptable precision to scale to uW */
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->power.sensors[channel],
> + 1000000, val);
> + break;
> + case hwmon_fan:
> + ret = macsmc_hwmon_read_fan(hwmon, attr, channel, val);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static int macsmc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_fan:
> + return macsmc_hwmon_write_fan(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t macsmc_hwmon_fan_is_visible(const void *data, u32 attr,
> + int channel)
> +{
> + const struct macsmc_hwmon *hwmon = data;
> +
> + if (fan_control && attr == hwmon_fan_target &&
> + hwmon->fan.fans[channel].mode.macsmc_key != 0)
> + return 0644;
> +
> + return 0444;
This should return 0 for non-existing fans.
> +}
> +
> +static umode_t macsmc_hwmon_is_visible(const void *data,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + switch (type) {
> + case hwmon_fan:
> + return macsmc_hwmon_fan_is_visible(data, attr, channel);
> + default:
> + break;
> + }
> +
> + return 0444;
If those attributes exist they must be supported. Otherwise return 0.
> +}
> +
> +static const struct hwmon_ops macsmc_hwmon_ops = {
> + .is_visible = macsmc_hwmon_is_visible,
> + .read = macsmc_hwmon_read,
> + .read_string = macsmc_hwmon_read_label,
> + .write = macsmc_hwmon_write,
> +};
> +
> +/*
> + * Get the key metadata, including key data type, from the SMC.
> + */
> +static int macsmc_hwmon_parse_key(struct device *dev, struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor,
> + const char *key)
> +{
> + int ret;
> +
> + ret = apple_smc_get_key_info(smc, _SMC_KEY(key), &sensor->info);
> + if (ret) {
> + dev_err(dev, "Failed to retrieve key info for %s\n", key);
> + return ret;
> + }
> +
> + sensor->macsmc_key = _SMC_KEY(key);
> +
> + return 0;
> +}
> +
> +/*
> + * A sensor is a single key-value pair as made available by the SMC.
> + * The devicetree gives us the SMC key ID and a friendly name where the
> + * purpose of the sensor is known.
> + */
> +static int macsmc_hwmon_create_sensor(struct device *dev, struct apple_smc *smc,
> + struct device_node *sensor_node,
> + struct macsmc_hwmon_sensor *sensor)
> +{
> + const char *key, *label;
> + int ret;
> +
> + ret = of_property_read_string(sensor_node, "apple,key-id", &key);
> + if (ret) {
> + dev_err(dev, "Could not find apple,key-id in sensor node\n");
> + return ret;
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, sensor, key);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_string(sensor_node, "label", &label);
> + if (ret) {
> + dev_err(dev, "No label found for sensor %s\n", key);
> + return ret;
> + }
> +
> + strscpy_pad(sensor->label, label, sizeof(sensor->label));
> +
> + return 0;
> +}
> +
> +/*
> + * Fan data is exposed by the SMC as multiple sensors.
> + *
> + * The devicetree schema reuses apple,key-id for the actual fan speed sensor.
> + * Min, max and target keys do not need labels, so we can reuse label
> + * for naming the entire fan.
> + */
> +static int macsmc_hwmon_create_fan(struct device *dev, struct apple_smc *smc,
> + struct device_node *fan_node,
> + struct macsmc_hwmon_fan *fan)
> +{
> + const char *label, *now, *min, *max, *set, *mode;
> + int ret;
> +
> + ret = of_property_read_string(fan_node, "apple,key-id", &now);
> + if (ret) {
> + dev_err(dev, "apple,key-id not found in fan node!\n");
> + return -EINVAL;
return ret;
Never overwrite error numbers.
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_string(fan_node, "label", &label);
> + if (ret) {
> + dev_err(dev, "No label found for fan %s\n", now);
> + return ret;
> + }
> +
> + strscpy_pad(fan->label, label, sizeof(fan->label));
> +
> + fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
> +
> + /* The following keys are not required to simply monitor fan speed */
> + if (!of_property_read_string(fan_node, "apple,fan-minimum", &min)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->min, min);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_MIN;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-maximum", &max)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->max, max);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_MAX;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-target", &set)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->set, set);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_TARGET;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-mode", &mode)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->mode, mode);
> + if (ret)
> + return ret;
> + }
> +
> + /* Initialise fan control mode to automatic */
> + fan->manual = false;
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon,
> + struct device_node *hwmon_node)
> +{
> + struct device_node *key_node __maybe_unused;
> + u32 n_current = 0, n_fan = 0, n_power = 0, n_temperature = 0, n_voltage = 0;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "current-") {
> + n_current++;
> + }
> +
> + if (n_current) {
> + hwmon->curr.sensors = devm_kcalloc(hwmon->dev, n_current,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->curr.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "current-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->curr.sensors[hwmon->curr.count]))
> + hwmon->curr.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "fan-") {
> + n_fan++;
> + }
> +
> + if (n_fan) {
> + hwmon->fan.fans = devm_kcalloc(hwmon->dev, n_fan,
> + sizeof(struct macsmc_hwmon_fan), GFP_KERNEL);
> + if (!hwmon->fan.fans)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "fan-") {
> + if (!macsmc_hwmon_create_fan(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->fan.fans[hwmon->fan.count]))
> + hwmon->fan.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "power-") {
> + n_power++;
> + }
> +
> + if (n_power) {
> + hwmon->power.sensors = devm_kcalloc(hwmon->dev, n_power,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->power.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "power-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->power.sensors[hwmon->power.count]))
> + hwmon->power.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "temperature-") {
> + n_temperature++;
> + }
> +
> + if (n_temperature) {
> + hwmon->temp.sensors = devm_kcalloc(hwmon->dev, n_temperature,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->temp.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "temperature-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->temp.sensors[hwmon->temp.count]))
> + hwmon->temp.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "voltage-") {
> + n_voltage++;
> + }
> +
> + if (n_voltage) {
> + hwmon->volt.sensors = devm_kcalloc(hwmon->dev, n_voltage,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->volt.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "volt-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->volt.sensors[hwmon->volt.count]))
> + hwmon->volt.count++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Create NULL-terminated config arrays */
> +static void macsmc_hwmon_populate_configs(u32 *configs, u32 num_keys, u32 flags)
> +{
> + int idx;
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = flags;
> +}
> +
> +static void macsmc_hwmon_populate_fan_configs(u32 *configs, u32 num_keys,
> + struct macsmc_hwmon_fans *fans)
> +{
> + int idx;
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = fans->fans[idx].attrs;
> +}
> +
> +static const struct hwmon_channel_info *const macsmc_chip_channel_info =
> + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ);
> +
> +static int macsmc_hwmon_create_infos(struct macsmc_hwmon *hwmon)
> +{
> + struct hwmon_channel_info *channel_info;
> + int i = 0;
> +
> + /* chip */
> + hwmon->channel_infos[i++] = macsmc_chip_channel_info;
> +
> + if (hwmon->curr.count) {
> + channel_info = &hwmon->curr.channel_info;
> + channel_info->type = hwmon_curr;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->curr.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->curr.count,
> + (HWMON_C_INPUT | HWMON_C_LABEL));
Unnecessary ()
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->fan.count) {
> + channel_info = &hwmon->fan.channel_info;
> + channel_info->type = hwmon_fan;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->fan.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_fan_configs((u32 *)channel_info->config,
> + hwmon->fan.count,
> + &hwmon->fan);
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->power.count) {
> + channel_info = &hwmon->power.channel_info;
> + channel_info->type = hwmon_power;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->power.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->power.count,
> + (HWMON_P_INPUT | HWMON_P_LABEL));
Unnecessary (). Drop everywhere.
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->temp.count) {
> + channel_info = &hwmon->temp.channel_info;
> + channel_info->type = hwmon_temp;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->temp.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->temp.count,
> + (HWMON_T_INPUT | HWMON_T_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->volt.count) {
> + channel_info = &hwmon->volt.channel_info;
> + channel_info->type = hwmon_in;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->volt.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->volt.count,
> + (HWMON_I_INPUT | HWMON_I_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_probe(struct platform_device *pdev)
> +{
> + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> + struct macsmc_hwmon *hwmon;
> + int ret;
> +
> + /*
> + * The MFD driver will try to probe us unconditionally. Some devices
> + * with the SMC do not have hwmon capabilities. Only probe if we have
> + * a hwmon node.
> + */
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon),
> + GFP_KERNEL);
> + if (!hwmon)
> + return -ENOMEM;
> +
> + hwmon->dev = &pdev->dev;
> + hwmon->smc = smc;
> +
> + ret = macsmc_hwmon_populate_sensors(hwmon, hwmon->dev->of_node);
> + if (ret) {
> + dev_err(hwmon->dev, "Could not populate keys!\n");
> + return ret;
> + }
> +
> + if (!hwmon->curr.count && !hwmon->fan.count &&
> + !hwmon->power.count && !hwmon->temp.count &&
> + !hwmon->volt.count) {
> + dev_err(hwmon->dev,
> + "No valid keys found of any supported type");
> + return -ENODEV;
> + }
> +
> + ret = macsmc_hwmon_create_infos(hwmon);
> + if (ret)
> + return ret;
> +
> + hwmon->chip_info.ops = &macsmc_hwmon_ops;
> + hwmon->chip_info.info =
> + (const struct hwmon_channel_info *const *)&hwmon->channel_infos;
> +
> + hwmon->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
> + "macsmc_hwmon", hwmon,
> + &hwmon->chip_info, NULL);
> + if (IS_ERR(hwmon->hwmon_dev))
> + return dev_err_probe(hwmon->dev, PTR_ERR(hwmon->hwmon_dev),
> + "Probing SMC hwmon device failed\n");
> +
> + dev_info(hwmon->dev, "Registered SMC hwmon device. Sensors:");
> + dev_info(hwmon->dev,
> + "Current: %d, Fans: %d, Power: %d, Temperature: %d, Voltage: %d",
> + hwmon->curr.count, hwmon->fan.count,
> + hwmon->power.count, hwmon->temp.count,
> + hwmon->volt.count);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id macsmc_hwmon_of_table[] = {
> + { .compatible = "apple,smc-hwmon" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, macsmc_hwmon_of_table);
> +
> +static struct platform_driver macsmc_hwmon_driver = {
> + .probe = macsmc_hwmon_probe,
> + .driver = {
> + .name = "macsmc_hwmon",
> + .of_match_table = macsmc_hwmon_of_table,
> + },
> +};
> +module_platform_driver(macsmc_hwmon_driver);
> +
> +MODULE_DESCRIPTION("Apple Silicon SMC hwmon driver");
> +MODULE_AUTHOR("James Calligeros <jcalligeros99@gmail.com>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_ALIAS("platform:macsmc_hwmon");
^ permalink raw reply
* Re: [PATCH v2 00/11] mfd: macsmc: add rtc, hwmon and hid subdevices
From: Alexandre Belloni @ 2025-08-28 22:12 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, Mark Kettenis, Hector Martin
In-Reply-To: <20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com>
Hello,
On 27/08/2025 21:22:34+1000, James Calligeros wrote:
> Hi all,
>
> This series adds support for the remaining SMC subdevices. These are the
> RTC, hwmon, and HID devices. They are being submitted together as the RTC
> and hwmon drivers both require changes to the SMC DT schema.
>
How do you expect this to be merged? From what I get, I could just take
1 and 3 as there doesn't seem to be any actual dependencies on any of
the other patches and the MFD change doesn't depend on the RTC changes.
> The RTC driver is responsible for getting and setting the system clock,
> and requires an NVMEM cell. This series replaces Sven's original RTC driver
> submission [1].
>
> The hwmon function is an interesting one. While each Apple Silicon device
> exposes pretty similar sets of sensors, these all seem to be paired to
> different SMC keys in the firmware interface. This is true even when the
> sensors are on the SoC. For example, an M1 MacBook Pro will use different
> keys to access the LITTLE core temperature sensors to an M1 Mac mini. This
> necessitates describing which keys correspond to which sensors for each
> device individually, and populating the hwmon structs at runtime. We do
> this with a node in the device tree. This series includes only the keys
> for sensors which we know to be common to all devices. The SMC is also
> responsible for monitoring and controlling fan speeds on systems with fans,
> which we expose via the hwmon driver.
>
> The SMC also handles the hardware power button and lid switch. Power
> button presses and lid opening/closing are emitted as HID events, so we
> add a HID subdevice to handle them.
>
> Note that this series is based on a branch with three additional commits
> applied to add the parent SMC nodes to the relevant Devicetrees. This
> was done to silence build errors. The series applies cleanly to 6.17-rc1.
>
> Regards,
>
> James
>
> [1] https://lore.kernel.org/asahi/CAEg-Je84XxLWH7vznQmPRfjf6GxWOu75ZetwN7AdseAwfMLLrQ@mail.gmail.com/T/#t
>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> Changes in v2:
> - Added Rob's R-b tag to RTC DT binding
> - Removed redundant nesting from hwmon DT binding
> - Dedpulicated property definitions in hwmon DT schema
> - Made label a required property for hwmon DT nodes
> - Clarified semantics in hwmon DT schema definitions
> - Split mfd tree changes into separate commits
> - Fixed numerous style errors in hwmon driver
> - Addressed Guenter's initial feedback on the hwmon driver
> - Modified hwmon driver to reflect DT schema changes
> - Added compatible property to hwmon node
> - Link to v1: https://lore.kernel.org/r/20250819-macsmc-subdevs-v1-0-57df6c3e5f19@gmail.com
>
> ---
> Hector Martin (2):
> rtc: Add new rtc-macsmc driver for Apple Silicon Macs
> input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
>
> James Calligeros (7):
> dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
> mfd: macsmc: Wire up Apple SMC RTC subdevice
> hwmon: Add Apple Silicon SMC hwmon driver
> mfd: macsmc: Wire up Apple SMC hwmon subdevice
> mfd: macsmc: Wire up Apple SMC HID subdevice
> arm64: dts: apple: Add common hwmon sensors and fans
> arm64: dts: apple: t8103, t600x, t8112: Add common hwmon nodes to devices
>
> Sven Peter (2):
> dt-bindings: rtc: Add Apple SMC RTC
> arm64: dts: apple: t8103,t600x,t8112: Add SMC RTC node
>
> .../bindings/hwmon/apple,smc-hwmon.yaml | 132 ++++
> .../bindings/mfd/apple,smc.yaml | 45 ++
> .../bindings/rtc/apple,smc-rtc.yaml | 35 +
> MAINTAINERS | 5 +
> .../boot/dts/apple/hwmon-common.dtsi | 37 ++
> .../boot/dts/apple/hwmon-fan-dual.dtsi | 24 +
> arch/arm64/boot/dts/apple/hwmon-fan.dtsi | 19 +
> .../boot/dts/apple/hwmon-laptop.dtsi | 35 +
> .../boot/dts/apple/hwmon-mac-mini.dtsi | 17 +
> .../arm64/boot/dts/apple/t6001-j375c.dts | 2 +
> .../arm64/boot/dts/apple/t6002-j375d.dts | 2 +
> .../arm64/boot/dts/apple/t600x-die0.dtsi | 6 +
> .../boot/dts/apple/t600x-j314-j316.dtsi | 4 +
> .../arm64/boot/dts/apple/t600x-j375.dtsi | 2 +
> arch/arm64/boot/dts/apple/t8103-j274.dts | 2 +
> arch/arm64/boot/dts/apple/t8103-j293.dts | 3 +
> arch/arm64/boot/dts/apple/t8103-j313.dts | 2 +
> arch/arm64/boot/dts/apple/t8103-j456.dts | 2 +
> arch/arm64/boot/dts/apple/t8103-j457.dts | 2 +
> .../arm64/boot/dts/apple/t8103-jxxx.dtsi | 2 +
> arch/arm64/boot/dts/apple/t8103.dtsi | 6 +
> arch/arm64/boot/dts/apple/t8112-j413.dts | 2 +
> arch/arm64/boot/dts/apple/t8112-j473.dts | 2 +
> arch/arm64/boot/dts/apple/t8112-j493.dts | 3 +
> .../arm64/boot/dts/apple/t8112-jxxx.dtsi | 2 +
> arch/arm64/boot/dts/apple/t8112.dtsi | 6 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc_hwmon.c | 848 +++++++++++++++++++++++++
> drivers/input/misc/Kconfig | 11 +
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/macsmc-hid.c | 209 ++++++
> drivers/mfd/macsmc.c | 3 +
> drivers/rtc/Kconfig | 11 +
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-macsmc.c | 141 ++++
> 36 files changed, 1637 insertions(+)
> ---
> base-commit: 876d6a70b24869f96ebc8672caf86cb4bae72927
> change-id: 20250816-macsmc-subdevs-87032c017d0c
>
> Best regards,
> --
> James Calligeros <jcalligeros99@gmail.com>
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3] platform/x86: asus-wmi: map more keys on ExpertBook B9
From: Ilpo Järvinen @ 2025-08-28 16:18 UTC (permalink / raw)
To: Anton Khirnov
Cc: Hans de Goede, Corentin Chary, Luke D. Jones, Dmitry Torokhov,
linux-input, platform-driver-x86, LKML
In-Reply-To: <20250827152954.4844-1-anton@khirnov.net>
On Wed, 27 Aug 2025, Anton Khirnov wrote:
> * there is a dedicated "noise cancel" key in top row, between mic mute
> and PrintScreen; it sends 0xCA when pressed by itself (mapped to F13),
> 0xCB with Fn (mapped to F14)
> * Fn+f sends 0x9D; it is not documented in the manual, but some web
> search results mention "asus intelligent performance"; mapped to FN_F
>
> Signed-off-by: Anton Khirnov <anton@khirnov.net>
> ---
> Hi Hans,
> would you mind applying this version of the patch, with the Fn+space
> mapping left out for now?
Hi Anton,
These days I'm the one handling pdx86 patches. I took this into the
review-ilpo-fixes branch and reinstated Hans' ack.
My experience is that long delays are nothing unusual when interacting
with Dimitry so don't get discouraged by that when it comes to the
rest of v2 content. Once Dimitry has okay'ed the input side change, please
resubmit the rest.
--
i.
> Thanks
> ---
>
> drivers/platform/x86/asus-nb-wmi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index f84c3d03c1de..dba3c1488db2 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -618,6 +618,7 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
> { KE_KEY, 0x93, { KEY_SWITCHVIDEOMODE } }, /* SDSP LCD + CRT + TV + DVI */
> { KE_KEY, 0x95, { KEY_MEDIA } },
> { KE_KEY, 0x99, { KEY_PHONE } }, /* Conflicts with fan mode switch */
> + { KE_KEY, 0X9D, { KEY_FN_F } },
> { KE_KEY, 0xA0, { KEY_SWITCHVIDEOMODE } }, /* SDSP HDMI only */
> { KE_KEY, 0xA1, { KEY_SWITCHVIDEOMODE } }, /* SDSP LCD + HDMI */
> { KE_KEY, 0xA2, { KEY_SWITCHVIDEOMODE } }, /* SDSP CRT + HDMI */
> @@ -632,6 +633,8 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
> { KE_IGNORE, 0xC0, }, /* External display connect/disconnect notification */
> { KE_KEY, 0xC4, { KEY_KBDILLUMUP } },
> { KE_KEY, 0xC5, { KEY_KBDILLUMDOWN } },
> + { KE_KEY, 0xCA, { KEY_F13 } }, /* Noise cancelling on Expertbook B9 */
> + { KE_KEY, 0xCB, { KEY_F14 } }, /* Fn+noise-cancel */
> { KE_IGNORE, 0xC6, }, /* Ambient Light Sensor notification */
> { KE_IGNORE, 0xCF, }, /* AC mode */
> { KE_KEY, 0xFA, { KEY_PROG2 } }, /* Lid flip action */
>
^ permalink raw reply
* Re: [PATCH v6] platform/x86: Add WMI driver for Redmibook keyboard.
From: Ilpo Järvinen @ 2025-08-28 12:07 UTC (permalink / raw)
To: Gladyshev Ilya
Cc: w_armin, linux-input, nikita.nikita.krasnov, Armin Wolf,
Hans de Goede, linux-kernel, platform-driver-x86
In-Reply-To: <20250820174140.41410-1-foxido@foxido.dev>
On Wed, 20 Aug 2025 20:41:32 +0300, Gladyshev Ilya wrote:
> This driver implements support for various Fn keys (like Cut) and Xiaomi
> specific AI button.
>
>
Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.
The list of commits applied:
[1/1] platform/x86: Add WMI driver for Redmibook keyboard.
commit: 98aadf8e494d2bf66b15d0ef9646ad9fdb6ded15
--
i.
^ permalink raw reply
* [PATCH 5/5] Input: cros_ec_keyb - Defer probe until parent EC device is registered
From: Tzung-Bi Shih @ 2025-08-28 8:36 UTC (permalink / raw)
To: Dmitry Torokhov, Benson Leung; +Cc: tzungbi, linux-input, chrome-platform
In-Reply-To: <20250828083601.856083-1-tzungbi@kernel.org>
The `cros_ec_keyb` driver can be probed before the cros_ec_device has
completed the registration. This creates a race condition where
`cros_ec_keyb` might access uninitialized data.
Fix this by calling `cros_ec_device_registered()` to check the parent's
status. If the device is not yet ready, return -EPROBE_DEFER to ensure
the probe is retried later.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index c1e53d87c8a7..f7209c8ebbcc 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -705,6 +705,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
ec = dev_get_drvdata(pdev->dev.parent);
if (!ec)
return -EPROBE_DEFER;
+ /*
+ * Even if the cros_ec_device pointer is available, still need to check
+ * if the device is fully registered before using it.
+ */
+ if (!cros_ec_device_registered(ec))
+ return -EPROBE_DEFER;
ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL);
if (!ckdev)
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply related
* [PATCH 4/5] platform/chrome: cros_ec: Add a flag to track registration state
From: Tzung-Bi Shih @ 2025-08-28 8:36 UTC (permalink / raw)
To: Dmitry Torokhov, Benson Leung; +Cc: tzungbi, linux-input, chrome-platform
In-Reply-To: <20250828083601.856083-1-tzungbi@kernel.org>
Introduce a `registered` flag to the `struct cros_ec_device` to allow
callers to determine if the device has been fully registered and is
ready for use.
This is a preparatory step to prevent race conditions where other drivers
might try to access the device before it is fully registered or after
it has been unregistered.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/cros_ec.c | 7 +++++++
drivers/platform/chrome/cros_ec_proto.c | 15 +++++++++++++++
include/linux/platform_data/cros_ec_proto.h | 4 ++++
3 files changed, 26 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 61bcef8741db..1da79e3d215b 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -9,6 +9,7 @@
* battery charging and regulator control, firmware update.
*/
+#include <linux/cleanup.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/of_platform.h>
@@ -316,6 +317,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
goto exit;
}
+ scoped_guard(mutex, &ec_dev->lock)
+ ec_dev->registered = true;
+
dev_info(dev, "Chrome EC device registered\n");
/*
@@ -343,6 +347,9 @@ EXPORT_SYMBOL(cros_ec_register);
*/
void cros_ec_unregister(struct cros_ec_device *ec_dev)
{
+ scoped_guard(mutex, &ec_dev->lock)
+ ec_dev->registered = false;
+
if (ec_dev->mkbp_event_supported)
blocking_notifier_chain_unregister(&ec_dev->event_notifier,
&ec_dev->notifier_ready);
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 3e94a0a82173..1d8d9168ec1a 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -3,6 +3,7 @@
//
// Copyright (C) 2015 Google, Inc
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/limits.h>
@@ -1153,5 +1154,19 @@ int cros_ec_get_cmd_versions(struct cros_ec_device *ec_dev, u16 cmd)
}
EXPORT_SYMBOL_GPL(cros_ec_get_cmd_versions);
+/**
+ * cros_ec_device_registered - Return if the ec_dev is registered.
+ *
+ * @ec_dev: EC device
+ *
+ * Return: true if registered. Otherwise, false.
+ */
+bool cros_ec_device_registered(struct cros_ec_device *ec_dev)
+{
+ guard(mutex)(&ec_dev->lock);
+ return ec_dev->registered;
+}
+EXPORT_SYMBOL_GPL(cros_ec_device_registered);
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("ChromeOS EC communication protocol helpers");
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 4d96cffbb9e3..de14923720a5 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -128,6 +128,7 @@ struct cros_ec_command {
* @dout_size: Size of dout buffer to allocate (zero to use static dout).
* @wake_enabled: True if this device can wake the system from sleep.
* @suspended: True if this device had been suspended.
+ * @registered: True if this device had been registered.
* @cmd_xfer: Send command to EC and get response.
* Returns the number of bytes received if the communication
* succeeded, but that doesn't mean the EC was happy with the
@@ -186,6 +187,7 @@ struct cros_ec_device {
int dout_size;
bool wake_enabled;
bool suspended;
+ bool registered;
int (*cmd_xfer)(struct cros_ec_device *ec,
struct cros_ec_command *msg);
int (*pkt_xfer)(struct cros_ec_device *ec,
@@ -278,6 +280,8 @@ int cros_ec_cmd_readmem(struct cros_ec_device *ec_dev, u8 offset, u8 size, void
int cros_ec_get_cmd_versions(struct cros_ec_device *ec_dev, u16 cmd);
+bool cros_ec_device_registered(struct cros_ec_device *ec_dev);
+
/**
* cros_ec_get_time_ns() - Return time in ns.
*
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply related
* [PATCH 3/5] platform/chrome: cros_ec: Separate initialization from cros_ec_register()
From: Tzung-Bi Shih @ 2025-08-28 8:35 UTC (permalink / raw)
To: Dmitry Torokhov, Benson Leung; +Cc: tzungbi, linux-input, chrome-platform
In-Reply-To: <20250828083601.856083-1-tzungbi@kernel.org>
Move the initialization of the `struct cros_ec_device` from
cros_ec_register() into cros_ec_device_alloc().
This decouples device initialization from registration. By doing so,
the per-device lock is now available immediately after allocation,
allowing it to be used safely even before the device is fully
registered.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/cros_ec.c | 57 ++++++++++++++++---------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index da049068b6e9..61bcef8741db 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -30,6 +30,14 @@ static struct cros_ec_platform pd_p = {
.cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
};
+static void cros_ec_device_free(void *data)
+{
+ struct cros_ec_device *ec_dev = data;
+
+ mutex_destroy(&ec_dev->lock);
+ lockdep_unregister_key(&ec_dev->lockdep_key);
+}
+
struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
{
struct cros_ec_device *ec_dev;
@@ -45,7 +53,28 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
sizeof(struct ec_params_rwsig_action) +
EC_MAX_REQUEST_OVERHEAD;
+ ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
+ if (!ec_dev->din)
+ return NULL;
+
+ ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
+ if (!ec_dev->dout)
+ return NULL;
+
ec_dev->dev = dev;
+ ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
+ ec_dev->max_request = sizeof(struct ec_params_rwsig_action);
+ ec_dev->suspend_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT;
+
+ BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
+ BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->panic_notifier);
+
+ lockdep_register_key(&ec_dev->lockdep_key);
+ mutex_init(&ec_dev->lock);
+ lockdep_set_class(&ec_dev->lock, &ec_dev->lockdep_key);
+
+ if (devm_add_action_or_reset(dev, cros_ec_device_free, ec_dev))
+ return NULL;
return ec_dev;
}
@@ -200,29 +229,7 @@ static int cros_ec_ready_event(struct notifier_block *nb,
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
- int err = 0;
-
- BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
- BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->panic_notifier);
-
- ec_dev->max_request = sizeof(struct ec_params_hello);
- ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
- ec_dev->max_passthru = 0;
- ec_dev->ec = NULL;
- ec_dev->pd = NULL;
- ec_dev->suspend_timeout_ms = EC_HOST_SLEEP_TIMEOUT_DEFAULT;
-
- ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
- if (!ec_dev->din)
- return -ENOMEM;
-
- ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
- if (!ec_dev->dout)
- return -ENOMEM;
-
- lockdep_register_key(&ec_dev->lockdep_key);
- mutex_init(&ec_dev->lock);
- lockdep_set_class(&ec_dev->lock, &ec_dev->lockdep_key);
+ int err;
/* Send RWSIG continue to jump to RW for devices using RWSIG. */
err = cros_ec_rwsig_continue(ec_dev);
@@ -322,8 +329,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
exit:
platform_device_unregister(ec_dev->ec);
platform_device_unregister(ec_dev->pd);
- mutex_destroy(&ec_dev->lock);
- lockdep_unregister_key(&ec_dev->lockdep_key);
return err;
}
EXPORT_SYMBOL(cros_ec_register);
@@ -343,8 +348,6 @@ void cros_ec_unregister(struct cros_ec_device *ec_dev)
&ec_dev->notifier_ready);
platform_device_unregister(ec_dev->pd);
platform_device_unregister(ec_dev->ec);
- mutex_destroy(&ec_dev->lock);
- lockdep_unregister_key(&ec_dev->lockdep_key);
}
EXPORT_SYMBOL(cros_ec_unregister);
--
2.51.0.268.g9569e192d0-goog
^ permalink raw reply related
* [PATCH 2/5] platform/chrome: Centralize common cros_ec_device initialization
From: Tzung-Bi Shih @ 2025-08-28 8:35 UTC (permalink / raw)
To: Dmitry Torokhov, Benson Leung; +Cc: tzungbi, linux-input, chrome-platform
In-Reply-To: <20250828083601.856083-1-tzungbi@kernel.org>
Move the common initialization from protocol device drivers into central
cros_ec_device_alloc().
This removes duplicated code from each driver's probe function.
The buffer sizes are now calculated once, using the maximum possible
overhead required by any of the transport protocols, ensuring the
allocated buffers are sufficient for all cases.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/cros_ec.c | 9 +++++++++
drivers/platform/chrome/cros_ec_i2c.c | 5 -----
drivers/platform/chrome/cros_ec_ishtp.c | 4 ----
drivers/platform/chrome/cros_ec_lpc.c | 4 ----
drivers/platform/chrome/cros_ec_rpmsg.c | 4 ----
drivers/platform/chrome/cros_ec_spi.c | 5 -----
drivers/platform/chrome/cros_ec_uart.c | 4 ----
include/linux/platform_data/cros_ec_proto.h | 14 ++++++++++----
8 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 25283a148ab9..da049068b6e9 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -38,6 +38,15 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev)
if (!ec_dev)
return NULL;
+ ec_dev->din_size = sizeof(struct ec_host_response) +
+ sizeof(struct ec_response_get_protocol_info) +
+ EC_MAX_RESPONSE_OVERHEAD;
+ ec_dev->dout_size = sizeof(struct ec_host_request) +
+ sizeof(struct ec_params_rwsig_action) +
+ EC_MAX_REQUEST_OVERHEAD;
+
+ ec_dev->dev = dev;
+
return ec_dev;
}
EXPORT_SYMBOL(cros_ec_device_alloc);
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index ee3c5130ec3f..def1144a077e 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -297,16 +297,11 @@ static int cros_ec_i2c_probe(struct i2c_client *client)
return -ENOMEM;
i2c_set_clientdata(client, ec_dev);
- ec_dev->dev = dev;
ec_dev->priv = client;
ec_dev->irq = client->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_i2c;
ec_dev->phys_name = client->adapter->name;
- ec_dev->din_size = sizeof(struct ec_host_response_i2c) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request_i2c) +
- sizeof(struct ec_params_rwsig_action);
err = cros_ec_register(ec_dev);
if (err) {
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
index c102a796670c..4e74e702c5a2 100644
--- a/drivers/platform/chrome/cros_ec_ishtp.c
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -550,14 +550,10 @@ static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
client_data->ec_dev = ec_dev;
dev->driver_data = ec_dev;
- ec_dev->dev = dev;
ec_dev->priv = client_data->cros_ish_cl;
ec_dev->cmd_xfer = NULL;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
ec_dev->phys_name = dev_name(dev);
- ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct cros_ish_out_msg) + sizeof(struct ec_params_rwsig_action);
return cros_ec_register(ec_dev);
}
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 30fa89b81666..78cfff80cdea 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -642,14 +642,10 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
return -ENOMEM;
platform_set_drvdata(pdev, ec_dev);
- ec_dev->dev = dev;
ec_dev->phys_name = dev_name(dev);
ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_lpc;
ec_dev->cmd_readmem = cros_ec_lpc_readmem;
- ec_dev->din_size = sizeof(struct ec_host_response) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request) + sizeof(struct ec_params_rwsig_action);
ec_dev->priv = ec_lpc;
/*
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index 9ac2b923db6d..09bd9e49464e 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -224,14 +224,10 @@ static int cros_ec_rpmsg_probe(struct rpmsg_device *rpdev)
if (!ec_rpmsg)
return -ENOMEM;
- ec_dev->dev = dev;
ec_dev->priv = ec_rpmsg;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_rpmsg;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_rpmsg;
ec_dev->phys_name = dev_name(&rpdev->dev);
- ec_dev->din_size = sizeof(struct ec_host_response) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request) + sizeof(struct ec_params_rwsig_action);
dev_set_drvdata(dev, ec_dev);
ec_rpmsg->rpdev = rpdev;
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index c778300a4145..28fa82f8cb07 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -757,16 +757,11 @@ static int cros_ec_spi_probe(struct spi_device *spi)
cros_ec_spi_dt_probe(ec_spi, dev);
spi_set_drvdata(spi, ec_dev);
- ec_dev->dev = dev;
ec_dev->priv = ec_spi;
ec_dev->irq = spi->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_spi;
ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
- ec_dev->din_size = EC_MSG_PREAMBLE_COUNT +
- sizeof(struct ec_host_response) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request) + sizeof(struct ec_params_rwsig_action);
ec_spi->last_transfer_ns = ktime_get_ns();
diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
index 1a7511b1bbe3..d5b37414ff12 100644
--- a/drivers/platform/chrome/cros_ec_uart.c
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -276,14 +276,10 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
/* Initialize ec_dev for cros_ec */
ec_dev->phys_name = dev_name(dev);
- ec_dev->dev = dev;
ec_dev->priv = ec_uart;
ec_dev->irq = ec_uart->irq;
ec_dev->cmd_xfer = NULL;
ec_dev->pkt_xfer = cros_ec_uart_pkt_xfer;
- ec_dev->din_size = sizeof(struct ec_host_response) +
- sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request) + sizeof(struct ec_params_rwsig_action);
serdev_device_set_client_ops(serdev, &cros_ec_uart_client_ops);
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 3ec24f445c29..4d96cffbb9e3 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -33,12 +33,18 @@
/*
* Max bus-specific overhead incurred by request/responses.
- * I2C requires 1 additional byte for requests.
- * I2C requires 2 additional bytes for responses.
- * SPI requires up to 32 additional bytes for responses.
+ *
+ * Request:
+ * - I2C requires 1 byte (see struct ec_host_request_i2c).
+ * - ISHTP requires 4 bytes (see struct cros_ish_out_msg).
+ *
+ * Response:
+ * - I2C requires 2 bytes (see struct ec_host_response_i2c).
+ * - ISHTP requires 4 bytes (see struct cros_ish_in_msg).
+ * - SPI requires 32 bytes (see EC_MSG_PREAMBLE_COUNT).
*/
#define EC_PROTO_VERSION_UNKNOWN 0
-#define EC_MAX_REQUEST_OVERHEAD 1
+#define EC_MAX_REQUEST_OVERHEAD 4
#define EC_MAX_RESPONSE_OVERHEAD 32
/*
--
2.51.0.268.g9569e192d0-goog
^ 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