• Dave Gerlach's avatar
    PM / OPP: _of_add_opp_table_v2(): increment count only if OPP is added · deac8703
    Dave Gerlach authored
    Currently the _of_add_opp_table_v2 call loops through the OPP nodes in
    the operating-points-v2 table in the device tree and calls
    _opp_add_static_v2 for each to add them to the table. It counts each
    iteration through this loop as an added OPP, however there are cases
    where _opp_add_static_v2() returns 0 but no new OPP is added to the
    list.
    
    This can happen while adding duplicate OPP or if the OPP isn't supported
    by hardware.
    
    Because of this the count variable will contain the number of OPP nodes
    in the table in device tree but not necessarily the ones that are
    actually added.
    
    As this count value is what is checked to determine if there are any
    valid OPPs, if a platform has an operating-points-v2 table with all OPP
    nodes containing opp-supported-hw values that are not currently
    supported, then _of_add_opp_table_v2 will fail to abort as it should due
    to an empty table.
    
    Additionally, since commit 3ba98324 ("PM / OPP: Get
    performance state using genpd helper"), the same count variable is
    compared against the number of OPPs containing performance states and
    requires that either all or none have pstates set, however in the case
    of any opp table that has any entries that do not get added by
    _opp_add_static_v2 due to incompatible opp-supported-hw fields, these
    numbers will not match and _of_add_opp_table_v2 will incorrectly fail.
    
    We need to clearly identify all the three cases (success, failure,
    unsupported/duplicate OPPs) and then increment count only on success
    case. Change return type of _opp_add_static_v2() to return the pointer
    to the newly added OPP instead of an integer. This routine now returns a
    valid pointer if the OPP is really added, NULL for unsupported or
    duplicate OPPs, and error value cased as a pointer on errors.
    
    Ideally the fixes tag in this commit should point back to the commit
    that introduced OPP v2 initially, as that's where we started incorrectly
    accounting for duplicate OPPs:
    
    commit 27465902 ("PM / OPP: Add support to parse "operating-points-v2" bindings")
    
    But it wasn't a real problem until recently as the count was only used
    to check if any OPPs are added or not. And so this commit points to a
    rather recent commit where we added more code that depends on the value
    of "count".
    
    Fixes: 3ba98324 ("PM / OPP: Get performance state using genpd helper")
    Reported-by: default avatarDave Gerlach <d-gerlach@ti.com>
    Reported-by: default avatarNiklas Cassel <niklas.cassel@linaro.org>
    Tested-by: default avatarNiklas Cassel <niklas.cassel@linaro.org>
    Signed-off-by: default avatarDave Gerlach <d-gerlach@ti.com>
    Signed-off-by: default avatarViresh Kumar <viresh.kumar@linaro.org>
    deac8703
of.c 20.4 KB