Re: [PATCH 6.11 regression fix] power: supply: sysfs: Revert use power_supply_property_is_writeable()

["Thomas Weißschuh" <linux@weissschuh.net>]

Hi Hans,

On 2024-09-08 16:44:14+0000, Hans de Goede wrote:
> power_supply_property_is_writeable() contains the following check:
> 
>         if (atomic_read(&psy->use_cnt) <= 0 ||
>                         !psy->desc->property_is_writeable)
>                 return -ENODEV;
> 
> psy->use_cnt gets initialized to 0 and is incremented by
> __power_supply_register() after it calls device_add(); and thus after
> the driver core calls power_supply_attr_is_visible() to get the attr's
> permissions.
> 
> So when power_supply_attr_is_visible() runs psy->use_cnt is 0 failing
> the above check. This is causing all the attributes to have permissions
> of 444 even those which should be writable.
> 
> Move back to manually calling psy->desc->property_is_writeable() without
> the psy->use_cnt check to fix this.

Thanks for the fix!

OTOH the whole power_supply_attr_is_visible() is completely unused
outisde of the psy core. So instead we could unexport it, drop the use_cnt
check and use it again.
(All of this as part of the psy extension series, for now your revert
should be used)

What do you think?

> 
> Fixes: be6299c6e55e ("power: supply: sysfs: use power_supply_property_is_writeable()")
> Cc: Thomas Weißschuh <linux@weissschuh.net>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Note this is a straight-forward revert of be6299c6e55e
> ---
>  drivers/power/supply/power_supply_sysfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 6cd3fac1891b..fb9b67b5a9aa 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -387,7 +387,8 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
>  		int property = psy->desc->properties[i];
>  
>  		if (property == attrno) {
> -			if (power_supply_property_is_writeable(psy, property) > 0)
> +			if (psy->desc->property_is_writeable &&
> +			    psy->desc->property_is_writeable(psy, property) > 0)
>  				mode |= S_IWUSR;
>  
>  			return mode;
> -- 
> 2.46.0
>