[PATCH] phy: phy-uclass: Add a missing error handling path

[Jonas Karlman]

On 2023-07-02 20:47, Bhupesh Sharma wrote:
> Since function 'phy_get_counts()' can return NULL,
> add handling for that error path inside callers of
> this function.

Do you have any example where this can/has happened?

Counts should never be NULL in a normal working call flow. I am a bit
worried that this patch may hide some other bug or use of an
uninitialized phy struct.

The phy struct is initialized in generic_phy_get_by_index_nodev. That
function should fail when counts cannot be allocated.

Maybe generic_phy_get_by_index_nodev should unset phy->dev if it fails,
or generic_phy_valid should be extended to also check phy->counts.
That way generic_phy_valid would return false and these functions
should return earlier before trying to use counts.

Regards,
Jonas

> 
> Signed-off-by: Bhupesh Sharma <bhupesh.sharma at linaro.org>
> ---
>  drivers/phy/phy-uclass.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
> index 629ef3aa3d..c523a0b45e 100644
> --- a/drivers/phy/phy-uclass.c
> +++ b/drivers/phy/phy-uclass.c
> @@ -229,6 +229,11 @@ int generic_phy_init(struct phy *phy)
>  	if (!generic_phy_valid(phy))
>  		return 0;
>  	counts = phy_get_counts(phy);
> +	if (!counts) {
> +		debug("phy_get_counts returns NULL\n");
> +		return -ENODEV;
> +	}
> +
>  	if (counts->init_count > 0) {
>  		counts->init_count++;
>  		return 0;
> @@ -275,6 +280,11 @@ int generic_phy_exit(struct phy *phy)
>  	if (!generic_phy_valid(phy))
>  		return 0;
>  	counts = phy_get_counts(phy);
> +	if (!counts) {
> +		debug("phy_get_counts returns NULL\n");
> +		return -ENODEV;
> +	}
> +
>  	if (counts->init_count == 0)
>  		return 0;
>  	if (counts->init_count > 1) {
> @@ -305,6 +315,11 @@ int generic_phy_power_on(struct phy *phy)
>  	if (!generic_phy_valid(phy))
>  		return 0;
>  	counts = phy_get_counts(phy);
> +	if (!counts) {
> +		debug("phy_get_counts returns NULL\n");
> +		return -ENODEV;
> +	}
> +
>  	if (counts->power_on_count > 0) {
>  		counts->power_on_count++;
>  		return 0;
> @@ -341,6 +356,11 @@ int generic_phy_power_off(struct phy *phy)
>  	if (!generic_phy_valid(phy))
>  		return 0;
>  	counts = phy_get_counts(phy);
> +	if (!counts) {
> +		debug("phy_get_counts returns NULL\n");
> +		return -ENODEV;
> +	}
> +
>  	if (counts->power_on_count == 0)
>  		return 0;
>  	if (counts->power_on_count > 1) {