| Message ID | 20200303203559.23995-3-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | cpuidle: psci: Some fixes when using the hierarchical layout | expand |
On Tue, Mar 03, 2020 at 09:35:57PM +0100, Ulf Hansson wrote: > The current code intends to allow a PSCI PM domain to have none domain idle > states defined in DT. However, a few minor things needs to be fixed to make > this correctly supported, so let's do that. > > Reported-by: Benjamin Gaignard <benjamin.gaignard@st.com> > Fixes: a65a397f2451 ("cpuidle: psci: Add support for PM domains by using genpd") > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/cpuidle/cpuidle-psci-domain.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > index 423f03bbeb74..c34b12c4069a 100644 > --- a/drivers/cpuidle/cpuidle-psci-domain.c > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > @@ -49,6 +49,9 @@ static int __init psci_pd_parse_state_nodes(struct genpd_power_state *states, > int i, ret; > u32 psci_state, *psci_state_buf; > > + if (!states) > + return 0; > + Was any issue found ? Or just code inspection ? If states = NULL, state_count = 0, and I don't see anything blowing up. It may save couple of extra instruction execution. > for (i = 0; i < state_count; i++) { > ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode), > &psci_state); > @@ -96,6 +99,9 @@ static void psci_pd_free_states(struct genpd_power_state *states, > { > int i; > > + if (!states) > + return; > + Same here and kfree(NULL) is also valid. -- Regards, Sudeep
On Wed, 4 Mar 2020 at 11:50, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Tue, Mar 03, 2020 at 09:35:57PM +0100, Ulf Hansson wrote: > > The current code intends to allow a PSCI PM domain to have none domain idle > > states defined in DT. However, a few minor things needs to be fixed to make > > this correctly supported, so let's do that. > > > > Reported-by: Benjamin Gaignard <benjamin.gaignard@st.com> > > Fixes: a65a397f2451 ("cpuidle: psci: Add support for PM domains by using genpd") > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > > --- > > drivers/cpuidle/cpuidle-psci-domain.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c > > index 423f03bbeb74..c34b12c4069a 100644 > > --- a/drivers/cpuidle/cpuidle-psci-domain.c > > +++ b/drivers/cpuidle/cpuidle-psci-domain.c > > @@ -49,6 +49,9 @@ static int __init psci_pd_parse_state_nodes(struct genpd_power_state *states, > > int i, ret; > > u32 psci_state, *psci_state_buf; > > > > + if (!states) > > + return 0; > > + > > Was any issue found ? Or just code inspection ? If states = NULL, > state_count = 0, and I don't see anything blowing up. It may save couple > of extra instruction execution. Code inspection, the real problem was fixed in patch 1. > > > for (i = 0; i < state_count; i++) { > > ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode), > > &psci_state); > > @@ -96,6 +99,9 @@ static void psci_pd_free_states(struct genpd_power_state *states, > > { > > int i; > > > > + if (!states) > > + return; > > + > > Same here and kfree(NULL) is also valid. Yep, let's drop $subject patch from the series, it's not needed. Thanks for reviewing! Kind regards Uffe
diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c index 423f03bbeb74..c34b12c4069a 100644 --- a/drivers/cpuidle/cpuidle-psci-domain.c +++ b/drivers/cpuidle/cpuidle-psci-domain.c @@ -49,6 +49,9 @@ static int __init psci_pd_parse_state_nodes(struct genpd_power_state *states, int i, ret; u32 psci_state, *psci_state_buf; + if (!states) + return 0; + for (i = 0; i < state_count; i++) { ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode), &psci_state); @@ -96,6 +99,9 @@ static void psci_pd_free_states(struct genpd_power_state *states, { int i; + if (!states) + return; + for (i = 0; i < state_count; i++) kfree(states[i].data); kfree(states);
The current code intends to allow a PSCI PM domain to have none domain idle states defined in DT. However, a few minor things needs to be fixed to make this correctly supported, so let's do that. Reported-by: Benjamin Gaignard <benjamin.gaignard@st.com> Fixes: a65a397f2451 ("cpuidle: psci: Add support for PM domains by using genpd") Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/cpuidle/cpuidle-psci-domain.c | 6 ++++++ 1 file changed, 6 insertions(+)