Hi,
This marks the properties and usb_types entries in
struct power_supply_desc as const, so that drivers
can constify those tables.
-- Sebastian
Sebastian Reichel (4):
power: supply: core: Constify usb_types
power: supply: charger-manager: Prepare for const properties
power: supply: generic-adc-battery: Prepare for const properties
power: supply: core: Constify properties
drivers/power/supply/charger-manager.c | 40 ++++++++++++----------
drivers/power/supply/generic-adc-battery.c | 22 ++++++------
drivers/power/supply/power_supply_sysfs.c | 2 +-
include/linux/power_supply.h | 4 +--
4 files changed, 36 insertions(+), 32 deletions(-)
--
2.25.1
usb_types is a list of USB types supported by the
hardware, which does not change at runtime. Let's
mark it as const for improved security.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/power_supply_sysfs.c | 2 +-
include/linux/power_supply.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index f37ad4eae60b..45dafc1820ff 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -78,7 +78,7 @@ static const char * const power_supply_scope_text[] = {
};
static ssize_t power_supply_show_usb_type(struct device *dev,
- enum power_supply_usb_type *usb_types,
+ const enum power_supply_usb_type *usb_types,
ssize_t num_usb_types,
union power_supply_propval *value,
char *buf)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index dcd5a71e6c67..0392c9cc8f1c 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -223,7 +223,7 @@ struct power_supply_config {
struct power_supply_desc {
const char *name;
enum power_supply_type type;
- enum power_supply_usb_type *usb_types;
+ const enum power_supply_usb_type *usb_types;
size_t num_usb_types;
enum power_supply_property *properties;
size_t num_properties;
--
2.25.1
properties is a list of properties supported by the
driver. It is not modified by the power-supply core
and drivers are not supposed to change it once the
list has been registered. Let's mark it as const for
improved security.
Signed-off-by: Sebastian Reichel <[email protected]>
---
include/linux/power_supply.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 0392c9cc8f1c..6a34df65d4d1 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -225,7 +225,7 @@ struct power_supply_desc {
enum power_supply_type type;
const enum power_supply_usb_type *usb_types;
size_t num_usb_types;
- enum power_supply_property *properties;
+ const enum power_supply_property *properties;
size_t num_properties;
/*
--
2.25.1
This prepares the driver to work with the properties entry
in power_supply_desc marked as const.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/generic-adc-battery.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
index bc462d1ec963..caa829738ef7 100644
--- a/drivers/power/supply/generic-adc-battery.c
+++ b/drivers/power/supply/generic-adc-battery.c
@@ -241,6 +241,7 @@ static int gab_probe(struct platform_device *pdev)
struct power_supply_desc *psy_desc;
struct power_supply_config psy_cfg = {};
struct gab_platform_data *pdata = pdev->dev.platform_data;
+ enum power_supply_property *properties;
int ret = 0;
int chan;
int index = ARRAY_SIZE(gab_props);
@@ -268,16 +269,16 @@ static int gab_probe(struct platform_device *pdev)
* copying the static properties and allocating extra memory for holding
* the extra configurable properties received from platform data.
*/
- psy_desc->properties = kcalloc(ARRAY_SIZE(gab_props) +
- ARRAY_SIZE(gab_chan_name),
- sizeof(*psy_desc->properties),
- GFP_KERNEL);
- if (!psy_desc->properties) {
+ properties = kcalloc(ARRAY_SIZE(gab_props) +
+ ARRAY_SIZE(gab_chan_name),
+ sizeof(*properties),
+ GFP_KERNEL);
+ if (!properties) {
ret = -ENOMEM;
goto first_mem_fail;
}
- memcpy(psy_desc->properties, gab_props, sizeof(gab_props));
+ memcpy(properties, gab_props, sizeof(gab_props));
/*
* getting channel from iio and copying the battery properties
@@ -294,13 +295,11 @@ static int gab_probe(struct platform_device *pdev)
int index2;
for (index2 = 0; index2 < index; index2++) {
- if (psy_desc->properties[index2] ==
- gab_dyn_props[chan])
+ if (properties[index2] == gab_dyn_props[chan])
break; /* already known */
}
if (index2 == index) /* really new */
- psy_desc->properties[index++] =
- gab_dyn_props[chan];
+ properties[index++] = gab_dyn_props[chan];
any = true;
}
}
@@ -317,6 +316,7 @@ static int gab_probe(struct platform_device *pdev)
* as come channels may be not be supported by the device.So
* we need to take care of that.
*/
+ psy_desc->properties = properties;
psy_desc->num_properties = index;
adc_bat->psy = power_supply_register(&pdev->dev, psy_desc, &psy_cfg);
@@ -358,7 +358,7 @@ static int gab_probe(struct platform_device *pdev)
iio_channel_release(adc_bat->channel[chan]);
}
second_mem_fail:
- kfree(psy_desc->properties);
+ kfree(properties);
first_mem_fail:
return ret;
}
--
2.25.1
This prepares the driver to work with the properties entry
in power_supply_desc marked as const.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/charger-manager.c | 40 ++++++++++++++------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index a21e1a2673f8..a71e2ee81423 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1422,7 +1422,9 @@ static int charger_manager_prepare_sysfs(struct charger_manager *cm)
}
static int cm_init_thermal_data(struct charger_manager *cm,
- struct power_supply *fuel_gauge)
+ struct power_supply *fuel_gauge,
+ enum power_supply_property *properties,
+ size_t *num_properties)
{
struct charger_desc *desc = cm->desc;
union power_supply_propval val;
@@ -1433,9 +1435,8 @@ static int cm_init_thermal_data(struct charger_manager *cm,
POWER_SUPPLY_PROP_TEMP, &val);
if (!ret) {
- cm->charger_psy_desc.properties[cm->charger_psy_desc.num_properties] =
- POWER_SUPPLY_PROP_TEMP;
- cm->charger_psy_desc.num_properties++;
+ properties[*num_properties] = POWER_SUPPLY_PROP_TEMP;
+ (*num_properties)++;
cm->desc->measure_battery_temp = true;
}
#ifdef CONFIG_THERMAL
@@ -1446,9 +1447,8 @@ static int cm_init_thermal_data(struct charger_manager *cm,
return PTR_ERR(cm->tzd_batt);
/* Use external thermometer */
- cm->charger_psy_desc.properties[cm->charger_psy_desc.num_properties] =
- POWER_SUPPLY_PROP_TEMP_AMBIENT;
- cm->charger_psy_desc.num_properties++;
+ properties[*num_properties] = POWER_SUPPLY_PROP_TEMP_AMBIENT;
+ (*num_properties)++;
cm->desc->measure_battery_temp = true;
ret = 0;
}
@@ -1621,6 +1621,8 @@ static int charger_manager_probe(struct platform_device *pdev)
int j = 0;
union power_supply_propval val;
struct power_supply *fuel_gauge;
+ enum power_supply_property *properties;
+ size_t num_properties;
struct power_supply_config psy_cfg = {};
if (IS_ERR(desc)) {
@@ -1717,18 +1719,17 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->charger_psy_desc.name = cm->psy_name_buf;
/* Allocate for psy properties because they may vary */
- cm->charger_psy_desc.properties =
- devm_kcalloc(&pdev->dev,
+ properties = devm_kcalloc(&pdev->dev,
ARRAY_SIZE(default_charger_props) +
NUM_CHARGER_PSY_OPTIONAL,
- sizeof(enum power_supply_property), GFP_KERNEL);
- if (!cm->charger_psy_desc.properties)
+ sizeof(*properties), GFP_KERNEL);
+ if (!properties)
return -ENOMEM;
- memcpy(cm->charger_psy_desc.properties, default_charger_props,
+ memcpy(properties, default_charger_props,
sizeof(enum power_supply_property) *
ARRAY_SIZE(default_charger_props));
- cm->charger_psy_desc.num_properties = psy_default.num_properties;
+ num_properties = psy_default.num_properties;
/* Find which optional psy-properties are available */
fuel_gauge = power_supply_get_by_name(desc->psy_fuel_gauge);
@@ -1739,25 +1740,28 @@ static int charger_manager_probe(struct platform_device *pdev)
}
if (!power_supply_get_property(fuel_gauge,
POWER_SUPPLY_PROP_CHARGE_NOW, &val)) {
- cm->charger_psy_desc.properties[cm->charger_psy_desc.num_properties] =
+ properties[cm->charger_psy_desc.num_properties] =
POWER_SUPPLY_PROP_CHARGE_NOW;
- cm->charger_psy_desc.num_properties++;
+ num_properties++;
}
if (!power_supply_get_property(fuel_gauge,
POWER_SUPPLY_PROP_CURRENT_NOW,
&val)) {
- cm->charger_psy_desc.properties[cm->charger_psy_desc.num_properties] =
+ properties[cm->charger_psy_desc.num_properties] =
POWER_SUPPLY_PROP_CURRENT_NOW;
- cm->charger_psy_desc.num_properties++;
+ num_properties++;
}
- ret = cm_init_thermal_data(cm, fuel_gauge);
+ ret = cm_init_thermal_data(cm, fuel_gauge, properties, &num_properties);
if (ret) {
dev_err(&pdev->dev, "Failed to initialize thermal data\n");
cm->desc->measure_battery_temp = false;
}
power_supply_put(fuel_gauge);
+ cm->charger_psy_desc.properties = properties;
+ cm->charger_psy_desc.num_properties = num_properties;
+
INIT_DELAYED_WORK(&cm->fullbatt_vchk_work, fullbatt_vchk);
/* Register sysfs entry for charger(regulator) */
--
2.25.1
Hi Sebastian,
On 13/4/20 20:38, Sebastian Reichel wrote:
> properties is a list of properties supported by the
> driver. It is not modified by the power-supply core
> and drivers are not supposed to change it once the
> list has been registered. Let's mark it as const for
> improved security.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
Reviewed-by: Enric Balletbo i Serra <[email protected]>
> ---
> include/linux/power_supply.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 0392c9cc8f1c..6a34df65d4d1 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -225,7 +225,7 @@ struct power_supply_desc {
> enum power_supply_type type;
> const enum power_supply_usb_type *usb_types;
> size_t num_usb_types;
> - enum power_supply_property *properties;
> + const enum power_supply_property *properties;
> size_t num_properties;
>
> /*
>
Hi Sebastian,
I went through all the patches and Anyway I also tested with cros-usbpd-driver
and they look good to me. I am wondering if you shouldn't constify the users in
this series too, looks pretty trivial?
Cheers,
Enric
On 13/4/20 20:38, Sebastian Reichel wrote:
> Hi,
>
> This marks the properties and usb_types entries in
> struct power_supply_desc as const, so that drivers
> can constify those tables.
>
> -- Sebastian
>
> Sebastian Reichel (4):
> power: supply: core: Constify usb_types
> power: supply: charger-manager: Prepare for const properties
> power: supply: generic-adc-battery: Prepare for const properties
> power: supply: core: Constify properties
>
> drivers/power/supply/charger-manager.c | 40 ++++++++++++----------
> drivers/power/supply/generic-adc-battery.c | 22 ++++++------
> drivers/power/supply/power_supply_sysfs.c | 2 +-
> include/linux/power_supply.h | 4 +--
> 4 files changed, 36 insertions(+), 32 deletions(-)
>
Hi Sebastian,
On 13/4/20 20:38, Sebastian Reichel wrote:
> usb_types is a list of USB types supported by the
> hardware, which does not change at runtime. Let's
> mark it as const for improved security.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
Reviewed-by: Enric Balletbo i Serra <[email protected]>
> ---
> drivers/power/supply/power_supply_sysfs.c | 2 +-
> include/linux/power_supply.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index f37ad4eae60b..45dafc1820ff 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -78,7 +78,7 @@ static const char * const power_supply_scope_text[] = {
> };
>
> static ssize_t power_supply_show_usb_type(struct device *dev,
> - enum power_supply_usb_type *usb_types,
> + const enum power_supply_usb_type *usb_types,
> ssize_t num_usb_types,
> union power_supply_propval *value,
> char *buf)
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index dcd5a71e6c67..0392c9cc8f1c 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -223,7 +223,7 @@ struct power_supply_config {
> struct power_supply_desc {
> const char *name;
> enum power_supply_type type;
> - enum power_supply_usb_type *usb_types;
> + const enum power_supply_usb_type *usb_types;
> size_t num_usb_types;
> enum power_supply_property *properties;
> size_t num_properties;
>
Hi Sebastian,
On 13/4/20 20:38, Sebastian Reichel wrote:
> This prepares the driver to work with the properties entry
> in power_supply_desc marked as const.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
Reviewed-by: Enric Balletbo i Serra <[email protected]>
> ---
> drivers/power/supply/generic-adc-battery.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/power/supply/generic-adc-battery.c b/drivers/power/supply/generic-adc-battery.c
> index bc462d1ec963..caa829738ef7 100644
> --- a/drivers/power/supply/generic-adc-battery.c
> +++ b/drivers/power/supply/generic-adc-battery.c
> @@ -241,6 +241,7 @@ static int gab_probe(struct platform_device *pdev)
> struct power_supply_desc *psy_desc;
> struct power_supply_config psy_cfg = {};
> struct gab_platform_data *pdata = pdev->dev.platform_data;
> + enum power_supply_property *properties;
> int ret = 0;
> int chan;
> int index = ARRAY_SIZE(gab_props);
> @@ -268,16 +269,16 @@ static int gab_probe(struct platform_device *pdev)
> * copying the static properties and allocating extra memory for holding
> * the extra configurable properties received from platform data.
> */
> - psy_desc->properties = kcalloc(ARRAY_SIZE(gab_props) +
> - ARRAY_SIZE(gab_chan_name),
> - sizeof(*psy_desc->properties),
> - GFP_KERNEL);
> - if (!psy_desc->properties) {
> + properties = kcalloc(ARRAY_SIZE(gab_props) +
> + ARRAY_SIZE(gab_chan_name),
> + sizeof(*properties),
> + GFP_KERNEL);
> + if (!properties) {
> ret = -ENOMEM;
> goto first_mem_fail;
> }
>
> - memcpy(psy_desc->properties, gab_props, sizeof(gab_props));
> + memcpy(properties, gab_props, sizeof(gab_props));
>
> /*
> * getting channel from iio and copying the battery properties
> @@ -294,13 +295,11 @@ static int gab_probe(struct platform_device *pdev)
> int index2;
>
> for (index2 = 0; index2 < index; index2++) {
> - if (psy_desc->properties[index2] ==
> - gab_dyn_props[chan])
> + if (properties[index2] == gab_dyn_props[chan])
> break; /* already known */
> }
> if (index2 == index) /* really new */
> - psy_desc->properties[index++] =
> - gab_dyn_props[chan];
> + properties[index++] = gab_dyn_props[chan];
> any = true;
> }
> }
> @@ -317,6 +316,7 @@ static int gab_probe(struct platform_device *pdev)
> * as come channels may be not be supported by the device.So
> * we need to take care of that.
> */
> + psy_desc->properties = properties;
> psy_desc->num_properties = index;
>
> adc_bat->psy = power_supply_register(&pdev->dev, psy_desc, &psy_cfg);
> @@ -358,7 +358,7 @@ static int gab_probe(struct platform_device *pdev)
> iio_channel_release(adc_bat->channel[chan]);
> }
> second_mem_fail:
> - kfree(psy_desc->properties);
> + kfree(properties);
> first_mem_fail:
> return ret;
> }
>
Hi,
On Mon, Apr 13, 2020 at 08:38:49PM +0200, Sebastian Reichel wrote:
> This marks the properties and usb_types entries in
> struct power_supply_desc as const, so that drivers
> can constify those tables.
>
> Sebastian Reichel (4):
> power: supply: core: Constify usb_types
> power: supply: charger-manager: Prepare for const properties
> power: supply: generic-adc-battery: Prepare for const properties
> power: supply: core: Constify properties
>
> drivers/power/supply/charger-manager.c | 40 ++++++++++++----------
> drivers/power/supply/generic-adc-battery.c | 22 ++++++------
> drivers/power/supply/power_supply_sysfs.c | 2 +-
> include/linux/power_supply.h | 4 +--
> 4 files changed, 36 insertions(+), 32 deletions(-)
I merged the charger-manager and generic-adc-battery patches with
Enric's RB, but took over Michał Mirosław's variant for the other bits.
-- Sebastian