diff mbox series

[v2,2/4] power: supply: core: ease special formatting implementations

Message ID 20240303-power_supply-charge_behaviour_prop-v2-2-8ebb0a7c2409@weissschuh.net
State Accepted
Commit 521d75b4174e9cbfad73e7d0ac34fd6461b542d7
Headers show
Series power: supply: core: align charge_behaviour format with docs | expand

Commit Message

Thomas Weißschuh March 3, 2024, 3:31 p.m. UTC
By moving the conditional into the default-branch of the switch new
additions to the switch won't have to bypass the conditional.

This makes it easier to implement those special cases like the upcoming
change to the formatting of "charge_behaviour".

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/lkml/53082075-852f-4698-b354-ed30e7fd2683@redhat.com/
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/power/supply/power_supply_sysfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Sebastian Reichel March 6, 2024, 12:25 a.m. UTC | #1
Hi,

On Sun, Mar 03, 2024 at 04:31:14PM +0100, Thomas Weißschuh wrote:
> By moving the conditional into the default-branch of the switch new
> additions to the switch won't have to bypass the conditional.
> 
> This makes it easier to implement those special cases like the upcoming
> change to the formatting of "charge_behaviour".
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Link: https://lore.kernel.org/lkml/53082075-852f-4698-b354-ed30e7fd2683@redhat.com/
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---

Thanks, queued.

-- Sebastian

>  drivers/power/supply/power_supply_sysfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 977611e16373..10fec411794b 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -298,11 +298,6 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		}
>  	}
>  
> -	if (ps_attr->text_values_len > 0 &&
> -	    value.intval < ps_attr->text_values_len && value.intval >= 0) {
> -		return sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
> -	}
> -
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_USB_TYPE:
>  		ret = power_supply_show_usb_type(dev, psy->desc,
> @@ -312,7 +307,12 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		ret = sysfs_emit(buf, "%s\n", value.strval);
>  		break;
>  	default:
> -		ret = sysfs_emit(buf, "%d\n", value.intval);
> +		if (ps_attr->text_values_len > 0 &&
> +				value.intval < ps_attr->text_values_len && value.intval >= 0) {
> +			ret = sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
> +		} else {
> +			ret = sysfs_emit(buf, "%d\n", value.intval);
> +		}
>  	}
>  
>  	return ret;
> 
> -- 
> 2.44.0
> 
>
Hans de Goede March 27, 2024, 10:36 a.m. UTC | #2
Hi,

On 3/3/24 4:31 PM, Thomas Weißschuh wrote:
> By moving the conditional into the default-branch of the switch new
> additions to the switch won't have to bypass the conditional.
> 
> This makes it easier to implement those special cases like the upcoming
> change to the formatting of "charge_behaviour".
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Link: https://lore.kernel.org/lkml/53082075-852f-4698-b354-ed30e7fd2683@redhat.com/
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
>  drivers/power/supply/power_supply_sysfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 977611e16373..10fec411794b 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -298,11 +298,6 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		}
>  	}
>  
> -	if (ps_attr->text_values_len > 0 &&
> -	    value.intval < ps_attr->text_values_len && value.intval >= 0) {
> -		return sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
> -	}
> -
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_USB_TYPE:
>  		ret = power_supply_show_usb_type(dev, psy->desc,
> @@ -312,7 +307,12 @@ static ssize_t power_supply_show_property(struct device *dev,
>  		ret = sysfs_emit(buf, "%s\n", value.strval);
>  		break;
>  	default:
> -		ret = sysfs_emit(buf, "%d\n", value.intval);
> +		if (ps_attr->text_values_len > 0 &&
> +				value.intval < ps_attr->text_values_len && value.intval >= 0) {
> +			ret = sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
> +		} else {
> +			ret = sysfs_emit(buf, "%d\n", value.intval);
> +		}
>  	}
>  
>  	return ret;
>
diff mbox series

Patch

diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 977611e16373..10fec411794b 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -298,11 +298,6 @@  static ssize_t power_supply_show_property(struct device *dev,
 		}
 	}
 
-	if (ps_attr->text_values_len > 0 &&
-	    value.intval < ps_attr->text_values_len && value.intval >= 0) {
-		return sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
-	}
-
 	switch (psp) {
 	case POWER_SUPPLY_PROP_USB_TYPE:
 		ret = power_supply_show_usb_type(dev, psy->desc,
@@ -312,7 +307,12 @@  static ssize_t power_supply_show_property(struct device *dev,
 		ret = sysfs_emit(buf, "%s\n", value.strval);
 		break;
 	default:
-		ret = sysfs_emit(buf, "%d\n", value.intval);
+		if (ps_attr->text_values_len > 0 &&
+				value.intval < ps_attr->text_values_len && value.intval >= 0) {
+			ret = sysfs_emit(buf, "%s\n", ps_attr->text_values[value.intval]);
+		} else {
+			ret = sysfs_emit(buf, "%d\n", value.intval);
+		}
 	}
 
 	return ret;