| Message ID | 20230815085205.9868-2-quic_luoj@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | add clock controller of qca8386/qca8084 | expand |
On Tue, Aug 15, 2023 at 04:52:02PM +0800, Luo Jie wrote: > Add the clk_branch2_mdio_ops for supporting clock controller > where the hardware register is accessed by MDIO bus, and the > spin clock can't be used because of sleep during the MDIO spin clock? I believe you're trying to say that the underlying access to the MDIO bus can not be done in non-sleepable context and we can therefor not use enable/disable to operate it? > operation. > > The clock is enabled by the .prepare instead of .enable when > the clk_branch2_mdio_ops is used. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > drivers/clk/qcom/clk-branch.c | 7 +++++++ > drivers/clk/qcom/clk-branch.h | 1 + > 2 files changed, 8 insertions(+) > > diff --git a/drivers/clk/qcom/clk-branch.c b/drivers/clk/qcom/clk-branch.c > index fc4735f74f0f..5e08c026ca4a 100644 > --- a/drivers/clk/qcom/clk-branch.c > +++ b/drivers/clk/qcom/clk-branch.c > @@ -153,3 +153,10 @@ const struct clk_ops clk_branch_simple_ops = { > .is_enabled = clk_is_enabled_regmap, > }; > EXPORT_SYMBOL_GPL(clk_branch_simple_ops); > + > +const struct clk_ops clk_branch2_mdio_ops = { > + .prepare = clk_branch2_enable, > + .unprepare = clk_branch2_disable, I see none of the clocks specify halt_check, which would imply that these two calls just turns into clk_enable_regmap() and clk_disable_regmap(). So, isn't this then equivalent to clk_branch_simple_ops? Regards, Bjorn > + .is_prepared = clk_is_enabled_regmap, > +}; > +EXPORT_SYMBOL_GPL(clk_branch2_mdio_ops); > diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h > index 0cf800b9d08d..4b006e8eec5e 100644 > --- a/drivers/clk/qcom/clk-branch.h > +++ b/drivers/clk/qcom/clk-branch.h > @@ -85,6 +85,7 @@ extern const struct clk_ops clk_branch_ops; > extern const struct clk_ops clk_branch2_ops; > extern const struct clk_ops clk_branch_simple_ops; > extern const struct clk_ops clk_branch2_aon_ops; > +extern const struct clk_ops clk_branch2_mdio_ops; > > #define to_clk_branch(_hw) \ > container_of(to_clk_regmap(_hw), struct clk_branch, clkr) > -- > 2.17.1 >
On 8/18/2023 11:29 AM, Bjorn Andersson wrote: > On Tue, Aug 15, 2023 at 04:52:02PM +0800, Luo Jie wrote: >> Add the clk_branch2_mdio_ops for supporting clock controller >> where the hardware register is accessed by MDIO bus, and the >> spin clock can't be used because of sleep during the MDIO > > spin clock? > > I believe you're trying to say that the underlying access to the MDIO > bus can not be done in non-sleepable context and we can therefor not use > enable/disable to operate it? Hi Bjorn, Thanks for the review comments. yes, the MDIO operation can't be done in non-sleepable context, and enable/disable clock is using spin lock, so i enable the clock in .prepare ops. will fix this typo"spin clock" to "spin lock" > >> operation. >> >> The clock is enabled by the .prepare instead of .enable when >> the clk_branch2_mdio_ops is used. >> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com> >> --- >> drivers/clk/qcom/clk-branch.c | 7 +++++++ >> drivers/clk/qcom/clk-branch.h | 1 + >> 2 files changed, 8 insertions(+) >> >> diff --git a/drivers/clk/qcom/clk-branch.c b/drivers/clk/qcom/clk-branch.c >> index fc4735f74f0f..5e08c026ca4a 100644 >> --- a/drivers/clk/qcom/clk-branch.c >> +++ b/drivers/clk/qcom/clk-branch.c >> @@ -153,3 +153,10 @@ const struct clk_ops clk_branch_simple_ops = { >> .is_enabled = clk_is_enabled_regmap, >> }; >> EXPORT_SYMBOL_GPL(clk_branch_simple_ops); >> + >> +const struct clk_ops clk_branch2_mdio_ops = { >> + .prepare = clk_branch2_enable, >> + .unprepare = clk_branch2_disable, > > I see none of the clocks specify halt_check, which would imply that > these two calls just turns into clk_enable_regmap() and > clk_disable_regmap(). > > So, isn't this then equivalent to clk_branch_simple_ops? > > Regards, > Bjorn > Thanks Bjorn for pointing this, i will add the the halt_check in the next patch set, halt_check is applicable. >> + .is_prepared = clk_is_enabled_regmap, >> +}; >> +EXPORT_SYMBOL_GPL(clk_branch2_mdio_ops); >> diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h >> index 0cf800b9d08d..4b006e8eec5e 100644 >> --- a/drivers/clk/qcom/clk-branch.h >> +++ b/drivers/clk/qcom/clk-branch.h >> @@ -85,6 +85,7 @@ extern const struct clk_ops clk_branch_ops; >> extern const struct clk_ops clk_branch2_ops; >> extern const struct clk_ops clk_branch_simple_ops; >> extern const struct clk_ops clk_branch2_aon_ops; >> +extern const struct clk_ops clk_branch2_mdio_ops; >> >> #define to_clk_branch(_hw) \ >> container_of(to_clk_regmap(_hw), struct clk_branch, clkr) >> -- >> 2.17.1 >>
diff --git a/drivers/clk/qcom/clk-branch.c b/drivers/clk/qcom/clk-branch.c index fc4735f74f0f..5e08c026ca4a 100644 --- a/drivers/clk/qcom/clk-branch.c +++ b/drivers/clk/qcom/clk-branch.c @@ -153,3 +153,10 @@ const struct clk_ops clk_branch_simple_ops = { .is_enabled = clk_is_enabled_regmap, }; EXPORT_SYMBOL_GPL(clk_branch_simple_ops); + +const struct clk_ops clk_branch2_mdio_ops = { + .prepare = clk_branch2_enable, + .unprepare = clk_branch2_disable, + .is_prepared = clk_is_enabled_regmap, +}; +EXPORT_SYMBOL_GPL(clk_branch2_mdio_ops); diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h index 0cf800b9d08d..4b006e8eec5e 100644 --- a/drivers/clk/qcom/clk-branch.h +++ b/drivers/clk/qcom/clk-branch.h @@ -85,6 +85,7 @@ extern const struct clk_ops clk_branch_ops; extern const struct clk_ops clk_branch2_ops; extern const struct clk_ops clk_branch_simple_ops; extern const struct clk_ops clk_branch2_aon_ops; +extern const struct clk_ops clk_branch2_mdio_ops; #define to_clk_branch(_hw) \ container_of(to_clk_regmap(_hw), struct clk_branch, clkr)
Add the clk_branch2_mdio_ops for supporting clock controller where the hardware register is accessed by MDIO bus, and the spin clock can't be used because of sleep during the MDIO operation. The clock is enabled by the .prepare instead of .enable when the clk_branch2_mdio_ops is used. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- drivers/clk/qcom/clk-branch.c | 7 +++++++ drivers/clk/qcom/clk-branch.h | 1 + 2 files changed, 8 insertions(+)