On 2020/12/01 21:18, Ben Dooks wrote: > On 01/12/2020 12:00, Damien Le Moal wrote: >> On 2020/12/01 20:03, conor.dooley@xxxxxxxxxxxxx wrote: >>> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>> >>> This driver provides an interface for other drivers to access the >>> functions of the system controller on the Microchip PolarFire SoC. >>> >>> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>> --- >>> drivers/soc/Kconfig | 1 + >>> drivers/soc/Makefile | 1 + >>> drivers/soc/microchip/Kconfig | 10 ++ >>> drivers/soc/microchip/Makefile | 1 + >>> drivers/soc/microchip/mpfs_sys_controller.c | 135 ++++++++++++++++++++ >>> 5 files changed, 148 insertions(+) >>> create mode 100644 drivers/soc/microchip/Kconfig >>> create mode 100644 drivers/soc/microchip/Makefile >>> create mode 100644 drivers/soc/microchip/mpfs_sys_controller.c >>> >>> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig >>> index 425ab6f7e375..22cb097bcbdc 100644 >>> --- a/drivers/soc/Kconfig >>> +++ b/drivers/soc/Kconfig >>> @@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig" >>> source "drivers/soc/fsl/Kconfig" >>> source "drivers/soc/imx/Kconfig" >>> source "drivers/soc/ixp4xx/Kconfig" >>> +source "drivers/soc/microchip/Kconfig" >>> source "drivers/soc/mediatek/Kconfig" >>> source "drivers/soc/qcom/Kconfig" >>> source "drivers/soc/renesas/Kconfig" >>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile >>> index 36452bed86ef..fb084cf2d12e 100644 >>> --- a/drivers/soc/Makefile >>> +++ b/drivers/soc/Makefile >>> @@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_GEMINI) += gemini/ >>> obj-y += imx/ >>> obj-$(CONFIG_ARCH_IXP4XX) += ixp4xx/ >>> obj-$(CONFIG_SOC_XWAY) += lantiq/ >>> +obj-$(CONFIG_SOC_MICROCHIP_POLARFIRE) += microchip/ >>> obj-y += mediatek/ >>> obj-y += amlogic/ >>> obj-y += qcom/ >>> diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig >>> new file mode 100644 >>> index 000000000000..40e5203c8ba0 >>> --- /dev/null >>> +++ b/drivers/soc/microchip/Kconfig >>> @@ -0,0 +1,10 @@ >>> +config MPFS_SYS_CONTROLLER >>> + tristate "MPFS_SYS_CONTROLLER" >>> + depends on MPFS_MBOX >>> + help >>> + This driver adds support for the Polarfire SoC system controller. >>> + >>> + To compile this driver as a module, choose M here. the >>> + module will be called mpfs_system_controller. >>> + >>> + If unsure, say N. >>> diff --git a/drivers/soc/microchip/Makefile b/drivers/soc/microchip/Makefile >>> new file mode 100644 >>> index 000000000000..23b1f42a37db >>> --- /dev/null >>> +++ b/drivers/soc/microchip/Makefile >>> @@ -0,0 +1 @@ >>> +obj-$(CONFIG_MPFS_SYS_CONTROLLER) += mpfs_sys_controller.o >>> diff --git a/drivers/soc/microchip/mpfs_sys_controller.c b/drivers/soc/microchip/mpfs_sys_controller.c >>> new file mode 100644 >>> index 000000000000..875a0671e196 >>> --- /dev/null >>> +++ b/drivers/soc/microchip/mpfs_sys_controller.c >>> @@ -0,0 +1,135 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Microchip MPFS system controller driver >>> + * >>> + * Copyright (c) 2020 Microchip Corporation. All rights reserved. >>> + * >>> + * Author: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>> + * >>> + */ >>> + >>> +#include <linux/slab.h> >>> +#include <linux/module.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/of_platform.h> >>> +#include <linux/mailbox_client.h> >>> +#include <linux/platform_device.h> >>> +#include <soc/microchip/mpfs.h> >>> + >>> +static DEFINE_MUTEX(transaction_lock); >>> + >>> +struct mpfs_sys_controller { >>> + struct mbox_client client; >>> + struct mbox_chan *chan; >>> + struct completion c; >>> + u32 enabled; >>> + void *response; >>> + u16 response_size_bytes; >>> +}; >>> + >>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg, >>> + void *response, u16 response_size_bytes) >>> +{ >>> + int ret; >>> + >>> + mpfs_client->response = response; >>> + mpfs_client->response_size_bytes = response_size_bytes; >>> + >>> + mutex_lock_interruptible(&transaction_lock); >>> + >>> + reinit_completion(&mpfs_client->c); >>> + >>> + ret = mbox_send_message(mpfs_client->chan, msg); >>> + if (ret >= 0) { >>> + if (wait_for_completion_timeout(&mpfs_client->c, HZ)) { >>> + ret = 0; >>> + } else { >>> + ret = -ETIMEDOUT; >>> + dev_warn(mpfs_client->client.dev, "MPFS sys controller transaction timeout"); >>> + } >>> + } else { >>> + dev_err(mpfs_client->client.dev, >>> + "mpfs sys controller transaction returned %d\r\n", ret); >>> + } >>> + >>> + mutex_unlock(&transaction_lock); >>> + >>> + return ret; >>> +} >>> +EXPORT_SYMBOL(mpfs_blocking_transaction); >>> + >>> +static void rx_callback(struct mbox_client *client, void *msg) >>> +{ >>> + struct mpfs_sys_controller *mpfs_client = >>> + container_of(client, struct mpfs_sys_controller, client); >>> + >>> + memcpy(mpfs_client->response, (u8 *)msg, >>> + mpfs_client->response_size_bytes); >>> + >>> + complete(&mpfs_client->c); >>> +} >>> + >>> +static int mpfs_sys_controller_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + struct mpfs_sys_controller *mpfs_client; >>> + >>> + mpfs_client = devm_kzalloc(dev, sizeof(*mpfs_client), GFP_KERNEL); >>> + if (!mpfs_client) >>> + return -ENOMEM; >>> + >>> + mpfs_client->client.dev = dev; >>> + mpfs_client->client.rx_callback = rx_callback; >>> + mpfs_client->client.tx_block = 1U; >>> + >>> + mpfs_client->chan = mbox_request_channel_byname(&mpfs_client->client, >>> + "mbox-mpfs"); >>> + if (IS_ERR(mpfs_client->chan)) { >>> + int ret = PTR_ERR(mpfs_client->chan); >>> + >>> + if (ret != -EPROBE_DEFER) >>> + dev_err(dev, "Failed to get mbox channel: %d\n", ret); >>> + return ret; >> >> You can replace all of this with >> >> return dev_err_probe(dev, PTR_ERR(mpfs_client->chan), >> ""Failed to get mbox channel\n"); > > you'll spam the error log if you get a lot of probe defferals. > I think the code as is fine. Please look at that function code: it does not print anything if the error is -EPROBE_DEFER. -- Damien Le Moal Western Digital Research