lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sat, 5 Nov 2022 01:16:02 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
Cc:     agross@...nel.org, andersson@...nel.org,
        konrad.dybcio@...ainline.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        marijn.suijten@...ainline.org, kernel@...labora.com,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>
Subject: Re: [PATCH 2/2] soc: qcom: Add Qualcomm Ramp Controller driver

On Fri, 4 Nov 2022 at 16:35, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com> wrote:
>
> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...ainline.org>
>
> The Ramp Controller is used to program the sequence ID for pulse
> swallowing, enable sequence and linking sequence IDs for the CPU
> cores on some Qualcomm SoCs.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...ainline.org>
> ---
>  drivers/soc/qcom/Kconfig           |   9 +
>  drivers/soc/qcom/Makefile          |   1 +
>  drivers/soc/qcom/ramp_controller.c | 330 +++++++++++++++++++++++++++++
>  3 files changed, 340 insertions(+)
>  create mode 100644 drivers/soc/qcom/ramp_controller.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 024e420f1bb7..1e681f98bad4 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -95,6 +95,15 @@ config QCOM_QMI_HELPERS
>         tristate
>         depends on NET
>
> +config QCOM_RAMP_CTRL
> +       tristate "Qualcomm Ramp Controller driver"
> +       depends on ARCH_QCOM
> +       help
> +         The Ramp Controller is used to program the sequence ID for pulse
> +         swallowing, enable sequence and linking sequence IDs for the
> +         CPU cores on some Qualcomm SoCs.
> +         Say y here to enable support for the ramp controller.

Generic question. regarding this controller. If it is supposed to work
close to DVCS, etc. Does ramp controller need any programming when
changing speed and/or APC voltage?
Is it necessary to turn ramp on and off during the runtime?

