Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.7 11/14] Input: gpio_keys_polled - suppress deferred probe error for gpio
From: Sasha Levin @ 2024-03-11 18:36 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Uwe Kleine-König, Linus Walleij, Dmitry Torokhov,
	Sasha Levin, linux-input
In-Reply-To: <20240311183618.327694-1-sashal@kernel.org>

From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

[ Upstream commit 963465a33141d0d52338e77f80fe543d2c9dc053 ]

On a PC Engines APU our admins are faced with:

	$ dmesg | grep -c "gpio-keys-polled gpio-keys-polled: unable to claim gpio 0, err=-517"
	261

Such a message always appears when e.g. a new USB device is plugged in.

Suppress this message which considerably clutters the kernel log for
EPROBE_DEFER (i.e. -517).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20240305101042.10953-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/keyboard/gpio_keys_polled.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index ba00ecfbd343b..b41fd1240f431 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -315,12 +315,10 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 			error = devm_gpio_request_one(dev, button->gpio,
 					flags, button->desc ? : DRV_NAME);
-			if (error) {
-				dev_err(dev,
-					"unable to claim gpio %u, err=%d\n",
-					button->gpio, error);
-				return error;
-			}
+			if (error)
+				return dev_err_probe(dev, error,
+						     "unable to claim gpio %u\n",
+						     button->gpio);
 
 			bdata->gpiod = gpio_to_desc(button->gpio);
 			if (!bdata->gpiod) {
-- 
2.43.0


^ permalink raw reply related

* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Dmitry Torokhov @ 2024-03-11 16:18 UTC (permalink / raw)
  To: Karel Balej
  Cc: Krzysztof Kozlowski, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Liam Girdwood, Mark Brown, devicetree, linux-kernel,
	linux-input, Duje Mihanović, ~postmarketos/upstreaming,
	phone-devel
In-Reply-To: <CZQUKBQF1GZ9.3RSNW5WQBU9L6@gimli.ms.mff.cuni.cz>

On Mon, Mar 11, 2024 at 11:26:16AM +0100, Karel Balej wrote:
> Krzysztof Kozlowski, 2024-03-10T21:35:36+01:00:
> > On 10/03/2024 12:35, Karel Balej wrote:
> > > Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
> > >> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
> > >>> Dmitry,
> > >>>
> > >>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
> > >>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
> > >>>>> From: Karel Balej <balejk@matfyz.cz>
> > >>>>>
> > >>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
> > >>>>> driver to handle it. The driver currently only provides a basic support
> > >>>>> omitting additional functions found in the vendor version, such as long
> > >>>>> onkey and GPIO integration.
> > >>>>>
> > >>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> > >>>>> ---
> > >>>>>
> > >>>>> Notes:
> > >>>>>     RFC v3:
> > >>>>>     - Drop wakeup-source.
> > >>>>>     RFC v2:
> > >>>>>     - Address Dmitry's feedback:
> > >>>>>       - Sort includes alphabetically.
> > >>>>>       - Drop onkey->irq.
> > >>>>>       - ret -> err in irq_handler and no initialization.
> > >>>>>       - Break long lines and other formatting.
> > >>>>>       - Do not clobber platform_get_irq error.
> > >>>>>       - Do not set device parent manually.
> > >>>>>       - Use input_set_capability.
> > >>>>>       - Use the wakeup-source DT property.
> > >>>>>       - Drop of_match_table.
> > >>>>
> > >>>> I only said that you should not be using of_match_ptr(), but you still
> > >>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
> > >>>> proper module loading support.
> > >>>
> > >>> I removed of_match_table because I no longer need compatible for this --
> > >>> there are no device tree properties and the driver is being instantiated
> > >>> by the MFD driver.
> > >>>
> > >>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
> > >>> compiled as module? If that is the case, given what I write above, am I
> > >>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
> > >>> to use here?
> > >>
> > >> Yes, if uevent generated for the device is "platform:<name>" then
> > >> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
> > >> sets it up (OF modalias or platform), but you should be able to check
> > >> the format looking at the "uevent" attribute for your device in sysfs
> > >> (/sys/devices/bus/platform/...). 
> > > 
> > > The uevent is indeed platform.
> > > 
> > > But since there is only one device, perhaps having a device table is
> > > superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> > > fitting?
> >
> > Adding aliases for standard IDs and standard cases is almost never
> > correct. If you need module alias, it means your ID table is wrong (or
> > missing, which is usually wrong).
> >
> > > 
> > > Although I don't understand why this is even necessary when the driver
> > > name is such and the module is registered using
> > > `module_platform_driver`...
> >
> > ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
> 
> I think I understand the practical reasons. My point was that I would
> expect the alias to be added automatically even in the case that the
> device table is absent based solely on the driver name and the
> registration method (*module*_*platform*_driver). Why is that not the
> case? Obviously the driver name matching the mfd_cell name is sufficient
> for the driver to probe when it is built in so the name does seem to
> serve as some identification for the device just as a device table entry
> would.
> 
> Furthermore, drivers/input/serio/ioc3kbd.c does not seem to have an ID
> table either, nor a MODULE_ALIAS -- is that a mistake? If not, what
> mechanism causes the driver to probe when compiled as a module? It seems
> to me to effectively be the same setup as with my driver and that does
> not load automatically (because of the missing alias).

Yes, ioc3kbd is broken as far as module auto-loading goes. It probably
did not get noticed before because the driver is likely to be built-in
on the target architecture.

I'll take patches.

Thanks.

-- 
Dmitry

^ permalink raw reply

* [RFC PATCH v4 5/5] MAINTAINERS: add myself for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Add an entry to MAINTAINERS for the Marvell 88PM886 PMIC MFD, onkey and
regulator drivers.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v3:
    - Remove onkey bindings file.
    RFC v2:
    - Only mention 88PM886 in the commit message.
    - Add regulator driver.
    - Rename the entry.

 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec428..944f88c92df6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12949,6 +12949,15 @@ F:	drivers/net/dsa/mv88e6xxx/
 F:	include/linux/dsa/mv88e6xxx.h
 F:	include/linux/platform_data/mv88e6xxx.h
 
+MARVELL 88PM886 PMIC DRIVER
+M:	Karel Balej <balejk@matfyz.cz>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
+F:	drivers/input/misc/88pm886-onkey.c
+F:	drivers/mfd/88pm886.c
+F:	drivers/regulators/88pm886-regulator.c
+F:	include/linux/mfd/88pm886.h
+
 MARVELL ARMADA 3700 PHY DRIVERS
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
 S:	Maintained
-- 
2.44.0


^ permalink raw reply related

* [RFC PATCH v4 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM886 PMIC provides onkey among other things. Add client
driver to handle it. The driver currently only provides a basic support
omitting additional functions found in the vendor version, such as long
onkey and GPIO integration.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v4:
    - Reflect MFD driver changes:
      - chip->regmaps[...] -> chip->regmap
    - Address Dmitry's feedback:
      - Add ID table.
      - Add Ack.
    RFC v3:
    - Drop wakeup-source.
    RFC v2:
    - Address Dmitry's feedback:
      - Sort includes alphabetically.
      - Drop onkey->irq.
      - ret -> err in irq_handler and no initialization.
      - Break long lines and other formatting.
      - Do not clobber platform_get_irq error.
      - Do not set device parent manually.
      - Use input_set_capability.
      - Use the wakeup-source DT property.
      - Drop of_match_table.
      - Use more temporaries.
      - Use dev_err_probe.
    - Modify Kconfig description.

 drivers/input/misc/88pm886-onkey.c | 99 ++++++++++++++++++++++++++++++
 drivers/input/misc/Kconfig         |  7 +++
 drivers/input/misc/Makefile        |  1 +
 3 files changed, 107 insertions(+)
 create mode 100644 drivers/input/misc/88pm886-onkey.c

diff --git a/drivers/input/misc/88pm886-onkey.c b/drivers/input/misc/88pm886-onkey.c
new file mode 100644
index 000000000000..b0bfbd57d826
--- /dev/null
+++ b/drivers/input/misc/88pm886-onkey.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm886.h>
+
+struct pm886_onkey {
+	struct input_dev *idev;
+	struct pm886_chip *chip;
+};
+
+static irqreturn_t pm886_onkey_irq_handler(int irq, void *data)
+{
+	struct pm886_onkey *onkey = data;
+	struct regmap *regmap = onkey->chip->regmap;
+	struct input_dev *idev = onkey->idev;
+	struct device *parent = idev->dev.parent;
+	unsigned int val;
+	int err;
+
+	err = regmap_read(regmap, PM886_REG_STATUS1, &val);
+	if (err) {
+		dev_err(parent, "Failed to read status: %d\n", err);
+		return IRQ_NONE;
+	}
+	val &= PM886_ONKEY_STS1;
+
+	input_report_key(idev, KEY_POWER, val);
+	input_sync(idev);
+
+	return IRQ_HANDLED;
+}
+
+static int pm886_onkey_probe(struct platform_device *pdev)
+{
+	struct pm886_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct pm886_onkey *onkey;
+	struct input_dev *idev;
+	int irq, err;
+
+	onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+	if (!onkey)
+		return -ENOMEM;
+
+	onkey->chip = chip;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return dev_err_probe(dev, irq, "Failed to get IRQ\n");
+
+	idev = devm_input_allocate_device(dev);
+	if (!idev) {
+		dev_err(dev, "Failed to allocate input device\n");
+		return -ENOMEM;
+	}
+	onkey->idev = idev;
+
+	idev->name = "88pm886-onkey";
+	idev->phys = "88pm886-onkey/input0";
+	idev->id.bustype = BUS_I2C;
+
+	input_set_capability(idev, EV_KEY, KEY_POWER);
+
+	err = devm_request_threaded_irq(dev, irq, NULL, pm886_onkey_irq_handler,
+					IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey",
+					onkey);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to request IRQ\n");
+
+	err = input_register_device(idev);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to register input device\n");
+
+	return 0;
+}
+
+static const struct platform_device_id pm886_onkey_id_table[] = {
+	{ "88pm886-onkey", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, pm886_onkey_id_table);
+
+static struct platform_driver pm886_onkey_driver = {
+	.driver = {
+		.name = "88pm886-onkey",
+	},
+	.probe = pm886_onkey_probe,
+	.id_table = pm886_onkey_id_table,
+};
+module_platform_driver(pm886_onkey_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM886 onkey driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..16a079d9f0f2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -33,6 +33,13 @@ config INPUT_88PM80X_ONKEY
 	  To compile this driver as a module, choose M here: the module
 	  will be called 88pm80x_onkey.
 
+config INPUT_88PM886_ONKEY
+	tristate "Marvell 88PM886 onkey support"
+	depends on MFD_88PM886_PMIC
+	help
+	  Support the onkey of Marvell 88PM886 PMIC as an input device
+	  reporting power button status.
+
 config INPUT_AB8500_PONKEY
 	tristate "AB8500 Pon (PowerOn) Key"
 	depends on AB8500_CORE
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 04296a4abe8e..054a6dc1ac27 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -7,6 +7,7 @@
 
 obj-$(CONFIG_INPUT_88PM860X_ONKEY)	+= 88pm860x_onkey.o
 obj-$(CONFIG_INPUT_88PM80X_ONKEY)	+= 88pm80x_onkey.o
+obj-$(CONFIG_INPUT_88PM886_ONKEY)	+= 88pm886-onkey.o
 obj-$(CONFIG_INPUT_AB8500_PONKEY)	+= ab8500-ponkey.o
 obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
 obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
-- 
2.44.0


^ permalink raw reply related

* [RFC PATCH v4 3/5] regulator: add regulators driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Support the LDO and buck regulators of the Marvell 88PM886 PMIC.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
Please note that most of the regulators are not yet described: the
descriptions will be added to pm886_regulators in the same manner as the
already present ones before the series leaves the RFC state. In the
meantime, general comments on the driver will be appreciated.

Notes:
    RFC v4:
    - Initialize regulators regmap in the regulators driver.
    - Register all regulators at once.
    - Drop regulator IDs.
    - Add missing '\n' to dev_err_probe message.
    - Fix includes.
    - Add ID table.
    RFC v3:
    - Do not have a variable for each regulator -- define them all in the
      pm886_regulators array.
    - Use new regulators regmap index name.
    - Use dev_err_probe.
    RFC v2:
    - Drop of_compatible and related code.
    - Drop unused include.
    - Remove some abstraction: use only one regmap for all regulators and
      only mention 88PM886 in Kconfig description.
    - Reword commit message.

 drivers/regulator/88pm886-regulator.c | 215 ++++++++++++++++++++++++++
 drivers/regulator/Kconfig             |   6 +
 drivers/regulator/Makefile            |   1 +
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/regulator/88pm886-regulator.c

diff --git a/drivers/regulator/88pm886-regulator.c b/drivers/regulator/88pm886-regulator.c
new file mode 100644
index 000000000000..6a2ddb11a28a
--- /dev/null
+++ b/drivers/regulator/88pm886-regulator.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/linear_range.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/mfd/88pm886.h>
+
+#define PM886_PAGE_OFFSET_REGULATORS	1
+
+#define PM886_REG_LDO_EN1		0x09
+#define PM886_REG_LDO_EN2		0x0a
+
+#define PM886_REG_BUCK_EN		0x08
+
+#define PM886_REG_LDO1_VOUT		0x20
+#define PM886_REG_LDO2_VOUT		0x26
+#define PM886_REG_LDO3_VOUT		0x2c
+#define PM886_REG_LDO4_VOUT		0x32
+#define PM886_REG_LDO5_VOUT		0x38
+#define PM886_REG_LDO6_VOUT		0x3e
+#define PM886_REG_LDO7_VOUT		0x44
+#define PM886_REG_LDO8_VOUT		0x4a
+#define PM886_REG_LDO9_VOUT		0x50
+#define PM886_REG_LDO10_VOUT		0x56
+#define PM886_REG_LDO11_VOUT		0x5c
+#define PM886_REG_LDO12_VOUT		0x62
+#define PM886_REG_LDO13_VOUT		0x68
+#define PM886_REG_LDO14_VOUT		0x6e
+#define PM886_REG_LDO15_VOUT		0x74
+#define PM886_REG_LDO16_VOUT		0x7a
+
+#define PM886_REG_BUCK1_VOUT		0xa5
+#define PM886_REG_BUCK2_VOUT		0xb3
+#define PM886_REG_BUCK3_VOUT		0xc1
+#define PM886_REG_BUCK4_VOUT		0xcf
+#define PM886_REG_BUCK5_VOUT		0xdd
+
+#define PM886_LDO_VSEL_MASK		0x0f
+#define PM886_BUCK_VSEL_MASK		0x7f
+
+struct pm886_regulator {
+	struct regulator_desc desc;
+	int max_uA;
+};
+
+static int pm886_regulator_get_ilim(struct regulator_dev *rdev)
+{
+	struct pm886_regulator *data = rdev_get_drvdata(rdev);
+
+	if (!data) {
+		dev_err(&rdev->dev, "Failed to get regulator data\n");
+		return -EINVAL;
+	}
+	return data->max_uA;
+}
+
+static const struct regulator_ops pm886_ldo_ops = {
+	.list_voltage = regulator_list_voltage_table,
+	.map_voltage = regulator_map_voltage_iterate,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_current_limit = pm886_regulator_get_ilim,
+};
+
+static const struct regulator_ops pm886_buck_ops = {
+	.list_voltage = regulator_list_voltage_linear_range,
+	.map_voltage = regulator_map_voltage_linear_range,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.enable = regulator_enable_regmap,
+	.disable = regulator_disable_regmap,
+	.is_enabled = regulator_is_enabled_regmap,
+	.get_current_limit = pm886_regulator_get_ilim,
+};
+
+static const unsigned int pm886_ldo_volt_table1[] = {
+	1700000, 1800000, 1900000, 2500000, 2800000, 2900000, 3100000, 3300000,
+};
+
+static const unsigned int pm886_ldo_volt_table2[] = {
+	1200000, 1250000, 1700000, 1800000, 1850000, 1900000, 2500000, 2600000,
+	2700000, 2750000, 2800000, 2850000, 2900000, 3000000, 3100000, 3300000,
+};
+
+static const unsigned int pm886_ldo_volt_table3[] = {
+	1700000, 1800000, 1900000, 2000000, 2100000, 2500000, 2700000, 2800000,
+};
+
+static const struct linear_range pm886_buck_volt_ranges1[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+	REGULATOR_LINEAR_RANGE(1600000, 80, 84, 50000),
+};
+
+static const struct linear_range pm886_buck_volt_ranges2[] = {
+	REGULATOR_LINEAR_RANGE(600000, 0, 79, 12500),
+	REGULATOR_LINEAR_RANGE(1600000, 80, 114, 50000),
+};
+
+static struct pm886_regulator pm886_regulators[] = {
+	{
+		.desc = {
+			.name = "LDO2",
+			.regulators_node = "regulators",
+			.of_match = "ldo2",
+			.ops = &pm886_ldo_ops,
+			.type = REGULATOR_VOLTAGE,
+			.enable_reg = PM886_REG_LDO_EN1,
+			.enable_mask = BIT(1),
+			.volt_table = pm886_ldo_volt_table1,
+			.n_voltages = ARRAY_SIZE(pm886_ldo_volt_table1),
+			.vsel_reg = PM886_REG_LDO2_VOUT,
+			.vsel_mask = PM886_LDO_VSEL_MASK,
+		},
+		.max_uA = 100000,
+	},
+	{
+		.desc = {
+			.name = "LDO15",
+			.regulators_node = "regulators",
+			.of_match = "ldo15",
+			.ops = &pm886_ldo_ops,
+			.type = REGULATOR_VOLTAGE,
+			.enable_reg = PM886_REG_LDO_EN2,
+			.enable_mask = BIT(6),
+			.volt_table = pm886_ldo_volt_table2,
+			.n_voltages = ARRAY_SIZE(pm886_ldo_volt_table2),
+			.vsel_reg = PM886_REG_LDO15_VOUT,
+			.vsel_mask = PM886_LDO_VSEL_MASK,
+		},
+		.max_uA = 200000,
+	},
+	{
+		.desc = {
+			.name = "buck2",
+			.regulators_node = "regulators",
+			.of_match = "buck2",
+			.ops = &pm886_buck_ops,
+			.type = REGULATOR_VOLTAGE,
+			.n_voltages = 115,
+			.linear_ranges = pm886_buck_volt_ranges2,
+			.n_linear_ranges = ARRAY_SIZE(pm886_buck_volt_ranges2),
+			.vsel_reg = PM886_REG_BUCK2_VOUT,
+			.vsel_mask = PM886_BUCK_VSEL_MASK,
+			.enable_reg = PM886_REG_BUCK_EN,
+			.enable_mask = BIT(1),
+		},
+		.max_uA = 1200000,
+	},
+};
+
+static int pm886_regulator_probe(struct platform_device *pdev)
+{
+	struct pm886_chip *chip = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config rcfg = { };
+	struct pm886_regulator *regulator;
+	struct device *dev = &pdev->dev;
+	struct regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	struct i2c_client *page;
+	struct regmap *regmap;
+
+	page = devm_i2c_new_dummy_device(dev, chip->client->adapter,
+			chip->client->addr + PM886_PAGE_OFFSET_REGULATORS);
+	if (IS_ERR(page))
+		return dev_err_probe(dev, PTR_ERR(page),
+				"Failed to initialize regulators client\n");
+
+	regmap = devm_regmap_init_i2c(page, &pm886_i2c_regmap);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				"Failed to initialize regulators regmap\n");
+	rcfg.regmap = regmap;
+
+	rcfg.dev = dev->parent;
+
+	for (int i = 0; i < ARRAY_SIZE(pm886_regulators); i++) {
+		regulator = &pm886_regulators[i];
+		rdesc = &regulator->desc;
+		rcfg.driver_data = regulator;
+		rdev = devm_regulator_register(dev, rdesc, &rcfg);
+		if (IS_ERR(rdev))
+			return dev_err_probe(dev, PTR_ERR(rdev),
+					"Failed to register %s\n", rdesc->name);
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id pm886_regulator_id_table[] = {
+	{ "88pm886-regulator", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, pm886_regulator_id_table);
+
+static struct platform_driver pm886_regulator_driver = {
+	.driver = {
+		.name = "88pm886-regulator",
+	},
+	.probe = pm886_regulator_probe,
+	.id_table = pm886_regulator_id_table,
+};
+module_platform_driver(pm886_regulator_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM886 PMIC regulator driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 550145f82726..e8f504d4b9f6 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -91,6 +91,12 @@ config REGULATOR_88PM8607
 	help
 	  This driver supports 88PM8607 voltage regulator chips.
 
+config REGULATOR_88PM886
+	tristate "Marvell 88PM886 voltage regulators"
+	depends on MFD_88PM886_PMIC
+	help
+	  This driver implements support for Marvell 88PM886 voltage regulators.
+
 config REGULATOR_ACT8865
 	tristate "Active-semi act8865 voltage regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 46fb569e6be8..f30089b74b2e 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
 obj-$(CONFIG_REGULATOR_88PG86X) += 88pg86x.o
 obj-$(CONFIG_REGULATOR_88PM800) += 88pm800-regulator.o
 obj-$(CONFIG_REGULATOR_88PM8607) += 88pm8607.o
+obj-$(CONFIG_REGULATOR_88PM886) += 88pm886-regulator.o
 obj-$(CONFIG_REGULATOR_CROS_EC) += cros-ec-regulator.o
 obj-$(CONFIG_REGULATOR_CPCAP) += cpcap-regulator.o
 obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
-- 
2.44.0


^ permalink raw reply related

* [RFC PATCH v4 2/5] mfd: add driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM886 is a PMIC which provides various functions such as
onkey, battery, charger and regulators. It is found for instance in the
samsung,coreprimevelte smartphone with which this was tested. Implement
basic support to allow for the use of regulators and onkey.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v4:
    - Use MFD_CELL_* macros.
    - Address Lee's feedback:
      - Do not define regmap_config.val_bits and .reg_bits.
      - Drop everything regulator related except mfd_cell (regmap
        initialization, IDs enum etc.). Drop pm886_initialize_subregmaps.
      - Do not store regmap pointers as an array as there is now only one
        regmap. Also drop the corresponding enum.
      - Move regmap_config to the header as it is needed in the regulators
        driver.
      - pm886_chip.whoami -> chip_id
      - Reword chip ID mismatch error message and print the ID as
        hexadecimal.
      - Fix includes in include/linux/88pm886.h.
      - Drop the pm886_irq_number enum and define the (for the moment) only
        IRQ explicitly.
    - Have only one MFD cell for all regulators as they are now registered
      all at once in the regulators driver.
    - Reword commit message.
    - Make device table static and remove comma after the sentinel to signal
      that nothing should come after it.
    RFC v3:
    - Drop onkey cell .of_compatible.
    - Rename LDO page offset and regmap to REGULATORS.
    RFC v2:
    - Remove some abstraction.
    - Sort includes alphabetically and add linux/of.h.
    - Depend on OF, remove of_match_ptr and add MODULE_DEVICE_TABLE.
    - Use more temporaries and break long lines.
    - Do not initialize ret in probe.
    - Use the wakeup-source DT property.
    - Rename ret to err.
    - Address Lee's comments:
      - Drop patched in presets for base regmap and related defines.
      - Use full sentences in comments.
      - Remove IRQ comment.
      - Define regmap_config member values.
      - Rename data to sys_off_data.
      - Add _PMIC suffix to Kconfig.
      - Use dev_err_probe.
      - Do not store irq_data.
      - s/WHOAMI/CHIP_ID
      - Drop LINUX part of include guard name.
      - Merge in the regulator series modifications in order to have more
        devices and modify the commit message accordingly. Changes with
        respect to the original regulator series patches:
        - ret -> err
        - Add temporary for dev in pm88x_initialize_subregmaps.
        - Drop of_compatible for the regulators.
        - Do not duplicate LDO regmap for bucks.
    - Rewrite commit message.

 drivers/mfd/88pm886.c       | 149 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig         |  12 +++
 drivers/mfd/Makefile        |   1 +
 include/linux/mfd/88pm886.h |  38 +++++++++
 4 files changed, 200 insertions(+)
 create mode 100644 drivers/mfd/88pm886.c
 create mode 100644 include/linux/mfd/88pm886.h

diff --git a/drivers/mfd/88pm886.c b/drivers/mfd/88pm886.c
new file mode 100644
index 000000000000..88f21f813ec1
--- /dev/null
+++ b/drivers/mfd/88pm886.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+
+#include <linux/mfd/88pm886.h>
+
+#define PM886_REG_INT_STATUS1			0x05
+
+#define PM886_REG_INT_ENA_1			0x0a
+#define PM886_INT_ENA1_ONKEY			BIT(0)
+
+#define PM886_IRQ_ONKEY				0
+
+static struct regmap_irq pm886_regmap_irqs[] = {
+	REGMAP_IRQ_REG(PM886_IRQ_ONKEY, 0, PM886_INT_ENA1_ONKEY),
+};
+
+static struct regmap_irq_chip pm886_regmap_irq_chip = {
+	.name = "88pm886",
+	.irqs = pm886_regmap_irqs,
+	.num_irqs = ARRAY_SIZE(pm886_regmap_irqs),
+	.num_regs = 4,
+	.status_base = PM886_REG_INT_STATUS1,
+	.ack_base = PM886_REG_INT_STATUS1,
+	.unmask_base = PM886_REG_INT_ENA_1,
+};
+
+static struct resource pm886_onkey_resources[] = {
+	DEFINE_RES_IRQ_NAMED(PM886_IRQ_ONKEY, "88pm886-onkey"),
+};
+
+static struct mfd_cell pm886_devs[] = {
+	MFD_CELL_RES("88pm886-onkey", pm886_onkey_resources),
+	MFD_CELL_NAME("88pm886-regulator"),
+};
+
+static int pm886_power_off_handler(struct sys_off_data *sys_off_data)
+{
+	struct pm886_chip *chip = sys_off_data->cb_data;
+	struct regmap *regmap = chip->regmap;
+	struct device *dev = &chip->client->dev;
+	int err;
+
+	err = regmap_update_bits(regmap, PM886_REG_MISC_CONFIG1, PM886_SW_PDOWN,
+				PM886_SW_PDOWN);
+	if (err) {
+		dev_err(dev, "Failed to power off the device: %d\n", err);
+		return NOTIFY_BAD;
+	}
+	return NOTIFY_DONE;
+}
+
+static int pm886_setup_irq(struct pm886_chip *chip,
+		struct regmap_irq_chip_data **irq_data)
+{
+	struct regmap *regmap = chip->regmap;
+	struct device *dev = &chip->client->dev;
+	int err;
+
+	/* Set interrupt clearing mode to clear on write. */
+	err = regmap_update_bits(regmap, PM886_REG_MISC_CONFIG2,
+			PM886_INT_INV | PM886_INT_CLEAR | PM886_INT_MASK_MODE,
+			PM886_INT_WC);
+	if (err) {
+		dev_err(dev, "Failed to set interrupt clearing mode: %d\n", err);
+		return err;
+	}
+
+	err = devm_regmap_add_irq_chip(dev, regmap, chip->client->irq,
+					IRQF_ONESHOT, -1, &pm886_regmap_irq_chip,
+					irq_data);
+	if (err) {
+		dev_err(dev, "Failed to request IRQ: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int pm886_probe(struct i2c_client *client)
+{
+	struct regmap_irq_chip_data *irq_data;
+	struct device *dev = &client->dev;
+	struct pm886_chip *chip;
+	struct regmap *regmap;
+	unsigned int chip_id;
+	int err;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->client = client;
+	chip->chip_id = (uintptr_t)device_get_match_data(dev);
+	i2c_set_clientdata(client, chip);
+
+	regmap = devm_regmap_init_i2c(client, &pm886_i2c_regmap);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to initialize regmap\n");
+	chip->regmap = regmap;
+
+	err = regmap_read(regmap, PM886_REG_ID, &chip_id);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to read chip ID\n");
+
+	if (chip->chip_id != chip_id)
+		return dev_err_probe(dev, -EINVAL, "Unsupported chip: 0x%x\n", chip_id);
+
+	err = pm886_setup_irq(chip, &irq_data);
+	if (err)
+		return err;
+
+	err = devm_mfd_add_devices(dev, 0, pm886_devs, ARRAY_SIZE(pm886_devs),
+				NULL, 0, regmap_irq_get_domain(irq_data));
+	if (err)
+		return dev_err_probe(dev, err, "Failed to add devices\n");
+
+	err = devm_register_power_off_handler(dev, pm886_power_off_handler, chip);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to register power off handler\n");
+
+	device_init_wakeup(dev, device_property_read_bool(dev, "wakeup-source"));
+
+	return 0;
+}
+
+static const struct of_device_id pm886_of_match[] = {
+	{ .compatible = "marvell,88pm886-a1", .data = (void *)PM886_A1_CHIP_ID },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pm886_of_match);
+
+static struct i2c_driver pm886_i2c_driver = {
+	.driver = {
+		.name = "88pm886",
+		.of_match_table = pm886_of_match,
+	},
+	.probe = pm886_probe,
+};
+module_i2c_driver(pm886_i2c_driver);
+
+MODULE_DESCRIPTION("Marvell 88PM886 PMIC driver");
+MODULE_AUTHOR("Karel Balej <balejk@matfyz.cz>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e7a6e45b9fac..9ef921c59f30 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -794,6 +794,18 @@ config MFD_88PM860X
 	  select individual components like voltage regulators, RTC and
 	  battery-charger under the corresponding menus.
 
+config MFD_88PM886_PMIC
+	bool "Marvell 88PM886 PMIC"
+	depends on I2C=y
+	depends on OF
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	select MFD_CORE
+	help
+	  This enables support for Marvell 88PM886 Power Management IC.
+	  This includes the I2C driver and the core APIs _only_, you have to
+	  select individual components like onkey under the corresponding menus.
+
 config MFD_MAX14577
 	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..d016b7fed354 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -7,6 +7,7 @@
 obj-$(CONFIG_MFD_88PM860X)	+= 88pm860x.o
 obj-$(CONFIG_MFD_88PM800)	+= 88pm800.o 88pm80x.o
 obj-$(CONFIG_MFD_88PM805)	+= 88pm805.o 88pm80x.o
+obj-$(CONFIG_MFD_88PM886_PMIC)	+= 88pm886.o
 obj-$(CONFIG_MFD_ACT8945A)	+= act8945a.o
 obj-$(CONFIG_MFD_SM501)		+= sm501.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
diff --git a/include/linux/mfd/88pm886.h b/include/linux/mfd/88pm886.h
new file mode 100644
index 000000000000..a5e6524bb19d
--- /dev/null
+++ b/include/linux/mfd/88pm886.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __MFD_88PM886_H
+#define __MFD_88PM886_H
+
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+#define PM886_A1_CHIP_ID		0xa1
+
+#define PM886_REGMAP_CONF_MAX_REG	0xfe
+
+#define PM886_REG_ID			0x00
+
+#define PM886_REG_STATUS1		0x01
+#define PM886_ONKEY_STS1		BIT(0)
+
+#define PM886_REG_MISC_CONFIG1		0x14
+#define PM886_SW_PDOWN			BIT(5)
+
+#define PM886_REG_MISC_CONFIG2		0x15
+#define PM886_INT_INV			BIT(0)
+#define PM886_INT_CLEAR			BIT(1)
+#define PM886_INT_RC			0x00
+#define PM886_INT_WC			BIT(1)
+#define PM886_INT_MASK_MODE		BIT(2)
+
+struct pm886_chip {
+	struct i2c_client *client;
+	unsigned int chip_id;
+	struct regmap *regmap;
+};
+
+static const struct regmap_config pm886_i2c_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = PM886_REGMAP_CONF_MAX_REG,
+};
+#endif /* __MFD_88PM886_H */
-- 
2.44.0


^ permalink raw reply related

* [RFC PATCH v4 1/5] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240311160110.32185-1-karelb@gimli.ms.mff.cuni.cz>

From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM886 is a PMIC with several subdevices such as onkey,
regulators or battery and charger. It comes in at least two revisions,
A0 and A1 -- only A1 is described here at the moment.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Karel Balej <balejk@matfyz.cz>
---

Notes:
    RFC v4:
    - Address Krzysztof's comments:
      - Fix regulators indentation.
      - Add Krzysztof's trailer.
    RFC v3:
    - Add wakeup-source property.
    - Address Rob's feedback:
      - Move regulators into the MFD file.
      - Remove interrupt-controller and #interrupt-cells properties.
    RFC v2:
    - Address Rob's feedback:
      - Drop mention of 88PM880.
      - Make sure the file passes bindings check (add the necessary header
        and fix `interrupt-cells`).
      - Other small changes.
    - Add regulators. Changes with respect to the regulator RFC series:
      - Address Krzysztof's comments:
        - Drop unused compatible.
        - Fix sub-node pattern.

 .../bindings/mfd/marvell,88pm886-a1.yaml      | 76 +++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml

diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
new file mode 100644
index 000000000000..d6a71c912b76
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/marvell,88pm886-a1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell 88PM886 PMIC core
+
+maintainers:
+  - Karel Balej <balejk@matfyz.cz>
+
+description:
+  Marvell 88PM886 is a PMIC providing several functions such as onkey,
+  regulators or battery and charger.
+
+properties:
+  compatible:
+    const: marvell,88pm886-a1
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  wakeup-source: true
+
+  regulators:
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^(ldo(1[0-6]|[1-9])|buck[1-5])$":
+        type: object
+        $ref: /schemas/regulator/regulator.yaml#
+        description: LDO or buck regulator.
+        unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@30 {
+        compatible = "marvell,88pm886-a1";
+        reg = <0x30>;
+        interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&gic>;
+        wakeup-source;
+
+        regulators {
+          ldo2: ldo2 {
+            regulator-min-microvolt = <3100000>;
+            regulator-max-microvolt = <3300000>;
+          };
+
+          ldo15: ldo15 {
+            regulator-min-microvolt = <3300000>;
+            regulator-max-microvolt = <3300000>;
+          };
+
+          buck2: buck2 {
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+          };
+        };
+      };
+    };
+...
-- 
2.44.0


^ permalink raw reply related

* [RFC PATCH v4 0/5] initial support for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 15:51 UTC (permalink / raw)
  To: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Dmitry Torokhov, Liam Girdwood, Mark Brown,
	devicetree, linux-kernel, linux-input
  Cc: Duje Mihanović, ~postmarketos/upstreaming, phone-devel

From: Karel Balej <balejk@matfyz.cz>

Hello,

the following implements basic support for Marvell's 88PM886 PMIC which
is found for instance as a component of the samsung,coreprimevelte
smartphone which inspired this and also serves as a testing platform.

The code for the MFD is based primarily on this old series [1] with the
addition of poweroff based on the smartphone's downstream kernel tree
[2]. The onkey and regulators drivers are based on the latter. I am not
in possesion of the datasheet.

[1] https://lore.kernel.org/all/1434098601-3498-1-git-send-email-yizhang@marvell.com/
[2] https://github.com/CoderCharmander/g361f-kernel

Thank you and kind regards,
K. B.
---
RFC v4:
- RFC v3: https://lore.kernel.org/all/20240303101506.4187-1-karelb@gimli.ms.mff.cuni.cz/
RFC v3:
- Address Rob's feedback:
  - Drop onkey bindings patch.
- Rename PM88X -> PM886 everywhere.
- RFC v2: https://lore.kernel.org/all/20240211094609.2223-1-karelb@gimli.ms.mff.cuni.cz/
RFC v2:
- Merge with the regulators series to have multiple devices and thus
  justify the use of the MFD framework.
- Rebase on v6.8-rc3.
- Reorder patches.
- MFD RFC v1: https://lore.kernel.org/all/20231217131838.7569-1-karelb@gimli.ms.mff.cuni.cz/
- regulators RFC v1: https://lore.kernel.org/all/20231228100208.2932-1-karelb@gimli.ms.mff.cuni.cz/

Karel Balej (5):
  dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
  mfd: add driver for Marvell 88PM886 PMIC
  regulator: add regulators driver for Marvell 88PM886 PMIC
  input: add onkey driver for Marvell 88PM886 PMIC
  MAINTAINERS: add myself for Marvell 88PM886 PMIC

 .../bindings/mfd/marvell,88pm886-a1.yaml      |  76 +++++++
 MAINTAINERS                                   |   9 +
 drivers/input/misc/88pm886-onkey.c            |  99 ++++++++
 drivers/input/misc/Kconfig                    |   7 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/mfd/88pm886.c                         | 149 ++++++++++++
 drivers/mfd/Kconfig                           |  12 +
 drivers/mfd/Makefile                          |   1 +
 drivers/regulator/88pm886-regulator.c         | 215 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   6 +
 drivers/regulator/Makefile                    |   1 +
 include/linux/mfd/88pm886.h                   |  38 ++++
 12 files changed, 614 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm886-a1.yaml
 create mode 100644 drivers/input/misc/88pm886-onkey.c
 create mode 100644 drivers/mfd/88pm886.c
 create mode 100644 drivers/regulator/88pm886-regulator.c
 create mode 100644 include/linux/mfd/88pm886.h

-- 
2.44.0


^ permalink raw reply

* Re: [PATCH v2 0/2] map Fn + R key on newer Lenovo Yogas and Legions
From: Ilpo Järvinen @ 2024-03-11 13:28 UTC (permalink / raw)
  To: Gergo Koteles
  Cc: Ike Panhc, Hans de Goede, Dmitry Torokhov, platform-driver-x86,
	LKML, linux-input
In-Reply-To: <cover.1710065750.git.soyer@irl.hu>

On Sun, 10 Mar 2024, Gergo Koteles wrote:

> Hi All,
> 
> This patch series adds a new KEY_REFRESH_RATE_TOGGLE input event code 
> and maps the Fn + R key to it in the ideapad-laptop driver.
> 
> It affects two WMI keycodes. I couldn't try the 0x0a.
> 
> Regards,
> Gergo
> 
> Changes in v2:
>  - use KEY_REFRESH_RATE_TOGGLE instead of KEY_FN_R
> 
> [1]: https://lore.kernel.org/all/cover.1708399689.git.soyer@irl.hu/
> 
> Gergo Koteles (2):
>   Input: allocate keycode for Display refresh rate toggle
>   platform/x86: ideapad-laptop: map Fn + R key to
>     KEY_REFRESH_RATE_TOGGLE

Hi,

As mentioned in the other thread, please redo this on top of 
pdx86/for-next.

-- 
 i.


^ permalink raw reply

* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 12:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Dmitry Torokhov
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel, linux-input,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <ce1cac6f-afb9-4388-b709-bfaee0feb525@linaro.org>

Krzysztof Kozlowski, 2024-03-11T11:41:53+01:00:
> On 11/03/2024 11:26, Karel Balej wrote:
> > Krzysztof Kozlowski, 2024-03-10T21:35:36+01:00:
> >> On 10/03/2024 12:35, Karel Balej wrote:
> >>> Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
> >>>> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
> >>>>> Dmitry,
> >>>>>
> >>>>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
> >>>>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
> >>>>>>> From: Karel Balej <balejk@matfyz.cz>
> >>>>>>>
> >>>>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
> >>>>>>> driver to handle it. The driver currently only provides a basic support
> >>>>>>> omitting additional functions found in the vendor version, such as long
> >>>>>>> onkey and GPIO integration.
> >>>>>>>
> >>>>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> >>>>>>> ---
> >>>>>>>
> >>>>>>> Notes:
> >>>>>>>     RFC v3:
> >>>>>>>     - Drop wakeup-source.
> >>>>>>>     RFC v2:
> >>>>>>>     - Address Dmitry's feedback:
> >>>>>>>       - Sort includes alphabetically.
> >>>>>>>       - Drop onkey->irq.
> >>>>>>>       - ret -> err in irq_handler and no initialization.
> >>>>>>>       - Break long lines and other formatting.
> >>>>>>>       - Do not clobber platform_get_irq error.
> >>>>>>>       - Do not set device parent manually.
> >>>>>>>       - Use input_set_capability.
> >>>>>>>       - Use the wakeup-source DT property.
> >>>>>>>       - Drop of_match_table.
> >>>>>>
> >>>>>> I only said that you should not be using of_match_ptr(), but you still
> >>>>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
> >>>>>> proper module loading support.
> >>>>>
> >>>>> I removed of_match_table because I no longer need compatible for this --
> >>>>> there are no device tree properties and the driver is being instantiated
> >>>>> by the MFD driver.
> >>>>>
> >>>>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
> >>>>> compiled as module? If that is the case, given what I write above, am I
> >>>>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
> >>>>> to use here?
> >>>>
> >>>> Yes, if uevent generated for the device is "platform:<name>" then
> >>>> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
> >>>> sets it up (OF modalias or platform), but you should be able to check
> >>>> the format looking at the "uevent" attribute for your device in sysfs
> >>>> (/sys/devices/bus/platform/...). 
> >>>
> >>> The uevent is indeed platform.
> >>>
> >>> But since there is only one device, perhaps having a device table is
> >>> superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> >>> fitting?
> >>
> >> Adding aliases for standard IDs and standard cases is almost never
> >> correct. If you need module alias, it means your ID table is wrong (or
> >> missing, which is usually wrong).
> >>
> >>>
> >>> Although I don't understand why this is even necessary when the driver
> >>> name is such and the module is registered using
> >>> `module_platform_driver`...
> >>
> >> ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
> > 
> > I think I understand the practical reasons. My point was that I would
> > expect the alias to be added automatically even in the case that the
> > device table is absent based solely on the driver name and the
> > registration method (*module*_*platform*_driver). Why is that not the
> > case? Obviously the driver name matching the mfd_cell name is sufficient
>
> You mean add it automatically by macro-magic based on presence of
> id_table and/or of_match_table?

Yes, that plus: if id_table is present, use that for module aliases,
otherwise use driver name. In fact, I checked the platform core and it
seems to proceed in exactly this way when determining whether there is a
match. And while autoloading and probing are two different things, I
assume that we always want to consider a module for autoloading when we
know that it will probe because we have a compatible device.

> That's a good question. I cannot find answer why not, except that maybe
> no one ever wrote it...
>
>
> > for the driver to probe when it is built in so the name does seem to
> > serve as some identification for the device just as a device table entry
> > would.
> > 
> > Furthermore, drivers/input/serio/ioc3kbd.c does not seem to have an ID
> > table either, nor a MODULE_ALIAS -- is that a mistake? If not, what
> > mechanism causes the driver to probe when compiled as a module? It seems
>
> You are now mixing two different things: probing of driver (so bind) and
> module auto-loading.

Yes, sorry, I meant autoloading.

> Probing is done also by driver name. Auto-loading,
> not sure, maybe by name as well?

Well probably not, otherwise it would work here too, no? Unless there
are some fundamental differences in this between PCI and platform
drivers. But the input driver is platform too and is required through
the MFD cell, so I think it should be the same scenario.

> However it is also likely that
> auto-loading is broken. Several drivers had such issues in the past.

OK, I see, thank you. I think this was the main source of my confusion
because I looked at other drivers for reference when trying to
understand which properties (name/device table) are necessary for what
action (probing/autoloading).

> Best regards,
> Krzysztof

Thank you again, kind regards,
K. B.

^ permalink raw reply

* Re: [PATCH 0/2] map Fn + R key on newer Yogas and Legions
From: Ilpo Järvinen @ 2024-03-11 10:53 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Ike Panhc, Hans de Goede, Philipp Jungkamp, Gergo Koteles,
	platform-driver-x86, LKML, linux-input
In-Reply-To: <Ze0PSaOQSJMxL_Ln@google.com>

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

On Sat, 9 Mar 2024, Dmitry Torokhov wrote:

> On Mon, Feb 26, 2024 at 03:27:33PM +0200, Ilpo Järvinen wrote:
> > On Tue, 20 Feb 2024 04:39:34 +0100, Gergo Koteles wrote:
> > 
> > > This patch series adds a new KEY_FN_R input event code and map the
> > > Fn + R key to it in the ideapad-laptop driver.
> > > 
> > > It affects two WMI keycodes and I couldn't try the 0x0a, but I couldn't
> > > find any indication that the refresh rate toggle should not be Fn + R.
> > > 
> > > Regards,
> > > Gergo
> > > 
> > > [...]
> > 
> > 
> > Thank you for your contribution, it has been applied to my local
> > review-ilpo branch. Note it will show up in the public
> > platform-drivers-x86/review-ilpo branch only once I've pushed my
> > local branch there, which might take a while.
> > 
> > The list of commits applied:
> > [1/2] Input: allocate keycode for Fn + R
> >       commit: 4e45fa464aeef4e803412b5dcce73aad48c94b0e
> 
> I am sorry for the delay, but instead of defining a generic name we should define
> a proper keycode for concrete action even if nothing is printed on a
> particular key on a particular device.
> 
> Please drop this patch.

Dropping them at this point would bit of a hassle due to backmerges of
fixes branch.

Gergo please make new changes on top of for-next for the change into the 
proper keycode (I see up sent a new series with assumption that I drop 
the other patches).

-- 
 i.

^ permalink raw reply

* [dtor-input:next] BUILD SUCCESS 2d77f70bb7180060072b9c2406d67e4b3872af15
From: kernel test robot @ 2024-03-11 10:42 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 2d77f70bb7180060072b9c2406d67e4b3872af15  Input: imagis - add touch key support

elapsed time: 727m

configs tested: 200
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig   gcc  
alpha                            allyesconfig   gcc  
alpha                               defconfig   gcc  
arc                              alldefconfig   gcc  
arc                              allmodconfig   gcc  
arc                               allnoconfig   gcc  
arc                              allyesconfig   gcc  
arc                          axs101_defconfig   gcc  
arc                                 defconfig   gcc  
arc                   randconfig-001-20240311   gcc  
arc                   randconfig-002-20240311   gcc  
arm                              alldefconfig   gcc  
arm                              allmodconfig   gcc  
arm                               allnoconfig   clang
arm                              allyesconfig   gcc  
arm                                 defconfig   clang
arm                           h3600_defconfig   gcc  
arm                           imxrt_defconfig   clang
arm                        neponset_defconfig   gcc  
arm                       netwinder_defconfig   gcc  
arm                   randconfig-001-20240311   clang
arm                   randconfig-002-20240311   gcc  
arm                   randconfig-003-20240311   clang
arm                   randconfig-004-20240311   clang
arm                        shmobile_defconfig   gcc  
arm                       spear13xx_defconfig   gcc  
arm64                            allmodconfig   clang
arm64                             allnoconfig   gcc  
arm64                               defconfig   gcc  
arm64                 randconfig-001-20240311   clang
arm64                 randconfig-002-20240311   clang
arm64                 randconfig-003-20240311   clang
arm64                 randconfig-004-20240311   clang
csky                             allmodconfig   gcc  
csky                              allnoconfig   gcc  
csky                             allyesconfig   gcc  
csky                                defconfig   gcc  
csky                  randconfig-001-20240311   gcc  
csky                  randconfig-002-20240311   gcc  
hexagon                          allmodconfig   clang
hexagon                           allnoconfig   clang
hexagon                          allyesconfig   clang
hexagon                             defconfig   clang
hexagon               randconfig-001-20240311   clang
hexagon               randconfig-002-20240311   clang
i386                             allmodconfig   gcc  
i386                              allnoconfig   gcc  
i386                             allyesconfig   gcc  
i386         buildonly-randconfig-001-20240311   clang
i386         buildonly-randconfig-002-20240311   clang
i386         buildonly-randconfig-003-20240311   clang
i386         buildonly-randconfig-004-20240311   gcc  
i386         buildonly-randconfig-005-20240311   clang
i386         buildonly-randconfig-006-20240311   clang
i386                                defconfig   clang
i386                  randconfig-001-20240311   gcc  
i386                  randconfig-002-20240311   gcc  
i386                  randconfig-003-20240311   clang
i386                  randconfig-004-20240311   clang
i386                  randconfig-005-20240311   gcc  
i386                  randconfig-006-20240311   clang
i386                  randconfig-011-20240311   gcc  
i386                  randconfig-012-20240311   gcc  
i386                  randconfig-013-20240311   clang
i386                  randconfig-014-20240311   gcc  
i386                  randconfig-015-20240311   clang
i386                  randconfig-016-20240311   clang
loongarch                        allmodconfig   gcc  
loongarch                         allnoconfig   gcc  
loongarch                        allyesconfig   gcc  
loongarch                           defconfig   gcc  
loongarch             randconfig-001-20240311   gcc  
loongarch             randconfig-002-20240311   gcc  
m68k                             allmodconfig   gcc  
m68k                              allnoconfig   gcc  
m68k                             allyesconfig   gcc  
m68k                          amiga_defconfig   gcc  
m68k                         apollo_defconfig   gcc  
m68k                                defconfig   gcc  
m68k                          hp300_defconfig   gcc  
m68k                          multi_defconfig   gcc  
m68k                        stmark2_defconfig   gcc  
microblaze                       allmodconfig   gcc  
microblaze                        allnoconfig   gcc  
microblaze                       allyesconfig   gcc  
microblaze                          defconfig   gcc  
mips                             allmodconfig   gcc  
mips                              allnoconfig   gcc  
mips                             allyesconfig   gcc  
mips                     decstation_defconfig   gcc  
nios2                            allmodconfig   gcc  
nios2                             allnoconfig   gcc  
nios2                            allyesconfig   gcc  
nios2                               defconfig   gcc  
nios2                 randconfig-001-20240311   gcc  
nios2                 randconfig-002-20240311   gcc  
openrisc                         allmodconfig   gcc  
openrisc                          allnoconfig   gcc  
openrisc                         allyesconfig   gcc  
openrisc                            defconfig   gcc  
openrisc                       virt_defconfig   gcc  
parisc                           allmodconfig   gcc  
parisc                            allnoconfig   gcc  
parisc                           allyesconfig   gcc  
parisc                              defconfig   gcc  
parisc                randconfig-001-20240311   gcc  
parisc                randconfig-002-20240311   gcc  
parisc64                            defconfig   gcc  
powerpc                    adder875_defconfig   gcc  
powerpc                          allmodconfig   gcc  
powerpc                           allnoconfig   gcc  
powerpc                          allyesconfig   clang
powerpc                     ep8248e_defconfig   gcc  
powerpc                 linkstation_defconfig   clang
powerpc                     ppa8548_defconfig   gcc  
powerpc               randconfig-001-20240311   gcc  
powerpc               randconfig-002-20240311   clang
powerpc               randconfig-003-20240311   gcc  
powerpc                    sam440ep_defconfig   gcc  
powerpc                     skiroot_defconfig   clang
powerpc                    socrates_defconfig   gcc  
powerpc                     tqm5200_defconfig   gcc  
powerpc                         wii_defconfig   gcc  
powerpc64             randconfig-001-20240311   clang
powerpc64             randconfig-002-20240311   clang
powerpc64             randconfig-003-20240311   gcc  
riscv                            allmodconfig   clang
riscv                             allnoconfig   gcc  
riscv                            allyesconfig   clang
riscv                               defconfig   clang
riscv                    nommu_k210_defconfig   clang
riscv                 randconfig-001-20240311   clang
riscv                 randconfig-002-20240311   gcc  
riscv                          rv32_defconfig   clang
s390                             allmodconfig   clang
s390                              allnoconfig   clang
s390                             allyesconfig   gcc  
s390                                defconfig   clang
s390                  randconfig-001-20240311   clang
s390                  randconfig-002-20240311   gcc  
sh                               allmodconfig   gcc  
sh                                allnoconfig   gcc  
sh                               allyesconfig   gcc  
sh                         apsh4a3a_defconfig   gcc  
sh                                  defconfig   gcc  
sh                            hp6xx_defconfig   gcc  
sh                    randconfig-001-20240311   gcc  
sh                    randconfig-002-20240311   gcc  
sh                          rsk7203_defconfig   gcc  
sh                           se7722_defconfig   gcc  
sparc                            allmodconfig   gcc  
sparc                             allnoconfig   gcc  
sparc                            allyesconfig   gcc  
sparc                               defconfig   gcc  
sparc64                          allmodconfig   gcc  
sparc64                          allyesconfig   gcc  
sparc64                             defconfig   gcc  
sparc64               randconfig-001-20240311   gcc  
sparc64               randconfig-002-20240311   gcc  
um                               allmodconfig   clang
um                                allnoconfig   clang
um                               allyesconfig   gcc  
um                                  defconfig   clang
um                             i386_defconfig   gcc  
um                    randconfig-001-20240311   gcc  
um                    randconfig-002-20240311   clang
um                           x86_64_defconfig   clang
x86_64                            allnoconfig   clang
x86_64                           allyesconfig   clang
x86_64       buildonly-randconfig-001-20240311   clang
x86_64       buildonly-randconfig-002-20240311   clang
x86_64       buildonly-randconfig-003-20240311   clang
x86_64       buildonly-randconfig-004-20240311   gcc  
x86_64       buildonly-randconfig-005-20240311   clang
x86_64       buildonly-randconfig-006-20240311   gcc  
x86_64                              defconfig   gcc  
x86_64                randconfig-001-20240311   clang
x86_64                randconfig-002-20240311   clang
x86_64                randconfig-003-20240311   gcc  
x86_64                randconfig-004-20240311   gcc  
x86_64                randconfig-005-20240311   gcc  
x86_64                randconfig-006-20240311   clang
x86_64                randconfig-011-20240311   clang
x86_64                randconfig-012-20240311   clang
x86_64                randconfig-013-20240311   clang
x86_64                randconfig-014-20240311   gcc  
x86_64                randconfig-015-20240311   clang
x86_64                randconfig-016-20240311   gcc  
x86_64                randconfig-071-20240311   gcc  
x86_64                randconfig-072-20240311   clang
x86_64                randconfig-073-20240311   clang
x86_64                randconfig-074-20240311   gcc  
x86_64                randconfig-075-20240311   clang
x86_64                randconfig-076-20240311   clang
x86_64                          rhel-8.3-rust   clang
x86_64                               rhel-8.3   gcc  
xtensa                            allnoconfig   gcc  
xtensa                           allyesconfig   gcc  
xtensa                randconfig-001-20240311   gcc  
xtensa                randconfig-002-20240311   gcc  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Krzysztof Kozlowski @ 2024-03-11 10:41 UTC (permalink / raw)
  To: Karel Balej, Dmitry Torokhov
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel, linux-input,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <CZQUKBQF1GZ9.3RSNW5WQBU9L6@gimli.ms.mff.cuni.cz>

On 11/03/2024 11:26, Karel Balej wrote:
> Krzysztof Kozlowski, 2024-03-10T21:35:36+01:00:
>> On 10/03/2024 12:35, Karel Balej wrote:
>>> Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
>>>> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
>>>>> Dmitry,
>>>>>
>>>>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
>>>>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
>>>>>>> From: Karel Balej <balejk@matfyz.cz>
>>>>>>>
>>>>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
>>>>>>> driver to handle it. The driver currently only provides a basic support
>>>>>>> omitting additional functions found in the vendor version, such as long
>>>>>>> onkey and GPIO integration.
>>>>>>>
>>>>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
>>>>>>> ---
>>>>>>>
>>>>>>> Notes:
>>>>>>>     RFC v3:
>>>>>>>     - Drop wakeup-source.
>>>>>>>     RFC v2:
>>>>>>>     - Address Dmitry's feedback:
>>>>>>>       - Sort includes alphabetically.
>>>>>>>       - Drop onkey->irq.
>>>>>>>       - ret -> err in irq_handler and no initialization.
>>>>>>>       - Break long lines and other formatting.
>>>>>>>       - Do not clobber platform_get_irq error.
>>>>>>>       - Do not set device parent manually.
>>>>>>>       - Use input_set_capability.
>>>>>>>       - Use the wakeup-source DT property.
>>>>>>>       - Drop of_match_table.
>>>>>>
>>>>>> I only said that you should not be using of_match_ptr(), but you still
>>>>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
>>>>>> proper module loading support.
>>>>>
>>>>> I removed of_match_table because I no longer need compatible for this --
>>>>> there are no device tree properties and the driver is being instantiated
>>>>> by the MFD driver.
>>>>>
>>>>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
>>>>> compiled as module? If that is the case, given what I write above, am I
>>>>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
>>>>> to use here?
>>>>
>>>> Yes, if uevent generated for the device is "platform:<name>" then
>>>> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
>>>> sets it up (OF modalias or platform), but you should be able to check
>>>> the format looking at the "uevent" attribute for your device in sysfs
>>>> (/sys/devices/bus/platform/...). 
>>>
>>> The uevent is indeed platform.
>>>
>>> But since there is only one device, perhaps having a device table is
>>> superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
>>> fitting?
>>
>> Adding aliases for standard IDs and standard cases is almost never
>> correct. If you need module alias, it means your ID table is wrong (or
>> missing, which is usually wrong).
>>
>>>
>>> Although I don't understand why this is even necessary when the driver
>>> name is such and the module is registered using
>>> `module_platform_driver`...
>>
>> ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
> 
> I think I understand the practical reasons. My point was that I would
> expect the alias to be added automatically even in the case that the
> device table is absent based solely on the driver name and the
> registration method (*module*_*platform*_driver). Why is that not the
> case? Obviously the driver name matching the mfd_cell name is sufficient

You mean add it automatically by macro-magic based on presence of
id_table and/or of_match_table?

That's a good question. I cannot find answer why not, except that maybe
no one ever wrote it...


> for the driver to probe when it is built in so the name does seem to
> serve as some identification for the device just as a device table entry
> would.
> 
> Furthermore, drivers/input/serio/ioc3kbd.c does not seem to have an ID
> table either, nor a MODULE_ALIAS -- is that a mistake? If not, what
> mechanism causes the driver to probe when compiled as a module? It seems

You are now mixing two different things: probing of driver (so bind) and
module auto-loading. Probing is done also by driver name. Auto-loading,
not sure, maybe by name as well? However it is also likely that
auto-loading is broken. Several drivers had such issues in the past.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
From: Charles Keepax @ 2024-03-11 10:30 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, lee, broonie, patches, linux-sound, linux-input,
	devicetree
In-Reply-To: <Ze5E1KxRltUTX4R6@nixie71>

On Sun, Mar 10, 2024 at 06:40:04PM -0500, Jeff LaBundy wrote:
> On Fri, Mar 08, 2024 at 10:24:19PM +0000, James Ogletree wrote:
> > +static void cs40l50_start_dsp(const struct firmware *bin, void *context)
> > +{
> > +	struct cs40l50 *cs40l50 = context;
> > +	int err;
> > +
> > +	/* Wavetable is optional; start DSP regardless */
> > +	cs40l50->bin = bin;
> > +
> > +	mutex_lock(&cs40l50->lock);
> 
> It seems the mutex is used only to prevent interrupt handling while the DSP
> is being configured; can't we just call disable_irq() and enable_irq() here?
> 

The trouble with diabling the IRQ is it masks the IRQ line. That
means if the IRQ is shared with other devices you will be
silencing their IRQs for the duration as well, which is slightly
rude. Especially given the driver requests the IRQ with the
SHARED flag I would be inclinded to stick with the mutex unless
there are other reasons not to.

> > +static int cs40l50_irq_init(struct cs40l50 *cs40l50)
> > +{
> > +	struct device *dev = cs40l50->dev;
> > +	int err, i, virq;
> > +
> > +	err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
> > +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> > +				       &cs40l50_irq_chip, &cs40l50->irq_data);
> > +	if (err) {
> > +		dev_err(dev, "Failed adding IRQ chip\n");
> 
> I don't see any need for individual prints in this function, since the call
> to cs40l50_irq_init() is already followed by a call to dev_err_probe().

I would probably suggest keeping these individual prints here and
removing the one in cs40l50_probe, it is nicer to know exactly
what failed when something goes wrong. That said at least the
devm_request_threaded_irq can probe defer so that print should be
a dev_err_probe since this function is only called on the probe
path.

> > +	dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
> 
> This should be dev_dbg().
> 

Not sure what the concenus is on this, but personally I greatly
prefer these device ID prints being infos. Nice to always have
some indication of the device and its version.

Thanks,
Charles

^ permalink raw reply

* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-11 10:26 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Dmitry Torokhov
  Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Liam Girdwood, Mark Brown, devicetree, linux-kernel, linux-input,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <3601a374-4161-40e1-8a80-9bbfdae5bd8a@linaro.org>

Krzysztof Kozlowski, 2024-03-10T21:35:36+01:00:
> On 10/03/2024 12:35, Karel Balej wrote:
> > Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
> >> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
> >>> Dmitry,
> >>>
> >>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
> >>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
> >>>>> From: Karel Balej <balejk@matfyz.cz>
> >>>>>
> >>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
> >>>>> driver to handle it. The driver currently only provides a basic support
> >>>>> omitting additional functions found in the vendor version, such as long
> >>>>> onkey and GPIO integration.
> >>>>>
> >>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> >>>>> ---
> >>>>>
> >>>>> Notes:
> >>>>>     RFC v3:
> >>>>>     - Drop wakeup-source.
> >>>>>     RFC v2:
> >>>>>     - Address Dmitry's feedback:
> >>>>>       - Sort includes alphabetically.
> >>>>>       - Drop onkey->irq.
> >>>>>       - ret -> err in irq_handler and no initialization.
> >>>>>       - Break long lines and other formatting.
> >>>>>       - Do not clobber platform_get_irq error.
> >>>>>       - Do not set device parent manually.
> >>>>>       - Use input_set_capability.
> >>>>>       - Use the wakeup-source DT property.
> >>>>>       - Drop of_match_table.
> >>>>
> >>>> I only said that you should not be using of_match_ptr(), but you still
> >>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
> >>>> proper module loading support.
> >>>
> >>> I removed of_match_table because I no longer need compatible for this --
> >>> there are no device tree properties and the driver is being instantiated
> >>> by the MFD driver.
> >>>
> >>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
> >>> compiled as module? If that is the case, given what I write above, am I
> >>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
> >>> to use here?
> >>
> >> Yes, if uevent generated for the device is "platform:<name>" then
> >> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
> >> sets it up (OF modalias or platform), but you should be able to check
> >> the format looking at the "uevent" attribute for your device in sysfs
> >> (/sys/devices/bus/platform/...). 
> > 
> > The uevent is indeed platform.
> > 
> > But since there is only one device, perhaps having a device table is
> > superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> > fitting?
>
> Adding aliases for standard IDs and standard cases is almost never
> correct. If you need module alias, it means your ID table is wrong (or
> missing, which is usually wrong).
>
> > 
> > Although I don't understand why this is even necessary when the driver
> > name is such and the module is registered using
> > `module_platform_driver`...
>
> ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.

I think I understand the practical reasons. My point was that I would
expect the alias to be added automatically even in the case that the
device table is absent based solely on the driver name and the
registration method (*module*_*platform*_driver). Why is that not the
case? Obviously the driver name matching the mfd_cell name is sufficient
for the driver to probe when it is built in so the name does seem to
serve as some identification for the device just as a device table entry
would.

Furthermore, drivers/input/serio/ioc3kbd.c does not seem to have an ID
table either, nor a MODULE_ALIAS -- is that a mistake? If not, what
mechanism causes the driver to probe when compiled as a module? It seems
to me to effectively be the same setup as with my driver and that does
not load automatically (because of the missing alias).

> Just run `modinfo`.

Thank you very much,
K. B.

^ permalink raw reply

* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
From: Charles Keepax @ 2024-03-11 10:14 UTC (permalink / raw)
  To: Jeff LaBundy
  Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, lee, broonie, patches, linux-sound, linux-input,
	devicetree
In-Reply-To: <Ze4GEKkXoRA/4Sga@nixie71>

On Sun, Mar 10, 2024 at 02:12:16PM -0500, Jeff LaBundy wrote:
> On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> > +	switch (op_code) {
> > +	case CS_DSP_WSEQ_FULL:
> > +		cs_dsp_chunk_write(&ch, 32, op_new->address);
> > +		cs_dsp_chunk_write(&ch, 32, op_new->data);
> > +		break;
> > +	case CS_DSP_WSEQ_L16:
> > +	case CS_DSP_WSEQ_H16:
> > +		cs_dsp_chunk_write(&ch, 24, op_new->address);
> > +		cs_dsp_chunk_write(&ch, 16, op_new->data);
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
> > +		goto op_new_free;
> 
> There is no need to drop down and call devm_kfree() here; it's sufficient
> to simply return -EINVAL. The devres core will free any instances of
> cs_dsp_wseq_op when the driver is unbound.
> 
> I imagine you're calling devm_kfree() to protect against the case where
> the driver successfully probes, but the contents of the firmware are found
> to be invalid later. In that case, yes, this newly allocated memory will
> persist throughout the length of the driver's life.
> 
> That's an error condition though; if it happens, the customer will surely
> remove the module, correct the .wmfw or .bin file, then insert the module
> again. All we need to do here is make sure that memory does not leak after
> the module is removed, and device-managed allocation already handles this.
> 

I disagree here. This is the write function, there is no guarantee
this is even called at probe time. This is generic code going
into the DSP library and will presumably get re-used for different
purposes and on different parts in the future. Freeing on the error
path is much safer here.

> > +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> > +			    const struct reg_sequence *reg_seq, int num_regs,
> > +			    u8 op_code, bool update)
> > +{
> > +	int ret, i;
> > +
> > +	for (i = 0; i < num_regs; i++) {
> > +		ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
> > +					reg_seq[i].def, update, op_code);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> 
> This is absolutely a nit-pick, but in cs_dsp_wseq_init(), you use the pattern
> of breaking on error and always returning ret; here you return on error. Both
> are functionally equivalent but it would be nice to be consistent in terms of
> style.
> 
> If you do opt to match cs_dsp_wseq_multi_write() to cs_dsp_wseq_init(), I do
> think you'll need to initialize ret to zero here as well to avoid a compiler
> warning for the num_regs = 0 case.

I would be inclined to make cs_dsp_wseq_init function like this
one personally, rather than the other way around. Generally best
to return if there is no clean up to do.

Thanks,
Charles

^ permalink raw reply

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: syzbot @ 2024-03-11 10:08 UTC (permalink / raw)
  To: nogikh
  Cc: akpm, linux-input, linux-kernel, linux-mm, linux-usb, nogikh,
	pasha.tatashin, qiang.zhang1211, syzkaller-bugs
In-Reply-To: <CANp29Y40T7LrN07BnMiGE4u1EK4wo-FgxSGNBQ9dKOn=yajmdA@mail.gmail.com>

> On Mon, Mar 11, 2024 at 10:15 AM syzbot
> <syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com> wrote:
>>
>> >>
>> >> Hello,
>> >>
>> >> syzbot found the following issue on:
>> >>
>> >> HEAD commit:    90d35da658da Linux 6.8-rc7
>> >> git tree:       upstream
>> >> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
>> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
>> >> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
>> >> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
>> >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
>> >>
>> >> Downloadable assets:
>> >> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
>> >> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
>> >> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
>> >>
>> >> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> >> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com
>> >
>> > #syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>
> Please try to either put this whole command on one line or add : after
> #syz test (as in the example at the bottom of the report), then syzbot

want either no args or 2 args (repo, branch), got 12

> will parse the next line with the branch name as well.
>
> -- 
> Aleksandr
>
>>
>> want either no args or 2 args (repo, branch), got 1
>>
>> > master
>> >
>> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> > index 7019a40457a6..69e344f07e68 100644
>> > --- a/kernel/sched/core.c
>> > +++ b/kernel/sched/core.c
>> > @@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
>> >                  */
>> >                 touch_nmi_watchdog();
>> >                 touch_all_softlockup_watchdogs();
>> > +               rcu_cpu_stall_reset();
>> >                 if (state_filter_match(state_filter, p))
>> >                         sched_show_task(p);
>> >         }
>> >
>> >
>> >
>> >
>> >>
>> >> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
>> >>  1-....
>> >>  } 4831 jiffies s: 1849 root: 0x2/.
>> >> rcu: blocking rcu_node structures (internal RCU debug):
>> >> Sending NMI from CPU 0 to CPUs 1:
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >> NMI backtrace for cpu 1
>> >> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
>> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
>> >> RIP: 0010:format_decode+0x546/0x1bb0
>> >> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
>> >> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
>> >> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
>> >> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
>> >> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
>> >> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
>> >> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
>> >> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
>> >> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> >> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
>> >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> >> Call Trace:
>> >>  <NMI>
>> >>  </NMI>
>> >>  <IRQ>
>> >>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
>> >>  sprintf+0xda/0x120 lib/vsprintf.c:3028
>> >>  print_time kernel/printk/printk.c:1324 [inline]
>> >>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
>> >>  record_print_text kernel/printk/printk.c:1399 [inline]
>> >>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
>> >>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
>> >>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
>> >>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
>> >>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
>> >>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
>> >>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
>> >>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
>> >>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
>> >>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
>> >>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
>> >>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
>> >>  input_to_handler drivers/input/input.c:132 [inline]
>> >>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
>> >>  input_event_dispose drivers/input/input.c:378 [inline]
>> >>  input_handle_event drivers/input/input.c:406 [inline]
>> >>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
>> >>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
>> >>  expire_timers kernel/time/timer.c:1751 [inline]
>> >>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
>> >>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
>> >>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
>> >>  invoke_softirq kernel/softirq.c:427 [inline]
>> >>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
>> >>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
>> >>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
>> >>  </IRQ>
>> >>  <TASK>
>> >>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
>> >> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
>> >> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
>> >> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
>> >> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
>> >> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
>> >> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
>> >> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
>> >> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
>> >>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
>> >>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
>> >>  set_ptes include/linux/pgtable.h:241 [inline]
>> >>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
>> >>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
>> >>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
>> >>  do_fault_around mm/memory.c:4716 [inline]
>> >>  do_read_fault mm/memory.c:4749 [inline]
>> >>  do_fault mm/memory.c:4888 [inline]
>> >>  do_pte_missing mm/memory.c:3745 [inline]
>> >>  handle_pte_fault mm/memory.c:5164 [inline]
>> >>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
>> >>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
>> >>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
>> >>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
>> >>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
>> >>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
>> >> RIP: 0033:0x7f74dc087080
>> >> Code: Unable to access opcode bytes at 0x7f74dc087056.
>> >> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
>> >> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
>> >> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
>> >> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
>> >> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
>> >> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
>> >>  </TASK>
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
>> >> Workqueue:  0x0 (events_unbound)
>> >> Call Trace:
>> >>  <TASK>
>> >>  context_switch kernel/sched/core.c:5400 [inline]
>> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
>> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
>> >>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
>> >> Workqueue:  0x0 (bat_events)
>> >> Call Trace:
>> >>  <TASK>
>> >>  context_switch kernel/sched/core.c:5400 [inline]
>> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
>> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
>> >>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
>> >> Call Trace:
>> >>  <TASK>
>> >>  context_switch kernel/sched/core.c:5400 [inline]
>> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
>> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
>> >>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
>> >> Call Trace:
>> >>  <TASK>
>> >>  context_switch kernel/sched/core.c:5400 [inline]
>> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
>> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
>> >>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>> >>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
>> >> Call Trace:
>> >>  <TASK>
>> >>  context_switch kernel/sched/core.c:5400 [inline]
>> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
>> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
>> >>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>> >>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>> >>  </TASK>
>> >> task:ksoftirqd/0
>> >>
>> >>
>> >> ---
>> >> This report is generated by a bot. It may contain errors.
>> >> See https://goo.gl/tpsmEJ for more information about syzbot.
>> >> syzbot engineers can be reached at syzkaller@googlegroups.com.
>> >>
>> >> syzbot will keep track of this issue. See:
>> >> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>> >>
>> >> If the report is already addressed, let syzbot know by replying with:
>> >> #syz fix: exact-commit-title
>> >>
>> >> If you want syzbot to run the reproducer, reply with:
>> >> #syz test: git://repo/address.git branch-or-commit-hash
>> >> If you attach or paste a git patch, syzbot will apply it before testing.
>> >>
>> >> If you want to overwrite report's subsystems, reply with:
>> >> #syz set subsystems: new-subsystem
>> >> (See the list of subsystem names on the web dashboard)
>> >>
>> >> If the report is a duplicate of another one, reply with:
>> >> #syz dup: exact-subject-of-another-report
>> >>
>> >> If you want to undo deduplication, reply with:
>> >> #syz undup
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000007d76cf06135efac5%40google.com.

^ permalink raw reply

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: Aleksandr Nogikh @ 2024-03-11 10:08 UTC (permalink / raw)
  To: syzbot
  Cc: qiang.zhang1211, akpm, linux-input, linux-kernel, linux-mm,
	linux-usb, pasha.tatashin, syzkaller-bugs
In-Reply-To: <0000000000007d76cf06135efac5@google.com>

On Mon, Mar 11, 2024 at 10:15 AM syzbot
<syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com> wrote:
>
> >>
> >> Hello,
> >>
> >> syzbot found the following issue on:
> >>
> >> HEAD commit:    90d35da658da Linux 6.8-rc7
> >> git tree:       upstream
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
> >> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
> >> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
> >>
> >> Downloadable assets:
> >> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
> >> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
> >> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
> >>
> >> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> >> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com
> >
> > #syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

Please try to either put this whole command on one line or add : after
#syz test (as in the example at the bottom of the report), then syzbot
will parse the next line with the branch name as well.

-- 
Aleksandr

>
> want either no args or 2 args (repo, branch), got 1
>
> > master
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 7019a40457a6..69e344f07e68 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
> >                  */
> >                 touch_nmi_watchdog();
> >                 touch_all_softlockup_watchdogs();
> > +               rcu_cpu_stall_reset();
> >                 if (state_filter_match(state_filter, p))
> >                         sched_show_task(p);
> >         }
> >
> >
> >
> >
> >>
> >> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
> >>  1-....
> >>  } 4831 jiffies s: 1849 root: 0x2/.
> >> rcu: blocking rcu_node structures (internal RCU debug):
> >> Sending NMI from CPU 0 to CPUs 1:
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >> NMI backtrace for cpu 1
> >> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
> >> RIP: 0010:format_decode+0x546/0x1bb0
> >> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
> >> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
> >> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
> >> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
> >> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
> >> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
> >> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
> >> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
> >> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
> >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >> Call Trace:
> >>  <NMI>
> >>  </NMI>
> >>  <IRQ>
> >>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
> >>  sprintf+0xda/0x120 lib/vsprintf.c:3028
> >>  print_time kernel/printk/printk.c:1324 [inline]
> >>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
> >>  record_print_text kernel/printk/printk.c:1399 [inline]
> >>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
> >>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
> >>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
> >>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
> >>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
> >>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
> >>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
> >>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
> >>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
> >>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
> >>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
> >>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
> >>  input_to_handler drivers/input/input.c:132 [inline]
> >>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
> >>  input_event_dispose drivers/input/input.c:378 [inline]
> >>  input_handle_event drivers/input/input.c:406 [inline]
> >>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
> >>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
> >>  expire_timers kernel/time/timer.c:1751 [inline]
> >>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
> >>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
> >>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
> >>  invoke_softirq kernel/softirq.c:427 [inline]
> >>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
> >>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
> >>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
> >>  </IRQ>
> >>  <TASK>
> >>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
> >> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
> >> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
> >> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
> >> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
> >> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
> >> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
> >> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
> >> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
> >>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
> >>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
> >>  set_ptes include/linux/pgtable.h:241 [inline]
> >>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
> >>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
> >>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
> >>  do_fault_around mm/memory.c:4716 [inline]
> >>  do_read_fault mm/memory.c:4749 [inline]
> >>  do_fault mm/memory.c:4888 [inline]
> >>  do_pte_missing mm/memory.c:3745 [inline]
> >>  handle_pte_fault mm/memory.c:5164 [inline]
> >>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
> >>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
> >>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
> >>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
> >>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
> >>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
> >> RIP: 0033:0x7f74dc087080
> >> Code: Unable to access opcode bytes at 0x7f74dc087056.
> >> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
> >> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
> >> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
> >> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
> >> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
> >> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
> >>  </TASK>
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
> >> Workqueue:  0x0 (events_unbound)
> >> Call Trace:
> >>  <TASK>
> >>  context_switch kernel/sched/core.c:5400 [inline]
> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
> >>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
> >> Workqueue:  0x0 (bat_events)
> >> Call Trace:
> >>  <TASK>
> >>  context_switch kernel/sched/core.c:5400 [inline]
> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
> >>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
> >> Call Trace:
> >>  <TASK>
> >>  context_switch kernel/sched/core.c:5400 [inline]
> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
> >>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
> >> Call Trace:
> >>  <TASK>
> >>  context_switch kernel/sched/core.c:5400 [inline]
> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
> >>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
> >>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
> >> Call Trace:
> >>  <TASK>
> >>  context_switch kernel/sched/core.c:5400 [inline]
> >>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
> >>  __schedule_loop kernel/sched/core.c:6802 [inline]
> >>  schedule+0x149/0x260 kernel/sched/core.c:6817
> >>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
> >>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
> >>  kthread+0x2ef/0x390 kernel/kthread.c:388
> >>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >>  </TASK>
> >> task:ksoftirqd/0
> >>
> >>
> >> ---
> >> This report is generated by a bot. It may contain errors.
> >> See https://goo.gl/tpsmEJ for more information about syzbot.
> >> syzbot engineers can be reached at syzkaller@googlegroups.com.
> >>
> >> syzbot will keep track of this issue. See:
> >> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> >>
> >> If the report is already addressed, let syzbot know by replying with:
> >> #syz fix: exact-commit-title
> >>
> >> If you want syzbot to run the reproducer, reply with:
> >> #syz test: git://repo/address.git branch-or-commit-hash
> >> If you attach or paste a git patch, syzbot will apply it before testing.
> >>
> >> If you want to overwrite report's subsystems, reply with:
> >> #syz set subsystems: new-subsystem
> >> (See the list of subsystem names on the web dashboard)
> >>
> >> If the report is a duplicate of another one, reply with:
> >> #syz dup: exact-subject-of-another-report
> >>
> >> If you want to undo deduplication, reply with:
> >> #syz undup
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000007d76cf06135efac5%40google.com.

^ permalink raw reply

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: syzbot @ 2024-03-11  9:15 UTC (permalink / raw)
  To: qiang.zhang1211
  Cc: akpm, linux-input, linux-kernel, linux-mm, linux-usb,
	pasha.tatashin, qiang.zhang1211, syzkaller-bugs
In-Reply-To: <CALm+0cVun=SSczF2hJPJFZU85NMmntScWAFLL7e9pXJiVt+qWw@mail.gmail.com>

>>
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit:    90d35da658da Linux 6.8-rc7
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
>> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
>> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
>>
>> Downloadable assets:
>> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
>> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
>> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com
>
> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

want either no args or 2 args (repo, branch), got 1

> master
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 7019a40457a6..69e344f07e68 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
>                  */
>                 touch_nmi_watchdog();
>                 touch_all_softlockup_watchdogs();
> +               rcu_cpu_stall_reset();
>                 if (state_filter_match(state_filter, p))
>                         sched_show_task(p);
>         }
>
>
>
>
>>
>> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
>>  1-....
>>  } 4831 jiffies s: 1849 root: 0x2/.
>> rcu: blocking rcu_node structures (internal RCU debug):
>> Sending NMI from CPU 0 to CPUs 1:
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> NMI backtrace for cpu 1
>> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
>> RIP: 0010:format_decode+0x546/0x1bb0
>> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
>> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
>> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
>> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
>> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
>> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
>> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
>> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>>  <NMI>
>>  </NMI>
>>  <IRQ>
>>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
>>  sprintf+0xda/0x120 lib/vsprintf.c:3028
>>  print_time kernel/printk/printk.c:1324 [inline]
>>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
>>  record_print_text kernel/printk/printk.c:1399 [inline]
>>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
>>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
>>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
>>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
>>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
>>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
>>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
>>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
>>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
>>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
>>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
>>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
>>  input_to_handler drivers/input/input.c:132 [inline]
>>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
>>  input_event_dispose drivers/input/input.c:378 [inline]
>>  input_handle_event drivers/input/input.c:406 [inline]
>>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
>>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
>>  expire_timers kernel/time/timer.c:1751 [inline]
>>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
>>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
>>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
>>  invoke_softirq kernel/softirq.c:427 [inline]
>>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
>>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
>>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
>>  </IRQ>
>>  <TASK>
>>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
>> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
>> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
>> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
>> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
>> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
>> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
>> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
>> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
>>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
>>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
>>  set_ptes include/linux/pgtable.h:241 [inline]
>>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
>>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
>>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
>>  do_fault_around mm/memory.c:4716 [inline]
>>  do_read_fault mm/memory.c:4749 [inline]
>>  do_fault mm/memory.c:4888 [inline]
>>  do_pte_missing mm/memory.c:3745 [inline]
>>  handle_pte_fault mm/memory.c:5164 [inline]
>>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
>>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
>>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
>>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
>>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
>>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
>> RIP: 0033:0x7f74dc087080
>> Code: Unable to access opcode bytes at 0x7f74dc087056.
>> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
>> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
>> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
>> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
>> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
>> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
>>  </TASK>
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
>> Workqueue:  0x0 (events_unbound)
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
>> Workqueue:  0x0 (bat_events)
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:ksoftirqd/0
>>
>>
>> ---
>> This report is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this issue. See:
>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>>
>> If the report is already addressed, let syzbot know by replying with:
>> #syz fix: exact-commit-title
>>
>> If you want syzbot to run the reproducer, reply with:
>> #syz test: git://repo/address.git branch-or-commit-hash
>> If you attach or paste a git patch, syzbot will apply it before testing.
>>
>> If you want to overwrite report's subsystems, reply with:
>> #syz set subsystems: new-subsystem
>> (See the list of subsystem names on the web dashboard)
>>
>> If the report is a duplicate of another one, reply with:
>> #syz dup: exact-subject-of-another-report
>>
>> If you want to undo deduplication, reply with:
>> #syz undup
>>

^ permalink raw reply

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: Z qiang @ 2024-03-11  9:14 UTC (permalink / raw)
  To: syzbot
  Cc: akpm, linux-input, linux-kernel, linux-mm, linux-usb,
	pasha.tatashin, syzkaller-bugs
In-Reply-To: <0000000000006dbb0b06133aacee@google.com>

>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    90d35da658da Linux 6.8-rc7
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7019a40457a6..69e344f07e68 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
                 */
                touch_nmi_watchdog();
                touch_all_softlockup_watchdogs();
+               rcu_cpu_stall_reset();
                if (state_filter_match(state_filter, p))
                        sched_show_task(p);
        }




>
> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
>  1-....
>  } 4831 jiffies s: 1849 root: 0x2/.
> rcu: blocking rcu_node structures (internal RCU debug):
> Sending NMI from CPU 0 to CPUs 1:
>  kthread+0x2ef/0x390 kernel/kthread.c:388
> NMI backtrace for cpu 1
> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
> RIP: 0010:format_decode+0x546/0x1bb0
> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  <NMI>
>  </NMI>
>  <IRQ>
>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
>  sprintf+0xda/0x120 lib/vsprintf.c:3028
>  print_time kernel/printk/printk.c:1324 [inline]
>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
>  record_print_text kernel/printk/printk.c:1399 [inline]
>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
>  input_to_handler drivers/input/input.c:132 [inline]
>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
>  input_event_dispose drivers/input/input.c:378 [inline]
>  input_handle_event drivers/input/input.c:406 [inline]
>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
>  expire_timers kernel/time/timer.c:1751 [inline]
>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
>  invoke_softirq kernel/softirq.c:427 [inline]
>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
>  </IRQ>
>  <TASK>
>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
>  set_ptes include/linux/pgtable.h:241 [inline]
>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
>  do_fault_around mm/memory.c:4716 [inline]
>  do_read_fault mm/memory.c:4749 [inline]
>  do_fault mm/memory.c:4888 [inline]
>  do_pte_missing mm/memory.c:3745 [inline]
>  handle_pte_fault mm/memory.c:5164 [inline]
>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
> RIP: 0033:0x7f74dc087080
> Code: Unable to access opcode bytes at 0x7f74dc087056.
> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
>  </TASK>
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
> Workqueue:  0x0 (events_unbound)
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
> Workqueue:  0x0 (bat_events)
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:ksoftirqd/0
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>

^ permalink raw reply related

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: syzbot @ 2024-03-11  9:12 UTC (permalink / raw)
  To: qiang.zhang1211
  Cc: akpm, linux-input, linux-kernel, linux-mm, linux-usb,
	pasha.tatashin, qiang.zhang1211, syzkaller-bugs
In-Reply-To: <CALm+0cX+J-Cb_-Vd7i=2M3FiDa89BreqrdXYwMB=yeR6G7GGvw@mail.gmail.com>

>>
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit:    90d35da658da Linux 6.8-rc7
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
>> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
>> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
>>
>> Downloadable assets:
>> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
>> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
>> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com
>
>
> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

want either no args or 2 args (repo, branch), got 1

> master
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 7019a40457a6..69e344f07e68 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
>                  */
>                 touch_nmi_watchdog();
>                 touch_all_softlockup_watchdogs();
> +               rcu_cpu_stall_reset();
>                 if (state_filter_match(state_filter, p))
>                         sched_show_task(p);
>
>         }
>
>
>>
>> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
>>  1-....
>>  } 4831 jiffies s: 1849 root: 0x2/.
>> rcu: blocking rcu_node structures (internal RCU debug):
>> Sending NMI from CPU 0 to CPUs 1:
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>> NMI backtrace for cpu 1
>> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
>> RIP: 0010:format_decode+0x546/0x1bb0
>> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
>> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
>> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
>> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
>> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
>> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
>> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
>> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>>  <NMI>
>>  </NMI>
>>  <IRQ>
>>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
>>  sprintf+0xda/0x120 lib/vsprintf.c:3028
>>  print_time kernel/printk/printk.c:1324 [inline]
>>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
>>  record_print_text kernel/printk/printk.c:1399 [inline]
>>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
>>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
>>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
>>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
>>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
>>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
>>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
>>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
>>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
>>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
>>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
>>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
>>  input_to_handler drivers/input/input.c:132 [inline]
>>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
>>  input_event_dispose drivers/input/input.c:378 [inline]
>>  input_handle_event drivers/input/input.c:406 [inline]
>>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
>>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
>>  expire_timers kernel/time/timer.c:1751 [inline]
>>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
>>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
>>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
>>  invoke_softirq kernel/softirq.c:427 [inline]
>>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
>>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
>>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
>>  </IRQ>
>>  <TASK>
>>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
>> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
>> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
>> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
>> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
>> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
>> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
>> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
>> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
>>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
>>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
>>  set_ptes include/linux/pgtable.h:241 [inline]
>>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
>>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
>>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
>>  do_fault_around mm/memory.c:4716 [inline]
>>  do_read_fault mm/memory.c:4749 [inline]
>>  do_fault mm/memory.c:4888 [inline]
>>  do_pte_missing mm/memory.c:3745 [inline]
>>  handle_pte_fault mm/memory.c:5164 [inline]
>>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
>>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
>>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
>>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
>>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
>>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
>> RIP: 0033:0x7f74dc087080
>> Code: Unable to access opcode bytes at 0x7f74dc087056.
>> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
>> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
>> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
>> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
>> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
>> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
>>  </TASK>
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
>> Workqueue:  0x0 (events_unbound)
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
>> Workqueue:  0x0 (bat_events)
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
>> Call Trace:
>>  <TASK>
>>  context_switch kernel/sched/core.c:5400 [inline]
>>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>>  __schedule_loop kernel/sched/core.c:6802 [inline]
>>  schedule+0x149/0x260 kernel/sched/core.c:6817
>>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>>  kthread+0x2ef/0x390 kernel/kthread.c:388
>>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>>  </TASK>
>> task:ksoftirqd/0
>>
>>
>> ---
>> This report is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this issue. See:
>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>>
>> If the report is already addressed, let syzbot know by replying with:
>> #syz fix: exact-commit-title
>>
>> If you want syzbot to run the reproducer, reply with:
>> #syz test: git://repo/address.git branch-or-commit-hash
>> If you attach or paste a git patch, syzbot will apply it before testing.
>>
>> If you want to overwrite report's subsystems, reply with:
>> #syz set subsystems: new-subsystem
>> (See the list of subsystem names on the web dashboard)
>>
>> If the report is a duplicate of another one, reply with:
>> #syz dup: exact-subject-of-another-report
>>
>> If you want to undo deduplication, reply with:
>> #syz undup
>>

^ permalink raw reply

* Re: [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: Z qiang @ 2024-03-11  9:12 UTC (permalink / raw)
  To: syzbot
  Cc: akpm, linux-input, linux-kernel, linux-mm, linux-usb,
	pasha.tatashin, syzkaller-bugs
In-Reply-To: <0000000000006dbb0b06133aacee@google.com>

>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    90d35da658da Linux 6.8-rc7
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
> dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com


#syz test https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7019a40457a6..69e344f07e68 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9233,6 +9233,7 @@ void show_state_filter(unsigned int state_filter)
                 */
                touch_nmi_watchdog();
                touch_all_softlockup_watchdogs();
+               rcu_cpu_stall_reset();
                if (state_filter_match(state_filter, p))
                        sched_show_task(p);

        }


>
> rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
>  1-....
>  } 4831 jiffies s: 1849 root: 0x2/.
> rcu: blocking rcu_node structures (internal RCU debug):
> Sending NMI from CPU 0 to CPUs 1:
>  kthread+0x2ef/0x390 kernel/kthread.c:388
> NMI backtrace for cpu 1
> CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
> RIP: 0010:format_decode+0x546/0x1bb0
> Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
> RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
> RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
> RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
> RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
> R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
> R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
> FS:  0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  <NMI>
>  </NMI>
>  <IRQ>
>  vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
>  sprintf+0xda/0x120 lib/vsprintf.c:3028
>  print_time kernel/printk/printk.c:1324 [inline]
>  info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
>  record_print_text kernel/printk/printk.c:1399 [inline]
>  printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
>  console_emit_next_record kernel/printk/printk.c:2868 [inline]
>  console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
>  console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
>  vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
>  _printk+0xd5/0x120 kernel/printk/printk.c:2328
>  printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
>  show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
>  sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
>  show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
>  kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
>  kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
>  input_to_handler drivers/input/input.c:132 [inline]
>  input_pass_values+0x945/0x1200 drivers/input/input.c:161
>  input_event_dispose drivers/input/input.c:378 [inline]
>  input_handle_event drivers/input/input.c:406 [inline]
>  input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
>  call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
>  expire_timers kernel/time/timer.c:1751 [inline]
>  __run_timers+0x621/0x830 kernel/time/timer.c:2038
>  run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
>  __do_softirq+0x2bb/0x942 kernel/softirq.c:553
>  invoke_softirq kernel/softirq.c:427 [inline]
>  __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
>  irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
>  sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
>  </IRQ>
>  <TASK>
>  asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
> RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
> Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
> RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
> RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
> RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
> RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
> R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
> R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
>  __page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
>  page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
>  set_ptes include/linux/pgtable.h:241 [inline]
>  set_pte_range+0x885/0x8b0 mm/memory.c:4549
>  filemap_map_order0_folio mm/filemap.c:3513 [inline]
>  filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
>  do_fault_around mm/memory.c:4716 [inline]
>  do_read_fault mm/memory.c:4749 [inline]
>  do_fault mm/memory.c:4888 [inline]
>  do_pte_missing mm/memory.c:3745 [inline]
>  handle_pte_fault mm/memory.c:5164 [inline]
>  __handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
>  handle_mm_fault+0x27e/0x770 mm/memory.c:5470
>  do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
>  handle_page_fault arch/x86/mm/fault.c:1498 [inline]
>  exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
>  asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
> RIP: 0033:0x7f74dc087080
> Code: Unable to access opcode bytes at 0x7f74dc087056.
> RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
> RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
> RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
> RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
> R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
> R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
>  </TASK>
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/u4:0    state:I stack:24400 pid:11    tgid:11    ppid:2      flags:0x00004000
> Workqueue:  0x0 (events_unbound)
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/u4:1    state:I stack:23344 pid:12    tgid:12    ppid:2      flags:0x00004000
> Workqueue:  0x0 (bat_events)
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:kworker/R-mm_pe state:I stack:28752 pid:13    tgid:13    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:rcu_tasks_kthre state:I stack:27448 pid:14    tgid:14    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:rcu_tasks_trace state:I stack:27144 pid:15    tgid:15    ppid:2      flags:0x00004000
> Call Trace:
>  <TASK>
>  context_switch kernel/sched/core.c:5400 [inline]
>  __schedule+0x177f/0x49a0 kernel/sched/core.c:6727
>  __schedule_loop kernel/sched/core.c:6802 [inline]
>  schedule+0x149/0x260 kernel/sched/core.c:6817
>  rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
>  rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
>  kthread+0x2ef/0x390 kernel/kthread.c:388
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
>  </TASK>
> task:ksoftirqd/0
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>

^ permalink raw reply related

* Re: [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: Krzysztof Kozlowski @ 2024-03-11  6:31 UTC (permalink / raw)
  To: Jeff LaBundy, James Ogletree
  Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, patches, linux-sound, linux-input, devicetree
In-Reply-To: <Ze4KNBEsK5juzpNR@nixie71>

On 10/03/2024 20:29, Jeff LaBundy wrote:
> 
>> +  - vio-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
> 
> Nit: most device trees tend to use 8-column indentation as with kernel code.

If you meant DTS, then kernel coding style applies, which does not use
spaces. But this is a binding, so please use indentation as defined by
writing-schema: 2 or 4 spaces.


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
From: Jeff LaBundy @ 2024-03-10 23:40 UTC (permalink / raw)
  To: James Ogletree
  Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, patches, linux-sound, linux-input, devicetree
In-Reply-To: <20240308222421.188858-4-jogletre@opensource.cirrus.com>

Hi James,

Again, great work! I think we're almost there; I have just a few
trailing comments related to maintainability.

On Fri, Mar 08, 2024 at 10:24:19PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The MFD component registers and initializes the device.
> 
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
>  MAINTAINERS                 |   2 +
>  drivers/mfd/Kconfig         |  30 ++
>  drivers/mfd/Makefile        |   4 +
>  drivers/mfd/cs40l50-core.c  | 528 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/cs40l50-i2c.c   |  68 +++++
>  drivers/mfd/cs40l50-spi.c   |  68 +++++
>  include/linux/mfd/cs40l50.h | 144 ++++++++++
>  7 files changed, 844 insertions(+)
>  create mode 100644 drivers/mfd/cs40l50-core.c
>  create mode 100644 drivers/mfd/cs40l50-i2c.c
>  create mode 100644 drivers/mfd/cs40l50-spi.c
>  create mode 100644 include/linux/mfd/cs40l50.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b71017a187f8..69a9e0a3b968 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4940,6 +4940,8 @@ M:	Ben Bright <ben.bright@cirrus.com>
>  L:	patches@opensource.cirrus.com
>  S:	Supported
>  F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +F:	drivers/mfd/cs40l*
> +F:	include/linux/mfd/cs40l*
>  
>  CIRRUS LOGIC DSP FIRMWARE DRIVER
>  M:	Simon Trimmer <simont@opensource.cirrus.com>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 90ce58fd629e..6273c255f107 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2241,6 +2241,36 @@ config MCP_UCB1200_TS
>  
>  endmenu
>  
> +config MFD_CS40L50_CORE
> +	tristate
> +	select MFD_CORE
> +	select FW_CS_DSP
> +	select REGMAP_IRQ
> +
> +config MFD_CS40L50_I2C
> +	tristate "Cirrus Logic CS40L50 (I2C)"
> +	select REGMAP_I2C
> +	select MFD_CS40L50_CORE
> +	depends on I2C
> +	help
> +	  Select this to support the Cirrus Logic CS40L50 Haptic
> +	  Driver over I2C.
> +
> +	  This driver can be built as a module. If built as a module it will be
> +	  called "cs40l50-i2c".
> +
> +config MFD_CS40L50_SPI
> +	tristate "Cirrus Logic CS40L50 (SPI)"
> +	select REGMAP_SPI
> +	select MFD_CS40L50_CORE
> +	depends on SPI
> +	help
> +	  Select this to support the Cirrus Logic CS40L50 Haptic
> +	  Driver over SPI.
> +
> +	  This driver can be built as a module. If built as a module it will be
> +	  called "cs40l50-spi".
> +
>  config MFD_VEXPRESS_SYSREG
>  	tristate "Versatile Express System Registers"
>  	depends on VEXPRESS_CONFIG && GPIOLIB
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c66f07edcd0e..a8d18ba155d0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -88,6 +88,10 @@ obj-$(CONFIG_MFD_MADERA)	+= madera.o
>  obj-$(CONFIG_MFD_MADERA_I2C)	+= madera-i2c.o
>  obj-$(CONFIG_MFD_MADERA_SPI)	+= madera-spi.o
>  
> +obj-$(CONFIG_MFD_CS40L50_CORE)	+= cs40l50-core.o
> +obj-$(CONFIG_MFD_CS40L50_I2C)	+= cs40l50-i2c.o
> +obj-$(CONFIG_MFD_CS40L50_SPI)	+= cs40l50-spi.o
> +
>  obj-$(CONFIG_TPS6105X)		+= tps6105x.o
>  obj-$(CONFIG_TPS65010)		+= tps65010.o
>  obj-$(CONFIG_TPS6507X)		+= tps6507x.o
> diff --git a/drivers/mfd/cs40l50-core.c b/drivers/mfd/cs40l50-core.c
> new file mode 100644
> index 000000000000..92e67f80f36a
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-core.c
> @@ -0,0 +1,528 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/firmware/cirrus/wmfw.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +
> +static const struct mfd_cell cs40l50_devs[] = {
> +	{ .name = "cs40l50-codec", },
> +	{ .name = "cs40l50-vibra", },
> +};
> +
> +const struct regmap_config cs40l50_regmap = {
> +	.reg_bits =		32,
> +	.reg_stride =		4,
> +	.val_bits =		32,
> +	.reg_format_endian =	REGMAP_ENDIAN_BIG,
> +	.val_format_endian =	REGMAP_ENDIAN_BIG,
> +};
> +EXPORT_SYMBOL_GPL(cs40l50_regmap);
> +
> +static const char * const cs40l50_supplies[] = {
> +	"vp", "vio",
> +};

Strictly speaking, the VA supply must be called out in the binding as
a required supply; the device cannot function without it. I think you
have gotten away without handling VA because VA and VIO are likely tied
together on your board.

Are there any practical applications where VA and VIO would be split
apart on the board? For example, can VIO be tied to a 3.3-V supply in
support of 3.3-V logic, while VA remains powered by 1.8 V? If so, the
driver needs to provide a means to supply a separate VA regulator, and
the binding needs to specify that oneOf them is required.

If VA and VIO are recommended to be tied together, then I don't think
they need to be defined separately in the binding. We should only ask
customers to supply one 1.8-V regulator; perhaps the description can
mention how this regulator maps to the VA and VIO pins of the device.
For older devices like L25, VA and VIO were shorted inside the package.

The same goes for VAMP; it seems the intent here is to support internal
boost mode only, with VAMP and VBST shorted on the board. Therefore, I
don't see a need to define VAMP in the binding either. If you intend to
define VAMP in the binding, then the driver should support external
boost mode, otherwise the driver is incomplete. It seems unlikely that
customers would use external boost mode for a haptic application; this
is probably more common for audio applications that borrow the same die.

As I mention in the binding, VP should really be optional in terms of
regulator support; it's almost always connected to a battery, and not
a regulator.

> +
> +static const struct regmap_irq cs40l50_reg_irqs[] = {
> +	REGMAP_IRQ_REG(CS40L50_DSP_QUEUE_IRQ, CS40L50_IRQ1_INT_2_OFFSET,
> +		       CS40L50_DSP_QUEUE_MASK),
> +	REGMAP_IRQ_REG(CS40L50_AMP_SHORT_IRQ, CS40L50_IRQ1_INT_1_OFFSET,
> +		       CS40L50_AMP_SHORT_MASK),
> +	REGMAP_IRQ_REG(CS40L50_TEMP_ERR_IRQ, CS40L50_IRQ1_INT_8_OFFSET,
> +		       CS40L50_TEMP_ERR_MASK),
> +	REGMAP_IRQ_REG(CS40L50_BST_UVP_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> +		       CS40L50_BST_UVP_MASK),
> +	REGMAP_IRQ_REG(CS40L50_BST_SHORT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> +		       CS40L50_BST_SHORT_MASK),
> +	REGMAP_IRQ_REG(CS40L50_BST_ILIMIT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> +		       CS40L50_BST_ILIMIT_MASK),
> +	REGMAP_IRQ_REG(CS40L50_UVLO_VDDBATT_IRQ, CS40L50_IRQ1_INT_10_OFFSET,
> +		       CS40L50_UVLO_VDDBATT_MASK),
> +	REGMAP_IRQ_REG(CS40L50_GLOBAL_ERROR_IRQ, CS40L50_IRQ1_INT_18_OFFSET,
> +		       CS40L50_GLOBAL_ERROR_MASK),
> +};
> +
> +static struct regmap_irq_chip cs40l50_irq_chip = {
> +	.name =		"cs40l50",
> +
> +	.status_base =	CS40L50_IRQ1_INT_1,
> +	.mask_base =	CS40L50_IRQ1_MASK_1,
> +	.ack_base =	CS40L50_IRQ1_INT_1,
> +	.num_regs =	22,
> +
> +	.irqs =		cs40l50_reg_irqs,
> +	.num_irqs =	ARRAY_SIZE(cs40l50_reg_irqs),
> +
> +	.runtime_pm =	true,
> +};

No need for newlines in here.

> +
> +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val)
> +{
> +	int err, i;

It seems this series uses 'err', 'error' and 'ret' throughout. I know that
'error' is typically requested in the input subsystem, but I see no reason
not to stick with 'ret' for the rest of the series.

> +	u32 ack;
> +
> +	/* Device NAKs if hibernating, so optionally retry */
> +	for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
> +		err = regmap_write(regmap, CS40L50_DSP_QUEUE, val);
> +		if (!err)
> +			break;
> +
> +		usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
> +	}
> +
> +	/* If write never occurred, don't bother polling for ACK */
> +	if (i == CS40L50_DSP_TIMEOUT_COUNT) {
> +		dev_err(dev, "Timed out writing %#X to DSP: %d\n", val, err);
> +		return err;
> +	}
> +
> +	err = regmap_read_poll_timeout(regmap, CS40L50_DSP_QUEUE, ack, !ack,
> +				       CS40L50_DSP_POLL_US,
> +				       CS40L50_DSP_POLL_US * CS40L50_DSP_TIMEOUT_COUNT);
> +	if (err)
> +		dev_err(dev, "DSP did not ACK %#X: %d\n", val, err);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_dsp_write);
> +
> +static const struct cs_dsp_region cs40l50_dsp_regions[] = {
> +	{ .type = WMFW_HALO_PM_PACKED, .base = CS40L50_PMEM_0 },
> +	{ .type = WMFW_HALO_XM_PACKED, .base = CS40L50_XMEM_PACKED_0 },
> +	{ .type = WMFW_HALO_YM_PACKED, .base = CS40L50_YMEM_PACKED_0 },
> +	{ .type = WMFW_ADSP2_XM, .base = CS40L50_XMEM_UNPACKED24_0 },
> +	{ .type = WMFW_ADSP2_YM, .base = CS40L50_YMEM_UNPACKED24_0 },
> +};
> +
> +static const struct reg_sequence cs40l50_internal_vamp_config[] = {
> +	{ CS40L50_BST_LPMODE_SEL,	CS40L50_DCM_LOW_POWER },
> +	{ CS40L50_BLOCK_ENABLES2,	CS40L50_OVERTEMP_WARN },
> +};

