All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Svyatoslav Ryhel" <clamor95@gmail.com>
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v7 6/7] power: supply: Add driver for ASUS Transformer battery
Date: Mon, 18 May 2026 12:37:22 +0000	[thread overview]
Message-ID: <20260518123722.EE442C2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <20260518095907.36158-7-clamor95@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The driver lacks a MODULE_ALIAS declaration, which breaks module autoloading.
- [Low] Missing NULL check for the devm_kasprintf allocation of the power supply name.
--

commit ccc69cbdb16bd3e704318f7d07abbad8dc370844
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

power: supply: Add driver for ASUS Transformer battery

This commit introduces a new power supply driver for the ASUS Transformer
battery. It communicates with the embedded controller via I2C to read battery
status for both the tablet and mobile dock.

> diff --git a/drivers/power/supply/asus-transformer-ec-battery.c b/drivers/power/supply/asus-transformer-ec-battery.c
> new file mode 100644
> index 0000000000000..d08d155f72a3d
> --- /dev/null
> +++ b/drivers/power/supply/asus-transformer-ec-battery.c
[ ... ]
> +static int asus_ec_battery_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	cfg.fwnode = dev_fwnode(dev->parent);
> +	cfg.drv_data = priv;
> +
> +	memcpy(&priv->psy_desc, &asus_ec_battery_desc, sizeof(priv->psy_desc));
> +	priv->psy_desc.name = devm_kasprintf(dev, GFP_KERNEL, "%s-battery",
> +					     priv->ec->name);
> +
> +	priv->battery = devm_power_supply_register(dev, &priv->psy_desc, &cfg);
> +	if (IS_ERR(priv->battery))
> +		return dev_err_probe(dev, PTR_ERR(priv->battery),
> +				     "Failed to register power supply\n");

Is it possible for devm_kasprintf() to fail and return NULL under memory
pressure?

If so, priv->psy_desc.name would become NULL, and devm_power_supply_register()
would return ERR_PTR(-EINVAL) because it checks for !desc->name. This relies
on downstream validation and results in an incorrect error code being
propagated for a memory allocation failure instead of -ENOMEM.

Should there be an explicit NULL check here?

[ ... ]
> +static struct platform_driver asus_ec_battery_driver = {
> +	.driver = {
> +		.name = "asus-transformer-ec-battery",
> +		.pm = &asus_ec_battery_pm_ops,
> +	},
> +	.probe = asus_ec_battery_probe,
> +};
> +module_platform_driver(asus_ec_battery_driver);
> +
> +MODULE_AUTHOR("Michał Mirosław <mirq-linux@rere.qmqm.pl>");
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> +MODULE_DESCRIPTION("ASUS Transformer's battery driver");
> +MODULE_LICENSE("GPL");

Since this platform driver matches devices purely by name
("asus-transformer-ec-battery") and lacks an id_table or of_match_table,
will userspace tools like udev be able to automatically load this driver
when it is compiled as a module?

Should a MODULE_ALIAS("platform:asus-transformer-ec-battery"); declaration
be added to ensure module autoloading works correctly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518095907.36158-1-clamor95@gmail.com?part=6

  reply	other threads:[~2026-05-18 12:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18  9:59 [PATCH v7 0/7] mfd: Add support for Asus Transformer embedded controller Svyatoslav Ryhel
2026-05-18  9:59 ` [PATCH v7 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC Svyatoslav Ryhel
2026-05-18 10:09   ` sashiko-bot
2026-05-18  9:59 ` [PATCH v7 2/7] mfd: Add driver for ASUS Transformer embedded controller Svyatoslav Ryhel
2026-05-18 10:31   ` sashiko-bot
2026-05-18  9:59 ` [PATCH v7 3/7] input: serio: Add driver for ASUS Transformer dock keyboard and touchpad Svyatoslav Ryhel
2026-05-18 11:17   ` sashiko-bot
2026-05-18  9:59 ` [PATCH v7 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys Svyatoslav Ryhel
2026-05-18 11:46   ` sashiko-bot
2026-05-18  9:59 ` [PATCH v7 5/7] leds: Add driver for ASUS Transformer LEDs Svyatoslav Ryhel
2026-05-18 12:02   ` sashiko-bot
2026-05-18  9:59 ` [PATCH v7 6/7] power: supply: Add driver for ASUS Transformer battery Svyatoslav Ryhel
2026-05-18 12:37   ` sashiko-bot [this message]
2026-05-18  9:59 ` [PATCH v7 7/7] power: supply: Add charger driver for Asus Transformers Svyatoslav Ryhel
2026-05-27 14:46 ` [PATCH v7 0/7] mfd: Add support for Asus Transformer embedded controller Lee Jones
2026-05-28  5:10   ` Svyatoslav Ryhel

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20260518123722.EE442C2BCB7@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=clamor95@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

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

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