RE: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC Controller

[Mahapatra, Amit Kumar]

> -----Original Message-----
> From: Brad Larson <blarson@xxxxxxx>
> Sent: Monday, May 15, 2023 11:46 PM
> To: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; linux-
> spi@xxxxxxxxxxxxxxx; adrian.hunter@xxxxxxxxx; alcooperx@xxxxxxxxx;
> andy.shevchenko@xxxxxxxxx; arnd@xxxxxxxx; Larson, Bradley
> <Bradley.Larson@xxxxxxx>; brendan.higgins@xxxxxxxxx;
> briannorris@xxxxxxxxxxxx; catalin.marinas@xxxxxxx;
> conor+dt@xxxxxxxxxx; davidgow@xxxxxxxxxx; gsomlo@xxxxxxxxx;
> gerg@xxxxxxxxxxxxxx; hal.feng@xxxxxxxxxxxxxxxx; hasegawa-
> hitomi@xxxxxxxxxxx; j.neuschaefer@xxxxxxx; joel@xxxxxxxxx;
> kernel@xxxxxxxx; krzk@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> lee@xxxxxxxxxx; lee.jones@xxxxxxxxxx; broonie@xxxxxxxxxx;
> p.zabel@xxxxxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> samuel@xxxxxxxxxxxx; fancer.lancer@xxxxxxxxx;
> skhan@xxxxxxxxxxxxxxxxxxx; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@xxxxxxx>; Lendacky, Thomas
> <Thomas.Lendacky@xxxxxxx>; tonyhuang.sunplus@xxxxxxxxx;
> ulf.hansson@xxxxxxxxxx; vaishnav.a@xxxxxx; walker.chen@xxxxxxxxxxxxxxxx;
> will@xxxxxxxxxx; zhuyinbo@xxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: [PATCH v14 8/8] soc: amd: Add support for AMD Pensando SoC
> Controller
> 
> The Pensando SoC controller is a SPI connected companion device that is
> present in all Pensando SoC board designs.  The essential board management
> registers are accessed on chip select 0 with board mgmt IO support accessed
> using additional chip selects.
> 
> Signed-off-by: Brad Larson <blarson@xxxxxxx>
> ---
> 
> v14 changes:
> - Save 8 bytes of code size by swapping spi_device and reset_controller_dev
>   in penctrl_device
> - Code simplification and clarity from review inputs
> - Set penctrl_spi_driver.driver.name to match compatible pensando-elba-ctrl
> - Remove unused include in amd-pensando-ctrl.h
> - Rebase to linux-next 6.4.0-rc1 class_create() API change
> 
> v13 changes:
> - Update include list in pensando-ctrl.c
> - Change variable spi_dev to spi throughout
> - Removed unneeded variable initialization, simplification of
>   error checks, remove extra castings, and use dev_err_probe()
> - Sort the includes in amd-pensando-ctrl.h
> - Updates to cleanup if there is an error in penctrl_spi_probe()
> 
> v12 changes:
> - Fix gcc-12.1.0 warning:
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Link: https://lore.kernel.org/oe-kbuild-all/202303120925.SxLjwOd2-
> lkp@xxxxxxxxx/
> 
> v11 changes:
> - Fix the compatible to be specific 'amd,pensando-elba-ctrl'
> 
> v10 changes:
> - Different driver implementation specific to this Pensando controller device.
> - Moved to soc/amd directory under new name based on guidance.  This
> driver is
>   of no use to any design other than all Pensando SoC based cards.
> - Removed use of builtin_driver, can be built as a module.
> 
> v9 changes:
> - Previously patch 14/17
> - After the change to the device tree node and squashing
>   reset-cells into the parent simplified this to not use
>   any MFD API and move it to drivers/spi/pensando-sr.c.
> - Change the naming to remove elba since this driver is common
>   for all Pensando SoC designs .
> - Default yes SPI_PENSANDO_SR for ARCH_PENSANDO
> 
> ---
>  drivers/soc/Kconfig                    |   1 +
>  drivers/soc/Makefile                   |   1 +
>  drivers/soc/amd/Kconfig                |  16 ++
>  drivers/soc/amd/Makefile               |   2 +
>  drivers/soc/amd/pensando-ctrl.c        | 368 +++++++++++++++++++++++++
>  include/uapi/linux/amd-pensando-ctrl.h |  29 ++
>  6 files changed, 417 insertions(+)
>  create mode 100644 drivers/soc/amd/Kconfig  create mode 100644
> drivers/soc/amd/Makefile  create mode 100644 drivers/soc/amd/pensando-
> ctrl.c  create mode 100644 include/uapi/linux/amd-pensando-ctrl.h
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index
> 4e176280113a..9e023f74e47c 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -2,6 +2,7 @@
>  menu "SOC (System On Chip) specific Drivers"
> 
>  source "drivers/soc/actions/Kconfig"
> +source "drivers/soc/amd/Kconfig"
>  source "drivers/soc/amlogic/Kconfig"
>  source "drivers/soc/apple/Kconfig"
>  source "drivers/soc/aspeed/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index
> 3b0f9fb3b5c8..8914530f2721 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -4,6 +4,7 @@
>  #
> 
>  obj-$(CONFIG_ARCH_ACTIONS)	+= actions/
> +obj-y				+= amd/
>  obj-y				+= apple/
>  obj-y				+= aspeed/
>  obj-$(CONFIG_ARCH_AT91)		+= atmel/
> diff --git a/drivers/soc/amd/Kconfig b/drivers/soc/amd/Kconfig new file
> mode 100644 index 000000000000..011d5339d14e
> --- /dev/null
> +++ b/drivers/soc/amd/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only menu "AMD Pensando SoC drivers"
> +
> +config AMD_PENSANDO_CTRL
> +	tristate "AMD Pensando SoC Controller"
> +	depends on SPI_MASTER=y
> +	depends on (ARCH_PENSANDO && OF) || COMPILE_TEST
> +	default ARCH_PENSANDO
> +	select REGMAP_SPI
> +	select MFD_SYSCON
> +	help
> +	  Enables AMD Pensando SoC controller device support.  This is a SPI
> +	  attached companion device in all Pensando SoC board designs which
> +	  provides essential board control/status registers and management
> IO
> +	  support.
> +endmenu
> diff --git a/drivers/soc/amd/Makefile b/drivers/soc/amd/Makefile new file
> mode 100644 index 000000000000..a2de0424f68d
> --- /dev/null
> +++ b/drivers/soc/amd/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_AMD_PENSANDO_CTRL)	+= pensando-ctrl.o
> diff --git a/drivers/soc/amd/pensando-ctrl.c b/drivers/soc/amd/pensando-
> ctrl.c new file mode 100644 index 000000000000..a7ddd181dfe8
> --- /dev/null
> +++ b/drivers/soc/amd/pensando-ctrl.c
> @@ -0,0 +1,368 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Pensando SoC Controller
> + *
> + * Userspace interface and reset driver support for SPI connected
> +Pensando SoC
> + * controller device.  This device is present in all Pensando SoC
> +designs and
> + * contains board control/status registers and management IO support.
> + *
> + * Copyright 2023 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spi/spi.h>
> +
> +#include <linux/amd-pensando-ctrl.h>
> +
> +struct penctrl_device {
> +	struct reset_controller_dev rcdev;
> +	struct spi_device *spi;
> +};
> +
> +static DEFINE_MUTEX(spi_lock);
> +static dev_t penctrl_devt;
> +static struct penctrl_device *penctrl;
> +static struct class *penctrl_class;
> +
> +static long
> +penctrl_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) {
> +	void __user *in_arg = (void __user *)arg;
> +	struct penctrl_device *penctrl;
> +	u8 tx_buf[PENCTRL_MAX_MSG_LEN];
> +	u8 rx_buf[PENCTRL_MAX_MSG_LEN];
> +	struct spi_transfer t[2] = {};
> +	struct penctrl_spi_xfer *msg;
> +	struct spi_device *spi;
> +	unsigned int num_msgs;
> +	struct spi_message m;
> +	u32 size;
> +	int ret;
> +
> +	/* Check for a valid command */
> +	if (_IOC_TYPE(cmd) != PENCTRL_IOC_MAGIC)
> +		return -ENOTTY;
> +
> +	if (_IOC_NR(cmd) > PENCTRL_IOC_MAXNR)
> +		return -ENOTTY;
> +
> +	if (((_IOC_DIR(cmd) & _IOC_READ)) && !access_ok(in_arg,
> _IOC_SIZE(cmd)))
> +		return -EFAULT;
> +
> +	if (((_IOC_DIR(cmd) & _IOC_WRITE)) && !access_ok(in_arg,
> _IOC_SIZE(cmd)))
> +		return -EFAULT;
> +
> +	/* Get a reference to the SPI device */
> +	penctrl = filp->private_data;
> +	if (!penctrl)
> +		return -ESHUTDOWN;
> +
> +	spi = spi_dev_get(penctrl->spi);
> +	if (!spi)
> +		return -ESHUTDOWN;
> +
> +	/* Verify and prepare SPI message */
> +	size = _IOC_SIZE(cmd);
> +	num_msgs = size / sizeof(struct penctrl_spi_xfer);
> +	if (num_msgs > 2 || size == 0 || size % sizeof(struct
> penctrl_spi_xfer)) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +	msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
> +	if (IS_ERR(msg)) {
> +		ret = PTR_ERR(msg);
> +		goto out_unlock;
> +	}
> +	if (msg->len > PENCTRL_MAX_MSG_LEN) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	t[0].tx_buf = tx_buf;
> +	t[0].len = msg->len;
> +	if (copy_from_user(tx_buf, (void __user *)msg->tx_buf, msg->len))
> {
> +		ret = -EFAULT;
> +		goto out_unlock;
> +	}
> +	if (num_msgs > 1) {
> +		msg++;
> +		if (msg->len > PENCTRL_MAX_MSG_LEN) {
> +			ret = -EINVAL;
> +			goto out_unlock;
> +		}
> +		t[1].rx_buf = rx_buf;
> +		t[1].len = msg->len;
> +	}
> +	spi_message_init_with_transfers(&m, t, num_msgs);
> +
> +	/* Perform the transfer */
> +	mutex_lock(&spi_lock);
> +	ret = spi_sync(spi, &m);
> +	mutex_unlock(&spi_lock);
> +
> +	if (ret || (num_msgs == 1))
> +		goto out_unlock;
> +
> +	if (copy_to_user((void __user *)msg->rx_buf, rx_buf, msg->len))
> +		ret = -EFAULT;
> +
> +out_unlock:
> +	spi_dev_put(spi);
> +	return ret;
> +}
> +
> +static int penctrl_open(struct inode *inode, struct file *filp) {
> +	struct spi_device *spi;
> +	u8 current_cs;
> +
> +	filp->private_data = penctrl;
> +	current_cs = iminor(inode);
> +	spi = penctrl->spi;
> +	spi->chip_select = current_cs;

New set/get APIs for accessing spi->chip_select were introduced by 
https://github.com/torvalds/linux/commit/303feb3cc06ac0665d0ee9c1414941200e60e8a3
please use these APIs instead of accessing spi->chip_select dricetly. 

> +	spi->cs_gpiod = spi->controller->cs_gpiods[current_cs];
> +	spi_setup(spi);
> +	return stream_open(inode, filp);
> +}
> +
> +static int penctrl_release(struct inode *inode, struct file *filp) {
> +	filp->private_data = NULL;
> +	return 0;
> +}
> +
> +static const struct file_operations penctrl_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl = penctrl_ioctl,
> +	.open		= penctrl_open,
> +	.release	= penctrl_release,
> +	.llseek		= no_llseek,
> +};
> +
> +static int penctrl_regs_read(struct penctrl_device *penctrl, u32 reg,
> +u32 *val) {
> +	struct spi_device *spi = penctrl->spi;
> +	struct spi_transfer t[2] = {};
> +	struct spi_message m;
> +	u8 txbuf[3];
> +	u8 rxbuf[1];
> +	int ret;
> +
> +	txbuf[0] = PENCTRL_SPI_CMD_REGRD;
> +	txbuf[1] = reg;
> +	txbuf[2] = 0;
> +	t[0].tx_buf = txbuf;
> +	t[0].len = sizeof(txbuf);
> +
> +	rxbuf[0] = 0;
> +	t[1].rx_buf = rxbuf;
> +	t[1].len = sizeof(rxbuf);
> +
> +	spi_message_init_with_transfers(&m, t, ARRAY_SIZE(t));
> +	ret = spi_sync(spi, &m);
> +	if (ret == 0)
> +		*val = rxbuf[0];
> +
> +	return ret;
> +}
> +
> +static int penctrl_regs_write(struct penctrl_device *penctrl, u32 reg,
> +u32 val) {
> +	struct spi_device *spi = penctrl->spi;
> +	struct spi_transfer t = {};
> +	struct spi_message m;
> +	u8 txbuf[4];
> +
> +	txbuf[0] = PENCTRL_SPI_CMD_REGWR;
> +	txbuf[1] = reg;
> +	txbuf[2] = val;
> +	txbuf[3] = 0;
> +
> +	t.tx_buf = txbuf;
> +	t.len = sizeof(txbuf);
> +	spi_message_init_with_transfers(&m, &t, 1);
> +	return spi_sync(spi, &m);
> +}
> +
> +static int penctrl_reset_assert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct penctrl_device *penctrl =
> +		container_of(rcdev, struct penctrl_device, rcdev);
> +	struct spi_device *spi = penctrl->spi;
> +	unsigned int val;
> +	int ret;
> +
> +	mutex_lock(&spi_lock);
> +	spi->chip_select = 0;

