diff mbox series

[v4,10/21] soc: qcom: Add qcom's pstore minidump driver support

Message ID 1687955688-20809-11-git-send-email-quic_mojha@quicinc.com
State New
Headers show
Series None | expand

Commit Message

Mukesh Ojha June 28, 2023, 12:34 p.m. UTC 
This driver was inspired from the fact pstore ram region should be
fixed and boot firmware need to have awarness about this region,
so that it will be persistent across boot. But, there are many
QCOM SoC which does not support warm boot from hardware but they
have minidump support from the software, and for them, there is
no need of this pstore ram region to be fixed, but at the same
time have interest in the pstore frontends data. So, this driver
get the dynamic reserved region from the ram and register the
ramoops platform device.

 +---------+     +---------+   +--------+     +---------+
 | console |     | pmsg    |   | ftrace |     | dmesg   |
 +---------+     +---------+   +--------+     +---------+
       |             |             |              |
       |             |             |              |
       +------------------------------------------+
                          |
                         \ /
                  +----------------+
            (1)   |pstore frontends|
                  +----------------+
                          |
                         \ /
                 +------------------- +
            (2)  | pstore backend(ram)|
                 +--------------------+
                          |
                         \ /
                 +--------------------+
            (3)  |qcom_pstore_minidump|
                 +--------------------+
                          |
                         \ /
                   +---------------+
            (4)    | qcom_minidump |
                   +---------------+

This driver will route all the pstore front data to the stored
in qcom pstore reserved region and the reason of showing an
arrow from (3) to (4) as qcom_pstore_minidump driver will register
all the available frontends region with qcom minidump driver
in upcoming patch.

Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
---
 drivers/soc/qcom/Kconfig                | 12 +++++
 drivers/soc/qcom/Makefile               |  1 +
 drivers/soc/qcom/qcom_pstore_minidump.c | 85 +++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 drivers/soc/qcom/qcom_pstore_minidump.c

Comments

Rob Herring June 28, 2023, 10:57 p.m. UTC | #1 
On Wed, Jun 28, 2023 at 6:37 AM Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> This driver was inspired from the fact pstore ram region should be
> fixed and boot firmware need to have awarness about this region,
> so that it will be persistent across boot. But, there are many
> QCOM SoC which does not support warm boot from hardware but they
> have minidump support from the software, and for them, there is
> no need of this pstore ram region to be fixed, but at the same
> time have interest in the pstore frontends data. So, this driver
> get the dynamic reserved region from the ram and register the
> ramoops platform device.
>
>  +---------+     +---------+   +--------+     +---------+
>  | console |     | pmsg    |   | ftrace |     | dmesg   |
>  +---------+     +---------+   +--------+     +---------+
>        |             |             |              |
>        |             |             |              |
>        +------------------------------------------+
>                           |
>                          \ /
>                   +----------------+
>             (1)   |pstore frontends|
>                   +----------------+
>                           |
>                          \ /
>                  +------------------- +
>             (2)  | pstore backend(ram)|
>                  +--------------------+
>                           |
>                          \ /
>                  +--------------------+
>             (3)  |qcom_pstore_minidump|
>                  +--------------------+
>                           |
>                          \ /
>                    +---------------+
>             (4)    | qcom_minidump |
>                    +---------------+
>
> This driver will route all the pstore front data to the stored
> in qcom pstore reserved region and the reason of showing an
> arrow from (3) to (4) as qcom_pstore_minidump driver will register
> all the available frontends region with qcom minidump driver
> in upcoming patch.
>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
>  drivers/soc/qcom/Kconfig                | 12 +++++
>  drivers/soc/qcom/Makefile               |  1 +
>  drivers/soc/qcom/qcom_pstore_minidump.c | 85 +++++++++++++++++++++++++++++++++

drivers/soc/ is the dumping ground for things with no other place. As
this is a pstore driver, it belongs with pstore.

