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;
MIME-Version: 1.0
References: <1687955688-20809-1-git-send-email-quic_mojha@quicinc.com> <1687955688-20809-11-git-send-email-quic_mojha@quicinc.com>
In-Reply-To: <1687955688-20809-11-git-send-email-quic_mojha@quicinc.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Wed, 28 Jun 2023 16:57:20 -0600
Message-ID: <CAL_Jsq+O70mnreuS1m54RKM+uZu_z1L87RT8sKBYEw5uvowGJg@mail.gmail.com>
Subject: Re: [PATCH v4 10/21] soc: qcom: Add qcom's pstore minidump driver support
To:     Mukesh Ojha <quic_mojha@quicinc.com>
Cc:     corbet@lwn.net, agross@kernel.org, andersson@kernel.org,
        konrad.dybcio@linaro.org, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, keescook@chromium.org, tony.luck@intel.com,
        gpiccoli@igalia.com, mathieu.poirier@linaro.org,
        catalin.marinas@arm.com, will@kernel.org, linus.walleij@linaro.org,
        andy.shevchenko@gmail.com, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-hardening@vger.kernel.org,
        linux-remoteproc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jun 28, 2023 at 6:37=E2=80=AFAM 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 impl=
emented
>           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 an=
ywhere
> +         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                 +=3D ice.o
>  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)        +=3D qcom_ice.o
>  obj-$(CONFIG_QCOM_MINIDUMP) +=3D qcom_minidump.o
>  obj-$(CONFIG_QCOM_MINIDUMP_SMEM) +=3D qcom_minidump_smem.o
> +obj-$(CONFIG_QCOM_PSTORE_MINIDUMP) +=3D qcom_pstore_minidump.o
> diff --git a/drivers/soc/qcom/qcom_pstore_minidump.c b/drivers/soc/qcom/q=
com_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 reserv=
ed.
> + */
> +
> +#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 =3D 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 =3D of_parse_phandle(of_node, "memory-region", 0);
> +       if (!node)
> +               return -ENODEV;
> +
> +       rmem =3D 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 =3D devm_kzalloc(&pdev->dev, sizeof(*qcom_rdd), GFP_KERN=
EL);
> +       if (!qcom_rdd)
> +               return -ENOMEM;
> +
> +       pdata =3D &qcom_rdd->qcom_ramoops_pdata;
> +       pdata->mem_size =3D rmem->size;
> +       pdata->mem_address =3D rmem->base;
> +       ramoops_parse_dt(pdev, pdata);
> +
> +       qcom_rdd->ramoops_pdev =3D platform_device_register_data(NULL, "r=
amoops", -1,
> +                                                              pdata, siz=
eof(*pdata));
> +       if (IS_ERR(qcom_rdd->ramoops_pdev)) {
> +               ret =3D PTR_ERR(qcom_rdd->ramoops_pdev);
> +               dev_err(&pdev->dev, "could not create platform device: %l=
d\n", ret);
> +               qcom_rdd->ramoops_pdev =3D NULL;
> +       }
> +       platform_set_drvdata(pdev, qcom_rdd);
> +
> +       return ret;
> +}
> +
> +static void qcom_ramoops_remove(struct platform_device *pdev)
> +{
> +       struct qcom_ramoops_dd *qcom_rdd =3D platform_get_drvdata(pdev);
> +
> +       platform_device_unregister(qcom_rdd->ramoops_pdev);
> +       qcom_rdd->ramoops_pdev =3D NULL;
> +}
> +
> +static const struct of_device_id qcom_ramoops_of_match[] =3D {
> +       { .compatible =3D "qcom,ramoops"},
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_ramoops_of_match);
> +
> +static struct platform_driver qcom_ramoops_drv =3D {
> +       .driver         =3D {
> +               .name   =3D "qcom,ramoops",
> +               .of_match_table =3D qcom_ramoops_of_match,
> +       },
> +       .probe =3D qcom_ramoops_probe,
> +       .remove_new =3D qcom_ramoops_remove,
> +};
> +
> +module_platform_driver(qcom_ramoops_drv);
> +
> +MODULE_DESCRIPTION("Qualcomm ramoops minidump driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>