Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <be2329f9-747e-16e9-b212-74ff424c9a7c@intel.com>
Date:   Tue, 9 Aug 2022 10:06:35 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [PATCH v1] regmap: add generic indirect regmap support
Content-Language: en-US
To:     Tianfei Zhang <tianfei.zhang@intel.com>, <broonie@kernel.org>,
        <gregkh@linuxfoundation.org>, <rafael@kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     <hao.wu@intel.com>, <trix@redhat.com>, <yilun.xu@intel.com>,
        "Matthew Gerlach" <matthew.gerlach@linux.intel.com>
References: <20220607013755.594554-1-tianfei.zhang@intel.com>
From:   Russ Weight <russell.h.weight@intel.com>
In-Reply-To: <20220607013755.594554-1-tianfei.zhang@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WTRzdFFTUEJlMityRHkxOVh1VEppQ1d2MFVWeGxubjZaK1NpaWZRVVdZSXNl?=
 =?utf-8?B?VnkydmtQM1hSbnp0YXFNQlB1VXcxeXlydTlBcmpBY2lBWkY3Z0JJM1RDdGFV?=
 =?utf-8?B?N0YrRzBpMkpsWWgycWNTUFpmSHp1LzJJbkt5SDdQWjIrZmdHVHMvNXZyQjBu?=
 =?utf-8?B?ZVZxcjZUWWhQQjRYdVprZkFDRGdwdjd5YmJVYTB3bVBBQ2VNaWk0WmtWQ0JO?=
 =?utf-8?B?RkNzL1Q0R2NmQllGV1VTZzBaQjc4YVU2YndJajBkNDdvMWgzYWhMUHBpc0l1?=
 =?utf-8?B?MStQd1h5NVJmSkpDWnN2WFZWcHJXVndHRVJxV3FVZWk5ZTRyY0F0Ynp1bjlE?=
 =?utf-8?B?YVlyN3ZDK1ZwbHBMRFFFenVra1FKb3J4Tk5jaVJWVVpxTDlIVUtONk9Pc25s?=
 =?utf-8?B?alBoUjZCVXVNTE5maUl1ZnQxRGlZbXozbzlJVDhEajN0Z2FJMFBXTmlST1BJ?=
 =?utf-8?B?TDdySEdTUklWQ29Gdm9TZW5TaVpwSE5ZOFRyVVNuNzAyWlM0dCtyNitpOHVJ?=
 =?utf-8?B?SFRLbHhqQmhqVFZ5T3A2Vkg2aDhJeUIzeCtZM0NXazZCdWNJWUV1QmIzb0Qy?=
 =?utf-8?B?TmRnU3lRTWlEalY2T2k1TVlVQ1owenh6MWxHRGk1T3FVWEI0TGdVYk93bS9o?=
 =?utf-8?B?N1dzUCtZeG5SdkJla1p5WmMwT2hua1Q0Uk1iUnU0SFBxOGcxUFBGcURaUi93?=
 =?utf-8?B?MEQ3QUR1d2t1b0x5NGdSSit1ZloyRXpuNms3MndyOG1RNEk2Z0poempqeVZx?=
 =?utf-8?B?L0xOaU9iSG4vWmQ2TFFYbGtBTzFZT2I0aFN2NFlINkl6ek8rV0JISUZFSGlJ?=
 =?utf-8?B?dWFZRHIya0xnb3l2WFJtVjBzVS96RCtNTUpFV0ZKOVJVWGN4SUVlbk52MmJj?=
 =?utf-8?B?SklsbzNpdFRvN1NyM05EUnBXV2ZSU2JGazhjdzRhNkwwdEFqamVwdndHcTZL?=
 =?utf-8?B?MFVoRVNHVXdGWi9qN21qMk1lOGliZUp4ejhxb00rRjhHT045VVZMREZvOVMx?=
 =?utf-8?B?UmhBV2tPU2kxUzQrcFY1US91Qy9VNHpWTXFvTHNxRi94VXRJdWcwcXNZMzJC?=
 =?utf-8?B?RHVPOUhkR1YydHIzcU9zbmJOVnJ6aGVoNEp2ZXJmRXZoK3JIUTlmQk1UZjVE?=
 =?utf-8?B?dGQwTjUvT3BFYmQ5aFVzM29qQ004VlczTTdtdjArMG5lZmJLYVBFZmVZSjNE?=
 =?utf-8?B?STdlYTliWVVLNEFhRlJFNWF1SzF0bWM5OStHbC9Jbkh0VEt1NC9wVVJzNU5O?=
 =?utf-8?B?Y3E1eE5uZ3VxcDQvQ1dQdC9FQUdOMVJlUERzdEpmQ3RzVTNOUW9mYTFHRFR5?=
 =?utf-8?B?bEJxSjhvRmxVZlNZKzJJSzI5MDNBQVJ6T2FDRlRZZ2xKMVZ1Q09zTzB6NCs0?=
 =?utf-8?B?SE9JWk5ycUEyd3o0dzMvNkd0NHhtMGI1dG9WZWh3RnZjRlNRcURjSDVmMHEw?=
 =?utf-8?B?Sk94S1hQbGxhK0VlQkZ1R2wyaThhdi9oZFhwNTJ0OWNPeGlMa1VncEZlRkFv?=
 =?utf-8?B?Zmp4Y0FvcnkwZUhxQWVOa3V5TXRBWEY1Vkd0c21OTFdoc0MrM1VFQnJLZU9j?=
 =?utf-8?B?SE40QzVZNGh4M2dYZDRqV0VlamVVUWpIN2JLQzVrdENQZnJQM09ycDVEamtv?=
 =?utf-8?B?Y3JmWVhlN2dPSEZ0OHVtcjI0WTNNMHNERlU2QWdWcytndGJjdDNDQ29obWY3?=
 =?utf-8?B?eGExaHIxRkVlbitSRHUyTGJtUGwyaVppb1dTUjUyMFlvRkN3c1VJa1JkUjFN?=
 =?utf-8?B?WnZnYmwwSXhXR0hyZTB3Q1liR0diZVBzTEk2L3VtdnVlNy9GNWFBa0FYYlUv?=
 =?utf-8?B?OS83NkRPcHBEbk03TnlVRFpXbWoycDBIOU80TEdaWFRlck1EOGhsUWUrVHN1?=
 =?utf-8?B?M1R1dWFHMjlDRG5zeWNHcVZxVXVjb0I4VXJkNlMrWHpITC91M0QzOVh0eDF1?=
 =?utf-8?B?Z0tCaE1rK1RmK3dUbjFwWnd1TVVsUFV5dEIySXo0TkU0djFmRkdtQUFvNlE1?=
 =?utf-8?B?c0VrR2MrNmM3UnVESzIrVGtyWUN1eE0wb3gwQVhWRXVVT3hCcUplNkpHZTBY?=
 =?utf-8?B?Qkxid0t2WXFWU1NXUVhzbk13aWxmanNEZnBFd2RiM0VZWnJBVzFTdE56MDQr?=
 =?utf-8?B?SlFEbE1lL1J2a2hEWUFVbFNkYVVpSy95VGFEL3YveDRDRUR4L1BwaDNaTGJx?=
 =?utf-8?B?bmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3d7cefe0-0770-4174-0587-08da7a298939
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1899.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2022 17:06:44.3622
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qXEH+5XF2eaj8eKpqkmShtbrlY1S91ZkwQ0o4gWRJp/LGwwpWg3V+ghA0nBhNjq3CnfrLbTJ6S8BrM2by5zLlqWWwRGk3SuAKTRqRtNHyyw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1435
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 6/6/22 18:37, Tianfei Zhang wrote:
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>
> This patch adds support for regmap APIs that are intended to be used by
> the drivers of some devices which support generic indirect register access,
> for example PMCI (Platform Management Control Interface) device, HSSI
> (High Speed Serial Interface) device in FPGA.
>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> ---
>  drivers/base/regmap/Kconfig                   |   3 +
>  drivers/base/regmap/Makefile                  |   1 +
>  .../base/regmap/regmap-indirect-register.c    | 133 ++++++++++++++++++
>  include/linux/regmap.h                        |  12 ++
>  4 files changed, 149 insertions(+)
>  create mode 100644 drivers/base/regmap/regmap-indirect-register.c
>
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 159bac6c5046..4ea590604f8d 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -65,3 +65,6 @@ config REGMAP_I3C
>  config REGMAP_SPI_AVMM
>  	tristate
>  	depends on SPI
> +
> +config REGMAP_INDIRECT_REGISTER
> +	tristate
> diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> index 11facb32a027..504a32b79d8c 100644
> --- a/drivers/base/regmap/Makefile
> +++ b/drivers/base/regmap/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
>  obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
>  obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
>  obj-$(CONFIG_REGMAP_MDIO) += regmap-mdio.o
> +obj-$(CONFIG_REGMAP_INDIRECT_REGISTER) += regmap-indirect-register.o
> diff --git a/drivers/base/regmap/regmap-indirect-register.c b/drivers/base/regmap/regmap-indirect-register.c
> new file mode 100644
> index 000000000000..0b748e48bd87
> --- /dev/null
> +++ b/drivers/base/regmap/regmap-indirect-register.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Indirect Register Access.
> +//
> +// Copyright (C) 2022 Intel Corporation, Inc.
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define INDIRECT_CMD_OFF	0x0
> +#define INDIRECT_CMD_RD	BIT(0)
> +#define INDIRECT_CMD_WR	BIT(1)
> +#define INDIRECT_CMD_ACK	BIT(2)
> +
> +#define INDIRECT_ADDR_OFF	0x4
> +#define INDIRECT_RD_OFF	0x8
> +#define INDIRECT_WR_OFF	0xc
> +
> +#define INDIRECT_INT_US	1
> +#define INDIRECT_TIMEOUT_US	10000
> +
> +struct indirect_ctx {
> +	void __iomem *base;
> +	struct device *dev;
> +};
> +
> +static int indirect_bus_clr_cmd(struct indirect_ctx *ctx)
> +{
> +	unsigned int cmd;
> +	int ret;
> +
> +	writel(0, ctx->base + INDIRECT_CMD_OFF);
> +	ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> +				 (!cmd), INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> +	if (ret)
> +		dev_err(ctx->dev, "%s timed out on clearing cmd 0x%xn", __func__, cmd);
The line-return in the above dev_err() is missing a forward slash: \n

- Russ
> +
> +	return ret;
> +}
> +
> +static int indirect_bus_reg_read(void *context, unsigned int reg,
> +				 unsigned int *val)
> +{
> +	struct indirect_ctx *ctx = context;
> +	unsigned int cmd;
> +	int ret;
> +
> +	cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> +	if (cmd)
> +		dev_warn(ctx->dev, "%s non-zero cmd 0x%x\n", __func__, cmd);
> +
> +	writel(reg, ctx->base + INDIRECT_ADDR_OFF);
> +	writel(INDIRECT_CMD_RD, ctx->base + INDIRECT_CMD_OFF);
> +	ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> +				 (cmd & INDIRECT_CMD_ACK), INDIRECT_INT_US,
> +				 INDIRECT_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(ctx->dev, "%s timed out on reg 0x%x cmd 0x%x\n", __func__, reg, cmd);
> +		goto out;
> +	}
> +
> +	*val = readl(ctx->base + INDIRECT_RD_OFF);
> +
> +	if (indirect_bus_clr_cmd(ctx))
> +		ret = -ETIMEDOUT;
> +
> +out:
> +	return ret;
> +}
> +
> +static int indirect_bus_reg_write(void *context, unsigned int reg,
> +				  unsigned int val)
> +{
> +	struct indirect_ctx *ctx = context;
> +	unsigned int cmd;
> +	int ret;
> +
> +	cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> +	if (cmd)
> +		dev_warn(ctx->dev, "%s non-zero cmd 0x%x\n", __func__, cmd);
> +
> +	writel(val, ctx->base + INDIRECT_WR_OFF);
> +	writel(reg, ctx->base + INDIRECT_ADDR_OFF);
> +	writel(INDIRECT_CMD_WR, ctx->base + INDIRECT_CMD_OFF);
> +	ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> +				 (cmd & INDIRECT_CMD_ACK), INDIRECT_INT_US,
> +				 INDIRECT_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(ctx->dev, "%s timed out on reg 0x%x cmd 0x%x\n", __func__, reg, cmd);
> +		goto out;
> +	}
> +
> +	if (indirect_bus_clr_cmd(ctx))
> +		ret = -ETIMEDOUT;
> +
> +out:
> +	return ret;
> +}
> +
> +static const struct regmap_bus indirect_bus = {
> +	.reg_write = indirect_bus_reg_write,
> +	.reg_read =  indirect_bus_reg_read,
> +};
> +
> +/**
> + * devm_regmap_init_indirect_register - create a regmap for indirect register access
> + * @dev: device creating the regmap
> + * @base: __iomem point to base of memory with mailbox
> + * @cfg: regmap_config describing interface
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +struct regmap *devm_regmap_init_indirect_register(struct device *dev,
> +						  void __iomem *base,
> +						  struct regmap_config *cfg)
> +{
> +	struct indirect_ctx *ctx;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return NULL;
> +
> +	ctx->base = base;
> +	ctx->dev = dev;
> +
> +	return devm_regmap_init(dev, &indirect_bus, ctx, cfg);
> +}
> +EXPORT_SYMBOL_GPL(devm_regmap_init_indirect_register);
> +
> +MODULE_DESCRIPTION("Indirect Register Access");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index de81a94d7b30..72eb38883e88 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -670,6 +670,18 @@ struct regmap *__devm_regmap_init_spi_avmm(struct spi_device *spi,
>  					   const struct regmap_config *config,
>  					   struct lock_class_key *lock_key,
>  					   const char *lock_name);
> +/**
> + * devm_regmap_init_indirect_register - create a regmap for indirect register access
> + * @dev: device creating the regmap
> + * @base: __iomem point to base of memory with mailbox
> + * @cfg: regmap_config describing interface
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +struct regmap *devm_regmap_init_indirect_register(struct device *dev,
> +						  void __iomem *base,
> +						  struct regmap_config *cfg);
> +
>  /*
>   * Wrapper for regmap_init macros to include a unique lockdep key and name
>   * for each call. No-op if CONFIG_LOCKDEP is not set.