I recommend against aligning the second values here and below. If another
register with a long name is added later, the patch would touch more lines
than needed in order to restore the alignment. What you've done for the
cs40l50_dsp_regions[] array is safer.

> +
> +static const struct reg_sequence cs40l50_irq_mask_override[] = {
> +	{ CS40L50_IRQ1_MASK_2,	CS40L50_IRQ_MASK_2_OVERRIDE },
> +	{ CS40L50_IRQ1_MASK_20,	CS40L50_IRQ_MASK_20_OVERRIDE },
> +};
> +
> +static int cs40l50_configure_dsp(struct cs_dsp *dsp)
> +{
> +	struct cs40l50 *cs40l50 = container_of(dsp, struct cs40l50, dsp);
> +	u32 nwaves;
> +	int err;
> +
> +	/* Log number of effects if wavetable was loaded */
> +	if (cs40l50->bin) {

Is there any other clue you can use to discern whether a wavetable was
loaded? The memory at cs40l50->bin is gone now, and although you're not
dereferencing it anymore, another contributor might be fooled into doing
so down the road.

Maybe a boolean would be more maintainable; I don't feel strongly about
it though.

> +		err = regmap_read(dsp->regmap, CS40L50_NUM_WAVES, &nwaves);
> +		if (err)
> +			return err;
> +
> +		dev_info(dsp->dev, "%u effects loaded\n", nwaves);
> +	}
> +
> +	cs40l50->wseqs[CS40L50_PWR_ON].ctl = cs_dsp_get_ctl(dsp, "PM_PWR_ON_SEQ",
> +							    WMFW_ADSP2_XM,
> +							    CS40L50_PM_ALGO);
> +	if (!cs40l50->wseqs[CS40L50_PWR_ON].ctl) {
> +		dev_err(dsp->dev, "No control for power-on write sequence\n");
> +		return -ENOENT;
> +	}
> +
> +	/* Initialize the power-on write sequencer */
> +	err = cs_dsp_wseq_init(dsp, cs40l50->wseqs, 1);
> +	if (err) {
> +		dev_err(dsp->dev, "Failed to initialize power-on write sequence\n");
> +		return err;
> +	}
> +
> +	/* Configure internal V_AMP supply */
> +	err = regmap_multi_reg_write(dsp->regmap, cs40l50_internal_vamp_config,
> +				     ARRAY_SIZE(cs40l50_internal_vamp_config));
> +	if (err)
> +		return err;
> +
> +	err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
> +				      cs40l50_internal_vamp_config, CS_DSP_WSEQ_FULL,
> +				      ARRAY_SIZE(cs40l50_internal_vamp_config), false);
> +	if (err)
> +		return err;
> +
> +	/* Override firmware defaults for IRQ masks */
> +	err = regmap_multi_reg_write(dsp->regmap, cs40l50_irq_mask_override,
> +				     ARRAY_SIZE(cs40l50_irq_mask_override));
> +	if (err)
> +		return err;
> +
> +	err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
> +				      cs40l50_irq_mask_override, CS_DSP_WSEQ_FULL,
> +				      ARRAY_SIZE(cs40l50_irq_mask_override), false);
> +	if (err)
> +		return err;
> +
> +	/* Add child devices now that DSP is running */
> +	err = devm_mfd_add_devices(dsp->dev, PLATFORM_DEVID_NONE, cs40l50_devs,
> +				   ARRAY_SIZE(cs40l50_devs), NULL, 0, NULL);
> +	if (err)
> +		dev_err(dsp->dev, "Failed to add child devices: %d\n", err);
> +
> +	return err;
> +}
> +
> +static const struct cs_dsp_client_ops client_ops = {
> +	.post_run = cs40l50_configure_dsp,
> +};
> +
> +static void cs40l50_dsp_remove(void *data)
> +{
> +	cs_dsp_remove(data);
> +}
> +
> +static int cs40l50_dsp_init(struct cs40l50 *cs40l50)
> +{
> +	int err;
> +
> +	cs40l50->dsp.num = 1;
> +	cs40l50->dsp.type = WMFW_HALO;
> +	cs40l50->dsp.dev = cs40l50->dev;
> +	cs40l50->dsp.regmap = cs40l50->regmap;
> +	cs40l50->dsp.base = CS40L50_CORE_BASE;
> +	cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID;
> +	cs40l50->dsp.mem = cs40l50_dsp_regions;
> +	cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
> +	cs40l50->dsp.no_core_startstop = true;
> +	cs40l50->dsp.client_ops = &client_ops;
> +
> +	err = cs_dsp_halo_init(&cs40l50->dsp);
> +	if (err)
> +		return err;
> +
> +	return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove,
> +					&cs40l50->dsp);
> +}
> +
> +static void cs40l50_dsp_power_down(void *data)
> +{
> +	cs_dsp_power_down(data);
> +}
> +
> +static void cs40l50_dsp_stop(void *data)
> +{
> +	cs_dsp_stop(data);
> +}
> +
> +static void cs40l50_start_dsp(const struct firmware *bin, void *context)
> +{
> +	struct cs40l50 *cs40l50 = context;
> +	int err;
> +
> +	/* Wavetable is optional; start DSP regardless */
> +	cs40l50->bin = bin;
> +
> +	mutex_lock(&cs40l50->lock);

It seems the mutex is used only to prevent interrupt handling while the DSP
is being configured; can't we just call disable_irq() and enable_irq() here?

> +
> +	err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SHUTDOWN);
> +	if (err)
> +		goto err_mutex;
> +
> +	err = cs_dsp_power_up(&cs40l50->dsp, cs40l50->fw, "cs40l50.wmfw",
> +			      cs40l50->bin, "cs40l50.bin", "cs40l50");
> +	if (err)
> +		goto err_mutex;
> +
> +	err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_power_down,
> +				       &cs40l50->dsp);
> +	if (err)
> +		goto err_mutex;
> +
> +	err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SYSTEM_RESET);
> +	if (err)
> +		goto err_mutex;
> +
> +	err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
> +	if (err)
> +		goto err_mutex;
> +
> +	err = cs_dsp_run(&cs40l50->dsp);
> +	if (err)
> +		goto err_mutex;
> +
> +	err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_stop, &cs40l50->dsp);