Same here.

> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
> +	spi_setup(spi);
> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
> +		goto out_unlock;
> +	}
> +
> +	val |= BIT(6);
> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
> +	if (ret)
> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
> +
> +out_unlock:
> +	mutex_unlock(&spi_lock);
> +	return ret;
> +}
> +
> +static int penctrl_reset_deassert(struct reset_controller_dev *rcdev,
> +				  unsigned long id)
> +{
> +	struct penctrl_device *penctrl =
> +		container_of(rcdev, struct penctrl_device, rcdev);
> +	struct spi_device *spi = penctrl->spi;
> +	unsigned int val;
> +	int ret;
> +
> +	mutex_lock(&spi_lock);
> +	spi->chip_select = 0;

Same here.

Regards,
Amit

> +	spi->cs_gpiod = spi->controller->cs_gpiods[0];
> +	spi_setup(spi);
> +	ret = penctrl_regs_read(penctrl, PENCTRL_REG_CTRL0, &val);
> +	if (ret) {
> +		dev_err(&spi->dev, "error reading ctrl0 reg\n");
> +		goto out_unlock;
> +	}
> +
> +	val &= ~BIT(6);
> +	ret = penctrl_regs_write(penctrl, PENCTRL_REG_CTRL0, val);
> +	if (ret)
> +		dev_err(&spi->dev, "error writing ctrl0 reg\n");
> +
> +out_unlock:
> +	mutex_unlock(&spi_lock);
> +	return ret;
> +}
> +
> +static const struct reset_control_ops penctrl_reset_ops = {
> +	.assert = penctrl_reset_assert,
> +	.deassert = penctrl_reset_deassert,
> +};
> +
> +static int penctrl_spi_probe(struct spi_device *spi) {
> +	struct device *dev;
> +	struct cdev *cdev;
> +	u32 num_cs;
> +	int ret;
> +	u32 cs;
> +
> +	ret = device_property_read_u32(spi->dev.parent, "num-cs",
> &num_cs);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "number of chip-selects not defined\n");
> +
> +	ret = alloc_chrdev_region(&penctrl_devt, 0, num_cs, "penctrl");
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "failed to alloc chrdev region\n");
> +
> +	penctrl_class = class_create("penctrl");
> +	if (IS_ERR(penctrl_class)) {
> +		ret = dev_err_probe(&spi->dev, PTR_ERR(penctrl_class),
> +				    "failed to create class\n");
> +		goto unregister_chrdev;
> +	}
> +
> +	cdev = cdev_alloc();
> +	if (!cdev) {
> +		ret = dev_err_probe(&spi->dev, -ENOMEM,
> +				    "allocation of cdev failed\n");
> +		goto destroy_class;
> +	}
> +	cdev->owner = THIS_MODULE;
> +	cdev_init(cdev, &penctrl_fops);
> +
> +	ret = cdev_add(cdev, penctrl_devt, num_cs);
> +	if (ret) {
> +		ret = dev_err_probe(&spi->dev, ret,
> +				    "register of cdev failed\n");
> +		goto free_cdev;
> +	}
> +
> +	/* Allocate driver data */
> +	penctrl = kzalloc(sizeof(*penctrl), GFP_KERNEL);
> +	if (!penctrl) {
> +		ret = -ENOMEM;
> +		goto free_cdev;
> +	}
> +	penctrl->spi = spi;
> +	mutex_init(&spi_lock);
> +
> +	/* Create a device for each chip select */
> +	for (cs = 0; cs < num_cs; cs++) {
> +		dev = device_create(penctrl_class,
> +				    &spi->dev,
> +				    MKDEV(MAJOR(penctrl_devt), cs),
> +				    penctrl,
> +				    "penctrl0.%d",
> +				    cs);
> +		if (IS_ERR(dev)) {
> +			ret = dev_err_probe(&spi->dev, PTR_ERR(dev),
> +					    "error creating device\n");
> +			goto destroy_device;
> +		}
> +		dev_dbg(&spi->dev, "created device major %u, minor
> %d\n",
> +			MAJOR(penctrl_devt), cs);
> +	}
> +
> +	/* Register reset controller */
> +	penctrl->rcdev.dev = &spi->dev;
> +	penctrl->rcdev.ops = &penctrl_reset_ops;
> +	penctrl->rcdev.owner = THIS_MODULE;
> +	penctrl->rcdev.of_node = spi->dev.of_node;
> +	penctrl->rcdev.nr_resets = 1;
> +
> +	ret = reset_controller_register(&penctrl->rcdev);
> +	if (ret)
> +		return dev_err_probe(&spi->dev, ret,
> +				     "failed to register reset controller\n");
> +	return 0;
> +
> +destroy_device:
> +	for (cs = 0; cs < num_cs; cs++)
> +		device_destroy(penctrl_class,
> MKDEV(MAJOR(penctrl_devt), cs));
> +	kfree(penctrl);
> +free_cdev:
> +	cdev_del(cdev);
> +destroy_class:
> +	class_destroy(penctrl_class);
> +unregister_chrdev:
> +	unregister_chrdev(MAJOR(penctrl_devt), "penctrl");
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id penctrl_dt_match[] = {
> +	{ .compatible = "amd,pensando-elba-ctrl" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct spi_driver penctrl_spi_driver = {
> +	.probe = penctrl_spi_probe,
> +	.driver = {
> +		.name = "pensando-elba-ctrl",
> +		.of_match_table = penctrl_dt_match,
> +	},
> +};
> +module_spi_driver(penctrl_spi_driver);
> +
> +MODULE_AUTHOR("Brad Larson <blarson@xxxxxxx>");
> MODULE_DESCRIPTION("AMD
> +Pensando SoC Controller via SPI"); MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/amd-pensando-ctrl.h
> b/include/uapi/linux/amd-pensando-ctrl.h
> new file mode 100644
> index 000000000000..e5f9f0dfe146
> --- /dev/null
> +++ b/include/uapi/linux/amd-pensando-ctrl.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Userspace interface for /dev/penctrl
> + *
> + * This file can be used by applications that need to communicate
> + * with the AMD Pensando SoC controller device via the ioctl interface.
> + */
> +#ifndef _UAPI_LINUX_AMD_PENSANDO_CTRL_H #define
> +_UAPI_LINUX_AMD_PENSANDO_CTRL_H
> +
> +#include <linux/types.h>
> +
> +#define PENCTRL_SPI_CMD_REGRD	0x0b
> +#define PENCTRL_SPI_CMD_REGWR	0x02
> +#define PENCTRL_IOC_MAGIC	'k'
> +#define PENCTRL_IOC_MAXNR	0
> +#define PENCTRL_MAX_MSG_LEN	16
> +#define PENCTRL_MAX_REG		0xff
> +#define PENCTRL_REG_CTRL0	0x10
> +
> +struct penctrl_spi_xfer {
> +	__u64 tx_buf;
> +	__u64 rx_buf;
> +	__u32 len;
> +	__u32 speed_hz;
> +	__u64 compat;
> +};
> +
> +#endif /* _UAPI_LINUX_AMD_PENSANDO_CTRL_H */
> --
> 2.17.1