| Message ID | 20221115102217.6381-1-quic_shazhuss@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] clk: qcom: gcc-sc8280xp: add cxo as parent for three ufs ref clks | expand |
On Tue, Nov 15, 2022 at 03:52:17PM +0530, Shazad Hussain wrote: > Added parent_data as CXO for three UFS reference clocks named, > gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and > gcc_ufs_1_card_clkref_clk. The commit message should explain why this is needed rather than just state what is being done. For example, something along the lines of The three UFS reference clocks gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and gcc_ufs_1_card_clkref_clk are all sourced from CXO. Update the clock driver to describe this. should do. > Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") > Link: https://lore.kernel.org/lkml/Y2Tber39cHuOSR%2FW@hovoldconsulting.com/ > Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com> Tested-by: Johan Hovold <johan+linaro@kernel.org> And with a tweaked commit message you can also add: Reviewed-by: Johan Hovold <johan+linaro@kernel.org> >--- > Changes since v1: > - Renamed patch subject. Did not include Brian's T-b comment. > - Added parent data for two ufs phy ref clocks. > > v1 of this patch can be found at > https://lore.kernel.org/all/20221030142333.31019-1-quic_shazhuss@quicinc.com/ > > used below patches for verification on next-20221114 > https://lore.kernel.org/lkml/20221104092045.17410-2-johan+linaro@kernel.org/ > https://lore.kernel.org/lkml/20221104092045.17410-3-johan+linaro@kernel.org/ > https://lore.kernel.org/lkml/20221111113732.461881-1-thierry.reding@gmail.com/ > > drivers/clk/qcom/gcc-sc8280xp.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > index a18ed88f3b82..b3198784e1c3 100644 > --- a/drivers/clk/qcom/gcc-sc8280xp.c > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > @@ -5364,6 +5364,8 @@ static struct clk_branch gcc_ufs_1_card_clkref_clk = { > .enable_mask = BIT(0), > .hw.init = &(const struct clk_init_data) { > .name = "gcc_ufs_1_card_clkref_clk", > + .parent_data = &gcc_parent_data_tcxo, > + .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -5432,6 +5434,8 @@ static struct clk_branch gcc_ufs_card_clkref_clk = { > .enable_mask = BIT(0), > .hw.init = &(const struct clk_init_data) { > .name = "gcc_ufs_card_clkref_clk", > + .parent_data = &gcc_parent_data_tcxo, > + .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, > @@ -5848,6 +5852,8 @@ static struct clk_branch gcc_ufs_ref_clkref_clk = { > .enable_mask = BIT(0), > .hw.init = &(const struct clk_init_data) { > .name = "gcc_ufs_ref_clkref_clk", > + .parent_data = &gcc_parent_data_tcxo, > + .num_parents = 1, > .ops = &clk_branch2_ops, > }, > }, Johan
On Tue, Nov 15, 2022 at 01:08:11PM +0100, Johan Hovold wrote: > On Tue, Nov 15, 2022 at 03:52:17PM +0530, Shazad Hussain wrote: > > Added parent_data as CXO for three UFS reference clocks named, > > gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and > > gcc_ufs_1_card_clkref_clk. > > The commit message should explain why this is needed rather than just > state what is being done. For example, something along the lines of > > The three UFS reference clocks gcc_ufs_ref_clkref_clk, > gcc_ufs_card_clkref_clk and gcc_ufs_1_card_clkref_clk are all > sourced from CXO. > > Update the clock driver to describe this. > > should do. > > > Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") > > Link: https://lore.kernel.org/lkml/Y2Tber39cHuOSR%2FW@hovoldconsulting.com/ > > Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com> > > Tested-by: Johan Hovold <johan+linaro@kernel.org> > > And with a tweaked commit message you can also add: > > Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Thanks for the patch Shazad! Same as Johan, you can add my T-B: Tested-by: Andrew Halaney <ahalaney@redhat.com> and with a tweaked commit message my R-B (based on Bjorn's comments about hw documentation in v1): Reviewed-by: Andrew Halaney <ahalaney@redhat.com> > > >--- > > Changes since v1: > > - Renamed patch subject. Did not include Brian's T-b comment. > > - Added parent data for two ufs phy ref clocks. > > > > v1 of this patch can be found at > > https://lore.kernel.org/all/20221030142333.31019-1-quic_shazhuss@quicinc.com/ > > > > used below patches for verification on next-20221114 > > https://lore.kernel.org/lkml/20221104092045.17410-2-johan+linaro@kernel.org/ > > https://lore.kernel.org/lkml/20221104092045.17410-3-johan+linaro@kernel.org/ > > https://lore.kernel.org/lkml/20221111113732.461881-1-thierry.reding@gmail.com/ > > > > drivers/clk/qcom/gcc-sc8280xp.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c > > index a18ed88f3b82..b3198784e1c3 100644 > > --- a/drivers/clk/qcom/gcc-sc8280xp.c > > +++ b/drivers/clk/qcom/gcc-sc8280xp.c > > @@ -5364,6 +5364,8 @@ static struct clk_branch gcc_ufs_1_card_clkref_clk = { > > .enable_mask = BIT(0), > > .hw.init = &(const struct clk_init_data) { > > .name = "gcc_ufs_1_card_clkref_clk", > > + .parent_data = &gcc_parent_data_tcxo, > > + .num_parents = 1, > > .ops = &clk_branch2_ops, > > }, > > }, > > @@ -5432,6 +5434,8 @@ static struct clk_branch gcc_ufs_card_clkref_clk = { > > .enable_mask = BIT(0), > > .hw.init = &(const struct clk_init_data) { > > .name = "gcc_ufs_card_clkref_clk", > > + .parent_data = &gcc_parent_data_tcxo, > > + .num_parents = 1, > > .ops = &clk_branch2_ops, > > }, > > }, > > @@ -5848,6 +5852,8 @@ static struct clk_branch gcc_ufs_ref_clkref_clk = { > > .enable_mask = BIT(0), > > .hw.init = &(const struct clk_init_data) { > > .name = "gcc_ufs_ref_clkref_clk", > > + .parent_data = &gcc_parent_data_tcxo, > > + .num_parents = 1, > > .ops = &clk_branch2_ops, > > }, > > }, > > Johan >
On Tue, Nov 15, 2022 at 03:52:17PM +0530, Shazad Hussain wrote: > Added parent_data as CXO for three UFS reference clocks named, > gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and > gcc_ufs_1_card_clkref_clk. > > Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") > Link: https://lore.kernel.org/lkml/Y2Tber39cHuOSR%2FW@hovoldconsulting.com/ > Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com> Put this Link beneath the --- since Bjorn will add one that's specific to your patch when he adds it to his tree. With that change: Reviewed-by: Brian Masney <bmasney@redhat.com>
On Tue, Nov 15, 2022 at 07:50:22AM -0500, Brian Masney wrote: > On Tue, Nov 15, 2022 at 03:52:17PM +0530, Shazad Hussain wrote: > > Added parent_data as CXO for three UFS reference clocks named, > > gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and > > gcc_ufs_1_card_clkref_clk. > > > > Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") > > Link: https://lore.kernel.org/lkml/Y2Tber39cHuOSR%2FW@hovoldconsulting.com/ > > Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com> > > Put this Link beneath the --- since Bjorn will add one that's specific > to your patch when he adds it to his tree. I think it's ok to keep that Link if it was intended as a reference to the discussions leading up to this patch. There can be more than one Link tag, and this one is more in line with how Linus thinks they should be used than the ones added by tooling such as b4 when applying. Johan
On Tue, Nov 15, 2022 at 02:01:04PM +0100, Johan Hovold wrote: > > Put this Link beneath the --- since Bjorn will add one that's specific > > to your patch when he adds it to his tree. > > I think it's ok to keep that Link if it was intended as a reference to > the discussions leading up to this patch. There can be more than one > Link tag, and this one is more in line with how Linus thinks they should > be used than the ones added by tooling such as b4 when applying. OK, good to know. Thank you! Brian
diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c index a18ed88f3b82..b3198784e1c3 100644 --- a/drivers/clk/qcom/gcc-sc8280xp.c +++ b/drivers/clk/qcom/gcc-sc8280xp.c @@ -5364,6 +5364,8 @@ static struct clk_branch gcc_ufs_1_card_clkref_clk = { .enable_mask = BIT(0), .hw.init = &(const struct clk_init_data) { .name = "gcc_ufs_1_card_clkref_clk", + .parent_data = &gcc_parent_data_tcxo, + .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -5432,6 +5434,8 @@ static struct clk_branch gcc_ufs_card_clkref_clk = { .enable_mask = BIT(0), .hw.init = &(const struct clk_init_data) { .name = "gcc_ufs_card_clkref_clk", + .parent_data = &gcc_parent_data_tcxo, + .num_parents = 1, .ops = &clk_branch2_ops, }, }, @@ -5848,6 +5852,8 @@ static struct clk_branch gcc_ufs_ref_clkref_clk = { .enable_mask = BIT(0), .hw.init = &(const struct clk_init_data) { .name = "gcc_ufs_ref_clkref_clk", + .parent_data = &gcc_parent_data_tcxo, + .num_parents = 1, .ops = &clk_branch2_ops, }, },
Added parent_data as CXO for three UFS reference clocks named, gcc_ufs_ref_clkref_clk, gcc_ufs_card_clkref_clk and gcc_ufs_1_card_clkref_clk. Fixes: d65d005f9a6c ("clk: qcom: add sc8280xp GCC driver") Link: https://lore.kernel.org/lkml/Y2Tber39cHuOSR%2FW@hovoldconsulting.com/ Signed-off-by: Shazad Hussain <quic_shazhuss@quicinc.com> --- Changes since v1: - Renamed patch subject. Did not include Brian's T-b comment. - Added parent data for two ufs phy ref clocks. v1 of this patch can be found at https://lore.kernel.org/all/20221030142333.31019-1-quic_shazhuss@quicinc.com/ used below patches for verification on next-20221114 https://lore.kernel.org/lkml/20221104092045.17410-2-johan+linaro@kernel.org/ https://lore.kernel.org/lkml/20221104092045.17410-3-johan+linaro@kernel.org/ https://lore.kernel.org/lkml/20221111113732.461881-1-thierry.reding@gmail.com/ drivers/clk/qcom/gcc-sc8280xp.c | 6 ++++++ 1 file changed, 6 insertions(+)