In the case of L25, it was common to stop and re-start the DSP for either
of the following reasons:

1. Swap out .wmfw files. Eventually, different algorithms got so large that
   different .wmfw files were needed for different use-cases (e.g. A2H vs.
   f0 tracking). No matter how much RAM gets added to the HALO core, there
   is never enough! :)
2. Respond to spurious reset. With one customer in particular, the reset
   GPIO traveled over a flex cable, and the connector suffered some contact
   bounce during drop testing.

In general, things that should be able to be executed an infinite number of
times (e.g. register writes) should be logically separate from things that
should only be executed once, like a call to devm_add_action_or_reset().

The same is true for cs40l50_configure_dsp(); it contains a mix of register
writes, but then a call to devm_mfd_add_devices(). Please consider separating
some of these tasks so that rip-up is minimal in case you need to restore or
reconfigure the device on-the-fly at a later time.

> +err_mutex:
> +	mutex_unlock(&cs40l50->lock);
> +	release_firmware(cs40l50->bin);
> +	release_firmware(cs40l50->fw);
> +
> +	if (err)
> +		dev_err(cs40l50->dev, "Failed to start DSP: %d", err);

The fact that we have so much clean-up in the error path may suggest that
the bulk of this function belongs in a helper, e.g. __cs40l50_start_dsp(),
with return type of int.

