[PATCH] soc: add polarfire soc system controller

[PATCH] soc: add polarfire soc system controller

[conor.dooley]

From: Conor Dooley <conor.dooley@microchip.com>

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@microchip.com>
---
 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 | 119 ++++++++++++++++++++
 5 files changed, 132 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 e8a30c4c5aec..b33142e020e0 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -12,6 +12,7 @@ source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
+source "drivers/soc/microchip/Kconfig"
 source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/renesas/Kconfig"
 source "drivers/soc/rockchip/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index a05e9fbcd3e0..e3be151e391e 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -17,6 +17,7 @@ obj-y				+= ixp4xx/
 obj-$(CONFIG_SOC_XWAY)		+= lantiq/
 obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
 obj-y				+= mediatek/
+obj-y				+= microchip/
 obj-y				+= amlogic/
 obj-y				+= qcom/
 obj-y				+= renesas/
diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig
new file mode 100644
index 000000000000..eb656b33156b
--- /dev/null
+++ b/drivers/soc/microchip/Kconfig
@@ -0,0 +1,10 @@
+config POLARFIRE_SOC_SYS_CTRL
+	tristate "POLARFIRE_SOC_SYS_CTRL"
+	depends on POLARFIRE_SOC_MAILBOX
+	help
+	  This driver adds support for the PolarFire SoC (MPFS) 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..14489919fe4b
--- /dev/null
+++ b/drivers/soc/microchip/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL)	+= 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..3cfee997fa59
--- /dev/null
+++ b/drivers/soc/microchip/mpfs-sys-controller.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip PolarFire SoC (MPFS) system controller driver
+ *
+ * Copyright (c) 2020 Microchip Corporation. All rights reserved.
+ *
+ * Author: Conor Dooley <conor.dooley@microchip.com>
+ *
+ */
+
+#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;
+};
+
+int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
+{
+	int ret;
+
+	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\n");
+		}
+	} else {
+		dev_err(mpfs_client->client.dev,
+			"mpfs sys controller transaction returned %d\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);
+
+	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(&mpfs_client->client, 0);
+	if (IS_ERR(mpfs_client->chan))
+		return dev_err_probe(dev, PTR_ERR(mpfs_client->chan),
+				     "Failed to get mbox channel\n");
+
+	init_completion(&mpfs_client->c);
+
+	platform_set_drvdata(pdev, mpfs_client);
+
+	dev_info(&pdev->dev, "Registered MPFS system controller driver\n");
+
+	return 0;
+}
+
+struct mpfs_sys_controller *
+mpfs_sys_controller_get(struct device_node *mss_node)
+{
+	struct platform_device *pdev = of_find_device_by_node(mss_node);
+
+	if (!pdev)
+		return NULL;
+
+	return platform_get_drvdata(pdev);
+}
+EXPORT_SYMBOL(mpfs_sys_controller_get);
+
+static const struct of_device_id mpfs_sys_controller_of_match[] = {
+	{.compatible = "microchip,polarfire-soc-sys-controller", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mpfs_sys_controller_of_match);
+
+static struct platform_driver mpfs_sys_controller_driver = {
+	.driver = {
+		.name = "mpfs-sys-controller",
+		.of_match_table = mpfs_sys_controller_of_match,
+	},
+	.probe = mpfs_sys_controller_probe,
+};
+module_platform_driver(mpfs_sys_controller_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
+MODULE_DESCRIPTION("MPFS system controller driver");
-- 
2.33.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Conor.Dooley]

On 05/10/2021 13:47, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> 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@microchip.com>
Is there some extra CCs that I am missing on this patch that weren't 
picked up by getmaintainers,
and/or am I mistaken in thinking that the soc tree is arm only?
> ---
>   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 | 119 ++++++++++++++++++++
>   5 files changed, 132 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 e8a30c4c5aec..b33142e020e0 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -12,6 +12,7 @@ source "drivers/soc/imx/Kconfig"
>   source "drivers/soc/ixp4xx/Kconfig"
>   source "drivers/soc/litex/Kconfig"
>   source "drivers/soc/mediatek/Kconfig"
> +source "drivers/soc/microchip/Kconfig"
>   source "drivers/soc/qcom/Kconfig"
>   source "drivers/soc/renesas/Kconfig"
>   source "drivers/soc/rockchip/Kconfig"
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index a05e9fbcd3e0..e3be151e391e 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -17,6 +17,7 @@ obj-y				+= ixp4xx/
>   obj-$(CONFIG_SOC_XWAY)		+= lantiq/
>   obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
>   obj-y				+= mediatek/
> +obj-y				+= microchip/
>   obj-y				+= amlogic/
>   obj-y				+= qcom/
>   obj-y				+= renesas/
> diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig
> new file mode 100644
> index 000000000000..eb656b33156b
> --- /dev/null
> +++ b/drivers/soc/microchip/Kconfig
> @@ -0,0 +1,10 @@
> +config POLARFIRE_SOC_SYS_CTRL
> +	tristate "POLARFIRE_SOC_SYS_CTRL"
> +	depends on POLARFIRE_SOC_MAILBOX
> +	help
> +	  This driver adds support for the PolarFire SoC (MPFS) 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..14489919fe4b
> --- /dev/null
> +++ b/drivers/soc/microchip/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL)	+= 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..3cfee997fa59
> --- /dev/null
> +++ b/drivers/soc/microchip/mpfs-sys-controller.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip PolarFire SoC (MPFS) system controller driver
> + *
> + * Copyright (c) 2020 Microchip Corporation. All rights reserved.
> + *
> + * Author: Conor Dooley <conor.dooley@microchip.com>
> + *
> + */
> +
> +#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;
> +};
> +
> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
> +{
> +	int ret;
> +
> +	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\n");
> +		}
> +	} else {
> +		dev_err(mpfs_client->client.dev,
> +			"mpfs sys controller transaction returned %d\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);
> +
> +	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(&mpfs_client->client, 0);
> +	if (IS_ERR(mpfs_client->chan))
> +		return dev_err_probe(dev, PTR_ERR(mpfs_client->chan),
> +				     "Failed to get mbox channel\n");
> +
> +	init_completion(&mpfs_client->c);
> +
> +	platform_set_drvdata(pdev, mpfs_client);
> +
> +	dev_info(&pdev->dev, "Registered MPFS system controller driver\n");
> +
> +	return 0;
> +}
> +
> +struct mpfs_sys_controller *
> +mpfs_sys_controller_get(struct device_node *mss_node)
> +{
> +	struct platform_device *pdev = of_find_device_by_node(mss_node);
> +
> +	if (!pdev)
> +		return NULL;
> +
> +	return platform_get_drvdata(pdev);
> +}
> +EXPORT_SYMBOL(mpfs_sys_controller_get);
> +
> +static const struct of_device_id mpfs_sys_controller_of_match[] = {
> +	{.compatible = "microchip,polarfire-soc-sys-controller", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mpfs_sys_controller_of_match);
> +
> +static struct platform_driver mpfs_sys_controller_driver = {
> +	.driver = {
> +		.name = "mpfs-sys-controller",
> +		.of_match_table = mpfs_sys_controller_of_match,
> +	},
> +	.probe = mpfs_sys_controller_probe,
> +};
> +module_platform_driver(mpfs_sys_controller_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
> +MODULE_DESCRIPTION("MPFS system controller driver");


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Palmer Dabbelt]

On Thu, 21 Oct 2021 06:13:35 PDT (-0700), Conor.Dooley@microchip.com wrote:
> On 05/10/2021 13:47, conor.dooley@microchip.com wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> 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@microchip.com>
> Is there some extra CCs that I am missing on this patch that weren't 
> picked up by getmaintainers,
> and/or am I mistaken in thinking that the soc tree is arm only?

+Arnd, Olof, and the SOC list.  They probably understand this better 
than I do, we're kind of new to having SOCs in RISC-V land.

I guess I was assuming that someone maintained drivers/soc, but from 
poking around it seems like there's no entry for it and instead it's 
just a bunch of entries for the sub-directories.  As a result the 
scripts aren't picking up anyone to send these too, and I'd assuming 
that because they're not in arch/riscv that they're not for the RISC-V 
tree.

That said, it looks like I put the Kendryte stuff in there (so sorry if 
I screwed anything up).  I'm happy to take these via the RISC-V tree as 
well, I'm assuming that means there should be a MAINTAINERS entry for 
this new sub-directory so changes to it are less likely to get lost.  
Sorry if I was confusing before, I guess I forgot about how this fits 
together.

Arnd: aside from the lack of a maintainer, these generally look fine to 
me.  LMK if you were expecting this kind of stuff to go through the 
RISC-V tree.

Conor: if you're not comfortable maintaining this then I'm OK signing up 
for it.  I certainly don't want to maintain all RISC-V SOC drivers, but 
I'm vaguely responsible for this one so I'm not opposed to it.  
Obviously preferable on my end to have less of that, though, as it can 
get out of hand pretty quickly.

>> ---
>>   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 | 119 ++++++++++++++++++++
>>   5 files changed, 132 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 e8a30c4c5aec..b33142e020e0 100644
>> --- a/drivers/soc/Kconfig
>> +++ b/drivers/soc/Kconfig
>> @@ -12,6 +12,7 @@ source "drivers/soc/imx/Kconfig"
>>   source "drivers/soc/ixp4xx/Kconfig"
>>   source "drivers/soc/litex/Kconfig"
>>   source "drivers/soc/mediatek/Kconfig"
>> +source "drivers/soc/microchip/Kconfig"
>>   source "drivers/soc/qcom/Kconfig"
>>   source "drivers/soc/renesas/Kconfig"
>>   source "drivers/soc/rockchip/Kconfig"
>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
>> index a05e9fbcd3e0..e3be151e391e 100644
>> --- a/drivers/soc/Makefile
>> +++ b/drivers/soc/Makefile
>> @@ -17,6 +17,7 @@ obj-y				+= ixp4xx/
>>   obj-$(CONFIG_SOC_XWAY)		+= lantiq/
>>   obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
>>   obj-y				+= mediatek/
>> +obj-y				+= microchip/
>>   obj-y				+= amlogic/
>>   obj-y				+= qcom/
>>   obj-y				+= renesas/
>> diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig
>> new file mode 100644
>> index 000000000000..eb656b33156b
>> --- /dev/null
>> +++ b/drivers/soc/microchip/Kconfig
>> @@ -0,0 +1,10 @@
>> +config POLARFIRE_SOC_SYS_CTRL
>> +	tristate "POLARFIRE_SOC_SYS_CTRL"
>> +	depends on POLARFIRE_SOC_MAILBOX
>> +	help
>> +	  This driver adds support for the PolarFire SoC (MPFS) 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..14489919fe4b
>> --- /dev/null
>> +++ b/drivers/soc/microchip/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL)	+= 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..3cfee997fa59
>> --- /dev/null
>> +++ b/drivers/soc/microchip/mpfs-sys-controller.c
>> @@ -0,0 +1,119 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Microchip PolarFire SoC (MPFS) system controller driver
>> + *
>> + * Copyright (c) 2020 Microchip Corporation. All rights reserved.
>> + *
>> + * Author: Conor Dooley <conor.dooley@microchip.com>
>> + *
>> + */
>> +
>> +#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;
>> +};
>> +
>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
>> +{
>> +	int ret;
>> +
>> +	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\n");
>> +		}
>> +	} else {
>> +		dev_err(mpfs_client->client.dev,
>> +			"mpfs sys controller transaction returned %d\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);
>> +
>> +	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(&mpfs_client->client, 0);
>> +	if (IS_ERR(mpfs_client->chan))
>> +		return dev_err_probe(dev, PTR_ERR(mpfs_client->chan),
>> +				     "Failed to get mbox channel\n");
>> +
>> +	init_completion(&mpfs_client->c);
>> +
>> +	platform_set_drvdata(pdev, mpfs_client);
>> +
>> +	dev_info(&pdev->dev, "Registered MPFS system controller driver\n");
>> +
>> +	return 0;
>> +}
>> +
>> +struct mpfs_sys_controller *
>> +mpfs_sys_controller_get(struct device_node *mss_node)
>> +{
>> +	struct platform_device *pdev = of_find_device_by_node(mss_node);
>> +
>> +	if (!pdev)
>> +		return NULL;
>> +
>> +	return platform_get_drvdata(pdev);
>> +}
>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
>> +
>> +static const struct of_device_id mpfs_sys_controller_of_match[] = {
>> +	{.compatible = "microchip,polarfire-soc-sys-controller", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, mpfs_sys_controller_of_match);
>> +
>> +static struct platform_driver mpfs_sys_controller_driver = {
>> +	.driver = {
>> +		.name = "mpfs-sys-controller",
>> +		.of_match_table = mpfs_sys_controller_of_match,
>> +	},
>> +	.probe = mpfs_sys_controller_probe,
>> +};
>> +module_platform_driver(mpfs_sys_controller_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
>> +MODULE_DESCRIPTION("MPFS system controller driver");
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Arnd Bergmann]

On Thu, Oct 21, 2021 at 6:00 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> On Thu, 21 Oct 2021 06:13:35 PDT (-0700), Conor.Dooley@microchip.com wrote:
> > On 05/10/2021 13:47, conor.dooley@microchip.com wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> 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@microchip.com>
> > Is there some extra CCs that I am missing on this patch that weren't
> > picked up by getmaintainers, and/or am I mistaken in thinking that
> > the soc tree is arm only?

We can handle non-Arm SoCs as well, though most architecture
maintainers prefer to keep them merged through their trees.

> +Arnd, Olof, and the SOC list.  They probably understand this better
> than I do, we're kind of new to having SOCs in RISC-V land.
>
> I guess I was assuming that someone maintained drivers/soc, but from
> poking around it seems like there's no entry for it and instead it's
> just a bunch of entries for the sub-directories.  As a result the
> scripts aren't picking up anyone to send these too, and I'd assuming
> that because they're not in arch/riscv that they're not for the RISC-V
> tree.
>
> That said, it looks like I put the Kendryte stuff in there (so sorry if
> I screwed anything up).  I'm happy to take these via the RISC-V tree as
> well, I'm assuming that means there should be a MAINTAINERS entry for
> this new sub-directory so changes to it are less likely to get lost.
> Sorry if I was confusing before, I guess I forgot about how this fits
> together.
>
> Arnd: aside from the lack of a maintainer, these generally look fine to
> me.  LMK if you were expecting this kind of stuff to go through the
> RISC-V tree.

It probably helps avoid merge conflicts to go through the soc tree,
as there are generally more changes for arm specific socs in there.

However, we usually take pull requests from platform maintainers,
not individual patches. For Microchip's ARM based platforms, those
patches would go through the AT91/SAMA5 maintainers (added to
Cc). You can ask them if they are willing to take future patches for
the polarfire soc as well and forward them to soc@kernel.org along
with the other stuff.

> >> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
> >> +{
> >> +    int ret;
> >> +
> >> +    mutex_lock_interruptible(&transaction_lock);

When you do a mutex_lock_interruptible(), you have to check its return code and
handle the interruption, usually by passing down -EINTR to the caller.

> >> +struct mpfs_sys_controller *
> >> +mpfs_sys_controller_get(struct device_node *mss_node)
> >> +{
> >> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
> >> +
> >> +    if (!pdev)
> >> +            return NULL;
> >> +
> >> +    return platform_get_drvdata(pdev);
> >> +}
> >> +EXPORT_SYMBOL(mpfs_sys_controller_get);

There should probably be a check in here to ensure that this is actually
a system controller and it's bound do this driver, rather than returning a
random device's driver data.

It might also help to make this take a phandle instead of a device node
for lookup, to spare the client the extra phandle to node conversion.

       Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Conor.Dooley]

On 21/10/2021 17:00, Palmer Dabbelt wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
>
> On Thu, 21 Oct 2021 06:13:35 PDT (-0700), Conor.Dooley@microchip.com 
> wrote:
>> On 05/10/2021 13:47, conor.dooley@microchip.com wrote:
>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>
>>> 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@microchip.com>
>> Is there some extra CCs that I am missing on this patch that weren't
>> picked up by getmaintainers,
>> and/or am I mistaken in thinking that the soc tree is arm only?
>
> +Arnd, Olof, and the SOC list.  They probably understand this better
> than I do, we're kind of new to having SOCs in RISC-V land.
>
> I guess I was assuming that someone maintained drivers/soc, but from
> poking around it seems like there's no entry for it and instead it's
> just a bunch of entries for the sub-directories.  As a result the
> scripts aren't picking up anyone to send these too, and I'd assuming
> that because they're not in arch/riscv that they're not for the RISC-V
> tree.
>
> That said, it looks like I put the Kendryte stuff in there (so sorry if
> I screwed anything up).  I'm happy to take these via the RISC-V tree as
> well, I'm assuming that means there should be a MAINTAINERS entry for
> this new sub-directory so changes to it are less likely to get lost.
> Sorry if I was confusing before, I guess I forgot about how this fits
> together.
>
> Arnd: aside from the lack of a maintainer, these generally look fine to
> me.  LMK if you were expecting this kind of stuff to go through the
> RISC-V tree.
>
> Conor: if you're not comfortable maintaining this then I'm OK signing up
> for it.  I certainly don't want to maintain all RISC-V SOC drivers, but
> I'm vaguely responsible for this one so I'm not opposed to it.
> Obviously preferable on my end to have less of that, though, as it can
> get out of hand pretty quickly.
This was the patch that I had previously mentioned got dropped from
another series, and that series added a maintainers entry for our riscv
stuff - including drivers/soc/microchip/ and include/soc/microchip/.
I'm not currently listed as maintainer but I have a series to submit
with device tree changes etc and I will add myself to the entry since
I 'm the one who has been reading/responding here.
>
>>> ---
>>>   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 | 119 
>>> ++++++++++++++++++++
>>>   5 files changed, 132 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 e8a30c4c5aec..b33142e020e0 100644
>>> --- a/drivers/soc/Kconfig
>>> +++ b/drivers/soc/Kconfig
>>> @@ -12,6 +12,7 @@ source "drivers/soc/imx/Kconfig"
>>>   source "drivers/soc/ixp4xx/Kconfig"
>>>   source "drivers/soc/litex/Kconfig"
>>>   source "drivers/soc/mediatek/Kconfig"
>>> +source "drivers/soc/microchip/Kconfig"
>>>   source "drivers/soc/qcom/Kconfig"
>>>   source "drivers/soc/renesas/Kconfig"
>>>   source "drivers/soc/rockchip/Kconfig"
>>> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
>>> index a05e9fbcd3e0..e3be151e391e 100644
>>> --- a/drivers/soc/Makefile
>>> +++ b/drivers/soc/Makefile
>>> @@ -17,6 +17,7 @@ obj-y                              += ixp4xx/
>>>   obj-$(CONFIG_SOC_XWAY)             += lantiq/
>>>   obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
>>>   obj-y                              += mediatek/
>>> +obj-y                               += microchip/
>>>   obj-y                              += amlogic/
>>>   obj-y                              += qcom/
>>>   obj-y                              += renesas/
>>> diff --git a/drivers/soc/microchip/Kconfig 
>>> b/drivers/soc/microchip/Kconfig
>>> new file mode 100644
>>> index 000000000000..eb656b33156b
>>> --- /dev/null
>>> +++ b/drivers/soc/microchip/Kconfig
>>> @@ -0,0 +1,10 @@
>>> +config POLARFIRE_SOC_SYS_CTRL
>>> +    tristate "POLARFIRE_SOC_SYS_CTRL"
>>> +    depends on POLARFIRE_SOC_MAILBOX
>>> +    help
>>> +      This driver adds support for the PolarFire SoC (MPFS) 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..14489919fe4b
>>> --- /dev/null
>>> +++ b/drivers/soc/microchip/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL)        += 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..3cfee997fa59
>>> --- /dev/null
>>> +++ b/drivers/soc/microchip/mpfs-sys-controller.c
>>> @@ -0,0 +1,119 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Microchip PolarFire SoC (MPFS) system controller driver
>>> + *
>>> + * Copyright (c) 2020 Microchip Corporation. All rights reserved.
>>> + *
>>> + * Author: Conor Dooley <conor.dooley@microchip.com>
>>> + *
>>> + */
>>> +
>>> +#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;
>>> +};
>>> +
>>> +int mpfs_blocking_transaction(struct mpfs_sys_controller 
>>> *mpfs_client, void *msg)
>>> +{
>>> +    int ret;
>>> +
>>> +    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\n");
>>> +            }
>>> +    } else {
>>> +            dev_err(mpfs_client->client.dev,
>>> +                    "mpfs sys controller transaction returned 
>>> %d\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);
>>> +
>>> +    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(&mpfs_client->client, 0);
>>> +    if (IS_ERR(mpfs_client->chan))
>>> +            return dev_err_probe(dev, PTR_ERR(mpfs_client->chan),
>>> +                                 "Failed to get mbox channel\n");
>>> +
>>> +    init_completion(&mpfs_client->c);
>>> +
>>> +    platform_set_drvdata(pdev, mpfs_client);
>>> +
>>> +    dev_info(&pdev->dev, "Registered MPFS system controller 
>>> driver\n");
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +struct mpfs_sys_controller *
>>> +mpfs_sys_controller_get(struct device_node *mss_node)
>>> +{
>>> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
>>> +
>>> +    if (!pdev)
>>> +            return NULL;
>>> +
>>> +    return platform_get_drvdata(pdev);
>>> +}
>>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
>>> +
>>> +static const struct of_device_id mpfs_sys_controller_of_match[] = {
>>> +    {.compatible = "microchip,polarfire-soc-sys-controller", },
>>> +    {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, mpfs_sys_controller_of_match);
>>> +
>>> +static struct platform_driver mpfs_sys_controller_driver = {
>>> +    .driver = {
>>> +            .name = "mpfs-sys-controller",
>>> +            .of_match_table = mpfs_sys_controller_of_match,
>>> +    },
>>> +    .probe = mpfs_sys_controller_probe,
>>> +};
>>> +module_platform_driver(mpfs_sys_controller_driver);
>>> +
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
>>> +MODULE_DESCRIPTION("MPFS system controller driver");
>>
>>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Conor.Dooley]

resending, think i accidentally clicked a formatting option and the mail 
got converted to html.

On 21/10/2021 19:34, Arnd Bergmann wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, Oct 21, 2021 at 6:00 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> On Thu, 21 Oct 2021 06:13:35 PDT (-0700), Conor.Dooley@microchip.com wrote:
>
>> +Arnd, Olof, and the SOC list.  They probably understand this better
>> than I do, we're kind of new to having SOCs in RISC-V land.
>>
>> I guess I was assuming that someone maintained drivers/soc, but from
>> poking around it seems like there's no entry for it and instead it's
>> just a bunch of entries for the sub-directories.  As a result the
>> scripts aren't picking up anyone to send these too, and I'd assuming
>> that because they're not in arch/riscv that they're not for the RISC-V
>> tree.
>>
>> That said, it looks like I put the Kendryte stuff in there (so sorry if
>> I screwed anything up).  I'm happy to take these via the RISC-V tree as
>> well, I'm assuming that means there should be a MAINTAINERS entry for
>> this new sub-directory so changes to it are less likely to get lost.
>> Sorry if I was confusing before, I guess I forgot about how this fits
>> together.
>>
>> Arnd: aside from the lack of a maintainer, these generally look fine to
>> me.  LMK if you were expecting this kind of stuff to go through the
>> RISC-V tree.
> 
> It probably helps avoid merge conflicts to go through the soc tree,
> as there are generally more changes for arm specific socs in there.
Yeah, that sounds like a good idea. Spoke to Nicolas this morning and 
will send a revised version of this w/ your comments addressed and 
future polarfire soc additions via the at91/sam tree.
> 
> However, we usually take pull requests from platform maintainers,
> not individual patches. For Microchip's ARM based platforms, those
> patches would go through the AT91/SAMA5 maintainers (added to
> Cc). You can ask them if they are willing to take future patches for
> the polarfire soc as well and forward them to soc@kernel.org along
> with the other stuff.
> 
>>>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    mutex_lock_interruptible(&transaction_lock);
> 
> When you do a mutex_lock_interruptible(), you have to check its return code and
> handle the interruption, usually by passing down -EINTR to the caller.
> 
>>>> +struct mpfs_sys_controller *
>>>> +mpfs_sys_controller_get(struct device_node *mss_node)
>>>> +{
>>>> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
>>>> +
>>>> +    if (!pdev)
>>>> +            return NULL;
>>>> +
>>>> +    return platform_get_drvdata(pdev);
>>>> +}
>>>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
> 
> There should probably be a check in here to ensure that this is actually
> a system controller and it's bound do this driver, rather than returning a
> random device's driver data.
> 
> It might also help to make this take a phandle instead of a device node
> for lookup, to spare the client the extra phandle to node conversion.
> 
>         Arnd
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Conor.Dooley]

On 21/10/2021 19:34, Arnd Bergmann wrote:
>>>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    mutex_lock_interruptible(&transaction_lock);
> 
> When you do a mutex_lock_interruptible(), you have to check its return code and
> handle the interruption, usually by passing down -EINTR to the caller.
> 
>>>> +struct mpfs_sys_controller *
>>>> +mpfs_sys_controller_get(struct device_node *mss_node)
>>>> +{
>>>> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
>>>> +
>>>> +    if (!pdev)
>>>> +            return NULL;
>>>> +
>>>> +    return platform_get_drvdata(pdev);
>>>> +}
>>>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
> 
> There should probably be a check in here to ensure that this is actually
> a system controller and it's bound do this driver, rather than returning a
> random device's driver data >
> It might also help to make this take a phandle instead of a device node
> for lookup, to spare the client the extra phandle to node conversion.
Finally got around to this again, is it sufficient to just check that 
platform_get_drvdata returns non null, or should I also check if the 
pdev matches the drivers compatible strings? Only found one example of 
the latter and a mix of the former & what I had done when I went looking 
around at other drivers.
Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: add polarfire soc system controller

[Arnd Bergmann]

On Mon, Nov 8, 2021 at 4:19 PM <Conor.Dooley@microchip.com> wrote:
>
> On 21/10/2021 19:34, Arnd Bergmann wrote:
> >>>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
> >>>> +{
> >>>> +    int ret;
> >>>> +
> >>>> +    mutex_lock_interruptible(&transaction_lock);
> >
> > When you do a mutex_lock_interruptible(), you have to check its return code and
> > handle the interruption, usually by passing down -EINTR to the caller.
> >
> >>>> +struct mpfs_sys_controller *
> >>>> +mpfs_sys_controller_get(struct device_node *mss_node)
> >>>> +{
> >>>> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
> >>>> +
> >>>> +    if (!pdev)
> >>>> +            return NULL;
> >>>> +
> >>>> +    return platform_get_drvdata(pdev);
> >>>> +}
> >>>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
> >
> > There should probably be a check in here to ensure that this is actually
> > a system controller and it's bound do this driver, rather than returning a
> > random device's driver data >
> > It might also help to make this take a phandle instead of a device node
> > for lookup, to spare the client the extra phandle to node conversion.
>
> Finally got around to this again, is it sufficient to just check that
> platform_get_drvdata returns non null, or should I also check if the
> pdev matches the drivers compatible strings? Only found one example of
> the latter and a mix of the former & what I had done when I went looking
> around at other drivers.

Neither of those helps at all, it could still be a random other device.

I would check that the device is bound to mpfs_sys_controller_driver here,
that should be the easiest way. Looking at it some more, I suspect the
reference counting needs to be improved as well, to ensure that the
device is not going away here. You only hold a reference on the
of_node (which you apparently never release either), but not on the device.

    Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv