[3/7] soc: qcom: add QCOM PBS driver

Message ID 20230621185949.2068-4-quic_amelende@quicinc.com
State New
Headers
Series Add support for LUT PPG |

Commit Message

Anjelique Melendez June 21, 2023, 6:59 p.m. UTC
  Add the Qualcomm PBS (Programmable Boot Sequencer) driver. The QCOM PBS
driver supports configuring software PBS trigger events through PBS RAM
on Qualcomm Technologies, Inc (QTI) PMICs.

Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
 drivers/soc/qcom/Kconfig          |   9 +
 drivers/soc/qcom/Makefile         |   1 +
 drivers/soc/qcom/qcom-pbs.c       | 343 ++++++++++++++++++++++++++++++
 include/linux/soc/qcom/qcom-pbs.h |  36 ++++
 4 files changed, 389 insertions(+)
 create mode 100644 drivers/soc/qcom/qcom-pbs.c
 create mode 100644 include/linux/soc/qcom/qcom-pbs.h
  

Comments

Krzysztof Kozlowski June 24, 2023, 9:55 a.m. UTC | #1
On 21/06/2023 20:59, Anjelique Melendez wrote:
> Add the Qualcomm PBS (Programmable Boot Sequencer) driver. The QCOM PBS
> driver supports configuring software PBS trigger events through PBS RAM
> on Qualcomm Technologies, Inc (QTI) PMICs.
> 
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
>  drivers/soc/qcom/Kconfig          |   9 +
>  drivers/soc/qcom/Makefile         |   1 +
>  drivers/soc/qcom/qcom-pbs.c       | 343 ++++++++++++++++++++++++++++++
>  include/linux/soc/qcom/qcom-pbs.h |  36 ++++
>  4 files changed, 389 insertions(+)
>  create mode 100644 drivers/soc/qcom/qcom-pbs.c
>  create mode 100644 include/linux/soc/qcom/qcom-pbs.h
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index a491718f8064..226b668f4690 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -260,6 +260,15 @@ config QCOM_APR
>  	  used by audio driver to configure QDSP6
>  	  ASM, ADM and AFE modules.
>  
> +config QCOM_PBS
> +	tristate "PBS trigger support for Qualcomm PMIC"
> +	depends on SPMI
> +	help
> +	  This driver supports configuring software programmable boot sequencer (PBS)
> +	  trigger event through PBS RAM on Qualcomm Technologies, Inc. PMICs.
> +	  This module provides the APIs to the client drivers that wants to send the
> +	  PBS trigger event to the PBS RAM.
> +
>  config QCOM_ICC_BWMON
>  	tristate "QCOM Interconnect Bandwidth Monitor driver"
>  	depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 0f43a88b4894..4e154af3877a 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -31,5 +31,6 @@ obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
>  obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
>  obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
>  obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=	kryo-l2-accessors.o
> +obj-$(CONFIG_QCOM_PBS) += qcom-pbs.o
>  obj-$(CONFIG_QCOM_ICC_BWMON)	+= icc-bwmon.o
>  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)	+= ice.o
> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
> new file mode 100644
> index 000000000000..4a2bb7ff8031
> --- /dev/null
> +++ b/drivers/soc/qcom/qcom-pbs.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define pr_fmt(fmt)	"PBS: %s: " fmt, __func__
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/spmi.h>
> +#include <linux/soc/qcom/qcom-pbs.h>
> +
> +#define PBS_CLIENT_TRIG_CTL		0x42
> +#define PBS_CLIENT_SW_TRIG_BIT		BIT(7)
> +#define PBS_CLIENT_SCRATCH1		0x50
> +#define PBS_CLIENT_SCRATCH2		0x51
> +
> +static LIST_HEAD(pbs_dev_list);
> +static DEFINE_MUTEX(pbs_list_lock);

No file-scope variables. Drop both. You don't even need it.

> +
> +struct pbs_dev {
> +	struct device		*dev;
> +	struct device_node	*dev_node;
> +	struct regmap		*regmap;
> +	struct mutex		lock;
> +	struct list_head	link;
> +
> +	u32			base;
> +};
> +
> +static int qcom_pbs_read(struct pbs_dev *pbs, u32 address, u8 *val)
> +{
> +	int ret;
> +
> +	address += pbs->base;
> +	ret = regmap_bulk_read(pbs->regmap, address, val, 1);
> +	if (ret)
> +		pr_err("Failed to read address=%#x sid=%#x ret=%d\n",

dev_err

> +			address, to_spmi_device(pbs->dev->parent)->usid, ret);
> +
> +	return ret;
> +}
> +
> +static int qcom_pbs_write(struct pbs_dev *pbs, u16 address, u8 val)
> +{
> +	int ret;
> +
> +	address += pbs->base;
> +	ret = regmap_bulk_write(pbs->regmap, address, &val, 1);
> +	if (ret < 0)
> +		pr_err("Failed to write address=%#x sid=%#x ret=%d\n",
> +			  address, to_spmi_device(pbs->dev->parent)->usid, ret);
> +	else
> +		pr_debug("Wrote %#x to addr %#x\n", val, address);

No, there is regmap debug for this. Drop such debug statements from the
driver.

Actually the error print is also wrong, IMO.

> +
> +	return ret;
> +}
> +
> +static int qcom_pbs_masked_write(struct pbs_dev *pbs, u16 address, u8 mask, u8 val)
> +{
> +	int ret;
> +
> +	address += pbs->base;
> +	ret = regmap_update_bits(pbs->regmap, address, mask, val);
> +	if (ret < 0)
> +		pr_err("Failed to write address=%#x ret=%d\n", address, ret);
> +	else
> +		pr_debug("Wrote %#x to addr %#x\n", val, address);

Drop

> +
> +	return ret;
> +}
> +
> +static int qcom_pbs_wait_for_ack(struct pbs_dev *pbs, u8 bit_pos)
> +{
> +	u16 retries = 2000, delay = 1000;
> +	int ret;
> +	u8 val;
> +
> +	while (retries--) {
> +		ret = qcom_pbs_read(pbs, PBS_CLIENT_SCRATCH2, &val);
> +		if (ret < 0)
> +			return ret;
> +
> +		if (val == 0xFF) {
> +			/* PBS error - clear SCRATCH2 register */
> +			ret = qcom_pbs_write(pbs, PBS_CLIENT_SCRATCH2, 0);
> +			if (ret < 0)
> +				return ret;
> +
> +			pr_err("NACK from PBS for bit %u\n", bit_pos);
> +			return -EINVAL;
> +		}
> +
> +		if (val & BIT(bit_pos)) {
> +			pr_debug("PBS sequence for bit %u executed!\n", bit_pos);

dev_dbg

> +			break;
> +		}
> +
> +		usleep_range(delay, delay + 100);
> +	}
> +
> +	if (!retries) {
> +		pr_err("Timeout for PBS ACK/NACK for bit %u\n", bit_pos);

dev_err
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * qcom_pbs_trigger_single_event() - Trigger PBS sequence without using bitmap.
> + * @pbs: Pointer to PBS device
> + *
> + * This function is used to trigger the PBS that is hooked on the
> + * SW_TRIGGER directly in PBS client.
> + *
> + * Return: 0 on success, < 0 on failure
> + */
> +int qcom_pbs_trigger_single_event(struct pbs_dev *pbs)
> +{
> +	int ret = 0;
> +
> +	if (IS_ERR_OR_NULL(pbs))
> +		return -EINVAL;
> +
> +	mutex_lock(&pbs->lock);
> +	ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_TRIG_CTL, PBS_CLIENT_SW_TRIG_BIT,
> +				PBS_CLIENT_SW_TRIG_BIT);
> +	if (ret < 0)
> +		pr_err("Failed to write register %x ret=%d\n", PBS_CLIENT_TRIG_CTL, ret);

dev_* everywhere

> +	mutex_unlock(&pbs->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_pbs_trigger_single_event);
> +

...

> +/**
> + * get_pbs_client_device() - Get the PBS device used by client
> + * @dev: Client device
> + *
> + * This function is used to get the PBS device that is being
> + * used by the client.
> + *
> + * Returns: pbs_dev on success, ERR_PTR on failure
> + */
> +struct pbs_dev *get_pbs_client_device(struct device *dev)
> +{
> +	struct device_node *pbs_dev_node;
> +	struct pbs_dev *pbs;
> +
> +	pbs_dev_node = of_parse_phandle(dev->of_node, "qcom,pbs-client", 0);
> +	if (!pbs_dev_node) {
> +		pr_err("Missing qcom,pbs-client property\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	mutex_lock(&pbs_list_lock);
> +	list_for_each_entry(pbs, &pbs_dev_list, link) {

It does not make sense. You have the reference to the device, so you
have the pbs (via container_of). Don't add some
global-list-lookup-functions.

Look for example at Abel Vesa's ICE patchset.

> +		if (pbs_dev_node == pbs->dev_node) {
> +			of_node_put(pbs_dev_node);
> +			mutex_unlock(&pbs_list_lock);
> +			return pbs;
> +		}
> +	}
> +	mutex_unlock(&pbs_list_lock);

Where is device_link handling?

> +
> +	pr_debug("Unable to find PBS dev_node\n");
> +	of_node_put(pbs_dev_node);
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL(get_pbs_client_device);
> +
> +static int qcom_pbs_probe(struct platform_device *pdev)
> +{
> +	struct pbs_dev *pbs;
> +	u32 val;
> +	int ret;
> +
> +	pbs = devm_kzalloc(&pdev->dev, sizeof(*pbs), GFP_KERNEL);
> +	if (!pbs)
> +		return -ENOMEM;
> +
> +	pbs->dev = &pdev->dev;
> +	pbs->dev_node = pdev->dev.of_node;

Why do you need to store it?

> +	pbs->regmap = dev_get_regmap(pbs->dev->parent, NULL);
> +	if (!pbs->regmap) {
> +		dev_err(pbs->dev, "Couldn't get parent's regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = device_property_read_u32(pbs->dev, "reg", &val);
> +	if (ret < 0) {
> +		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
> +		return ret;
> +	}
> +
> +	pbs->base = val;
> +	mutex_init(&pbs->lock);
> +
> +	platform_set_drvdata(pdev, pbs);
> +
> +	mutex_lock(&pbs_list_lock);
> +	list_add(&pbs->link, &pbs_dev_list);
> +	mutex_unlock(&pbs_list_lock);
> +
> +	return 0;
> +}
> +
> +static int qcom_pbs_remove(struct platform_device *pdev)
> +{
> +	struct pbs_dev *pbs = platform_get_drvdata(pdev);
> +
> +	mutex_lock(&pbs_list_lock);
> +	list_del(&pbs->link);
> +	mutex_unlock(&pbs_list_lock);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id qcom_pbs_match_table[] = {
> +	{ .compatible = "qcom,pbs" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_pbs_match_table);
> +
> +static struct platform_driver qcom_pbs_driver = {
> +	.driver = {
> +		.name		= "qcom-pbs",
> +		.of_match_table	= qcom_pbs_match_table,
> +	},
> +	.probe = qcom_pbs_probe,
> +	.remove = qcom_pbs_remove,
> +};
> +module_platform_driver(qcom_pbs_driver)
> +
> +MODULE_DESCRIPTION("QCOM PBS DRIVER");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:qcom-pbs");

Drop alias. Not needed. If you need it, you have missing ID table.

> diff --git a/include/linux/soc/qcom/qcom-pbs.h b/include/linux/soc/qcom/qcom-pbs.h
> new file mode 100644
> index 000000000000..4b065951686a
> --- /dev/null



Best regards,
Krzysztof
  
Krzysztof Kozlowski June 24, 2023, 10:01 a.m. UTC | #2
On 24/06/2023 11:55, Krzysztof Kozlowski wrote:
>> +/**
>> + * get_pbs_client_device() - Get the PBS device used by client
>> + * @dev: Client device
>> + *
>> + * This function is used to get the PBS device that is being
>> + * used by the client.
>> + *
>> + * Returns: pbs_dev on success, ERR_PTR on failure
>> + */
>> +struct pbs_dev *get_pbs_client_device(struct device *dev)
>> +{
>> +	struct device_node *pbs_dev_node;
>> +	struct pbs_dev *pbs;
>> +
>> +	pbs_dev_node = of_parse_phandle(dev->of_node, "qcom,pbs-client", 0);
>> +	if (!pbs_dev_node) {
>> +		pr_err("Missing qcom,pbs-client property\n");
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	mutex_lock(&pbs_list_lock);
>> +	list_for_each_entry(pbs, &pbs_dev_list, link) {
> 
> It does not make sense. You have the reference to the device, so you
> have the pbs (via container_of). Don't add some
> global-list-lookup-functions.
> 
> Look for example at Abel Vesa's ICE patchset.

To be specific:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/qcom/ice.c?h=v6.4-rc7#n293

(+CC Abel)

Best regards,
Krzysztof
  
Anjelique Melendez June 29, 2023, 12:48 a.m. UTC | #3
On 6/24/2023 2:55 AM, Krzysztof Kozlowski wrote:
> On 21/06/2023 20:59, Anjelique Melendez wrote:
>> Add the Qualcomm PBS (Programmable Boot Sequencer) driver. The QCOM PBS
>> driver supports configuring software PBS trigger events through PBS RAM
>> on Qualcomm Technologies, Inc (QTI) PMICs.
>>
>> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
>> ---
>>  drivers/soc/qcom/Kconfig          |   9 +
>>  drivers/soc/qcom/Makefile         |   1 +
>>  drivers/soc/qcom/qcom-pbs.c       | 343 ++++++++++++++++++++++++++++++
>>  include/linux/soc/qcom/qcom-pbs.h |  36 ++++
>>  4 files changed, 389 insertions(+)
>>  create mode 100644 drivers/soc/qcom/qcom-pbs.c
>>  create mode 100644 include/linux/soc/qcom/qcom-pbs.h
>>
>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>> index a491718f8064..226b668f4690 100644
>> --- a/drivers/soc/qcom/Kconfig
>> +++ b/drivers/soc/qcom/Kconfig
>> @@ -260,6 +260,15 @@ config QCOM_APR
>>  	  used by audio driver to configure QDSP6
>>  	  ASM, ADM and AFE modules.
>>  
>> +config QCOM_PBS
>> +	tristate "PBS trigger support for Qualcomm PMIC"
>> +	depends on SPMI
>> +	help
>> +	  This driver supports configuring software programmable boot sequencer (PBS)
>> +	  trigger event through PBS RAM on Qualcomm Technologies, Inc. PMICs.
>> +	  This module provides the APIs to the client drivers that wants to send the
>> +	  PBS trigger event to the PBS RAM.
>> +
>>  config QCOM_ICC_BWMON
>>  	tristate "QCOM Interconnect Bandwidth Monitor driver"
>>  	depends on ARCH_QCOM || COMPILE_TEST
>> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
>> index 0f43a88b4894..4e154af3877a 100644
>> --- a/drivers/soc/qcom/Makefile
>> +++ b/drivers/soc/qcom/Makefile
>> @@ -31,5 +31,6 @@ obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
>>  obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
>>  obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
>>  obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=	kryo-l2-accessors.o
>> +obj-$(CONFIG_QCOM_PBS) += qcom-pbs.o
>>  obj-$(CONFIG_QCOM_ICC_BWMON)	+= icc-bwmon.o
>>  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)	+= ice.o
>> diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
>> new file mode 100644
>> index 000000000000..4a2bb7ff8031
>> --- /dev/null
>> +++ b/drivers/soc/qcom/qcom-pbs.c
>> @@ -0,0 +1,343 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#define pr_fmt(fmt)	"PBS: %s: " fmt, __func__
>> +
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spmi.h>
>> +#include <linux/soc/qcom/qcom-pbs.h>
>> +
>> +#define PBS_CLIENT_TRIG_CTL		0x42
>> +#define PBS_CLIENT_SW_TRIG_BIT		BIT(7)
>> +#define PBS_CLIENT_SCRATCH1		0x50
>> +#define PBS_CLIENT_SCRATCH2		0x51
>> +
>> +static LIST_HEAD(pbs_dev_list);
>> +static DEFINE_MUTEX(pbs_list_lock);
> 
> No file-scope variables. Drop both. You don't even need it.
Ack
> 
>> +
>> +struct pbs_dev {
>> +	struct device		*dev;
>> +	struct device_node	*dev_node;
>> +	struct regmap		*regmap;
>> +	struct mutex		lock;
>> +	struct list_head	link;
>> +
>> +	u32			base;
>> +};
>> +
>> +static int qcom_pbs_read(struct pbs_dev *pbs, u32 address, u8 *val)
>> +{
>> +	int ret;
>> +
>> +	address += pbs->base;
>> +	ret = regmap_bulk_read(pbs->regmap, address, val, 1);
>> +	if (ret)
>> +		pr_err("Failed to read address=%#x sid=%#x ret=%d\n",
> 
> dev_err
Ack
> 
>> +			address, to_spmi_device(pbs->dev->parent)->usid, ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static int qcom_pbs_write(struct pbs_dev *pbs, u16 address, u8 val)
>> +{
>> +	int ret;
>> +
>> +	address += pbs->base;
>> +	ret = regmap_bulk_write(pbs->regmap, address, &val, 1);
>> +	if (ret < 0)
>> +		pr_err("Failed to write address=%#x sid=%#x ret=%d\n",
>> +			  address, to_spmi_device(pbs->dev->parent)->usid, ret);
>> +	else
>> +		pr_debug("Wrote %#x to addr %#x\n", val, address);
> 
> No, there is regmap debug for this. Drop such debug statements from the
> driver.
Ack
> 
> Actually the error print is also wrong, IMO> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int qcom_pbs_masked_write(struct pbs_dev *pbs, u16 address, u8 mask, u8 val)
>> +{
>> +	int ret;
>> +
>> +	address += pbs->base;
>> +	ret = regmap_update_bits(pbs->regmap, address, mask, val);
>> +	if (ret < 0)
>> +		pr_err("Failed to write address=%#x ret=%d\n", address, ret);
>> +	else
>> +		pr_debug("Wrote %#x to addr %#x\n", val, address);
> 
> Drop
Ack
> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int qcom_pbs_wait_for_ack(struct pbs_dev *pbs, u8 bit_pos)
>> +{
>> +	u16 retries = 2000, delay = 1000;
>> +	int ret;
>> +	u8 val;
>> +
>> +	while (retries--) {
>> +		ret = qcom_pbs_read(pbs, PBS_CLIENT_SCRATCH2, &val);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		if (val == 0xFF) {
>> +			/* PBS error - clear SCRATCH2 register */
>> +			ret = qcom_pbs_write(pbs, PBS_CLIENT_SCRATCH2, 0);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			pr_err("NACK from PBS for bit %u\n", bit_pos);
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (val & BIT(bit_pos)) {
>> +			pr_debug("PBS sequence for bit %u executed!\n", bit_pos);
> 
> dev_dbg
Ack
> 
>> +			break;
>> +		}
>> +
>> +		usleep_range(delay, delay + 100);
>> +	}
>> +
>> +	if (!retries) {
>> +		pr_err("Timeout for PBS ACK/NACK for bit %u\n", bit_pos);
> 
> dev_err
Ack
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * qcom_pbs_trigger_single_event() - Trigger PBS sequence without using bitmap.
>> + * @pbs: Pointer to PBS device
>> + *
>> + * This function is used to trigger the PBS that is hooked on the
>> + * SW_TRIGGER directly in PBS client.
>> + *
>> + * Return: 0 on success, < 0 on failure
>> + */
>> +int qcom_pbs_trigger_single_event(struct pbs_dev *pbs)
>> +{
>> +	int ret = 0;
>> +
>> +	if (IS_ERR_OR_NULL(pbs))
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&pbs->lock);
>> +	ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_TRIG_CTL, PBS_CLIENT_SW_TRIG_BIT,
>> +				PBS_CLIENT_SW_TRIG_BIT);
>> +	if (ret < 0)
>> +		pr_err("Failed to write register %x ret=%d\n", PBS_CLIENT_TRIG_CTL, ret);
> 
> dev_* everywhere
Ack
> 
>> +	mutex_unlock(&pbs->lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(qcom_pbs_trigger_single_event);
>> +
> 
> ...
qcom_pbs_trigger_single_event() is used in our downstream haptics driver,
will remove for now and add it back when haptics driver get upstreamed
> 
>> +/**
>> + * get_pbs_client_device() - Get the PBS device used by client
>> + * @dev: Client device
>> + *
>> + * This function is used to get the PBS device that is being
>> + * used by the client.
>> + *
>> + * Returns: pbs_dev on success, ERR_PTR on failure
>> + */
>> +struct pbs_dev *get_pbs_client_device(struct device *dev)
>> +{
>> +	struct device_node *pbs_dev_node;
>> +	struct pbs_dev *pbs;
>> +
>> +	pbs_dev_node = of_parse_phandle(dev->of_node, "qcom,pbs-client", 0);
>> +	if (!pbs_dev_node) {
>> +		pr_err("Missing qcom,pbs-client property\n");
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +
>> +	mutex_lock(&pbs_list_lock);
>> +	list_for_each_entry(pbs, &pbs_dev_list, link) {
> 
> It does not make sense. You have the reference to the device, so you
> have the pbs (via container_of). Don't add some
> global-list-lookup-functions.
> 
> Look for example at Abel Vesa's ICE patchset.
> 
>> +		if (pbs_dev_node == pbs->dev_node) {
>> +			of_node_put(pbs_dev_node);
>> +			mutex_unlock(&pbs_list_lock);
>> +			return pbs;
>> +		}
>> +	}
>> +	mutex_unlock(&pbs_list_lock);
> 
> Where is device_link handling?
Thank you for pointing me to Abel's ICE patchset! Will be updating patch to
use container_of as well as having device_link_add().
> 
>> +
>> +	pr_debug("Unable to find PBS dev_node\n");
>> +	of_node_put(pbs_dev_node);
>> +	return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +EXPORT_SYMBOL(get_pbs_client_device);
>> +
>> +static int qcom_pbs_probe(struct platform_device *pdev)
>> +{
>> +	struct pbs_dev *pbs;
>> +	u32 val;
>> +	int ret;
>> +
>> +	pbs = devm_kzalloc(&pdev->dev, sizeof(*pbs), GFP_KERNEL);
>> +	if (!pbs)
>> +		return -ENOMEM;
>> +
>> +	pbs->dev = &pdev->dev;
>> +	pbs->dev_node = pdev->dev.of_node;
> 
> Why do you need to store it?
Ack - removing storing dev_node
> 
>> +	pbs->regmap = dev_get_regmap(pbs->dev->parent, NULL);
>> +	if (!pbs->regmap) {
>> +		dev_err(pbs->dev, "Couldn't get parent's regmap\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = device_property_read_u32(pbs->dev, "reg", &val);
>> +	if (ret < 0) {
>> +		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	pbs->base = val;
>> +	mutex_init(&pbs->lock);
>> +
>> +	platform_set_drvdata(pdev, pbs);
>> +
>> +	mutex_lock(&pbs_list_lock);
>> +	list_add(&pbs->link, &pbs_dev_list);
>> +	mutex_unlock(&pbs_list_lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_pbs_remove(struct platform_device *pdev)
>> +{
>> +	struct pbs_dev *pbs = platform_get_drvdata(pdev);
>> +
>> +	mutex_lock(&pbs_list_lock);
>> +	list_del(&pbs->link);
>> +	mutex_unlock(&pbs_list_lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id qcom_pbs_match_table[] = {
>> +	{ .compatible = "qcom,pbs" },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_pbs_match_table);
>> +
>> +static struct platform_driver qcom_pbs_driver = {
>> +	.driver = {
>> +		.name		= "qcom-pbs",
>> +		.of_match_table	= qcom_pbs_match_table,
>> +	},
>> +	.probe = qcom_pbs_probe,
>> +	.remove = qcom_pbs_remove,
>> +};
>> +module_platform_driver(qcom_pbs_driver)
>> +
>> +MODULE_DESCRIPTION("QCOM PBS DRIVER");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:qcom-pbs");
> 
> Drop alias. Not needed. If you need it, you have missing ID table.
Ack
> 
>> diff --git a/include/linux/soc/qcom/qcom-pbs.h b/include/linux/soc/qcom/qcom-pbs.h
>> new file mode 100644
>> index 000000000000..4b065951686a
>> --- /dev/null
> 
> 
> 
> Best regards,
> Krzysztof
> 

Thanks,
Anjelque
  

Patch

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a491718f8064..226b668f4690 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -260,6 +260,15 @@  config QCOM_APR
 	  used by audio driver to configure QDSP6
 	  ASM, ADM and AFE modules.
 
+config QCOM_PBS
+	tristate "PBS trigger support for Qualcomm PMIC"
+	depends on SPMI
+	help
+	  This driver supports configuring software programmable boot sequencer (PBS)
+	  trigger event through PBS RAM on Qualcomm Technologies, Inc. PMICs.
+	  This module provides the APIs to the client drivers that wants to send the
+	  PBS trigger event to the PBS RAM.
+
 config QCOM_ICC_BWMON
 	tristate "QCOM Interconnect Bandwidth Monitor driver"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 0f43a88b4894..4e154af3877a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -31,5 +31,6 @@  obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
 obj-$(CONFIG_QCOM_RPMHPD) += rpmhpd.o
 obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
 obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=	kryo-l2-accessors.o
+obj-$(CONFIG_QCOM_PBS) += qcom-pbs.o
 obj-$(CONFIG_QCOM_ICC_BWMON)	+= icc-bwmon.o
 obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)	+= ice.o
diff --git a/drivers/soc/qcom/qcom-pbs.c b/drivers/soc/qcom/qcom-pbs.c
new file mode 100644
index 000000000000..4a2bb7ff8031
--- /dev/null
+++ b/drivers/soc/qcom/qcom-pbs.c
@@ -0,0 +1,343 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define pr_fmt(fmt)	"PBS: %s: " fmt, __func__
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spmi.h>
+#include <linux/soc/qcom/qcom-pbs.h>
+
+#define PBS_CLIENT_TRIG_CTL		0x42
+#define PBS_CLIENT_SW_TRIG_BIT		BIT(7)
+#define PBS_CLIENT_SCRATCH1		0x50
+#define PBS_CLIENT_SCRATCH2		0x51
+
+static LIST_HEAD(pbs_dev_list);
+static DEFINE_MUTEX(pbs_list_lock);
+
+struct pbs_dev {
+	struct device		*dev;
+	struct device_node	*dev_node;
+	struct regmap		*regmap;
+	struct mutex		lock;
+	struct list_head	link;
+
+	u32			base;
+};
+
+static int qcom_pbs_read(struct pbs_dev *pbs, u32 address, u8 *val)
+{
+	int ret;
+
+	address += pbs->base;
+	ret = regmap_bulk_read(pbs->regmap, address, val, 1);
+	if (ret)
+		pr_err("Failed to read address=%#x sid=%#x ret=%d\n",
+			address, to_spmi_device(pbs->dev->parent)->usid, ret);
+
+	return ret;
+}
+
+static int qcom_pbs_write(struct pbs_dev *pbs, u16 address, u8 val)
+{
+	int ret;
+
+	address += pbs->base;
+	ret = regmap_bulk_write(pbs->regmap, address, &val, 1);
+	if (ret < 0)
+		pr_err("Failed to write address=%#x sid=%#x ret=%d\n",
+			  address, to_spmi_device(pbs->dev->parent)->usid, ret);
+	else
+		pr_debug("Wrote %#x to addr %#x\n", val, address);
+
+	return ret;
+}
+
+static int qcom_pbs_masked_write(struct pbs_dev *pbs, u16 address, u8 mask, u8 val)
+{
+	int ret;
+
+	address += pbs->base;
+	ret = regmap_update_bits(pbs->regmap, address, mask, val);
+	if (ret < 0)
+		pr_err("Failed to write address=%#x ret=%d\n", address, ret);
+	else
+		pr_debug("Wrote %#x to addr %#x\n", val, address);
+
+	return ret;
+}
+
+static int qcom_pbs_wait_for_ack(struct pbs_dev *pbs, u8 bit_pos)
+{
+	u16 retries = 2000, delay = 1000;
+	int ret;
+	u8 val;
+
+	while (retries--) {
+		ret = qcom_pbs_read(pbs, PBS_CLIENT_SCRATCH2, &val);
+		if (ret < 0)
+			return ret;
+
+		if (val == 0xFF) {
+			/* PBS error - clear SCRATCH2 register */
+			ret = qcom_pbs_write(pbs, PBS_CLIENT_SCRATCH2, 0);
+			if (ret < 0)
+				return ret;
+
+			pr_err("NACK from PBS for bit %u\n", bit_pos);
+			return -EINVAL;
+		}
+
+		if (val & BIT(bit_pos)) {
+			pr_debug("PBS sequence for bit %u executed!\n", bit_pos);
+			break;
+		}
+
+		usleep_range(delay, delay + 100);
+	}
+
+	if (!retries) {
+		pr_err("Timeout for PBS ACK/NACK for bit %u\n", bit_pos);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+/**
+ * qcom_pbs_trigger_single_event() - Trigger PBS sequence without using bitmap.
+ * @pbs: Pointer to PBS device
+ *
+ * This function is used to trigger the PBS that is hooked on the
+ * SW_TRIGGER directly in PBS client.
+ *
+ * Return: 0 on success, < 0 on failure
+ */
+int qcom_pbs_trigger_single_event(struct pbs_dev *pbs)
+{
+	int ret = 0;
+
+	if (IS_ERR_OR_NULL(pbs))
+		return -EINVAL;
+
+	mutex_lock(&pbs->lock);
+	ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_TRIG_CTL, PBS_CLIENT_SW_TRIG_BIT,
+				PBS_CLIENT_SW_TRIG_BIT);
+	if (ret < 0)
+		pr_err("Failed to write register %x ret=%d\n", PBS_CLIENT_TRIG_CTL, ret);
+	mutex_unlock(&pbs->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_pbs_trigger_single_event);
+
+/**
+ * qcom_pbs_trigger_event() - Trigger the PBS RAM sequence
+ * @pbs: Pointer to PBS device
+ * @bitmap: bitmap
+ *
+ * This function is used to trigger the PBS RAM sequence to be
+ * executed by the client driver.
+ *
+ * The PBS trigger sequence involves
+ * 1. setting the PBS sequence bit in PBS_CLIENT_SCRATCH1
+ * 2. Initiating the SW PBS trigger
+ * 3. Checking the equivalent bit in PBS_CLIENT_SCRATCH2 for the
+ *    completion of the sequence.
+ * 4. If PBS_CLIENT_SCRATCH2 == 0xFF, the PBS sequence failed to execute
+ *
+ * Returns: 0 on success, < 0 on failure
+ */
+int qcom_pbs_trigger_event(struct pbs_dev *pbs, u8 bitmap)
+{
+	u8 val, mask;
+	u16 bit_pos;
+	int ret;
+
+	if (!bitmap) {
+		pr_err("Invalid bitmap passed by client\n");
+		return -EINVAL;
+	}
+
+	if (IS_ERR_OR_NULL(pbs))
+		return -EINVAL;
+
+	mutex_lock(&pbs->lock);
+	ret = qcom_pbs_read(pbs, PBS_CLIENT_SCRATCH2, &val);
+	if (ret < 0)
+		goto out;
+
+	if (val == 0xFF) {
+		/* PBS error - clear SCRATCH2 register */
+		ret = qcom_pbs_write(pbs, PBS_CLIENT_SCRATCH2, 0);
+		if (ret < 0)
+			goto out;
+	}
+
+	for (bit_pos = 0; bit_pos < 8; bit_pos++) {
+		if (bitmap & BIT(bit_pos)) {
+			/*
+			 * Clear the PBS sequence bit position in
+			 * PBS_CLIENT_SCRATCH2 mask register.
+			 */
+			ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_SCRATCH2, BIT(bit_pos), 0);
+			if (ret < 0)
+				goto error;
+
+			/*
+			 * Set the PBS sequence bit position in
+			 * PBS_CLIENT_SCRATCH1 register.
+			 */
+			val = mask = BIT(bit_pos);
+			ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_SCRATCH1, mask, val);
+			if (ret < 0)
+				goto error;
+
+			/* Initiate the SW trigger */
+			val = mask = PBS_CLIENT_SW_TRIG_BIT;
+			ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_TRIG_CTL, mask, val);
+			if (ret < 0)
+				goto error;
+
+			ret = qcom_pbs_wait_for_ack(pbs, bit_pos);
+			if (ret < 0)
+				goto error;
+
+			/*
+			 * Clear the PBS sequence bit position in
+			 * PBS_CLIENT_SCRATCH1 register.
+			 */
+			ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_SCRATCH1, BIT(bit_pos), 0);
+			if (ret < 0)
+				goto error;
+
+			/*
+			 * Clear the PBS sequence bit position in
+			 * PBS_CLIENT_SCRATCH2 mask register.
+			 */
+			ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_SCRATCH2, BIT(bit_pos), 0);
+			if (ret < 0)
+				goto error;
+		}
+	}
+
+error:
+	/* Clear all the requested bitmap */
+	ret = qcom_pbs_masked_write(pbs, PBS_CLIENT_SCRATCH1, bitmap, 0);
+
+out:
+	mutex_unlock(&pbs->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_pbs_trigger_event);
+
+/**
+ * get_pbs_client_device() - Get the PBS device used by client
+ * @dev: Client device
+ *
+ * This function is used to get the PBS device that is being
+ * used by the client.
+ *
+ * Returns: pbs_dev on success, ERR_PTR on failure
+ */
+struct pbs_dev *get_pbs_client_device(struct device *dev)
+{
+	struct device_node *pbs_dev_node;
+	struct pbs_dev *pbs;
+
+	pbs_dev_node = of_parse_phandle(dev->of_node, "qcom,pbs-client", 0);
+	if (!pbs_dev_node) {
+		pr_err("Missing qcom,pbs-client property\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	mutex_lock(&pbs_list_lock);
+	list_for_each_entry(pbs, &pbs_dev_list, link) {
+		if (pbs_dev_node == pbs->dev_node) {
+			of_node_put(pbs_dev_node);
+			mutex_unlock(&pbs_list_lock);
+			return pbs;
+		}
+	}
+	mutex_unlock(&pbs_list_lock);
+
+	pr_debug("Unable to find PBS dev_node\n");
+	of_node_put(pbs_dev_node);
+	return ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL(get_pbs_client_device);
+
+static int qcom_pbs_probe(struct platform_device *pdev)
+{
+	struct pbs_dev *pbs;
+	u32 val;
+	int ret;
+
+	pbs = devm_kzalloc(&pdev->dev, sizeof(*pbs), GFP_KERNEL);
+	if (!pbs)
+		return -ENOMEM;
+
+	pbs->dev = &pdev->dev;
+	pbs->dev_node = pdev->dev.of_node;
+	pbs->regmap = dev_get_regmap(pbs->dev->parent, NULL);
+	if (!pbs->regmap) {
+		dev_err(pbs->dev, "Couldn't get parent's regmap\n");
+		return -EINVAL;
+	}
+
+	ret = device_property_read_u32(pbs->dev, "reg", &val);
+	if (ret < 0) {
+		dev_err(pbs->dev, "Couldn't find reg, ret = %d\n", ret);
+		return ret;
+	}
+
+	pbs->base = val;
+	mutex_init(&pbs->lock);
+
+	platform_set_drvdata(pdev, pbs);
+
+	mutex_lock(&pbs_list_lock);
+	list_add(&pbs->link, &pbs_dev_list);
+	mutex_unlock(&pbs_list_lock);
+
+	return 0;
+}
+
+static int qcom_pbs_remove(struct platform_device *pdev)
+{
+	struct pbs_dev *pbs = platform_get_drvdata(pdev);
+
+	mutex_lock(&pbs_list_lock);
+	list_del(&pbs->link);
+	mutex_unlock(&pbs_list_lock);
+
+	return 0;
+}
+
+static const struct of_device_id qcom_pbs_match_table[] = {
+	{ .compatible = "qcom,pbs" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_pbs_match_table);
+
+static struct platform_driver qcom_pbs_driver = {
+	.driver = {
+		.name		= "qcom-pbs",
+		.of_match_table	= qcom_pbs_match_table,
+	},
+	.probe = qcom_pbs_probe,
+	.remove = qcom_pbs_remove,
+};
+module_platform_driver(qcom_pbs_driver)
+
+MODULE_DESCRIPTION("QCOM PBS DRIVER");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:qcom-pbs");
diff --git a/include/linux/soc/qcom/qcom-pbs.h b/include/linux/soc/qcom/qcom-pbs.h
new file mode 100644
index 000000000000..4b065951686a
--- /dev/null
+++ b/include/linux/soc/qcom/qcom-pbs.h
@@ -0,0 +1,36 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _QCOM_PBS_H
+#define _QCOM_PBS_H
+
+#include <linux/errno.h>
+#include <linux/types.h>
+
+struct device_node;
+struct pbs_dev;
+
+#if IS_ENABLED(CONFIG_QCOM_PBS)
+int qcom_pbs_trigger_event(struct pbs_dev *pbs, u8 bitmap);
+int qcom_pbs_trigger_single_event(struct pbs_dev *pbs);
+struct pbs_dev *get_pbs_client_device(struct device *client_dev);
+#else
+static inline int qcom_pbs_trigger_event(struct pbs_dev *pbs, u8 bitmap)
+{
+	return -ENODEV;
+}
+
+static inline int qcom_pbs_trigger_single_event(struct pbs_dev *pbs)
+{
+	return -ENODEV;
+}
+
+static inline struct pbs_dev *get_pbs_client_device(struct device *client_dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
+#endif