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

Bhupesh Sharma bhupesh.sharma at linaro.org
Wed Jul 5 06:48:28 CEST 2023

Hi Jonas,

On Mon, 3 Jul 2023 at 01:18, Jonas Karlman <jonas at kwiboo.se> wrote:
> 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?

Yes, it happened while I was writing a UFS Host Controller driver for
u-boot and tried to initialize the UFS PHY via the generic u-boot PHY
CLASS utility functions, namely:

/* get phy */
ret = generic_phy_get_by_name(hba->dev, "ufsphy", &phy);

/* phy initialization */
ret = generic_phy_init(&phy);
 /* power on phy */
ret = generic_phy_power_on(&phy);

> 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.

I agree, but I found that if the UFS Host Controller driver would mess
up the phy call sequences while it was in a 'power_up_sequence',
instead of using the standard UCLASS_PHY
'generic_phy_get_by_index_nodev' flow, it might get a counts value
NULL and then the PHY driver would silently crash while setting /
accessing members of 'counts'.

int generic_phy_init(struct phy *phy)
    counts = phy_get_counts(phy);
    if (counts->init_count > 0) {
        // crash

> 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.

I agree, this error handling should be here for counts being
uninitialized, whether we do it per-function or at the top level.. As
I can see several users of the flow I described in the u-boot code
itself (for setting up a PHY), for e.g.:




More information about the U-Boot mailing list