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.sha...@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) {