>  3 files changed, 98 insertions(+)
>  create mode 100644 drivers/soc/qcom/qcom_pstore_minidump.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 1834213fd652..fbf08e30feda 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -306,4 +306,16 @@ config QCOM_MINIDUMP_SMEM
>
>           This config should be enabled if the low level minidump is implemented
>           as part of SMEM.
> +
> +config QCOM_PSTORE_MINIDUMP
> +       tristate "Pstore support for QCOM Minidump"
> +       depends on ARCH_QCOM
> +       depends on PSTORE_RAM
> +       depends on QCOM_MINIDUMP
> +       help
> +         Enablement of this driver ensures that ramoops region can be anywhere
> +         reserved in ram instead of being fixed address which needs boot firmware
> +         awareness. So, this driver creates plaform device and registers available
> +         frontend region with the Qualcomm's minidump driver.
> +
>  endmenu
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 737d868757ac..1ab59c1b364d 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -36,3 +36,4 @@ qcom_ice-objs                 += ice.o
>  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)        += qcom_ice.o
>  obj-$(CONFIG_QCOM_MINIDUMP) += qcom_minidump.o
>  obj-$(CONFIG_QCOM_MINIDUMP_SMEM) += qcom_minidump_smem.o
> +obj-$(CONFIG_QCOM_PSTORE_MINIDUMP) += qcom_pstore_minidump.o
> diff --git a/drivers/soc/qcom/qcom_pstore_minidump.c b/drivers/soc/qcom/qcom_pstore_minidump.c
> new file mode 100644
> index 000000000000..b07cd10340df
> --- /dev/null
> +++ b/drivers/soc/qcom/qcom_pstore_minidump.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_device.h>

You probably don't need this include. Use the actual includes you need
and don't rely on implicit includes (because I'm trying to remove
those).

> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/pstore_ram.h>
> +
> +struct qcom_ramoops_dd {
> +       struct ramoops_platform_data qcom_ramoops_pdata;
> +       struct platform_device *ramoops_pdev;
> +};
> +
> +static int qcom_ramoops_probe(struct platform_device *pdev)
> +{
> +       struct device_node *of_node = pdev->dev.of_node;
> +       struct qcom_ramoops_dd *qcom_rdd;
> +       struct ramoops_platform_data *pdata;
> +       struct reserved_mem *rmem;
> +       struct device_node *node;
> +       long ret;
> +
> +       node = of_parse_phandle(of_node, "memory-region", 0);
> +       if (!node)
> +               return -ENODEV;
> +
> +       rmem = of_reserved_mem_lookup(node);
> +       of_node_put(node);
> +       if (!rmem) {
> +               dev_err(&pdev->dev, "failed to locate DT /reserved-memory resource\n");
> +               return -EINVAL;
> +       }
> +
> +       qcom_rdd = devm_kzalloc(&pdev->dev, sizeof(*qcom_rdd), GFP_KERNEL);
> +       if (!qcom_rdd)
> +               return -ENOMEM;
> +
> +       pdata = &qcom_rdd->qcom_ramoops_pdata;
> +       pdata->mem_size = rmem->size;
> +       pdata->mem_address = rmem->base;
> +       ramoops_parse_dt(pdev, pdata);
> +
> +       qcom_rdd->ramoops_pdev = platform_device_register_data(NULL, "ramoops", -1,
> +                                                              pdata, sizeof(*pdata));
> +       if (IS_ERR(qcom_rdd->ramoops_pdev)) {
> +               ret = PTR_ERR(qcom_rdd->ramoops_pdev);
> +               dev_err(&pdev->dev, "could not create platform device: %ld\n", ret);
> +               qcom_rdd->ramoops_pdev = NULL;
> +       }
> +       platform_set_drvdata(pdev, qcom_rdd);
> +
> +       return ret;
> +}
> +
> +static void qcom_ramoops_remove(struct platform_device *pdev)
> +{
> +       struct qcom_ramoops_dd *qcom_rdd = platform_get_drvdata(pdev);
> +
> +       platform_device_unregister(qcom_rdd->ramoops_pdev);
> +       qcom_rdd->ramoops_pdev = NULL;
> +}
> +
> +static const struct of_device_id qcom_ramoops_of_match[] = {
> +       { .compatible = "qcom,ramoops"},
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_ramoops_of_match);
> +
> +static struct platform_driver qcom_ramoops_drv = {
> +       .driver         = {
> +               .name   = "qcom,ramoops",
> +               .of_match_table = qcom_ramoops_of_match,
> +       },
> +       .probe = qcom_ramoops_probe,
> +       .remove_new = qcom_ramoops_remove,
> +};
> +
> +module_platform_driver(qcom_ramoops_drv);
> +
> +MODULE_DESCRIPTION("Qualcomm ramoops minidump driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
Rob Herring July 5, 2023, 11:27 p.m. UTC | #2 
On Thu, Jun 29, 2023 at 3:16 AM Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
>
>
> On 6/29/2023 4:27 AM, Rob Herring wrote:
> > On Wed, Jun 28, 2023 at 6:37 AM Mukesh Ojha <quic_mojha@quicinc.com> wrote:
> >>
> >> This driver was inspired from the fact pstore ram region should be
> >> fixed and boot firmware need to have awarness about this region,
> >> so that it will be persistent across boot. But, there are many
> >> QCOM SoC which does not support warm boot from hardware but they
> >> have minidump support from the software, and for them, there is
> >> no need of this pstore ram region to be fixed, but at the same
> >> time have interest in the pstore frontends data. So, this driver
> >> get the dynamic reserved region from the ram and register the
> >> ramoops platform device.
> >>
> >>   +---------+     +---------+   +--------+     +---------+
> >>   | console |     | pmsg    |   | ftrace |     | dmesg   |
> >>   +---------+     +---------+   +--------+     +---------+
> >>         |             |             |              |
> >>         |             |             |              |
> >>         +------------------------------------------+
> >>                            |
> >>                           \ /
> >>                    +----------------+
> >>              (1)   |pstore frontends|
> >>                    +----------------+
> >>                            |
> >>                           \ /
> >>                   +------------------- +
> >>              (2)  | pstore backend(ram)|
> >>                   +--------------------+
> >>                            |
> >>                           \ /
> >>                   +--------------------+
> >>              (3)  |qcom_pstore_minidump|
> >>                   +--------------------+
> >>                            |
> >>                           \ /
> >>                     +---------------+
> >>              (4)    | qcom_minidump |
> >>                     +---------------+
> >>
> >> This driver will route all the pstore front data to the stored
> >> in qcom pstore reserved region and the reason of showing an
> >> arrow from (3) to (4) as qcom_pstore_minidump driver will register
> >> all the available frontends region with qcom minidump driver
> >> in upcoming patch.
> >>
> >> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> >> ---
> >>   drivers/soc/qcom/Kconfig                | 12 +++++
> >>   drivers/soc/qcom/Makefile               |  1 +
> >>   drivers/soc/qcom/qcom_pstore_minidump.c | 85 +++++++++++++++++++++++++++++++++
> >
> > drivers/soc/ is the dumping ground for things with no other place. As
> > this is a pstore driver, it belongs with pstore.
>
> The inspiration of this driver was taken from
> drivers/platform/chrome/chromeos_pstore.c, do you think that is misplaced ?

The difference is really that's nothing more than platform specific
logic to instantiate a normal ramoops device. It's kind of ugly, yes,
but it's still just a normal ramoops device in the end. Your case is
that plus all the extra parts for minidump.

Rob
diff mbox series

Patch

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 1834213fd652..fbf08e30feda 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -306,4 +306,16 @@  config QCOM_MINIDUMP_SMEM
 
 	  This config should be enabled if the low level minidump is implemented
 	  as part of SMEM.
+
+config QCOM_PSTORE_MINIDUMP
+	tristate "Pstore support for QCOM Minidump"
+	depends on ARCH_QCOM
+	depends on PSTORE_RAM
+	depends on QCOM_MINIDUMP
+	help
+	  Enablement of this driver ensures that ramoops region can be anywhere
+	  reserved in ram instead of being fixed address which needs boot firmware
+	  awareness. So, this driver creates plaform device and registers available
+	  frontend region with the Qualcomm's minidump driver.
+
 endmenu
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 737d868757ac..1ab59c1b364d 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -36,3 +36,4 @@  qcom_ice-objs			+= ice.o
 obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)	+= qcom_ice.o
 obj-$(CONFIG_QCOM_MINIDUMP) += qcom_minidump.o
 obj-$(CONFIG_QCOM_MINIDUMP_SMEM) += qcom_minidump_smem.o
+obj-$(CONFIG_QCOM_PSTORE_MINIDUMP) += qcom_pstore_minidump.o
diff --git a/drivers/soc/qcom/qcom_pstore_minidump.c b/drivers/soc/qcom/qcom_pstore_minidump.c
new file mode 100644
index 000000000000..b07cd10340df
--- /dev/null
+++ b/drivers/soc/qcom/qcom_pstore_minidump.c
@@ -0,0 +1,85 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/pstore_ram.h>
+
+struct qcom_ramoops_dd {
+	struct ramoops_platform_data qcom_ramoops_pdata;
+	struct platform_device *ramoops_pdev;
+};
+
+static int qcom_ramoops_probe(struct platform_device *pdev)
+{
+	struct device_node *of_node = pdev->dev.of_node;
+	struct qcom_ramoops_dd *qcom_rdd;
+	struct ramoops_platform_data *pdata;
+	struct reserved_mem *rmem;
+	struct device_node *node;
+	long ret;
+
+	node = of_parse_phandle(of_node, "memory-region", 0);
+	if (!node)
+		return -ENODEV;
+
+	rmem = of_reserved_mem_lookup(node);
+	of_node_put(node);
+	if (!rmem) {
+		dev_err(&pdev->dev, "failed to locate DT /reserved-memory resource\n");
+		return -EINVAL;
+	}
+
+	qcom_rdd = devm_kzalloc(&pdev->dev, sizeof(*qcom_rdd), GFP_KERNEL);
+	if (!qcom_rdd)
+		return -ENOMEM;
+
+	pdata = &qcom_rdd->qcom_ramoops_pdata;
+	pdata->mem_size = rmem->size;
+	pdata->mem_address = rmem->base;
+	ramoops_parse_dt(pdev, pdata);
+
+	qcom_rdd->ramoops_pdev = platform_device_register_data(NULL, "ramoops", -1,
+							       pdata, sizeof(*pdata));
+	if (IS_ERR(qcom_rdd->ramoops_pdev)) {
+		ret = PTR_ERR(qcom_rdd->ramoops_pdev);
+		dev_err(&pdev->dev, "could not create platform device: %ld\n", ret);
+		qcom_rdd->ramoops_pdev = NULL;
+	}
+	platform_set_drvdata(pdev, qcom_rdd);
+
+	return ret;
+}
+
+static void qcom_ramoops_remove(struct platform_device *pdev)
+{
+	struct qcom_ramoops_dd *qcom_rdd = platform_get_drvdata(pdev);
+
+	platform_device_unregister(qcom_rdd->ramoops_pdev);
+	qcom_rdd->ramoops_pdev = NULL;
+}
+
+static const struct of_device_id qcom_ramoops_of_match[] = {
+	{ .compatible = "qcom,ramoops"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_ramoops_of_match);
+
+static struct platform_driver qcom_ramoops_drv = {
+	.driver		= {
+		.name	= "qcom,ramoops",
+		.of_match_table = qcom_ramoops_of_match,
+	},
+	.probe = qcom_ramoops_probe,
+	.remove_new = qcom_ramoops_remove,
+};
+
+module_platform_driver(qcom_ramoops_drv);
+
+MODULE_DESCRIPTION("Qualcomm ramoops minidump driver");
+MODULE_LICENSE("GPL");