> +}
> +
> +static void cs40l50_request_firmware(const struct firmware *fw, void *context)
> +{
> +	struct cs40l50 *cs40l50 = context;
> +
> +	if (!fw) {
> +		dev_err(cs40l50->dev, "No firmware file found\n");
> +		return;
> +	}
> +
> +	cs40l50->fw = fw;
> +
> +	if (request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_WT,
> +				    cs40l50->dev, GFP_KERNEL, cs40l50,
> +				    cs40l50_start_dsp)) {
> +		dev_err(cs40l50->dev, "Failed to request %s\n", CS40L50_WT);

Please record the return value of request_firmware_nowait() and print it
in the error message. It makes it much easier for Luddites such as myself
to debug ;)

> +		release_firmware(cs40l50->fw);
> +	}
> +}
> +
> +struct cs40l50_irq {
> +	const char *name;
> +	int virq;
> +};
> +
> +static struct cs40l50_irq cs40l50_irqs[] = {
> +	{ "DSP", },
> +	{ "Global", },
> +	{ "Boost UVLO", },
> +	{ "Boost current limit", },
> +	{ "Boost short", },
> +	{ "Boost undervolt", },
> +	{ "Overtemp", },
> +	{ "Amp short", },
> +};
> +
> +static const struct reg_sequence cs40l50_err_rls[] = {
> +	{ CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_SET },
> +	{ CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_CLEAR },
> +};
> +
> +static irqreturn_t cs40l50_hw_err(int irq, void *data)
> +{
> +	struct cs40l50 *cs40l50 = data;
> +	int err = 0, i;
> +
> +	mutex_lock(&cs40l50->lock);
> +
> +	/* Log hardware interrupt and execute error release sequence */
> +	for (i = 1; i < ARRAY_SIZE(cs40l50_irqs); i++) {
> +		if (cs40l50_irqs[i].virq == irq) {
> +			dev_err(cs40l50->dev, "%s error\n", cs40l50_irqs[i].name);
> +			err = regmap_multi_reg_write(cs40l50->regmap, cs40l50_err_rls,
> +						     ARRAY_SIZE(cs40l50_err_rls));
> +			goto exit;

There is no need for a goto label here; just break.

> +		}
> +	}
> +exit:
> +	mutex_unlock(&cs40l50->lock);
> +	return IRQ_RETVAL(!err);
> +}
> +
> +static irqreturn_t cs40l50_dsp_queue(int irq, void *data)
> +{
> +	struct cs40l50 *cs40l50 = data;
> +	u32 rd_ptr, val, wt_ptr;
> +	int err = 0;
> +
> +	mutex_lock(&cs40l50->lock);
> +
> +	/* Read from DSP queue, log, and update read pointer */
> +	while (!err) {
> +		err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_WT, &wt_ptr);
> +		if (err)
> +			goto exit;

Same here, and throughout; just break out of the loop.

> +
> +		err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, &rd_ptr);
> +		if (err)
> +			goto exit;
> +
> +		/* Check if queue is empty */
> +		if (wt_ptr == rd_ptr)
> +			goto exit;
> +
> +		err = regmap_read(cs40l50->regmap, rd_ptr, &val);
> +		if (err)
> +			goto exit;
> +
> +		dev_dbg(cs40l50->dev, "DSP payload: %#X", val);
> +
> +		rd_ptr += sizeof(u32);
> +
> +		if (rd_ptr > CS40L50_DSP_QUEUE_END)
> +			rd_ptr = CS40L50_DSP_QUEUE_BASE;
> +
> +		err =  regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, rd_ptr);
> +		if (err)
> +			goto exit;
> +	}
> +exit:
> +	mutex_unlock(&cs40l50->lock);
> +
> +	return IRQ_RETVAL(!err);
> +}
> +
> +static int cs40l50_irq_init(struct cs40l50 *cs40l50)
> +{
> +	struct device *dev = cs40l50->dev;
> +	int err, i, virq;
> +
> +	err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> +				       &cs40l50_irq_chip, &cs40l50->irq_data);
> +	if (err) {
> +		dev_err(dev, "Failed adding IRQ chip\n");

I don't see any need for individual prints in this function, since the call
to cs40l50_irq_init() is already followed by a call to dev_err_probe().

> +		return err;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
> +		virq = regmap_irq_get_virq(cs40l50->irq_data, i);
> +		if (virq < 0) {
> +			dev_err(dev, "Failed getting %s\n", cs40l50_irqs[i].name);
> +			return virq;
> +		}
> +
> +		cs40l50_irqs[i].virq = virq;
> +
> +		/* Handle DSP and hardware interrupts separately */
> +		err = devm_request_threaded_irq(dev, virq, NULL,
> +						i ? cs40l50_hw_err : cs40l50_dsp_queue,
> +						IRQF_ONESHOT | IRQF_SHARED,
> +						cs40l50_irqs[i].name, cs40l50);
> +		if (err) {
> +			dev_err(dev, "Failed requesting %s IRQ\n",
> +				cs40l50_irqs[i].name);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int cs40l50_get_model(struct cs40l50 *cs40l50)
> +{
> +	int err;
> +
> +	err = regmap_read(cs40l50->regmap, CS40L50_DEVID, &cs40l50->devid);
> +	if (err)
> +		return err;
> +
> +	if (cs40l50->devid != CS40L50_DEVID_A)
> +		return -EINVAL;
> +
> +	err = regmap_read(cs40l50->regmap, CS40L50_REVID, &cs40l50->revid);
> +	if (err)
> +		return err;
> +
> +	if (cs40l50->revid < CS40L50_REVID_B0)
> +		return -EINVAL;
> +
> +	dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);

This should be dev_dbg().

> +
> +	return 0;
> +}
> +
> +static int cs40l50_pm_runtime_setup(struct device *dev)
> +{
> +	int err;
> +
> +	pm_runtime_set_autosuspend_delay(dev, CS40L50_AUTOSUSPEND_MS);
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_get_noresume(dev);
> +	err = pm_runtime_set_active(dev);
> +	if (err)
> +		return err;
> +
> +	return devm_pm_runtime_enable(dev);
> +}
> +
> +int cs40l50_probe(struct cs40l50 *cs40l50)
> +{
> +	struct device *dev = cs40l50->dev;
> +	int err;
> +
> +	mutex_init(&cs40l50->lock);
> +
> +	cs40l50->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(cs40l50->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(cs40l50->reset_gpio),
> +				     "Failed getting reset GPIO\n");
> +
> +	err = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(cs40l50_supplies),
> +					     cs40l50_supplies);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed getting supplies\n");
> +
> +	/* Ensure minimum reset pulse width */
> +	usleep_range(CS40L50_RESET_PULSE_US, CS40L50_RESET_PULSE_US + 100);
> +
> +	gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
> +
> +	/* Wait for control port to be ready */
> +	usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 100);
> +
> +	err = cs40l50_dsp_init(cs40l50);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to initialize DSP\n");
> +
> +	err = cs40l50_pm_runtime_setup(dev);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to initialize runtime PM\n");
> +
> +	err = cs40l50_get_model(cs40l50);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to get part number\n");

