| 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
| ||
|
Date: Fri, 4 Nov 2022 15:06:34 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
agross@...nel.org
Cc: 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
Il 04/11/22 15:04, Krzysztof Kozlowski ha scritto:
> On 04/11/2022 09:35, AngeloGioacchino Del Regno 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
>
> I propose:
> depends on ARCH_QCOM && ARM || COMPILE_TEST
>
> I don't think it is used on ARM64 SoCs, so let's make life of distros
> easier.
>
Agreed.
>> + 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.
>> +
>> 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)
>
> Drop spaces before #define
>
>> +
>> +#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;
>
> I didn't check much, but can these be pointers to const?
>
Yeah, const is the way. Will do.
>> + 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;
>> +};
>> +
>
> (...)
>
>> +
>> +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);
>
> Does it really have to be arch initcall? Cannot be module platform driver?
>
Cannot be platform driver. This has to initialize as early as possible, or
booting will be unstable in some cases (big cluster enabled).
Cheers!
Angelo
Powered by blists - more mailing lists