> +
>  config QCOM_RMTFS_MEM
>         tristate "Qualcomm Remote Filesystem memory driver"
>         depends on ARCH_QCOM
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index d66604aff2b0..6e02333c4080 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_QCOM_OCMEM)      += ocmem.o
>  obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
>  obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
>  qmi_helpers-y  += qmi_encdec.o qmi_interface.o
> +obj-$(CONFIG_QCOM_RAMP_CTRL)   += ramp_controller.o
>  obj-$(CONFIG_QCOM_RMTFS_MEM)   += rmtfs_mem.o
>  obj-$(CONFIG_QCOM_RPMH)                += qcom_rpmh.o
>  qcom_rpmh-y                    += rpmh-rsc.o
> diff --git a/drivers/soc/qcom/ramp_controller.c b/drivers/soc/qcom/ramp_controller.c
> new file mode 100644
> index 000000000000..e28679b545d1
> --- /dev/null
> +++ b/drivers/soc/qcom/ramp_controller.c
> @@ -0,0 +1,330 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm Ramp Controller driver
> + * Copyright (c) 2022, AngeloGioacchino Del Regno
> + *                     <angelogioacchino.delregno@...labora.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#define RC_UPDATE_EN           BIT(0)
> +#define RC_ROOT_EN             BIT(1)
> +
> +#define RC_REG_CFG_UPDATE      0x60
> + #define RC_CFG_UPDATE_EN      BIT(8)
> + #define RC_CFG_ACK            GENMASK(31, 16)
> +
> +#define RC_DCVS_CFG_SID                2
> +#define RC_LINK_SID            3
> +#define RC_LMH_SID             6
> +#define RC_DFS_SID             14
> +
> +#define RC_UPDATE_TIMEOUT_US   500
> +
> +/**
> + * struct qcom_ramp_controller_desc - SoC specific parameters
> + * @cfg_dfs_sid:      Dynamic Frequency Scaling SID configuration
> + * @cfg_link_sid:     Link SID configuration
> + * @cfg_lmh_sid:      Limits Management hardware SID configuration
> + * @cfg_ramp_pre_en:  Ramp Controller pre-enable sequence
> + * @cfg_ramp_en:      Ramp Controller enable sequence
> + * @cfg_ramp_post_en: Ramp Controller post-enable sequence
> + * @cfg_ramp_dis:     Ramp Controller disable sequence
> + * @cmd_reg:          Command register offset
> + * @num_dfs_sids:     Number of DFS SIDs (max 8)
> + * @num_link_sids:    Number of Link SIDs (max 3)
> + * @num_lmh_sids:     Number of LMh SIDs (max 8)
> + */
> +struct qcom_ramp_controller_desc {
> +       struct reg_sequence *cfg_dfs_sid;
> +       struct reg_sequence *cfg_link_sid;
> +       struct reg_sequence *cfg_lmh_sid;
> +       struct reg_sequence *cfg_ramp_pre_en;
> +       struct reg_sequence *cfg_ramp_en;
> +       struct reg_sequence *cfg_ramp_post_en;
> +       struct reg_sequence *cfg_ramp_dis;
> +       u8 cmd_reg;
> +       u8 num_dfs_sids;
> +       u8 num_link_sids;
> +       u8 num_lmh_sids;
> +};
> +
> +/**
> + * struct qcom_ramp_controller - Main driver structure
> + * @regmap: Regmap handle
> + * @desc:   SoC specific parameters
> + */
> +struct qcom_ramp_controller {
> +       struct regmap *regmap;
> +       const struct qcom_ramp_controller_desc *desc;
> +};
> +
> +/**
> + * rc_wait_for_update() - Wait for Ramp Controller root update
> + * @qrc: Main driver structure
> + *
> + * Return: Zero for success or negative number for failure
> + */
> +static int rc_wait_for_update(struct qcom_ramp_controller *qrc)
> +{
> +       const struct qcom_ramp_controller_desc *d = qrc->desc;
> +       struct regmap *r = qrc->regmap;
> +       u32 val;
> +       int ret;
> +
> +       ret = regmap_set_bits(r, d->cmd_reg, RC_ROOT_EN);
> +       if (ret)
> +               return ret;
> +
> +       return regmap_read_poll_timeout(r, d->cmd_reg, val, !(val & RC_UPDATE_EN),
> +                                       1, RC_UPDATE_TIMEOUT_US);
> +}
> +
> +/**
> + * rc_set_cfg_update() - Ramp Controller configuration update
> + * @qrc: Main driver structure
> + * @ce: Configuration entry to update
> + *
> + * Return: Zero for success or negative number for failure
> + */
> +static int rc_set_cfg_update(struct qcom_ramp_controller *qrc, u8 ce)
> +{
> +       const struct qcom_ramp_controller_desc *d = qrc->desc;
> +       struct regmap *r = qrc->regmap;
> +       u32 ack, val;
> +       int ret;
> +
> +       /* The ack bit is between bits 16-31 of RC_REG_CFG_UPDATE */
> +       ack = FIELD_PREP(RC_CFG_ACK, BIT(ce));
> +
> +       /* Write the configuration type first... */
> +       ret = regmap_set_bits(r, d->cmd_reg + RC_REG_CFG_UPDATE, ce);
> +       if (ret)
> +               return ret;
> +
> +       /* ...and after that, enable the update bit to sync the changes */
> +       ret = regmap_set_bits(r, d->cmd_reg + RC_REG_CFG_UPDATE, RC_CFG_UPDATE_EN);
> +       if (ret)
> +               return ret;
> +
> +       /* Wait for the changes to go through */
> +       ret = regmap_read_poll_timeout(r, d->cmd_reg + RC_REG_CFG_UPDATE, val,
> +                                      val & ack, 1, RC_UPDATE_TIMEOUT_US);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Configuration update success! The CFG_UPDATE register will not be
> +        * cleared automatically upon applying the configuration, so we have
> +        * to do that manually in order to leave the ramp controller in a
> +        * predictable and clean state.
> +        */
> +       ret = regmap_write(r, d->cmd_reg + RC_REG_CFG_UPDATE, 0);
> +       if (ret)
> +               return ret;
> +
> +       /* Wait for the update bit cleared ack */
> +       return regmap_read_poll_timeout(r, d->cmd_reg + RC_REG_CFG_UPDATE,
> +                                       val, !(val & RC_CFG_ACK), 1,
> +                                       RC_UPDATE_TIMEOUT_US);
> +}
> +
> +/**
> + * rc_write_cfg - Send configuration sequence
> + * @qrc: Main driver structure
> + * @seq: Register sequence to send before asking for update
> + * @ce: Configuration SID
> + * @nsids: Total number of SIDs
> + *
> + * Returns: Zero for success or negative number for error
> + */
> +static int rc_write_cfg(struct qcom_ramp_controller *qrc, struct reg_sequence *seq,
> +                       u16 ce, u8 nsids)
> +{
> +       int ret;
> +       u8 i;
> +
> +       /* Check if, and wait until the ramp controller is ready */
> +       ret = rc_wait_for_update(qrc);
> +       if (ret)
> +               return ret;
> +
> +       /* Write the sequence */
> +       ret = regmap_multi_reg_write(qrc->regmap, seq, nsids);
> +       if (ret)
> +               return ret;
> +
> +       /* Pull the trigger: do config update starting from the last sid */
> +       for (i = 0; i < nsids; i++) {
> +               ret = rc_set_cfg_update(qrc, (u8)ce - i);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * rc_ramp_ctrl_enable() - Enable Ramp up/down Control
> + * @qrc: Main driver structure
> + *
> + * Return: Zero for success or negative number for error
> + */
> +static int rc_ramp_ctrl_enable(struct qcom_ramp_controller *qrc)
> +{
> +       const struct qcom_ramp_controller_desc *d = qrc->desc;
> +       int ret;
> +
> +       ret = rc_write_cfg(qrc, d->cfg_ramp_pre_en, RC_DCVS_CFG_SID, 1);
> +       if (ret)
> +               return ret;
> +
> +       ret = rc_write_cfg(qrc, d->cfg_ramp_en, RC_DCVS_CFG_SID, 1);
> +       if (ret)
> +               return ret;
> +
> +       return rc_write_cfg(qrc, d->cfg_ramp_post_en, RC_DCVS_CFG_SID, 1);
> +}
> +
> +/**
> + * qcom_ramp_controller_start() - Initialize and start the ramp controller
> + * @qrc: Main driver structure
> + *
> + * The Ramp Controller needs to be initialized by programming the relevant
> + * registers with SoC-specific configuration: once programming is done,
> + * the hardware will take care of the rest (no further handling required).
> + *
> + * Return: Zero for success or negative number for error
> + */
> +static int qcom_ramp_controller_start(struct qcom_ramp_controller *qrc)
> +{
> +       const struct qcom_ramp_controller_desc *d = qrc->desc;
> +       int ret;
> +
> +       /* Program LMH, DFS, Link SIDs */
> +       ret = rc_write_cfg(qrc, d->cfg_lmh_sid, RC_LMH_SID, d->num_lmh_sids);
> +       if (ret)
> +               return ret;
> +
> +       ret = rc_write_cfg(qrc, d->cfg_dfs_sid, RC_DFS_SID, d->num_dfs_sids);
> +       if (ret)
> +               return ret;
> +
> +       ret = rc_write_cfg(qrc, d->cfg_link_sid, RC_LINK_SID, d->num_link_sids);
> +       if (ret)
> +               return ret;
> +
> +       /* Everything is ready! Enable the ramp up/down control */
> +       return rc_ramp_ctrl_enable(qrc);
> +}
> +
> +static const struct regmap_config qrc_regmap_config = {
> +       .reg_bits = 32,
> +       .reg_stride = 4,
> +       .val_bits = 32,
> +       .max_register = 0x68,
> +       .fast_io = true,
> +};
> +
> +static const struct qcom_ramp_controller_desc msm8976_rc_cfg = {
> +       .cfg_dfs_sid = (struct reg_sequence[]) {
> +               { 0x10, 0xfefebff7 },
> +               { 0x14, 0xfdff7fef },
> +               { 0x18, 0xfbffdefb },
> +               { 0x1c, 0xb69b5555 },
> +               { 0x20, 0x24929249 },
> +               { 0x24, 0x49241112 },
> +               { 0x28, 0x11112111 },
> +               { 0x2c, 0x8102 },

Does the vendor kernel contain register names for these items? If so,
it might be good to include them here.

> +       },
> +       .cfg_link_sid = (struct reg_sequence[]) {
> +               { 0x40, 0xfc987 },
> +       },
> +       .cfg_lmh_sid = (struct reg_sequence[]) {
> +               { 0x30, 0x77706db },
> +               { 0x34, 0x5550249 },
> +               { 0x38, 0x111 },
> +       },
> +       .cfg_ramp_pre_en = (struct reg_sequence[]) {
> +               { 0x50, 0x800 },
> +       },
> +       .cfg_ramp_en = (struct reg_sequence[]) {
> +               { 0x50, 0xc00 },
> +       },
> +       .cfg_ramp_post_en = (struct reg_sequence[]) {
> +               { 0x50, 0x400 },
> +       },
> +       .cfg_ramp_dis = (struct reg_sequence[]) {
> +               { 0x50, 0x0 },
> +       },
> +       .cmd_reg = 0x0,
> +
> +       .num_dfs_sids = 8,
> +       .num_lmh_sids = 3,
> +       .num_link_sids = 1,

I see. It might be better to have this as ARRAY_SIZE of the corresponding item.

> +};
> +
> +static int qcom_ramp_controller_probe(struct platform_device *pdev)
> +{
> +       struct qcom_ramp_controller *qrc;
> +       void __iomem *base;
> +
> +       base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       qrc = devm_kmalloc(&pdev->dev, sizeof(*qrc), GFP_KERNEL);
> +       if (!qrc)
> +               return -ENOMEM;
> +
> +       qrc->desc = device_get_match_data(&pdev->dev);
> +       if (!qrc)
> +               return -EINVAL;
> +
> +       qrc->regmap = devm_regmap_init_mmio(&pdev->dev, base, &qrc_regmap_config);
> +       if (IS_ERR(qrc->regmap))
> +               return PTR_ERR(qrc->regmap);
> +
> +       platform_set_drvdata(pdev, qrc);
> +
> +       return qcom_ramp_controller_start(qrc);
> +}
> +
> +static int qcom_ramp_controller_remove(struct platform_device *pdev)
> +{
> +       struct qcom_ramp_controller *qrc = platform_get_drvdata(pdev);
> +
> +       return rc_write_cfg(qrc, qrc->desc->cfg_ramp_dis, RC_DCVS_CFG_SID, 1);
> +}
> +
> +static const struct of_device_id qcom_ramp_controller_match_table[] = {
> +       { .compatible = "qcom,msm8976-ramp-controller", .data = &msm8976_rc_cfg },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_ramp_controller_match_table);
> +
> +static struct platform_driver qcom_ramp_controller_driver = {
> +       .driver = {
> +               .name = "qcom-ramp-controller",
> +               .of_match_table = qcom_ramp_controller_match_table,
> +               .suppress_bind_attrs = true,
> +       },
> +       .probe  = qcom_ramp_controller_probe,
> +       .remove = qcom_ramp_controller_remove,
> +};
> +
> +static int __init qcom_ramp_controller_init(void)
> +{
> +       return platform_driver_register(&qcom_ramp_controller_driver);
> +}
> +arch_initcall(qcom_ramp_controller_init);
> +
> +MODULE_AUTHOR("AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>");
> +MODULE_DESCRIPTION("Qualcomm Ramp Controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.37.2
>


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