I think cs40l50_get_model() should be called directly after usleep_range() is
called; there is no use in doing any other initialization if we can't talk I2C
or SPI, or the model is invalid.

> +
> +	err = cs40l50_irq_init(cs40l50);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to request IRQs\n");
> +
> +	err = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_FW,
> +				      dev, GFP_KERNEL, cs40l50, cs40l50_request_firmware);
> +	if (err)
> +		return dev_err_probe(dev, err, "Failed to request %s\n", CS40L50_FW);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_probe);
> +
> +int cs40l50_remove(struct cs40l50 *cs40l50)
> +{
> +	gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_remove);
> +
> +static int cs40l50_runtime_suspend(struct device *dev)
> +{
> +	struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
> +
> +	return regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE, CS40L50_ALLOW_HIBER);
> +}
> +
> +static int cs40l50_runtime_resume(struct device *dev)
> +{
> +	struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
> +
> +	return cs40l50_dsp_write(dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
> +}
> +
> +EXPORT_GPL_DEV_PM_OPS(cs40l50_pm_ops) = {
> +	RUNTIME_PM_OPS(cs40l50_runtime_suspend, cs40l50_runtime_resume, NULL)
> +};
> +
> +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(FW_CS_DSP);
> diff --git a/drivers/mfd/cs40l50-i2c.c b/drivers/mfd/cs40l50-i2c.c
> new file mode 100644
> index 000000000000..639be743d956
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-i2c.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/cs40l50.h>
> +
> +static int cs40l50_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct cs40l50 *cs40l50;
> +
> +	cs40l50 = devm_kzalloc(&i2c->dev, sizeof(*cs40l50), GFP_KERNEL);
> +	if (!cs40l50)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, cs40l50);
> +
> +	cs40l50->dev = &i2c->dev;
> +	cs40l50->irq = i2c->irq;
> +
> +	cs40l50->regmap = devm_regmap_init_i2c(i2c, &cs40l50_regmap);
> +	if (IS_ERR(cs40l50->regmap))
> +		return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
> +				     "Failed to initialize register map\n");
> +
> +	return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_i2c_remove(struct i2c_client *i2c)
> +{
> +	struct cs40l50 *cs40l50 = i2c_get_clientdata(i2c);
> +
> +	cs40l50_remove(cs40l50);
> +}
> +
> +static const struct i2c_device_id cs40l50_id_i2c[] = {
> +	{ "cs40l50" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, cs40l50_id_i2c);
> +
> +static const struct of_device_id cs40l50_of_match[] = {
> +	{ .compatible = "cirrus,cs40l50" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, cs40l50_of_match);
> +
> +static struct i2c_driver cs40l50_i2c_driver = {
> +	.driver = {
> +		.name = "cs40l50",
> +		.of_match_table = cs40l50_of_match,
> +		.pm = pm_ptr(&cs40l50_pm_ops),
> +	},
> +	.id_table = cs40l50_id_i2c,
> +	.probe = cs40l50_i2c_probe,
> +	.remove = cs40l50_i2c_remove,
> +};
> +module_i2c_driver(cs40l50_i2c_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 I2C Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c
> new file mode 100644
> index 000000000000..53526b595a0d
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-spi.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/spi/spi.h>
> +
> +static int cs40l50_spi_probe(struct spi_device *spi)
> +{
> +	struct cs40l50 *cs40l50;
> +
> +	cs40l50 = devm_kzalloc(&spi->dev, sizeof(*cs40l50), GFP_KERNEL);
> +	if (!cs40l50)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, cs40l50);
> +
> +	cs40l50->dev = &spi->dev;
> +	cs40l50->irq = spi->irq;
> +
> +	cs40l50->regmap = devm_regmap_init_spi(spi, &cs40l50_regmap);
> +	if (IS_ERR(cs40l50->regmap))
> +		return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
> +				     "Failed to initialize register map\n");
> +
> +	return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_spi_remove(struct spi_device *spi)
> +{
> +	struct cs40l50 *cs40l50 = spi_get_drvdata(spi);
> +
> +	cs40l50_remove(cs40l50);
> +}
> +
> +static const struct spi_device_id cs40l50_id_spi[] = {
> +	{ "cs40l50" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(spi, cs40l50_id_spi);
> +
> +static const struct of_device_id cs40l50_of_match[] = {
> +	{ .compatible = "cirrus,cs40l50" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, cs40l50_of_match);
> +
> +static struct spi_driver cs40l50_spi_driver = {
> +	.driver = {
> +		.name = "cs40l50",
> +		.of_match_table = cs40l50_of_match,
> +		.pm = pm_ptr(&cs40l50_pm_ops),
> +	},
> +	.id_table = cs40l50_id_spi,
> +	.probe = cs40l50_spi_probe,
> +	.remove = cs40l50_spi_remove,
> +};
> +module_spi_driver(cs40l50_spi_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 SPI Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h
> new file mode 100644
> index 000000000000..cc0661926918
> --- /dev/null
> +++ b/include/linux/mfd/cs40l50.h
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#ifndef __MFD_CS40L50_H__
> +#define __MFD_CS40L50_H__
> +
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/regmap.h>
> +
> +/* Power Supply Configuration */
> +#define CS40L50_BLOCK_ENABLES2		0x201C
> +#define CS40L50_ERR_RLS			0x2034
> +#define CS40L50_PWRMGT_CTL		0x2900
> +#define CS40L50_BST_LPMODE_SEL		0x3810
> +#define CS40L50_DCM_LOW_POWER		0x1
> +#define CS40L50_OVERTEMP_WARN		0x4000010
> +
> +/* Interrupts */
> +#define CS40L50_IRQ1_INT_1		0xE010
> +#define CS40L50_IRQ1_BASE		CS40L50_IRQ1_INT_1
> +#define CS40L50_IRQ1_INT_2		0xE014
> +#define CS40L50_IRQ1_INT_8		0xE02C
> +#define CS40L50_IRQ1_INT_9		0xE030
> +#define CS40L50_IRQ1_INT_10		0xE034
> +#define CS40L50_IRQ1_INT_18		0xE054
> +#define CS40L50_IRQ1_MASK_1		0xE090
> +#define CS40L50_IRQ1_MASK_2		0xE094
> +#define CS40L50_IRQ1_MASK_20		0xE0DC
> +#define CS40L50_IRQ1_INT_1_OFFSET	(CS40L50_IRQ1_INT_1 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_2_OFFSET	(CS40L50_IRQ1_INT_2 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_8_OFFSET	(CS40L50_IRQ1_INT_8 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_9_OFFSET	(CS40L50_IRQ1_INT_9 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_10_OFFSET	(CS40L50_IRQ1_INT_10 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_18_OFFSET	(CS40L50_IRQ1_INT_18 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ_MASK_2_OVERRIDE	0xFFDF7FFF
> +#define CS40L50_IRQ_MASK_20_OVERRIDE	0x15C01000
> +#define CS40L50_AMP_SHORT_MASK		BIT(31)
> +#define CS40L50_DSP_QUEUE_MASK		BIT(21)
> +#define CS40L50_TEMP_ERR_MASK		BIT(31)
> +#define CS40L50_BST_UVP_MASK		BIT(6)
> +#define CS40L50_BST_SHORT_MASK		BIT(7)
> +#define CS40L50_BST_ILIMIT_MASK		BIT(18)
> +#define CS40L50_UVLO_VDDBATT_MASK	BIT(16)
> +#define CS40L50_GLOBAL_ERROR_MASK	BIT(15)
> +
> +enum cs40l50_irq_list {
> +	CS40L50_DSP_QUEUE_IRQ,
> +	CS40L50_GLOBAL_ERROR_IRQ,
> +	CS40L50_UVLO_VDDBATT_IRQ,
> +	CS40L50_BST_ILIMIT_IRQ,
> +	CS40L50_BST_SHORT_IRQ,
> +	CS40L50_BST_UVP_IRQ,
> +	CS40L50_TEMP_ERR_IRQ,
> +	CS40L50_AMP_SHORT_IRQ,
> +};
> +
> +/* DSP */
> +#define CS40L50_XMEM_PACKED_0		0x2000000
> +#define CS40L50_XMEM_UNPACKED24_0	0x2800000
> +#define CS40L50_SYS_INFO_ID		0x25E0000
> +#define CS40L50_RAM_INIT		0x28021DC
> +#define CS40L50_DSP_QUEUE_WT		0x28042C8
> +#define CS40L50_DSP_QUEUE_RD		0x28042CC
> +#define CS40L50_NUM_WAVES		0x2805C18
> +#define CS40L50_CORE_BASE		0x2B80000
> +#define CS40L50_CCM_CORE_CONTROL	0x2BC1000
> +#define CS40L50_YMEM_PACKED_0		0x2C00000
> +#define CS40L50_YMEM_UNPACKED24_0	0x3400000
> +#define CS40L50_PMEM_0			0x3800000
> +#define CS40L50_MEM_RDY_HW		0x2
> +#define CS40L50_RAM_INIT_FLAG		0x1

I didn't see this #define used anywhere; please double check this one
and others.

> +#define CS40L50_CLOCK_DISABLE		0x80
> +#define CS40L50_CLOCK_ENABLE		0x281
> +#define CS40L50_DSP_POLL_US		1000
> +#define CS40L50_DSP_TIMEOUT_COUNT	100
> +#define CS40L50_RESET_PULSE_US		2200
> +#define CS40L50_CP_READY_US		3100
> +#define CS40L50_AUTOSUSPEND_MS		2000
> +#define CS40L50_PM_ALGO			0x9F206
> +#define CS40L50_GLOBAL_ERR_RLS_SET	BIT(11)
> +#define CS40L50_GLOBAL_ERR_RLS_CLEAR	0
> +
> +enum cs40l50_wseqs {
> +	CS40L50_PWR_ON,
> +	CS40L50_STANDBY,
> +	CS40L50_ACTIVE,
> +	CS40L50_NUM_WSEQS,
> +};
> +
> +/* DSP Queue */
> +#define CS40L50_DSP_QUEUE_BASE		0x11004
> +#define CS40L50_DSP_QUEUE_END		0x1101C
> +#define CS40L50_DSP_QUEUE		0x11020
> +#define CS40L50_PREVENT_HIBER		0x2000003
> +#define CS40L50_ALLOW_HIBER		0x2000004
> +#define CS40L50_SHUTDOWN		0x2000005
> +#define CS40L50_SYSTEM_RESET		0x02000007
> +#define CS40L50_START_I2S		0x3000002
> +#define CS40L50_OWT_PUSH		0x3000008
> +#define CS40L50_STOP_PLAYBACK		0x5000000
> +#define CS40L50_OWT_DELETE		0xD000000
> +
> +/* Firmware files */
> +#define CS40L50_FW			"cs40l50.wmfw"
> +#define CS40L50_WT			"cs40l50.bin"
> +
> +/* Device */
> +#define CS40L50_DEVID			0x0
> +#define CS40L50_REVID			0x4
> +#define CS40L50_DEVID_A			0x40A50
> +#define CS40L50_REVID_B0		0xB0
> +
> +struct cs40l50 {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct mutex lock;
> +	struct cs_dsp dsp;
> +	struct gpio_desc *reset_gpio;
> +	struct regmap_irq_chip_data *irq_data;
> +	const struct firmware *fw;
> +	const struct firmware *bin;
> +	struct cs_dsp_wseq wseqs[CS40L50_NUM_WSEQS];
> +	int irq;
> +	u32 devid;
> +	u32 revid;
> +};
> +
> +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val);
> +int cs40l50_probe(struct cs40l50 *cs40l50);
> +int cs40l50_remove(struct cs40l50 *cs40l50);
> +
> +extern const struct regmap_config cs40l50_regmap;
> +extern const struct dev_pm_ops cs40l50_pm_ops;
> +
> +#endif /* __MFD_CS40L50_H__ */
> -- 
> 2.25.1
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply

* [PATCH v3] HID: nintendo: use ida for LED player id
From: Martino Fontana @ 2024-03-10 22:12 UTC (permalink / raw)
  To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
  Cc: Martino Fontana, Ryan McClelland

Previously, the leds pattern would just increment with every controller
connected. This wouldn't take into consideration when controllers are
disconnected. The same controller could be connected and disconnected
with the pattern increasing player count each time.

This patch changes it by using an ID allocator in order to assign the
player id, the same way hid-playstation does.

Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
Changes for v2:

ida_free now frees the correct id, instead of an id that got moduloed.

Changes for v3:

Destroy allocator when removing driver.

 drivers/hid/hid-nintendo.c | 41 ++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 997c3a1adaca..8a8ae7881469 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -28,6 +28,7 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/hid.h>
+#include <linux/idr.h>
 #include <linux/input.h>
 #include <linux/jiffies.h>
 #include <linux/leds.h>
@@ -427,6 +428,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
 struct joycon_ctlr {
 	struct hid_device *hdev;
 	struct input_dev *input;
+	u32 player_id;
 	struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
 	struct led_classdev home_led;
 	enum joycon_ctlr_state ctlr_state;
@@ -1930,7 +1932,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
 	return ret;
 }
 
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
+static DEFINE_IDA(nintendo_player_id_allocator);
+
 static int joycon_leds_create(struct joycon_ctlr *ctlr)
 {
 	struct hid_device *hdev = ctlr->hdev;
@@ -1941,20 +1944,19 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
 	char *name;
 	int ret;
 	int i;
-	unsigned long flags;
 	int player_led_pattern;
-	static int input_num;
-
-	/*
-	 * Set the player leds based on controller number
-	 * Because there is no standard concept of "player number", the pattern
-	 * number will simply increase by 1 every time a controller is connected.
-	 */
-	spin_lock_irqsave(&joycon_input_num_spinlock, flags);
-	player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
-	spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
 
 	/* configure the player LEDs */
+	ctlr->player_id = U32_MAX;
+	ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
+	if (ret < 0) {
+		hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
+		goto home_led;
+	}
+	ctlr->player_id = ret;
+	player_led_pattern = ret % JC_NUM_LED_PATTERNS;
+	hid_info(ctlr->hdev, "assigned player %d led pattern", player_led_pattern + 1);
+
 	for (i = 0; i < JC_NUM_LEDS; i++) {
 		name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
 				      d_name,
@@ -2423,6 +2425,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
 	destroy_workqueue(ctlr->rumble_queue);
+	ida_free(&nintendo_player_id_allocator, ctlr->player_id);
 
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
@@ -2468,7 +2471,19 @@ static struct hid_driver nintendo_hid_driver = {
 	.resume		= nintendo_hid_resume,
 #endif
 };
-module_hid_driver(nintendo_hid_driver);
+static int __init nintendo_init(void)
+{
+	return hid_register_driver(&nintendo_hid_driver);
+}
+
+static void __exit nintendo_exit(void)
+{
+	hid_unregister_driver(&nintendo_hid_driver);
+	ida_destroy(&nintendo_player_id_allocator);
+}
+
+module_init(nintendo_init);
+module_exit(nintendo_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
-- 
2.43.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox