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%[email protected]/
Signed-off-by: Shazad Hussain <[email protected]>
---
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/[email protected]/
used below patches for verification on next-20221114
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/
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,
},
},
--
2.38.0
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%[email protected]/
> Signed-off-by: Shazad Hussain <[email protected]>
Tested-by: Johan Hovold <[email protected]>
And with a tweaked commit message you can also add:
Reviewed-by: Johan Hovold <[email protected]>
>---
> 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/[email protected]/
>
> used below patches for verification on next-20221114
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
> https://lore.kernel.org/lkml/[email protected]/
>
> 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%[email protected]/
> > Signed-off-by: Shazad Hussain <[email protected]>
>
> Tested-by: Johan Hovold <[email protected]>
>
> And with a tweaked commit message you can also add:
>
> Reviewed-by: Johan Hovold <[email protected]>
Thanks for the patch Shazad! Same as Johan, you can add my T-B:
Tested-by: Andrew Halaney <[email protected]>
and with a tweaked commit message my R-B (based on Bjorn's comments
about hw documentation in v1):
Reviewed-by: Andrew Halaney <[email protected]>
>
> >---
> > 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/[email protected]/
> >
> > used below patches for verification on next-20221114
> > https://lore.kernel.org/lkml/[email protected]/
> > https://lore.kernel.org/lkml/[email protected]/
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > 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%[email protected]/
> Signed-off-by: Shazad Hussain <[email protected]>
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 <[email protected]>
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%[email protected]/
> > Signed-off-by: Shazad Hussain <[email protected]>
>
> 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