On Mon, Apr 25, 2016 at 06:29:35PM -0700, Stephen Boyd wrote:
> On 04/25, Andy Gross wrote:
> > This patch converts the Qualcomm SCM firmware driver into a platform
> > driver.
> >
> > Signed-off-by: Andy Gross <andy.gross@xxxxxxxxxx>
> > ---
> > arch/arm64/Kconfig.platforms | 1 +
> > drivers/firmware/qcom_scm.c | 163 ++++++++++++++++++++++++++++++++++++++++---
> > 2 files changed, 155 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> > index efa77c1..6f0876f 100644
> > --- a/arch/arm64/Kconfig.platforms
> > +++ b/arch/arm64/Kconfig.platforms
> > @@ -76,6 +76,7 @@ config ARCH_MVEBU
> > config ARCH_QCOM
> > bool "Qualcomm Platforms"
> > select PINCTRL
> > + select QCOM_SCM
>
> So far we've left this selection up to the consumer drivers of
> the qcom_scm_*() APIs. Any reason why that's changing here? I
> don't see mention in the commit text.
We can leave it that way.
>
> > help
> > This enables support for the ARMv8 based Qualcomm chipsets.
> >
> > +
> > +/**
> > + * qcom_scm_is_available() - Checks if SCM is available
> > + */
> > +bool qcom_scm_is_available(void)
> > +{
> > + return !!__scm;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_is_available);
>
> What's the planned user of this? If we need it can we bury it
> inside the qcom_scm_*() functions?
Hmmmm doing a little searching, I don't see this being used anymore. I'll drop
it for now.
> > +
> > +static int qcom_scm_probe(struct platform_device *pdev)
> > +{
> [...]
> > +
> > + /* vote for max clk rate for highest performance */
> > + rate = clk_round_rate(scm->core_clk, INT_MAX);
> > + ret = clk_set_rate(scm->core_clk, rate);
>
> You can just do clk_set_rate(scm->core_clk, INT_MAX) and it will
> round internally for you and do the right thing.
I'll change this to do that.
> > + if (ret)
> > + return ret;
> > +
> > + __scm = scm;
> > + __scm->dev = &pdev->dev;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id qcom_scm_dt_match[] = {
> > + { .compatible = "qcom,scm-apq8064",},
> > + { .compatible = "qcom,scm-apq8084",},
> > + { .compatible = "qcom,scm-msm8916",},
> > + { .compatible = "qcom,scm-msm8974",},
> > + {},
>
> Nitpick: drop , here because it's always going to be the last
> entry.
will fix.
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
> > +
> > +static struct platform_driver qcom_scm_driver = {
> > + .driver = {
> > + .name = "qcom_scm",
> > + .of_match_table = qcom_scm_dt_match,
> > + },
> > + .probe = qcom_scm_probe,
> > +};
> > +
> > +builtin_platform_driver(qcom_scm_driver);
> > +
> > +static int __init qcom_scm_init(void)
> > +{
> > + struct device_node *np;
> > +
> > + np = of_find_node_by_name(NULL, "firmware");
> > + if (!np)
> > + return -ENODEV;
> > +
> > + return of_platform_populate(np, qcom_scm_dt_match, NULL, NULL);
> > +
>
> Weird newline and also we need an of_node_put() on the firmware
> node at the end of this function.
Ah thanks for catching that.
Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
