[PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[Dmitry Baryshkov]

Switch _qcom_cc_register_board_clk() to use parent_hws.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---

Changes since v1:
- Properly fix the clock's parent data in case parent clock is present
  in the dtsi.

---
 drivers/clk/qcom/common.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 75f09e6e057e..9f2bd37c9540 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -133,7 +133,7 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
 	struct device_node *node = NULL;
 	struct device_node *clocks_node;
 	struct clk_fixed_factor *factor;
-	struct clk_fixed_rate *fixed;
+	struct clk_fixed_rate *fixed = NULL;
 	struct clk_init_data init_data = { };
 	int ret;
 
@@ -161,6 +161,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
 	of_node_put(node);
 
 	if (add_factor) {
+		struct clk_parent_data parent_data;
+
 		factor = devm_kzalloc(dev, sizeof(*factor), GFP_KERNEL);
 		if (!factor)
 			return -EINVAL;
@@ -168,8 +170,13 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
 		factor->mult = factor->div = 1;
 		factor->hw.init = &init_data;
 
+		if (fixed)
+			parent_data.hw = &fixed->hw;
+		else
+			parent_data.name = path;
+
 		init_data.name = name;
-		init_data.parent_names = &path;
+		init_data.parent_data = &parent_data;
 		init_data.num_parents = 1;
 		init_data.flags = 0;
 		init_data.ops = &clk_fixed_factor_ops;
-- 
2.39.0

Re: [PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[Bjorn Andersson]

On Wed, Dec 28, 2022 at 10:35:55PM +0200, Dmitry Baryshkov wrote:
> Switch _qcom_cc_register_board_clk() to use parent_hws.
> 

There's more to this patch then this short sentence, please describe it
further.

And given Dan's reported-by, I must assume that there's an issue with
the current code. Is there a Fixes?

Regards,
Bjorn

> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> 
> Changes since v1:
> - Properly fix the clock's parent data in case parent clock is present
>   in the dtsi.
> 
> ---
>  drivers/clk/qcom/common.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 75f09e6e057e..9f2bd37c9540 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -133,7 +133,7 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
>  	struct device_node *node = NULL;
>  	struct device_node *clocks_node;
>  	struct clk_fixed_factor *factor;
> -	struct clk_fixed_rate *fixed;
> +	struct clk_fixed_rate *fixed = NULL;
>  	struct clk_init_data init_data = { };
>  	int ret;
>  
> @@ -161,6 +161,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
>  	of_node_put(node);
>  
>  	if (add_factor) {
> +		struct clk_parent_data parent_data;
> +
>  		factor = devm_kzalloc(dev, sizeof(*factor), GFP_KERNEL);
>  		if (!factor)
>  			return -EINVAL;
> @@ -168,8 +170,13 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
>  		factor->mult = factor->div = 1;
>  		factor->hw.init = &init_data;
>  
> +		if (fixed)
> +			parent_data.hw = &fixed->hw;
> +		else
> +			parent_data.name = path;
> +
>  		init_data.name = name;
> -		init_data.parent_names = &path;
> +		init_data.parent_data = &parent_data;
>  		init_data.num_parents = 1;
>  		init_data.flags = 0;
>  		init_data.ops = &clk_fixed_factor_ops;
> -- 
> 2.39.0
> 

Re: [PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[Dmitry Baryshkov]

On Wed, 11 Jan 2023 at 06:47, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Wed, Dec 28, 2022 at 10:35:55PM +0200, Dmitry Baryshkov wrote:
> > Switch _qcom_cc_register_board_clk() to use parent_hws.
> >
>
> There's more to this patch then this short sentence, please describe it
> further.

True, I'll fix it for v3.

>
> And given Dan's reported-by, I must assume that there's an issue with
> the current code. Is there a Fixes?

No. Dan reported an issue with v1, thus his Reported-by was included into v2.

>
> Regards,
> Bjorn
>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >
> > Changes since v1:
> > - Properly fix the clock's parent data in case parent clock is present
> >   in the dtsi.
> >
> > ---
> >  drivers/clk/qcom/common.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> > index 75f09e6e057e..9f2bd37c9540 100644
> > --- a/drivers/clk/qcom/common.c
> > +++ b/drivers/clk/qcom/common.c
> > @@ -133,7 +133,7 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> >       struct device_node *node = NULL;
> >       struct device_node *clocks_node;
> >       struct clk_fixed_factor *factor;
> > -     struct clk_fixed_rate *fixed;
> > +     struct clk_fixed_rate *fixed = NULL;
> >       struct clk_init_data init_data = { };
> >       int ret;
> >
> > @@ -161,6 +161,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> >       of_node_put(node);
> >
> >       if (add_factor) {
> > +             struct clk_parent_data parent_data;
> > +
> >               factor = devm_kzalloc(dev, sizeof(*factor), GFP_KERNEL);
> >               if (!factor)
> >                       return -EINVAL;
> > @@ -168,8 +170,13 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> >               factor->mult = factor->div = 1;
> >               factor->hw.init = &init_data;
> >
> > +             if (fixed)
> > +                     parent_data.hw = &fixed->hw;
> > +             else
> > +                     parent_data.name = path;
> > +
> >               init_data.name = name;
> > -             init_data.parent_names = &path;
> > +             init_data.parent_data = &parent_data;
> >               init_data.num_parents = 1;
> >               init_data.flags = 0;
> >               init_data.ops = &clk_fixed_factor_ops;
> > --
> > 2.39.0
> >



-- 
With best wishes
Dmitry

Re: [PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[Bjorn Andersson]

On Wed, Jan 11, 2023 at 06:49:29AM +0200, Dmitry Baryshkov wrote:
> On Wed, 11 Jan 2023 at 06:47, Bjorn Andersson <andersson@kernel.org> wrote:
> >
> > On Wed, Dec 28, 2022 at 10:35:55PM +0200, Dmitry Baryshkov wrote:
> > > Switch _qcom_cc_register_board_clk() to use parent_hws.
> > >
> >
> > There's more to this patch then this short sentence, please describe it
> > further.
> 
> True, I'll fix it for v3.
> 
> >
> > And given Dan's reported-by, I must assume that there's an issue with
> > the current code. Is there a Fixes?
> 
> No. Dan reported an issue with v1, thus his Reported-by was included into v2.
> 

I though it meant "this corrects an issue that was Reported-by".

IMHO including "issues was fixed during code review after being
Reported-by" diminishing the value of this trailer - in particular since
we don't use it broadly to capture anyone else's review feedback.


PS. I'm definitely in favour of introducing a trailer to give Dan, and
others, credit for reviews.

Regards,
Bjorn

> >
> > Regards,
> > Bjorn
> >
> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >
> > > Changes since v1:
> > > - Properly fix the clock's parent data in case parent clock is present
> > >   in the dtsi.
> > >
> > > ---
> > >  drivers/clk/qcom/common.c | 11 +++++++++--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> > > index 75f09e6e057e..9f2bd37c9540 100644
> > > --- a/drivers/clk/qcom/common.c
> > > +++ b/drivers/clk/qcom/common.c
> > > @@ -133,7 +133,7 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> > >       struct device_node *node = NULL;
> > >       struct device_node *clocks_node;
> > >       struct clk_fixed_factor *factor;
> > > -     struct clk_fixed_rate *fixed;
> > > +     struct clk_fixed_rate *fixed = NULL;
> > >       struct clk_init_data init_data = { };
> > >       int ret;
> > >
> > > @@ -161,6 +161,8 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> > >       of_node_put(node);
> > >
> > >       if (add_factor) {
> > > +             struct clk_parent_data parent_data;
> > > +
> > >               factor = devm_kzalloc(dev, sizeof(*factor), GFP_KERNEL);
> > >               if (!factor)
> > >                       return -EINVAL;
> > > @@ -168,8 +170,13 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
> > >               factor->mult = factor->div = 1;
> > >               factor->hw.init = &init_data;
> > >
> > > +             if (fixed)
> > > +                     parent_data.hw = &fixed->hw;
> > > +             else
> > > +                     parent_data.name = path;
> > > +
> > >               init_data.name = name;
> > > -             init_data.parent_names = &path;
> > > +             init_data.parent_data = &parent_data;
> > >               init_data.num_parents = 1;
> > >               init_data.flags = 0;
> > >               init_data.ops = &clk_fixed_factor_ops;
> > > --
> > > 2.39.0
> > >
> 
> 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v2] clk: qcom: common: use parent_hws in _qcom_cc_register_board_clk()

[Dmitry Baryshkov]

On 17/01/2023 05:30, Bjorn Andersson wrote:
> On Wed, Jan 11, 2023 at 06:49:29AM +0200, Dmitry Baryshkov wrote:
>> On Wed, 11 Jan 2023 at 06:47, Bjorn Andersson <andersson@kernel.org> wrote:
>>>
>>> On Wed, Dec 28, 2022 at 10:35:55PM +0200, Dmitry Baryshkov wrote:
>>>> Switch _qcom_cc_register_board_clk() to use parent_hws.
>>>>
>>>
>>> There's more to this patch then this short sentence, please describe it
>>> further.
>>
>> True, I'll fix it for v3.
>>
>>>
>>> And given Dan's reported-by, I must assume that there's an issue with
>>> the current code. Is there a Fixes?
>>
>> No. Dan reported an issue with v1, thus his Reported-by was included into v2.
>>
> 
> I though it meant "this corrects an issue that was Reported-by".
> 
> IMHO including "issues was fixed during code review after being
> Reported-by" diminishing the value of this trailer - in particular since
> we don't use it broadly to capture anyone else's review feedback.

I particularly use it for the cases when the issue was reported by 
testing robot or by Dan (or Dan's robot), since these emails explicitly 
ask for such trailer.

> 
> 
> PS. I'm definitely in favour of introducing a trailer to give Dan, and
> others, credit for reviews.

Any particular suggestion?

-- 
With best wishes
Dmitry