| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <foytwrb2sihficgrlcmxclnzm6mmpxqyjrfdkzardx3mkivmvg@b3hfby3fw2fm>
Date: Fri, 6 Dec 2024 01:50:53 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Armin Wolf <W_Armin@....de>, Hans de Goede <hdegoede@...hat.com>,
Thomas Weißschuh <thomas@...ssschuh.net>, Benson Leung <bleung@...omium.org>,
Guenter Roeck <groeck@...omium.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
chrome-platform@...ts.linux.dev, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v5 3/4] power: supply: test-power: implement a power
supply extension
Hi,
On Thu, Dec 05, 2024 at 09:46:37PM +0100, Thomas Weißschuh wrote:
> Allow easy testing of the new power supply extension functionality.
>
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> ---
This looks good to me apart from the function / variable names. For
example power_supply_ext_get_property() looks like a function from
the power-supply core and not from the test driver :)
I suggest replacing 'power_supply_ext_' prefixes with 'test_power_ext_'
and 'struct power_supply_ext power_supply_ext' with
'struct power_supply_ext test_power_ext'.
Greetings,
-- Sebastian
> drivers/power/supply/test_power.c | 111 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 111 insertions(+)
>
> diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
> index 442ceb7795e1d84e34da2801d228d53fb67e08d9..1e15d3d192ef4040839aa9740b06f5ab78b6b6d3 100644
> --- a/drivers/power/supply/test_power.c
> +++ b/drivers/power/supply/test_power.c
> @@ -37,6 +37,7 @@ static int battery_charge_counter = -1000;
> static int battery_current = -1600;
> static enum power_supply_charge_behaviour battery_charge_behaviour =
> POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO;
> +static bool battery_hook;
>
> static bool module_initialized;
>
> @@ -238,6 +239,85 @@ static const struct power_supply_config test_power_configs[] = {
> },
> };
>
> +static int power_supply_ext_manufacture_year = 1234;
> +static int power_supply_ext_temp_max = 1000;
> +static const enum power_supply_property power_supply_ext_props[] = {
> + POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
> + POWER_SUPPLY_PROP_TEMP_MAX,
> +};
> +
> +static int power_supply_ext_get_property(struct power_supply *psy,
> + const struct power_supply_ext *ext,
> + void *ext_data,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
> + val->intval = power_supply_ext_manufacture_year;
> + break;
> + case POWER_SUPPLY_PROP_TEMP_MAX:
> + val->intval = power_supply_ext_temp_max;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int power_supply_ext_set_property(struct power_supply *psy,
> + const struct power_supply_ext *ext,
> + void *ext_data,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + switch (psp) {
> + case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
> + power_supply_ext_manufacture_year = val->intval;
> + break;
> + case POWER_SUPPLY_PROP_TEMP_MAX:
> + power_supply_ext_temp_max = val->intval;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int power_supply_ext_property_is_writeable(struct power_supply *psy,
> + const struct power_supply_ext *ext,
> + void *ext_data,
> + enum power_supply_property psp)
> +{
> + return true;
> +}
> +
> +static const struct power_supply_ext power_supply_ext = {
> + .properties = power_supply_ext_props,
> + .num_properties = ARRAY_SIZE(power_supply_ext_props),
> + .get_property = power_supply_ext_get_property,
> + .set_property = power_supply_ext_set_property,
> + .property_is_writeable = power_supply_ext_property_is_writeable,
> +};
> +
> +static void test_battery_configure_battery_hook(bool enable)
> +{
> + struct power_supply *psy;
> +
> + psy = test_power_supplies[TEST_BATTERY];
> +
> + if (enable) {
> + if (power_supply_register_extension(psy, &power_supply_ext, NULL)) {
> + pr_err("registering battery extension failed\n");
> + return;
> + }
> + } else {
> + power_supply_unregister_extension(psy, &power_supply_ext);
> + }
> +
> + battery_hook = enable;
> +}
> +
> static int __init test_power_init(void)
> {
> int i;
> @@ -258,6 +338,8 @@ static int __init test_power_init(void)
> }
> }
>
> + test_battery_configure_battery_hook(true);
> +
> module_initialized = true;
> return 0;
> failed:
> @@ -524,6 +606,26 @@ static int param_set_battery_current(const char *key,
>
> #define param_get_battery_current param_get_int
>
> +static int param_set_battery_hook(const char *key,
> + const struct kernel_param *kp)
> +{
> + bool old_battery_hook;
> + int ret;
> +
> + old_battery_hook = battery_hook;
> +
> + ret = param_set_bool(key, kp);
> + if (ret)
> + return ret;
> +
> + if (old_battery_hook != battery_hook)
> + test_battery_configure_battery_hook(battery_hook);
> +
> + return 0;
> +}
> +
> +#define param_get_battery_hook param_get_bool
> +
> static const struct kernel_param_ops param_ops_ac_online = {
> .set = param_set_ac_online,
> .get = param_get_ac_online,
> @@ -574,6 +676,11 @@ static const struct kernel_param_ops param_ops_battery_current = {
> .get = param_get_battery_current,
> };
>
> +static const struct kernel_param_ops param_ops_battery_hook = {
> + .set = param_set_battery_hook,
> + .get = param_get_battery_hook,
> +};
> +
> #define param_check_ac_online(name, p) __param_check(name, p, void);
> #define param_check_usb_online(name, p) __param_check(name, p, void);
> #define param_check_battery_status(name, p) __param_check(name, p, void);
> @@ -584,6 +691,7 @@ static const struct kernel_param_ops param_ops_battery_current = {
> #define param_check_battery_voltage(name, p) __param_check(name, p, void);
> #define param_check_battery_charge_counter(name, p) __param_check(name, p, void);
> #define param_check_battery_current(name, p) __param_check(name, p, void);
> +#define param_check_battery_hook(name, p) __param_check(name, p, void);
>
>
> module_param(ac_online, ac_online, 0644);
> @@ -621,6 +729,9 @@ MODULE_PARM_DESC(battery_charge_counter,
> module_param(battery_current, battery_current, 0644);
> MODULE_PARM_DESC(battery_current, "battery current (milliampere)");
>
> +module_param(battery_hook, battery_hook, 0644);
> +MODULE_PARM_DESC(battery_hook, "battery hook");
> +
> MODULE_DESCRIPTION("Power supply driver for testing");
> MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@...il.com>");
> MODULE_LICENSE("GPL");
>
> --
> 2.47.1
>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists