From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> To: Po-Kai Chi <pk.chi@mediatek.com>, Matthias Brugger <matthias.bgg@gmail.com> Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org, wsd_upstream@mediatek.com, CC Hwang <cc.hwang@mediatek.com> Subject: Re: [PATCH v2 2/4] memory: mediatek: add DRAM controller driver Date: Tue, 4 May 2021 10:05:40 -0400 [thread overview] Message-ID: <6f7472a2-3be2-e1dc-8e0f-83f6b1cdf61f@canonical.com> (raw) In-Reply-To: <1618565538-6972-3-git-send-email-pk.chi@mediatek.com> On 16/04/2021 05:32, Po-Kai Chi wrote: > MediaTek DRAM controller (DRAMC) driver provides cross-platform features > as below: > > 1. provide APIs for low power feature queries > 2. create sysfs to pass the DRAM information to user-space > > Signed-off-by: Po-Kai Chi <pk.chi@mediatek.com> > --- > drivers/memory/Kconfig | 1 + > drivers/memory/Makefile | 1 + > drivers/memory/mediatek/Kconfig | 9 + > drivers/memory/mediatek/Makefile | 3 + > drivers/memory/mediatek/mtk-dramc.c | 711 +++++++++++++++++++++++++++++++++++ > include/memory/mediatek/dramc.h | 18 + > 6 files changed, 743 insertions(+) > create mode 100644 drivers/memory/mediatek/Kconfig > create mode 100644 drivers/memory/mediatek/Makefile > create mode 100644 drivers/memory/mediatek/mtk-dramc.c > create mode 100644 include/memory/mediatek/dramc.h > > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig > index 72c0df1..056e906 100644 > --- a/drivers/memory/Kconfig > +++ b/drivers/memory/Kconfig > @@ -225,6 +225,7 @@ config STM32_FMC2_EBI > devices (like SRAM, ethernet adapters, FPGAs, LCD displays, ...) on > SOCs containing the FMC2 External Bus Interface. > > +source "drivers/memory/mediatek/Kconfig" Please first group existing Mediatek driver there. It's messy. > source "drivers/memory/samsung/Kconfig" > source "drivers/memory/tegra/Kconfig" > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > index bc7663e..cd4f8cf 100644 > --- a/drivers/memory/Makefile > +++ b/drivers/memory/Makefile > @@ -25,6 +25,7 @@ obj-$(CONFIG_PL353_SMC) += pl353-smc.o > obj-$(CONFIG_RENESAS_RPCIF) += renesas-rpc-if.o > obj-$(CONFIG_STM32_FMC2_EBI) += stm32-fmc2-ebi.o > > +obj-$(CONFIG_MTK_DRAMC) += mediatek/ > obj-$(CONFIG_SAMSUNG_MC) += samsung/ > obj-$(CONFIG_TEGRA_MC) += tegra/ > obj-$(CONFIG_TI_EMIF_SRAM) += ti-emif-sram.o > diff --git a/drivers/memory/mediatek/Kconfig b/drivers/memory/mediatek/Kconfig > new file mode 100644 > index 0000000..a1618b0 > --- /dev/null > +++ b/drivers/memory/mediatek/Kconfig > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +config MTK_DRAMC > + tristate "MediaTek DRAMC driver" > + help > + This selects the MediaTek(R) DRAMC driver. > + Provide the API for DRAMC low power scenario, and the interface > + for reporting DRAM information, e.g. DRAM mode register (MR) for > + DRAM vendor ID, temperature, and density. > diff --git a/drivers/memory/mediatek/Makefile b/drivers/memory/mediatek/Makefile > new file mode 100644 > index 0000000..632be48 > --- /dev/null > +++ b/drivers/memory/mediatek/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_MTK_DRAMC) += mtk-dramc.o > diff --git a/drivers/memory/mediatek/mtk-dramc.c b/drivers/memory/mediatek/mtk-dramc.c > new file mode 100644 > index 0000000..155b3b7 > --- /dev/null > +++ b/drivers/memory/mediatek/mtk-dramc.c > @@ -0,0 +1,711 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/printk.h> > +#include <linux/slab.h> > +#include <linux/io.h> > +#include <memory/mediatek/dramc.h> > + > +#define DRAMC_DRV_NAME "mtk-dramc" What does this define bring? What's the benefit? > + > +struct mr_info_t { > + unsigned int mr_index; > + unsigned int mr_value; > +}; > + > +/* (...) > + > +static struct platform_driver dramc_drv = { > + .probe = dramc_probe, > + .remove = dramc_remove, > + .driver = { > + .name = DRAMC_DRV_NAME, > + .owner = THIS_MODULE, NAK, this is so old mistake... No point to review - run Smatch, sparse, checkpatch and coccinelle. Fix all the errors and then resubmit. > + .of_match_table = mtk_dramc_of_ids, > + }, > +}; > + > +static int __init dramc_drv_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&dramc_drv); > + if (ret) { > + pr_info("%s: init fail, ret 0x%x\n", __func__, ret); > + return ret; > + } > + > + return ret; > +} > + > +static void __exit dramc_drv_exit(void) > +{ > + platform_driver_unregister(&dramc_drv); > +} > + > +module_init(dramc_drv_init); > +module_exit(dramc_drv_exit); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("MediaTek DRAMC Driver"); > +MODULE_AUTHOR("Po-Kai Chi <pk.chi@mediatek.com>"); > diff --git a/include/memory/mediatek/dramc.h b/include/memory/mediatek/dramc.h > new file mode 100644 > index 0000000..c8d200f > --- /dev/null > +++ b/include/memory/mediatek/dramc.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#ifndef __DRAMC_H__ Extend the header guard - MEMORY_MEDIATEK_DRAMC > +#define __DRAMC_H__ Best regards, Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> To: Po-Kai Chi <pk.chi@mediatek.com>, Matthias Brugger <matthias.bgg@gmail.com> Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org, wsd_upstream@mediatek.com, CC Hwang <cc.hwang@mediatek.com> Subject: Re: [PATCH v2 2/4] memory: mediatek: add DRAM controller driver Date: Tue, 4 May 2021 10:05:40 -0400 [thread overview] Message-ID: <6f7472a2-3be2-e1dc-8e0f-83f6b1cdf61f@canonical.com> (raw) In-Reply-To: <1618565538-6972-3-git-send-email-pk.chi@mediatek.com> On 16/04/2021 05:32, Po-Kai Chi wrote: > MediaTek DRAM controller (DRAMC) driver provides cross-platform features > as below: > > 1. provide APIs for low power feature queries > 2. create sysfs to pass the DRAM information to user-space > > Signed-off-by: Po-Kai Chi <pk.chi@mediatek.com> > --- > drivers/memory/Kconfig | 1 + > drivers/memory/Makefile | 1 + > drivers/memory/mediatek/Kconfig | 9 + > drivers/memory/mediatek/Makefile | 3 + > drivers/memory/mediatek/mtk-dramc.c | 711 +++++++++++++++++++++++++++++++++++ > include/memory/mediatek/dramc.h | 18 + > 6 files changed, 743 insertions(+) > create mode 100644 drivers/memory/mediatek/Kconfig > create mode 100644 drivers/memory/mediatek/Makefile > create mode 100644 drivers/memory/mediatek/mtk-dramc.c > create mode 100644 include/memory/mediatek/dramc.h > > diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig > index 72c0df1..056e906 100644 > --- a/drivers/memory/Kconfig > +++ b/drivers/memory/Kconfig > @@ -225,6 +225,7 @@ config STM32_FMC2_EBI > devices (like SRAM, ethernet adapters, FPGAs, LCD displays, ...) on > SOCs containing the FMC2 External Bus Interface. > > +source "drivers/memory/mediatek/Kconfig" Please first group existing Mediatek driver there. It's messy. > source "drivers/memory/samsung/Kconfig" > source "drivers/memory/tegra/Kconfig" > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > index bc7663e..cd4f8cf 100644 > --- a/drivers/memory/Makefile > +++ b/drivers/memory/Makefile > @@ -25,6 +25,7 @@ obj-$(CONFIG_PL353_SMC) += pl353-smc.o > obj-$(CONFIG_RENESAS_RPCIF) += renesas-rpc-if.o > obj-$(CONFIG_STM32_FMC2_EBI) += stm32-fmc2-ebi.o > > +obj-$(CONFIG_MTK_DRAMC) += mediatek/ > obj-$(CONFIG_SAMSUNG_MC) += samsung/ > obj-$(CONFIG_TEGRA_MC) += tegra/ > obj-$(CONFIG_TI_EMIF_SRAM) += ti-emif-sram.o > diff --git a/drivers/memory/mediatek/Kconfig b/drivers/memory/mediatek/Kconfig > new file mode 100644 > index 0000000..a1618b0 > --- /dev/null > +++ b/drivers/memory/mediatek/Kconfig > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +config MTK_DRAMC > + tristate "MediaTek DRAMC driver" > + help > + This selects the MediaTek(R) DRAMC driver. > + Provide the API for DRAMC low power scenario, and the interface > + for reporting DRAM information, e.g. DRAM mode register (MR) for > + DRAM vendor ID, temperature, and density. > diff --git a/drivers/memory/mediatek/Makefile b/drivers/memory/mediatek/Makefile > new file mode 100644 > index 0000000..632be48 > --- /dev/null > +++ b/drivers/memory/mediatek/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_MTK_DRAMC) += mtk-dramc.o > diff --git a/drivers/memory/mediatek/mtk-dramc.c b/drivers/memory/mediatek/mtk-dramc.c > new file mode 100644 > index 0000000..155b3b7 > --- /dev/null > +++ b/drivers/memory/mediatek/mtk-dramc.c > @@ -0,0 +1,711 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/platform_device.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/of_device.h> > +#include <linux/printk.h> > +#include <linux/slab.h> > +#include <linux/io.h> > +#include <memory/mediatek/dramc.h> > + > +#define DRAMC_DRV_NAME "mtk-dramc" What does this define bring? What's the benefit? > + > +struct mr_info_t { > + unsigned int mr_index; > + unsigned int mr_value; > +}; > + > +/* (...) > + > +static struct platform_driver dramc_drv = { > + .probe = dramc_probe, > + .remove = dramc_remove, > + .driver = { > + .name = DRAMC_DRV_NAME, > + .owner = THIS_MODULE, NAK, this is so old mistake... No point to review - run Smatch, sparse, checkpatch and coccinelle. Fix all the errors and then resubmit. > + .of_match_table = mtk_dramc_of_ids, > + }, > +}; > + > +static int __init dramc_drv_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&dramc_drv); > + if (ret) { > + pr_info("%s: init fail, ret 0x%x\n", __func__, ret); > + return ret; > + } > + > + return ret; > +} > + > +static void __exit dramc_drv_exit(void) > +{ > + platform_driver_unregister(&dramc_drv); > +} > + > +module_init(dramc_drv_init); > +module_exit(dramc_drv_exit); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("MediaTek DRAMC Driver"); > +MODULE_AUTHOR("Po-Kai Chi <pk.chi@mediatek.com>"); > diff --git a/include/memory/mediatek/dramc.h b/include/memory/mediatek/dramc.h > new file mode 100644 > index 0000000..c8d200f > --- /dev/null > +++ b/include/memory/mediatek/dramc.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (c) 2021 MediaTek Inc. > + */ > + > +#ifndef __DRAMC_H__ Extend the header guard - MEMORY_MEDIATEK_DRAMC > +#define __DRAMC_H__ Best regards, Krzysztof _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2021-05-04 14:05 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-16 9:32 [PATCH v2] memory: mediatek: add DRAM controller driver Po-Kai Chi 2021-04-16 9:32 ` Po-Kai Chi 2021-04-16 9:32 ` [PATCH v2 1/4] dt-bindings: memory: Add binding for MediaTek DRAM Controller Po-Kai Chi 2021-04-16 9:32 ` Po-Kai Chi 2021-05-04 14:00 ` Krzysztof Kozlowski 2021-05-04 14:00 ` Krzysztof Kozlowski 2021-04-16 9:32 ` [PATCH v2 2/4] memory: mediatek: add DRAM controller driver Po-Kai Chi 2021-04-16 9:32 ` Po-Kai Chi 2021-04-20 18:28 ` Rob Herring 2021-04-20 18:28 ` Rob Herring 2021-04-27 5:13 ` Po-Kai Chi 2021-04-27 5:13 ` Po-Kai Chi 2021-05-04 14:05 ` Krzysztof Kozlowski [this message] 2021-05-04 14:05 ` Krzysztof Kozlowski 2021-04-16 9:32 ` [PATCH v2 3/4] arm64: dts: add DRAMC node for MT6779 Po-Kai Chi 2021-04-16 9:32 ` Po-Kai Chi 2021-04-16 9:32 ` [PATCH v2 4/4] arm64: defconfig: Enable MediaTek DRAMC common driver Po-Kai Chi 2021-04-16 9:32 ` Po-Kai Chi 2021-05-04 14:06 ` Krzysztof Kozlowski 2021-05-04 14:06 ` Krzysztof Kozlowski 2021-05-04 13:56 ` [PATCH v2] memory: mediatek: add DRAM controller driver Krzysztof Kozlowski 2021-05-04 13:56 ` Krzysztof Kozlowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6f7472a2-3be2-e1dc-8e0f-83f6b1cdf61f@canonical.com \ --to=krzysztof.kozlowski@canonical.com \ --cc=cc.hwang@mediatek.com \ --cc=devicetree@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=matthias.bgg@gmail.com \ --cc=pk.chi@mediatek.com \ --cc=wsd_upstream@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.