* Re: [PATCH 4/4] arm64: dts: mt6797: Fix mmsys node name
From: Chun-Kuang Hu @ 2020-05-19 15:06 UTC (permalink / raw)
To: matthias.bgg
Cc: Rob Herring, Matthias Brugger, Michael Turquette, Stephen Boyd,
Kate Stewart, devicetree, Greg Kroah-Hartman, linux-kernel,
moderated list:ARM/Mediatek SoC support, Linux ARM, mtk01761,
Thomas Gleixner, linux-clk, Allison Randal
In-Reply-To: <20200518113156.25009-4-matthias.bgg@kernel.org>
Hi, Matthias:
<matthias.bgg@kernel.org> 於 2020年5月18日 週一 下午7:33寫道：
>
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> Node names are supposed to match the class of the device. The
> mmsys node is a syscon as it provides more then just a clock controller.
> Update the name.
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> ---
>
> arch/arm64/boot/dts/mediatek/mt6797.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt6797.dtsi b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> index 136ef9527a0d..3efd032481ce 100644
> --- a/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt6797.dtsi
> @@ -233,7 +233,7 @@ uart3: serial@11005000 {
> status = "disabled";
> };
>
> - mmsys: mmsys_config@14000000 {
> + mmsys: syscon@14000000 {
> compatible = "mediatek,mt6797-mmsys", "syscon";
> reg = <0 0x14000000 0 0x1000>;
> #clock-cells = <1>;
> --
> 2.26.2
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 3/4] clk/soc: mediatek: mt6779: Bind clock driver from platform device
From: Chun-Kuang Hu @ 2020-05-19 15:10 UTC (permalink / raw)
To: matthias.bgg
Cc: Rob Herring, Matthias Brugger, Michael Turquette, Stephen Boyd,
Kate Stewart, devicetree, Greg Kroah-Hartman, linux-kernel,
moderated list:ARM/Mediatek SoC support, Linux ARM, mtk01761,
Thomas Gleixner, linux-clk, Allison Randal
In-Reply-To: <20200518113156.25009-3-matthias.bgg@kernel.org>
Hi, Matthias:
<matthias.bgg@kernel.org> 於 2020年5月18日 週一 下午7:33寫道：
>
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> The mmsys driver is now the top level entry point for the multimedia
> system (mmsys), we bind the clock driver by creating a platform device.
> We also bind the MediaTek DRM driver which is not yet implement and
> therefor will errror out for now.
>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
>
> drivers/clk/mediatek/clk-mt6779-mm.c | 9 ++-------
> drivers/soc/mediatek/mtk-mmsys.c | 8 ++++++++
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt6779-mm.c b/drivers/clk/mediatek/clk-mt6779-mm.c
> index fb5fbb8e3e41..059c1a41ac7a 100644
> --- a/drivers/clk/mediatek/clk-mt6779-mm.c
> +++ b/drivers/clk/mediatek/clk-mt6779-mm.c
> @@ -84,15 +84,11 @@ static const struct mtk_gate mm_clks[] = {
> GATE_MM1(CLK_MM_DISP_OVL_FBDC, "mm_disp_ovl_fbdc", "mm_sel", 16),
> };
>
> -static const struct of_device_id of_match_clk_mt6779_mm[] = {
> - { .compatible = "mediatek,mt6779-mmsys", },
> - {}
> -};
> -
> static int clk_mt6779_mm_probe(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> + struct device_node *node = dev->parent->of_node;
> struct clk_onecell_data *clk_data;
> - struct device_node *node = pdev->dev.of_node;
>
> clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
>
> @@ -106,7 +102,6 @@ static struct platform_driver clk_mt6779_mm_drv = {
> .probe = clk_mt6779_mm_probe,
> .driver = {
> .name = "clk-mt6779-mm",
> - .of_match_table = of_match_clk_mt6779_mm,
> },
> };
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index fee64c8d3020..dc15808cf3a3 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -88,6 +88,10 @@ static const struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
> .clk_driver = "clk-mt2712-mm",
> };
>
> +static const struct mtk_mmsys_driver_data mt6779_mmsys_driver_data = {
> + .clk_driver = "clk-mt6779-mm",
This patch looks good to me, but I've one question: why drm driver
just use single "mediatek-drm" for sub driver name, but clock driver
has different name for each SoC?
Could we just use single name for clock driver such as "mediatek-clk-mm"?
Regards,
Chun-Kuang.
> +};
> +
> static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
> .clk_driver = "clk-mt6797-mm",
> };
> @@ -343,6 +347,10 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
> .compatible = "mediatek,mt2712-mmsys",
> .data = &mt2712_mmsys_driver_data,
> },
> + {
> + .compatible = "mediatek,mt6779-mmsys",
> + .data = &mt6779_mmsys_driver_data,
> + },
> {
> .compatible = "mediatek,mt6797-mmsys",
> .data = &mt6797_mmsys_driver_data,
> --
> 2.26.2
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v8 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
From: kbuild test robot @ 2020-05-19 14:47 UTC (permalink / raw)
To: Ramuthevar,Vadivel MuruganX, linux-kernel, linux-mtd, devicetree
Cc: kbuild-all, miquel.raynal, richard, vigneshr, arnd,
brendanhiggins, tglx, boris.brezillon, anders.roxell, masonccyang
In-Reply-To: <20200519043750.47789-3-vadivel.muruganx.ramuthevar@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 3258 bytes --]
Hi "Ramuthevar,Vadivel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on soc/for-next linus/master linux/master v5.7-rc6 next-20200518]
[cannot apply to mtd/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Ramuthevar-Vadivel-MuruganX/mtd-rawnand-Add-NAND-controller-support-on-Intel-LGM-SoC/20200519-191652
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-allmodconfig (attached as .config)
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-193-gb8fad4bc-dirty
# save the attached .config to linux build tree
make C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/mtd/nand/raw/intel-nand-controller.c:439:24: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int val @@ got restricted __be3unsigned int val @@
drivers/mtd/nand/raw/intel-nand-controller.c:439:24: sparse: expected unsigned int val
>> drivers/mtd/nand/raw/intel-nand-controller.c:439:24: sparse: got restricted __be32 [usertype]
drivers/mtd/nand/raw/intel-nand-controller.c:441:24: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected unsigned int val @@ got restricted __be3unsigned int val @@
drivers/mtd/nand/raw/intel-nand-controller.c:441:24: sparse: expected unsigned int val
drivers/mtd/nand/raw/intel-nand-controller.c:441:24: sparse: got restricted __be32 [usertype]
vim +439 drivers/mtd/nand/raw/intel-nand-controller.c
420
421 static int ebu_nand_write_page_hwecc(struct nand_chip *chip, const u8 *buf,
422 int oob_required, int page)
423 {
424 struct mtd_info *mtd = nand_to_mtd(chip);
425 struct ebu_nand_controller *ebu_host = nand_get_controller_data(chip);
426 void __iomem *int_sta = ebu_host->hsnand + HSNAND_INT_STA;
427 int ret, val, x;
428
429 ebu_nand_trigger(ebu_host, page, NAND_CMD_SEQIN);
430
431 ret = ebu_dma_start(ebu_host, DMA_MEM_TO_DEV, buf, mtd->writesize);
432 if (ret)
433 return ret;
434
435 if (oob_required) {
436 const u8 *pdata;
437
438 pdata = chip->oob_poi;
> 439 writel(cpu_to_be32(*pdata++), ebu_host->hsnand + HSNAND_CMSG_0);
440
441 writel(cpu_to_be32(*pdata), ebu_host->hsnand + HSNAND_CMSG_1);
442 }
443
444 ret = readl_poll_timeout_atomic(int_sta, val,
445 !(val & HSNAND_INT_STA_WR_C), 10, 1000);
446 if (ret)
447 return -EIO;
448
449 x = readl(ebu_host->hsnand + HSNAND_CTL);
450 x &= ~HSNAND_CTL_GO;
451 writel(x, ebu_host->hsnand + HSNAND_CTL);
452
453 return 0;
454 }
455
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74014 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] hwmon: (ina2xx) Add support for ina260
From: Michal Simek @ 2020-05-19 15:28 UTC (permalink / raw)
To: Guenter Roeck, Michal Simek, Franz Forstmayr
Cc: Jean Delvare, Rob Herring, Mark Rutland, Jonathan Corbet,
linux-hwmon, devicetree, linux-kernel, linux-doc
In-Reply-To: <616f7b6e-0e11-7c76-3baa-5b90a0967a91@roeck-us.net>
On 19. 05. 20 16:14, Guenter Roeck wrote:
> On 5/18/20 10:21 PM, Michal Simek wrote:
>> On 26. 02. 20 3:16, Guenter Roeck wrote:
>>> On 2/24/20 3:26 PM, Franz Forstmayr wrote:
>>>> Add initial support for INA260 power monitor with integrated shunt.
>>>> Registers are different from other INA2xx devices, that's why a small
>>>> translation table is used.
>>>>
>>>> Signed-off-by: Franz Forstmayr <forstmayr.franz@gmail.com>
>>>
>>> I think the chip is sufficiently different to other chips that a separate
>>> driver would make much more sense than adding support to the existing
>>> driver.
>>> There is no calibration, registers are different, the retry logic is
>>> not needed. A new driver could use the with_info API and would be much
>>> simpler while at the same time not messing up the existing driver.
>>
>> Isn't it also better to switch to IIO framework?
>> As we discussed in past there are two ina226 drivers. One in hwmon and
>> second based on IIO framework (more advance one?) and would be good to
>> deprecate hwmon one.
>
> "More advanced" is relative. The ina2xx driver in iio doesn't support
> alert limits (which is queued in the hwmon driver for 5.8), and the
> iio->hwmon bridge doesn't support it either. On top of that, there are
> existing users of the hwmon driver, which would have to be converted
> first. As for ina260, it would be up to the implementer to determine
> if alert limit support is needed or not, and which API would be
> appropriate for the intended use case.
Good to know. If ina260 is done as separate driver I am fine with it.
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH 04/17] dt-bindings: mmc: renesas,sdhi: Document r8a7742 support
From: Ulf Hansson @ 2020-05-19 15:29 UTC (permalink / raw)
To: Lad Prabhakar
Cc: Geert Uytterhoeven, Jens Axboe, Rob Herring, Wolfram Sang,
Sergei Shtylyov, David S. Miller, Wim Van Sebroeck, Guenter Roeck,
linux-ide, DTML, Linux Kernel Mailing List, linux-i2c,
linux-mmc@vger.kernel.org, netdev, Linux-Renesas, linux-watchdog,
Prabhakar
In-Reply-To: <1589555337-5498-5-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Fri, 15 May 2020 at 17:09, Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Document SDHI controller for RZ/G1H (R8A7742) SoC, which is compatible
> with R-Car Gen2 SoC family.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Applied for next, thanks!
Kind regards
Uffe
> ---
> Documentation/devicetree/bindings/mmc/renesas,sdhi.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.txt b/Documentation/devicetree/bindings/mmc/renesas,sdhi.txt
> index e6cc478..0ca9a62 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.txt
> @@ -7,6 +7,7 @@ Required properties:
> "renesas,sdhi-r7s9210" - SDHI IP on R7S9210 SoC
> "renesas,sdhi-r8a73a4" - SDHI IP on R8A73A4 SoC
> "renesas,sdhi-r8a7740" - SDHI IP on R8A7740 SoC
> + "renesas,sdhi-r8a7742" - SDHI IP on R8A7742 SoC
> "renesas,sdhi-r8a7743" - SDHI IP on R8A7743 SoC
> "renesas,sdhi-r8a7744" - SDHI IP on R8A7744 SoC
> "renesas,sdhi-r8a7745" - SDHI IP on R8A7745 SoC
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine
From: Enric Balletbo Serra @ 2020-05-19 15:30 UTC (permalink / raw)
To: Roger Lu
Cc: Kevin Hilman, Rob Herring, Nicolas Boichat, Stephen Boyd,
Mark Rutland, Nishanth Menon, Angus Lin,
devicetree@vger.kernel.org, Linux PM list, linux-kernel,
Xiaoqing Liu, YT Lee, Fan Chen,
moderated list:ARM/Mediatek SoC support, HenryC Chen,
Charles Yang, Matthias Brugger, Linux ARM
In-Reply-To: <20200518092403.22647-4-roger.lu@mediatek.com>
Hi Roger,
Thank you for your patch. I have the feeling that this driver is
complex and difficult to follow and I am wondering if it wouldn't be
better if you can send a version that simply adds basic functionality
for now. Some comments below.
Missatge de Roger Lu <roger.lu@mediatek.com> del dia dl., 18 de maig
2020 a les 11:25:
>
> The SVS (Smart Voltage Scaling) engine is a piece
> of hardware which is used to calculate optimized
> voltage values of several power domains,
> e.g. CPU/GPU/CCI, according to chip process corner,
> temperatures, and other factors. Then DVFS driver
> could apply those optimized voltage values to reduce
> power consumption.
>
> Signed-off-by: Roger Lu <roger.lu@mediatek.com>
> ---
> drivers/power/avs/Kconfig | 10 +
> drivers/power/avs/Makefile | 1 +
> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++
> include/linux/power/mtk_svs.h | 23 +
> 4 files changed, 2153 insertions(+)
> create mode 100644 drivers/power/avs/mtk_svs.c
> create mode 100644 include/linux/power/mtk_svs.h
>
> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
> index cdb4237bfd02..67089ac6040e 100644
> --- a/drivers/power/avs/Kconfig
> +++ b/drivers/power/avs/Kconfig
> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN
> Say y here to enable support io domains on Rockchip SoCs. It is
> necessary for the io domain setting of the SoC to match the
> voltage supplied by the regulators.
> +
> +config MTK_SVS
> + bool "MediaTek Smart Voltage Scaling(SVS)"
Can't be this a module? Why? In such case, you should use tristate option
> + depends on POWER_AVS && MTK_EFUSE && NVMEM
> + help
> + The SVS engine is a piece of hardware which is used to calculate
> + optimized voltage values of several power domains, e.g.
> + CPU clusters/GPU/CCI, according to chip process corner, temperatures,
> + and other factors. Then DVFS driver could apply those optimized voltage
> + values to reduce power consumption.
> diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile
> index 9007d05853e2..231adf078582 100644
> --- a/drivers/power/avs/Makefile
> +++ b/drivers/power/avs/Makefile
> @@ -2,3 +2,4 @@
> obj-$(CONFIG_POWER_AVS_OMAP) += smartreflex.o
> obj-$(CONFIG_QCOM_CPR) += qcom-cpr.o
> obj-$(CONFIG_ROCKCHIP_IODOMAIN) += rockchip-io-domain.o
> +obj-$(CONFIG_MTK_SVS) += mtk_svs.o
Will this driver be SoC specific or the idea is to support different
SoCs? If the answer to the first question is yes, please name the file
with the SoC prefix (i.e mt8183_svs). However, If the answer to the
second question is yes, make sure you prefix common
functions/structs/defines with a generic prefix mtk_svs but use the
SoC prefix for the ones you expect will be different between SoC, i.e
mt8183_svs_. This helps the readability of the driver. Also, try to
avoid too generic names.
> diff --git a/drivers/power/avs/mtk_svs.c b/drivers/power/avs/mtk_svs.c
> new file mode 100644
> index 000000000000..a4083b3ef175
> --- /dev/null
> +++ b/drivers/power/avs/mtk_svs.c
> @@ -0,0 +1,2119 @@
> +// SPDX-License-Identifier: GPL-2.0
I suspect you want this only GPLv2 compliant. Use GPL-2.0-only
> +/*
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#define pr_fmt(fmt) "[mtk_svs] " fmt
I don't see any reason to use pr_fmt in this driver. Use dev_*
functions instead and remove the above.
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/kthread.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_opp.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/power/mtk_svs.h>
> +#include <linux/proc_fs.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/seq_file.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/thermal.h>
> +#include <linux/uaccess.h>
> +
> +/* svs 1-line sw id */
> +#define SVS_CPU_LITTLE BIT(0)
> +#define SVS_CPU_BIG BIT(1)
> +#define SVS_CCI BIT(2)
> +#define SVS_GPU BIT(3)
> +
> +/* svs bank mode support */
> +#define SVSB_MODE_ALL_DISABLE (0)
nit: SVS_BMODE_?
> +#define SVSB_MODE_INIT01 BIT(1)
> +#define SVSB_MODE_INIT02 BIT(2)
> +#define SVSB_MODE_MON BIT(3)
> +
> +/* svs bank init01 condition */
> +#define SVSB_INIT01_VOLT_IGNORE BIT(1)
> +#define SVSB_INIT01_VOLT_INC_ONLY BIT(2)
> +
> +/* svs bank common setting */
> +#define HIGH_TEMP_MAX (U32_MAX)
nit: SVS_*
> +#define RUNCONFIG_DEFAULT (0x80000000)
Btw, there is any public datasheet where I can see those addresses and
registers and bit fields?
> +#define DC_SIGNED_BIT (0x8000)
> +#define INTEN_INIT0x (0x00005f01)
> +#define INTEN_MONVOPEN (0x00ff0000)
> +#define SVSEN_OFF (0x0)
> +#define SVSEN_MASK (0x7)
> +#define SVSEN_INIT01 (0x1)
> +#define SVSEN_INIT02 (0x5)
> +#define SVSEN_MON (0x2)
> +#define INTSTS_MONVOP (0x00ff0000)
> +#define INTSTS_COMPLETE (0x1)
> +#define INTSTS_CLEAN (0x00ffffff)
> +
> +#define proc_fops_rw(name) \
> + static int name ## _proc_open(struct inode *inode, \
> + struct file *file) \
> + { \
> + return single_open(file, name ## _proc_show, \
> + PDE_DATA(inode)); \
> + } \
> + static const struct proc_ops name ## _proc_fops = { \
> + .proc_open = name ## _proc_open, \
> + .proc_read = seq_read, \
> + .proc_lseek = seq_lseek, \
> + .proc_release = single_release, \
> + .proc_write = name ## _proc_write, \
> + }
> +
> +#define proc_fops_ro(name) \
> + static int name ## _proc_open(struct inode *inode, \
> + struct file *file) \
> + { \
> + return single_open(file, name ## _proc_show, \
> + PDE_DATA(inode)); \
> + } \
> + static const struct proc_ops name ## _proc_fops = { \
> + .proc_open = name ## _proc_open, \
> + .proc_read = seq_read, \
> + .proc_lseek = seq_lseek, \
> + .proc_release = single_release, \
> + }
> +
> +#define proc_entry(name) {__stringify(name), &name ## _proc_fops}
> +
/proc is usually the old way of exporting files to userspace, so
unless you have a really good reason use sysfs instead, or even
better, if it is only for debug purposes use debugfs. Also, you should
document the entries in Documentation.
> +static DEFINE_SPINLOCK(mtk_svs_lock);
> +struct mtk_svs;
> +
> +enum svsb_phase {
nit: mtk_svs_bphase?
> + SVSB_PHASE_INIT01 = 0,
nit: SVS_BPHASE_?
> + SVSB_PHASE_INIT02,
> + SVSB_PHASE_MON,
> + SVSB_PHASE_ERROR,
> +};
> +
> +enum reg_index {
nit: svs_reg_index?
> + TEMPMONCTL0 = 0,
> + TEMPMONCTL1,
> + TEMPMONCTL2,
> + TEMPMONINT,
> + TEMPMONINTSTS,
> + TEMPMONIDET0,
> + TEMPMONIDET1,
> + TEMPMONIDET2,
> + TEMPH2NTHRE,
> + TEMPHTHRE,
> + TEMPCTHRE,
> + TEMPOFFSETH,
> + TEMPOFFSETL,
> + TEMPMSRCTL0,
> + TEMPMSRCTL1,
> + TEMPAHBPOLL,
> + TEMPAHBTO,
> + TEMPADCPNP0,
> + TEMPADCPNP1,
> + TEMPADCPNP2,
> + TEMPADCMUX,
> + TEMPADCEXT,
> + TEMPADCEXT1,
> + TEMPADCEN,
> + TEMPPNPMUXADDR,
> + TEMPADCMUXADDR,
> + TEMPADCEXTADDR,
> + TEMPADCEXT1ADDR,
> + TEMPADCENADDR,
> + TEMPADCVALIDADDR,
> + TEMPADCVOLTADDR,
> + TEMPRDCTRL,
> + TEMPADCVALIDMASK,
> + TEMPADCVOLTAGESHIFT,
> + TEMPADCWRITECTRL,
> + TEMPMSR0,
> + TEMPMSR1,
> + TEMPMSR2,
> + TEMPADCHADDR,
> + TEMPIMMD0,
> + TEMPIMMD1,
> + TEMPIMMD2,
> + TEMPMONIDET3,
> + TEMPADCPNP3,
> + TEMPMSR3,
> + TEMPIMMD3,
> + TEMPPROTCTL,
> + TEMPPROTTA,
> + TEMPPROTTB,
> + TEMPPROTTC,
> + TEMPSPARE0,
> + TEMPSPARE1,
> + TEMPSPARE2,
> + TEMPSPARE3,
> + TEMPMSR0_1,
> + TEMPMSR1_1,
> + TEMPMSR2_1,
> + TEMPMSR3_1,
> + DESCHAR,
> + TEMPCHAR,
> + DETCHAR,
> + AGECHAR,
> + DCCONFIG,
> + AGECONFIG,
> + FREQPCT30,
> + FREQPCT74,
> + LIMITVALS,
> + VBOOT,
> + DETWINDOW,
> + CONFIG,
> + TSCALCS,
> + RUNCONFIG,
> + SVSEN,
> + INIT2VALS,
> + DCVALUES,
> + AGEVALUES,
> + VOP30,
> + VOP74,
> + TEMP,
> + INTSTS,
> + INTSTSRAW,
> + INTEN,
> + CHKINT,
> + CHKSHIFT,
> + STATUS,
> + VDESIGN30,
> + VDESIGN74,
> + DVT30,
> + DVT74,
> + AGECOUNT,
> + SMSTATE0,
> + SMSTATE1,
> + CTL0,
> + DESDETSEC,
> + TEMPAGESEC,
> + CTRLSPARE0,
> + CTRLSPARE1,
> + CTRLSPARE2,
> + CTRLSPARE3,
> + CORESEL,
> + THERMINTST,
> + INTST,
> + THSTAGE0ST,
> + THSTAGE1ST,
> + THSTAGE2ST,
> + THAHBST0,
> + THAHBST1,
> + SPARE0,
> + SPARE1,
> + SPARE2,
> + SPARE3,
> + THSLPEVEB,
> + reg_num,
> +};
> +
> +static const u32 svs_regs_v2[] = {
Is this SoC specific or shared between SoCs?
> + [TEMPMONCTL0] = 0x000,
> + [TEMPMONCTL1] = 0x004,
> + [TEMPMONCTL2] = 0x008,
> + [TEMPMONINT] = 0x00c,
> + [TEMPMONINTSTS] = 0x010,
> + [TEMPMONIDET0] = 0x014,
> + [TEMPMONIDET1] = 0x018,
> + [TEMPMONIDET2] = 0x01c,
> + [TEMPH2NTHRE] = 0x024,
> + [TEMPHTHRE] = 0x028,
> + [TEMPCTHRE] = 0x02c,
> + [TEMPOFFSETH] = 0x030,
> + [TEMPOFFSETL] = 0x034,
> + [TEMPMSRCTL0] = 0x038,
> + [TEMPMSRCTL1] = 0x03c,
> + [TEMPAHBPOLL] = 0x040,
> + [TEMPAHBTO] = 0x044,
> + [TEMPADCPNP0] = 0x048,
> + [TEMPADCPNP1] = 0x04c,
> + [TEMPADCPNP2] = 0x050,
> + [TEMPADCMUX] = 0x054,
> + [TEMPADCEXT] = 0x058,
> + [TEMPADCEXT1] = 0x05c,
> + [TEMPADCEN] = 0x060,
> + [TEMPPNPMUXADDR] = 0x064,
> + [TEMPADCMUXADDR] = 0x068,
> + [TEMPADCEXTADDR] = 0x06c,
> + [TEMPADCEXT1ADDR] = 0x070,
> + [TEMPADCENADDR] = 0x074,
> + [TEMPADCVALIDADDR] = 0x078,
> + [TEMPADCVOLTADDR] = 0x07c,
> + [TEMPRDCTRL] = 0x080,
> + [TEMPADCVALIDMASK] = 0x084,
> + [TEMPADCVOLTAGESHIFT] = 0x088,
> + [TEMPADCWRITECTRL] = 0x08c,
> + [TEMPMSR0] = 0x090,
> + [TEMPMSR1] = 0x094,
> + [TEMPMSR2] = 0x098,
> + [TEMPADCHADDR] = 0x09c,
> + [TEMPIMMD0] = 0x0a0,
> + [TEMPIMMD1] = 0x0a4,
> + [TEMPIMMD2] = 0x0a8,
> + [TEMPMONIDET3] = 0x0b0,
> + [TEMPADCPNP3] = 0x0b4,
> + [TEMPMSR3] = 0x0b8,
> + [TEMPIMMD3] = 0x0bc,
> + [TEMPPROTCTL] = 0x0c0,
> + [TEMPPROTTA] = 0x0c4,
> + [TEMPPROTTB] = 0x0c8,
> + [TEMPPROTTC] = 0x0cc,
> + [TEMPSPARE0] = 0x0f0,
> + [TEMPSPARE1] = 0x0f4,
> + [TEMPSPARE2] = 0x0f8,
> + [TEMPSPARE3] = 0x0fc,
> + [TEMPMSR0_1] = 0x190,
> + [TEMPMSR1_1] = 0x194,
> + [TEMPMSR2_1] = 0x198,
> + [TEMPMSR3_1] = 0x1b8,
> + [DESCHAR] = 0xc00,
> + [TEMPCHAR] = 0xc04,
> + [DETCHAR] = 0xc08,
> + [AGECHAR] = 0xc0c,
> + [DCCONFIG] = 0xc10,
> + [AGECONFIG] = 0xc14,
> + [FREQPCT30] = 0xc18,
> + [FREQPCT74] = 0xc1c,
> + [LIMITVALS] = 0xc20,
> + [VBOOT] = 0xc24,
> + [DETWINDOW] = 0xc28,
> + [CONFIG] = 0xc2c,
> + [TSCALCS] = 0xc30,
> + [RUNCONFIG] = 0xc34,
> + [SVSEN] = 0xc38,
> + [INIT2VALS] = 0xc3c,
> + [DCVALUES] = 0xc40,
> + [AGEVALUES] = 0xc44,
> + [VOP30] = 0xc48,
> + [VOP74] = 0xc4c,
> + [TEMP] = 0xc50,
> + [INTSTS] = 0xc54,
> + [INTSTSRAW] = 0xc58,
> + [INTEN] = 0xc5c,
> + [CHKINT] = 0xc60,
> + [CHKSHIFT] = 0xc64,
> + [STATUS] = 0xc68,
> + [VDESIGN30] = 0xc6c,
> + [VDESIGN74] = 0xc70,
> + [DVT30] = 0xc74,
> + [DVT74] = 0xc78,
> + [AGECOUNT] = 0xc7c,
> + [SMSTATE0] = 0xc80,
> + [SMSTATE1] = 0xc84,
> + [CTL0] = 0xc88,
> + [DESDETSEC] = 0xce0,
> + [TEMPAGESEC] = 0xce4,
> + [CTRLSPARE0] = 0xcf0,
> + [CTRLSPARE1] = 0xcf4,
> + [CTRLSPARE2] = 0xcf8,
> + [CTRLSPARE3] = 0xcfc,
> + [CORESEL] = 0xf00,
> + [THERMINTST] = 0xf04,
> + [INTST] = 0xf08,
> + [THSTAGE0ST] = 0xf0c,
> + [THSTAGE1ST] = 0xf10,
> + [THSTAGE2ST] = 0xf14,
> + [THAHBST0] = 0xf18,
> + [THAHBST1] = 0xf1c,
> + [SPARE0] = 0xf20,
> + [SPARE1] = 0xf24,
> + [SPARE2] = 0xf28,
> + [SPARE3] = 0xf2c,
> + [THSLPEVEB] = 0xf30,
> +};
> +
> +struct thermal_parameter {
In general, not only in this struct, would be good have some
documentation to have a better undestanding of the fields. That makes
the job of the reviewer a bit easier.
> + int adc_ge_t;
> + int adc_oe_t;
> + int ge;
> + int oe;
> + int gain;
> + int o_vtsabb;
> + int o_vtsmcu1;
> + int o_vtsmcu2;
> + int o_vtsmcu3;
> + int o_vtsmcu4;
> + int o_vtsmcu5;
> + int degc_cali;
> + int adc_cali_en_t;
> + int o_slope;
> + int o_slope_sign;
> + int ts_id;
> +};
> +
> +struct svs_bank_ops {
> + void (*set_freqs_pct)(struct mtk_svs *svs);
> + void (*get_vops)(struct mtk_svs *svs);
> +};
> +
> +struct svs_bank {
> + struct svs_bank_ops *ops;
> + struct completion init_completion;
> + struct device *dev;
> + struct regulator *buck;
> + struct mutex lock; /* Lock to protect update voltage process */
> + bool suspended;
> + bool mtcmos_request;
> + s32 volt_offset;
> + u32 mode_support;
> + u32 opp_freqs[16];
> + u32 freqs_pct[16];
> + u32 opp_volts[16];
> + u32 init02_volts[16];
> + u32 volts[16];
> + u32 reg_data[3][reg_num];
> + u32 freq_base;
> + u32 vboot;
> + u32 volt_step;
> + u32 volt_base;
> + u32 init01_volt_flag;
> + u32 phase;
> + u32 vmax;
> + u32 vmin;
> + u32 bts;
> + u32 mts;
> + u32 bdes;
> + u32 mdes;
> + u32 mtdes;
> + u32 dcbdet;
> + u32 dcmdet;
> + u32 dthi;
> + u32 dtlo;
> + u32 det_window;
> + u32 det_max;
> + u32 age_config;
> + u32 age_voffset_in;
> + u32 agem;
> + u32 dc_config;
> + u32 dc_voffset_in;
> + u32 dvt_fixed;
> + u32 vco;
> + u32 chk_shift;
> + u32 svs_temp;
> + u32 upper_temp_bound;
> + u32 lower_temp_bound;
> + u32 high_temp_threashold;
> + u32 high_temp_offset;
> + u32 low_temp_threashold;
> + u32 low_temp_offset;
> + u32 core_sel;
> + u32 opp_count;
> + u32 int_st;
> + u32 systemclk_en;
> + u32 sw_id;
> + u32 bank_id;
> + u32 ctl0;
> + u8 *of_compatible;
> + u8 *name;
> + u8 *tzone_name;
> + u8 *buck_name;
> +};
> +
> +struct svs_platform {
> + struct svs_bank *banks;
> + bool (*efuse_parsing)(struct mtk_svs *svs);
> + bool fake_efuse;
> + bool need_hw_reset;
> + const u32 *regs;
> + unsigned long irqflags;
> + u32 bank_num;
> + u32 efuse_num;
> + u32 efuse_check;
> + u32 thermal_efuse_num;
> + u8 *name;
> +};
> +
> +struct mtk_svs {
> + const struct svs_platform *platform;
> + struct svs_bank *bank;
> + struct device *dev;
> + void __iomem *base;
> + struct clk *main_clk;
> + u32 *efuse;
> + u32 *thermal_efuse;
> +};
> +
> +unsigned long claim_mtk_svs_lock(void)
> + __acquires(&mtk_svs_lock)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&mtk_svs_lock, flags);
> +
> + return flags;
> +}
> +EXPORT_SYMBOL_GPL(claim_mtk_svs_lock);
Is this used for an external module? AFAICS no, so no need to export
and you can replace directly the callers of this with the
spin_lock_irqsave directly. If you plan to use this in the future for
other modules, export when is really needed to be exported.
> +
> +void release_mtk_svs_lock(unsigned long flags)
> + __releases(&mtk_svs_lock)
> +{
> + spin_unlock_irqrestore(&mtk_svs_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(release_mtk_svs_lock);
> +
ditto
> +static u32 percent(u32 numerator, u32 denominator)
> +{
> + u32 pct;
> +
> + /* If not divide 1000, "numerator * 100" would be data overflow. */
> + numerator /= 1000;
> + denominator /= 1000;
> + pct = ((numerator * 100) + denominator - 1) / denominator;
> + pct &= GENMASK(7, 0);
> +
> + return pct;
> +}
> +
> +static u32 svs_readl(struct mtk_svs *svs, enum reg_index rg_i)
> +{
> + return readl(svs->base + svs->platform->regs[rg_i]);
> +}
> +
> +static void svs_writel(struct mtk_svs *svs, u32 val, enum reg_index rg_i)
> +{
> + writel(val, svs->base + svs->platform->regs[rg_i]);
> +}
> +
> +static void svs_switch_bank(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> +
> + svs_writel(svs, svsb->core_sel, CORESEL);
> +}
> +
> +static u32 svsb_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step,
> + u32 svsb_volt_base)
> +{
> + u32 u_volt;
> +
> + u_volt = (svsb_volt * svsb_volt_step) + svsb_volt_base;
> +
> + return u_volt;
> +}
> +
> +static int svsb_get_zone_temperature(struct svs_bank *svsb, int *tzone_temp)
> +{
> + struct thermal_zone_device *tzd;
> +
> + tzd = thermal_zone_get_zone_by_name(svsb->tzone_name);
> + if (IS_ERR(tzd))
> + return PTR_ERR(tzd);
> +
> + return thermal_zone_get_temp(tzd, tzone_temp);
> +}
> +
> +static int svsb_set_volts(struct svs_bank *svsb, bool force_update)
> +{
> + u32 i, svsb_volt, opp_volt, temp_offset = 0;
> + int tzone_temp, ret;
> +
> + mutex_lock(&svsb->lock);
> +
> + /*
> + * If bank is suspended, it means signed-off voltages are applied.
> + * Don't need to update opp voltage anymore.
> + */
> + if (svsb->suspended && !force_update) {
> + pr_notice("%s: bank is suspended\n", svsb->name);
Replace all pr_* for dev_* and Its better to be quiet, this messages
looks to me for a dev_dbg candidate, not a notice.
> + mutex_unlock(&svsb->lock);
> + return -EPERM;
> + }
> +
> + /* Get thermal effect */
> + if (svsb->phase == SVSB_PHASE_MON) {
> + if (svsb->svs_temp > svsb->upper_temp_bound &&
> + svsb->svs_temp < svsb->lower_temp_bound) {
> + pr_err("%s: svs_temp is abnormal (0x%x)?\n",
> + svsb->name, svsb->svs_temp);
dev_err and I am not sure if the error level is appropriate here
> + mutex_unlock(&svsb->lock);
> + return -EINVAL;
> + }
> +
> + ret = svsb_get_zone_temperature(svsb, &tzone_temp);
> + if (ret) {
> + pr_err("%s: cannot get zone \"%s\" temperature(%d)\n",
> + svsb->name, svsb->tzone_name, ret);
> + pr_err("%s: set signed-off voltage this time.\n",
> + svsb->name);
> + svsb->phase = SVSB_PHASE_ERROR;
> + }
> +
> + if (tzone_temp >= svsb->high_temp_threashold)
> + temp_offset += svsb->high_temp_offset;
> +
> + if (tzone_temp <= svsb->low_temp_threashold)
> + temp_offset += svsb->low_temp_offset;
> + }
> +
> + /* vmin <= svsb_volt (opp_volt) <= signed-off voltage */
> + for (i = 0; i < svsb->opp_count; i++) {
> + if (svsb->phase == SVSB_PHASE_MON) {
> + svsb_volt = max((svsb->volts[i] + svsb->volt_offset +
> + temp_offset), svsb->vmin);
> + opp_volt = svsb_volt_to_opp_volt(svsb_volt,
> + svsb->volt_step,
> + svsb->volt_base);
> + } else if (svsb->phase == SVSB_PHASE_INIT02) {
> + svsb_volt = max((svsb->init02_volts[i] +
> + svsb->volt_offset), svsb->vmin);
> + opp_volt = svsb_volt_to_opp_volt(svsb_volt,
> + svsb->volt_step,
> + svsb->volt_base);
> + } else if (svsb->phase == SVSB_PHASE_ERROR) {
> + opp_volt = svsb->opp_volts[i];
> + } else {
> + pr_err("%s: unknown phase: %u?\n",
> + svsb->name, svsb->phase);
I think that using a goto here and some places below will help the readability.
ret = -EINVAL;
goto unlock_mutex
[...]
unlock_mutex:
mutex_unlock(&svsb->lock);
return ret;
> + mutex_unlock(&svsb->lock);
> + return -EINVAL;
> + }
> +
> + opp_volt = min(opp_volt, svsb->opp_volts[i]);
> + ret = dev_pm_opp_adjust_voltage(svsb->dev, svsb->opp_freqs[i],
> + opp_volt, opp_volt,
> + svsb->opp_volts[i]);
> + if (ret) {
> + pr_err("%s: set voltage failed: %d\n", svsb->name, ret);
> + mutex_unlock(&svsb->lock);
> + return ret;
> + }
> + }
> +
> + mutex_unlock(&svsb->lock);
> +
> + return 0;
> +}
> +
> +static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx)
> +{
> + u32 vy;
> +
> + if (v0 == v1 || f0 == f1)
> + return v0;
> +
> + /* *100 to have decimal fraction factor, +99 for rounding up. */
> + vy = (v0 * 100) - ((((v0 - v1) * 100) / (f0 - f1)) * (f0 - fx));
> + vy = (vy + 99) / 100;
> +
> + return vy;
> +}
> +
> +static void svs_get_vops_v2(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + u32 temp, i;
> +
> + temp = svs_readl(svs, VOP30);
> + svsb->volts[6] = (temp >> 24) & GENMASK(7, 0);
> + svsb->volts[4] = (temp >> 16) & GENMASK(7, 0);
> + svsb->volts[2] = (temp >> 8) & GENMASK(7, 0);
> + svsb->volts[0] = (temp & GENMASK(7, 0));
> +
> + temp = svs_readl(svs, VOP74);
> + svsb->volts[14] = (temp >> 24) & GENMASK(7, 0);
> + svsb->volts[12] = (temp >> 16) & GENMASK(7, 0);
> + svsb->volts[10] = (temp >> 8) & GENMASK(7, 0);
> + svsb->volts[8] = (temp & GENMASK(7, 0));
> +
> + for (i = 0; i <= 7; i++) {
> + if (i < 7) {
> + svsb->volts[(i * 2) + 1] =
> + interpolate(svsb->freqs_pct[i * 2],
> + svsb->freqs_pct[(i + 1) * 2],
> + svsb->volts[i * 2],
> + svsb->volts[(i + 1) * 2],
> + svsb->freqs_pct[(i * 2) + 1]);
> + } else if (i == 7) {
> + svsb->volts[(i * 2) + 1] =
> + interpolate(svsb->freqs_pct[(i - 1) * 2],
> + svsb->freqs_pct[i * 2],
> + svsb->volts[(i - 1) * 2],
> + svsb->volts[i * 2],
> + svsb->freqs_pct[(i * 2) + 1]);
> + }
> + }
> +}
> +
> +static void svs_set_freqs_pct_v2(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> +
> + svs_writel(svs,
> + ((svsb->freqs_pct[6] << 24) & GENMASK(31, 24)) |
> + ((svsb->freqs_pct[4] << 16) & GENMASK(23, 16)) |
> + ((svsb->freqs_pct[2] << 8) & GENMASK(15, 8)) |
> + (svsb->freqs_pct[0] & GENMASK(7, 0)),
> + FREQPCT30);
> + svs_writel(svs,
> + ((svsb->freqs_pct[14] << 24) & GENMASK(31, 24)) |
> + ((svsb->freqs_pct[12] << 16) & GENMASK(23, 16)) |
> + ((svsb->freqs_pct[10] << 8) & GENMASK(15, 8)) |
> + ((svsb->freqs_pct[8]) & GENMASK(7, 0)),
> + FREQPCT74);
> +}
> +
> +static void svs_set_phase(struct mtk_svs *svs, u32 target_phase)
> +{
> + struct svs_bank *svsb = svs->bank;
> + u32 des_char, temp_char, det_char, limit_vals;
> + u32 init2vals, ts_calcs, val, filter, i;
> +
> + svs_switch_bank(svs);
> +
> + des_char = ((svsb->bdes << 8) & GENMASK(15, 8)) |
> + (svsb->mdes & GENMASK(7, 0));
> + svs_writel(svs, des_char, DESCHAR);
> +
> + temp_char = ((svsb->vco << 16) & GENMASK(23, 16)) |
> + ((svsb->mtdes << 8) & GENMASK(15, 8)) |
> + (svsb->dvt_fixed & GENMASK(7, 0));
> + svs_writel(svs, temp_char, TEMPCHAR);
> +
> + det_char = ((svsb->dcbdet << 8) & GENMASK(15, 8)) |
> + (svsb->dcmdet & GENMASK(7, 0));
> + svs_writel(svs, det_char, DETCHAR);
> +
> + svs_writel(svs, svsb->dc_config, DCCONFIG);
> + svs_writel(svs, svsb->age_config, AGECONFIG);
> +
> + if (!svsb->agem) {
> + svs_writel(svs, RUNCONFIG_DEFAULT, RUNCONFIG);
> + } else {
> + val = 0x0;
> +
> + for (i = 0; i < 24; i += 2) {
> + filter = 0x3 << i;
> +
> + if (!(svsb->age_config & filter))
> + val |= (0x1 << i);
> + else
> + val |= (svsb->age_config & filter);
> + }
> + svs_writel(svs, val, RUNCONFIG);
> + }
> +
> + svsb->ops->set_freqs_pct(svs);
> +
> + limit_vals = ((svsb->vmax << 24) & GENMASK(31, 24)) |
> + ((svsb->vmin << 16) & GENMASK(23, 16)) |
> + ((svsb->dthi << 8) & GENMASK(15, 8)) |
> + (svsb->dtlo & GENMASK(7, 0));
> + svs_writel(svs, limit_vals, LIMITVALS);
> + svs_writel(svs, (svsb->vboot & GENMASK(7, 0)), VBOOT);
> + svs_writel(svs, (svsb->det_window & GENMASK(15, 0)), DETWINDOW);
> + svs_writel(svs, (svsb->det_max & GENMASK(15, 0)), CONFIG);
> +
> + if (svsb->chk_shift)
> + svs_writel(svs, (svsb->chk_shift & GENMASK(7, 0)), CHKSHIFT);
> +
> + if (svsb->ctl0)
> + svs_writel(svs, svsb->ctl0, CTL0);
> +
> + svs_writel(svs, INTSTS_CLEAN, INTSTS);
> +
> + switch (target_phase) {
> + case SVSB_PHASE_INIT01:
> + svs_writel(svs, INTEN_INIT0x, INTEN);
> + svs_writel(svs, SVSEN_INIT01, SVSEN);
> + break;
> + case SVSB_PHASE_INIT02:
> + svs_writel(svs, INTEN_INIT0x, INTEN);
> + init2vals = ((svsb->age_voffset_in << 16) & GENMASK(31, 16)) |
> + (svsb->dc_voffset_in & GENMASK(15, 0));
> + svs_writel(svs, init2vals, INIT2VALS);
> + svs_writel(svs, SVSEN_INIT02, SVSEN);
> + break;
> + case SVSB_PHASE_MON:
> + ts_calcs = ((svsb->bts << 12) & GENMASK(23, 12)) |
> + (svsb->mts & GENMASK(11, 0));
> + svs_writel(svs, ts_calcs, TSCALCS);
> + svs_writel(svs, INTEN_MONVOPEN, INTEN);
> + svs_writel(svs, SVSEN_MON, SVSEN);
> + break;
> + default:
> + WARN_ON(1);
> + break;
> + }
> +}
> +
> +static inline void svs_init01_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_notice("%s: %s: VDN74:0x%08x, VDN30:0x%08x, DCVALUES:0x%08x\n",
> + svsb->name, __func__, svs_readl(svs, VDESIGN74),
> + svs_readl(svs, VDESIGN30), svs_readl(svs, DCVALUES));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVSB_PHASE_INIT01][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->phase = SVSB_PHASE_INIT01;
> + svsb->dc_voffset_in = ~(svs_readl(svs, DCVALUES) & GENMASK(15, 0)) + 1;
> + if (svsb->init01_volt_flag == SVSB_INIT01_VOLT_IGNORE)
> + svsb->dc_voffset_in = 0;
> + else if ((svsb->dc_voffset_in & DC_SIGNED_BIT) &&
> + (svsb->init01_volt_flag == SVSB_INIT01_VOLT_INC_ONLY))
> + svsb->dc_voffset_in = 0;
> +
> + svsb->age_voffset_in = svs_readl(svs, AGEVALUES) & GENMASK(15, 0);
> +
> + svs_writel(svs, SVSEN_OFF, SVSEN);
> + svs_writel(svs, INTSTS_COMPLETE, INTSTS);
> +
> + /* svs init01 clock gating */
> + svsb->core_sel &= ~svsb->systemclk_en;
> + complete(&svsb->init_completion);
> +}
> +
> +static inline void svs_init02_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_notice("%s: %s: VOP74:0x%08x, VOP30:0x%08x, DCVALUES:0x%08x\n",
> + svsb->name, __func__, svs_readl(svs, VOP74),
> + svs_readl(svs, VOP30), svs_readl(svs, DCVALUES));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVSB_PHASE_INIT02][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->phase = SVSB_PHASE_INIT02;
> + svsb->ops->get_vops(svs);
> + memcpy(svsb->init02_volts, svsb->volts, sizeof(u32) * svsb->opp_count);
> +
> + svs_writel(svs, SVSEN_OFF, SVSEN);
> + svs_writel(svs, INTSTS_COMPLETE, INTSTS);
> +
> + complete(&svsb->init_completion);
> +}
> +
> +static inline void svs_mon_mode_isr_handler(struct mtk_svs *svs)
> +{
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVSB_PHASE_MON][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->phase = SVSB_PHASE_MON;
> + svsb->svs_temp = svs_readl(svs, TEMP) & GENMASK(7, 0);
> + svsb->ops->get_vops(svs);
> +
> + svs_writel(svs, INTSTS_MONVOP, INTSTS);
> +}
> +
> +static inline void svs_error_isr_handler(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb = svs->bank;
> + enum reg_index rg_i;
> +
> + pr_err("%s(): %s(%s)", __func__, svsp->name, svsb->name);
> + pr_err("CORESEL(0x%x) = 0x%08x\n",
> + svsp->regs[CORESEL], svs_readl(svs, CORESEL)),
> + pr_err("SVSEN(0x%x) = 0x%08x, INTSTS(0x%x) = 0x%08x\n",
> + svsp->regs[SVSEN], svs_readl(svs, SVSEN),
> + svsp->regs[INTSTS], svs_readl(svs, INTSTS));
> + pr_err("SMSTATE0(0x%x) = 0x%08x, SMSTATE1(0x%x) = 0x%08x\n",
> + svsp->regs[SMSTATE0], svs_readl(svs, SMSTATE0),
> + svsp->regs[SMSTATE1], svs_readl(svs, SMSTATE1));
> + pr_err("TEMP(0x%x) = 0x%08x, TEMPMSR0(0x%x) = 0x%08x\n",
> + svsp->regs[TEMP], svs_readl(svs, TEMP),
> + svsp->regs[TEMPMSR0], svs_readl(svs, TEMPMSR0));
> + pr_err("TEMPMSR1(0x%x) = 0x%08x, TEMPMSR2(0x%x) = 0x%08x\n",
> + svsp->regs[TEMPMSR1], svs_readl(svs, TEMPMSR1),
> + svsp->regs[TEMPMSR2], svs_readl(svs, TEMPMSR2));
> + pr_err("TEMPMONCTL0(0x%x) = 0x%08x, TEMPMSRCTL1(0x%x) = 0x%08x\n",
> + svsp->regs[TEMPMONCTL0], svs_readl(svs, TEMPMONCTL0),
> + svsp->regs[TEMPMSRCTL1], svs_readl(svs, TEMPMSRCTL1));
> +
> + for (rg_i = TEMPMONCTL0; rg_i < reg_num; rg_i++)
> + svsb->reg_data[SVSB_PHASE_MON][rg_i] = svs_readl(svs, rg_i);
> +
> + svsb->mode_support = SVSB_MODE_ALL_DISABLE;
> +
> + if (svsb->phase != SVSB_PHASE_INIT01)
> + svsb->phase = SVSB_PHASE_ERROR;
> +
> + svs_writel(svs, SVSEN_OFF, SVSEN);
> + svs_writel(svs, INTSTS_CLEAN, INTSTS);
> +}
> +
> +static irqreturn_t svs_isr(int irq, void *data)
> +{
> + struct mtk_svs *svs = (struct mtk_svs *)data;
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb = NULL;
> + unsigned long flags;
> + u32 idx, int_sts, svs_en;
> +
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (svsb->int_st & svs_readl(svs, INTST))
> + continue;
> +
> + if (!svsb->suspended) {
> + svs_switch_bank(svs);
> + int_sts = svs_readl(svs, INTSTS);
> + svs_en = svs_readl(svs, SVSEN);
> +
> + if (int_sts == INTSTS_COMPLETE &&
> + ((svs_en & SVSEN_MASK) == SVSEN_INIT01))
> + svs_init01_isr_handler(svs);
> + else if ((int_sts == INTSTS_COMPLETE) &&
> + ((svs_en & SVSEN_MASK) == SVSEN_INIT02))
> + svs_init02_isr_handler(svs);
> + else if (!!(int_sts & INTSTS_MONVOP))
> + svs_mon_mode_isr_handler(svs);
> + else
> + svs_error_isr_handler(svs);
> + }
> +
> + break;
> + }
> + release_mtk_svs_lock(flags);
> +
> + if (svsb->phase != SVSB_PHASE_INIT01)
> + svsb_set_volts(svsb, false);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void svs_mon_mode(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags;
> + u32 idx;
> +
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!(svsb->mode_support & SVSB_MODE_MON))
> + continue;
> +
> + svs_set_phase(svs, SVSB_PHASE_MON);
> + }
> + release_mtk_svs_lock(flags);
> +}
> +
> +static int svs_init02(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags, time_left;
> + u32 idx;
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!(svsb->mode_support & SVSB_MODE_INIT02))
> + continue;
> +
> + reinit_completion(&svsb->init_completion);
> + flags = claim_mtk_svs_lock();
> + svs_set_phase(svs, SVSB_PHASE_INIT02);
> + release_mtk_svs_lock(flags);
> + time_left =
> + wait_for_completion_timeout(&svsb->init_completion,
> + msecs_to_jiffies(2000));
> + if (!time_left) {
> + pr_err("%s: init02 completion timeout\n", svsb->name);
> + return -EBUSY;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int svs_init01(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct pm_qos_request *qos_request;
> + unsigned long flags, time_left;
> + bool search_done;
> + int ret = -EPERM;
> + u32 opp_freqs, opp_vboot, buck_volt, idx, i;
> +
> + qos_request = kzalloc(sizeof(*qos_request), GFP_KERNEL);
> + if (!qos_request)
> + return -ENOMEM;
> +
> + /* Let CPUs leave idle-off state for initializing svs_init01. */
> + cpu_latency_qos_add_request(qos_request, 0);
> +
> + /*
> + * Sometimes two svs banks use the same buck.
> + * Therefore, we set each svs bank to vboot voltage first.
> + */
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + search_done = false;
> +
> + ret = regulator_set_mode(svsb->buck, REGULATOR_MODE_FAST);
> + if (ret)
> + pr_notice("%s: fail to set fast mode: %d\n",
> + svsb->name, ret);
> +
> + if (svsb->mtcmos_request) {
> + ret = regulator_enable(svsb->buck);
> + if (ret) {
> + pr_err("%s: fail to enable %s power: %d\n",
> + svsb->name, svsb->buck_name, ret);
> + goto init01_finish;
> + }
> +
> + ret = dev_pm_domain_attach(svsb->dev, false);
> + if (ret) {
> + pr_err("%s: attach pm domain fail: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> +
> + pm_runtime_enable(svsb->dev);
> + ret = pm_runtime_get_sync(svsb->dev);
> + if (ret < 0) {
> + pr_err("%s: turn mtcmos on fail: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> + }
> +
> + /*
> + * Find the fastest freq that can be run at vboot and
> + * fix to that freq until svs_init01 is done.
> + */
> + opp_vboot = svsb_volt_to_opp_volt(svsb->vboot,
> + svsb->volt_step,
> + svsb->volt_base);
> +
> + for (i = 0; i < svsb->opp_count; i++) {
> + opp_freqs = svsb->opp_freqs[i];
> + if (!search_done && svsb->opp_volts[i] <= opp_vboot) {
> + ret = dev_pm_opp_adjust_voltage(svsb->dev,
> + opp_freqs,
> + opp_vboot,
> + opp_vboot,
> + opp_vboot);
> + if (ret) {
> + pr_err("%s: set voltage failed: %d\n",
> + svsb->name, ret);
> + goto init01_finish;
> + }
> +
> + search_done = true;
> + } else {
> + dev_pm_opp_disable(svsb->dev,
> + svsb->opp_freqs[i]);
> + }
> + }
> + }
> +
> + /* svs bank init01 begins */
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> +
> + if (!(svsb->mode_support & SVSB_MODE_INIT01))
> + continue;
> +
> + opp_vboot = svsb_volt_to_opp_volt(svsb->vboot,
> + svsb->volt_step,
> + svsb->volt_base);
> +
> + buck_volt = regulator_get_voltage(svsb->buck);
> + if (buck_volt != opp_vboot) {
> + pr_err("%s: buck voltage: %u, expected vboot: %u\n",
> + svsb->name, buck_volt, opp_vboot);
> + ret = -EPERM;
> + goto init01_finish;
> + }
> +
> + init_completion(&svsb->init_completion);
> + flags = claim_mtk_svs_lock();
> + svs_set_phase(svs, SVSB_PHASE_INIT01);
> + release_mtk_svs_lock(flags);
> + time_left =
> + wait_for_completion_timeout(&svsb->init_completion,
> + msecs_to_jiffies(2000));
> + if (!time_left) {
> + pr_err("%s: init01 completion timeout\n", svsb->name);
> + ret = -EBUSY;
> + goto init01_finish;
> + }
> + }
> +
> +init01_finish:
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + for (i = 0; i < svsb->opp_count; i++)
> + dev_pm_opp_enable(svsb->dev, svsb->opp_freqs[i]);
> +
> + if (regulator_set_mode(svsb->buck, REGULATOR_MODE_NORMAL))
> + pr_notice("%s: fail to set normal mode: %d\n",
> + svsb->name, ret);
> +
> + if (svsb->mtcmos_request) {
> + if (pm_runtime_put_sync(svsb->dev))
> + pr_err("%s: turn mtcmos off fail: %d\n",
> + svsb->name, ret);
> + pm_runtime_disable(svsb->dev);
> + dev_pm_domain_detach(svsb->dev, 0);
> + if (regulator_disable(svsb->buck))
> + pr_err("%s: fail to disable %s power: %d\n",
> + svsb->name, svsb->buck_name, ret);
> + }
> + }
> +
> + cpu_latency_qos_remove_request(qos_request);
> + kfree(qos_request);
> +
> + return ret;
> +}
> +
> +static int svs_start(struct mtk_svs *svs)
> +{
> + int ret;
> +
> + ret = svs_init01(svs);
> + if (ret)
> + return ret;
> +
> + ret = svs_init02(svs);
> + if (ret)
> + return ret;
> +
> + svs_mon_mode(svs);
> +
> + return ret;
> +}
> +
> +static bool svs_mt8183_efuse_parsing(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct thermal_parameter tp;
> + struct svs_bank *svsb;
> + bool mon_mode_support = true;
> + int format[6], x_roomt[6], tb_roomt = 0;
> + struct nvmem_cell *cell;
> + size_t len;
> + u32 idx, i, ft_pgm, mts, temp0, temp1, temp2;
> +
> + if (svsp->fake_efuse) {
> + pr_notice("fake efuse\n");
> + svs->efuse[0] = 0x00310080;
> + svs->efuse[1] = 0xabfbf757;
> + svs->efuse[2] = 0x47c747c7;
> + svs->efuse[3] = 0xabfbf757;
> + svs->efuse[4] = 0xe7fca0ec;
> + svs->efuse[5] = 0x47bf4b88;
> + svs->efuse[6] = 0xabfb8fa5;
> + svs->efuse[7] = 0xabfb217b;
> + svs->efuse[8] = 0x4bf34be1;
> + svs->efuse[9] = 0xabfb670d;
> + svs->efuse[16] = 0xabfbc653;
> + svs->efuse[17] = 0x47f347e1;
> + svs->efuse[18] = 0xabfbd848;
> + }
> +
> + for (i = 0; i < svsp->efuse_num; i++)
> + if (svs->efuse[i])
> + pr_notice("M_HW_RES%d: 0x%08x\n", i, svs->efuse[i]);
> +
> + /* svs efuse parsing */
> + ft_pgm = (svs->efuse[0] >> 4) & GENMASK(3, 0);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + if (ft_pgm <= 1)
> + svsb->init01_volt_flag = SVSB_INIT01_VOLT_IGNORE;
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + svsb->bdes = svs->efuse[16] & GENMASK(7, 0);
> + svsb->mdes = (svs->efuse[16] >> 8) & GENMASK(7, 0);
> + svsb->dcbdet = (svs->efuse[16] >> 16) & GENMASK(7, 0);
> + svsb->dcmdet = (svs->efuse[16] >> 24) & GENMASK(7, 0);
> + svsb->mtdes = (svs->efuse[17] >> 16) & GENMASK(7, 0);
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 10;
> + else
> + svsb->volt_offset += 2;
> + break;
> + case SVS_CPU_BIG:
> + svsb->bdes = svs->efuse[18] & GENMASK(7, 0);
> + svsb->mdes = (svs->efuse[18] >> 8) & GENMASK(7, 0);
> + svsb->dcbdet = (svs->efuse[18] >> 16) & GENMASK(7, 0);
> + svsb->dcmdet = (svs->efuse[18] >> 24) & GENMASK(7, 0);
> + svsb->mtdes = svs->efuse[17] & GENMASK(7, 0);
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 15;
> + else
> + svsb->volt_offset += 12;
> + break;
> + case SVS_CCI:
> + svsb->bdes = svs->efuse[4] & GENMASK(7, 0);
> + svsb->mdes = (svs->efuse[4] >> 8) & GENMASK(7, 0);
> + svsb->dcbdet = (svs->efuse[4] >> 16) & GENMASK(7, 0);
> + svsb->dcmdet = (svs->efuse[4] >> 24) & GENMASK(7, 0);
> + svsb->mtdes = (svs->efuse[5] >> 16) & GENMASK(7, 0);
> +
> + if (ft_pgm <= 3)
> + svsb->volt_offset += 10;
> + else
> + svsb->volt_offset += 2;
> + break;
> + case SVS_GPU:
> + svsb->bdes = svs->efuse[6] & GENMASK(7, 0);
> + svsb->mdes = (svs->efuse[6] >> 8) & GENMASK(7, 0);
> + svsb->dcbdet = (svs->efuse[6] >> 16) & GENMASK(7, 0);
> + svsb->dcmdet = (svs->efuse[6] >> 24) & GENMASK(7, 0);
> + svsb->mtdes = svs->efuse[5] & GENMASK(7, 0);
> +
> + if (ft_pgm >= 2) {
> + svsb->freq_base = 800000000; /* 800MHz */
> + svsb->dvt_fixed = 2;
> + }
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (svsp->fake_efuse) {
> + svs->thermal_efuse[0] = 0x02873f69;
> + svs->thermal_efuse[1] = 0xa11d9142;
> + svs->thermal_efuse[2] = 0xa2526900;
> + } else {
> + /* Get thermal efuse by nvmem */
> + cell = nvmem_cell_get(svs->dev, "calibration-data");
> + if (IS_ERR(cell)) {
> + pr_err("no thermal efuse? disable mon mode\n");
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mode_support &= ~SVSB_MODE_MON;
> + }
> +
> + return true;
> + }
> +
> + svs->thermal_efuse = (u32 *)nvmem_cell_read(cell, &len);
> + nvmem_cell_put(cell);
> + }
> +
> + /* Thermal efuse parsing */
> + tp.adc_ge_t = (svs->thermal_efuse[1] >> 22) & GENMASK(9, 0);
> + tp.adc_oe_t = (svs->thermal_efuse[1] >> 12) & GENMASK(9, 0);
> +
> + tp.o_vtsmcu1 = (svs->thermal_efuse[0] >> 17) & GENMASK(8, 0);
> + tp.o_vtsmcu2 = (svs->thermal_efuse[0] >> 8) & GENMASK(8, 0);
> + tp.o_vtsmcu3 = svs->thermal_efuse[1] & GENMASK(8, 0);
> + tp.o_vtsmcu4 = (svs->thermal_efuse[2] >> 23) & GENMASK(8, 0);
> + tp.o_vtsmcu5 = (svs->thermal_efuse[2] >> 5) & GENMASK(8, 0);
> + tp.o_vtsabb = (svs->thermal_efuse[2] >> 14) & GENMASK(8, 0);
> +
> + tp.degc_cali = (svs->thermal_efuse[0] >> 1) & GENMASK(5, 0);
> + tp.adc_cali_en_t = svs->thermal_efuse[0] & BIT(0);
> + tp.o_slope_sign = (svs->thermal_efuse[0] >> 7) & BIT(0);
> +
> + tp.ts_id = (svs->thermal_efuse[1] >> 9) & BIT(0);
> + tp.o_slope = (svs->thermal_efuse[0] >> 26) & GENMASK(5, 0);
> +
> + if (tp.adc_cali_en_t == 1) {
> + if (!tp.ts_id)
> + tp.o_slope = 0;
> +
> + if ((tp.adc_ge_t < 265 || tp.adc_ge_t > 758) ||
> + (tp.adc_oe_t < 265 || tp.adc_oe_t > 758) ||
> + (tp.o_vtsmcu1 < -8 || tp.o_vtsmcu1 > 484) ||
> + (tp.o_vtsmcu2 < -8 || tp.o_vtsmcu2 > 484) ||
> + (tp.o_vtsmcu3 < -8 || tp.o_vtsmcu3 > 484) ||
> + (tp.o_vtsmcu4 < -8 || tp.o_vtsmcu4 > 484) ||
> + (tp.o_vtsmcu5 < -8 || tp.o_vtsmcu5 > 484) ||
> + (tp.o_vtsabb < -8 || tp.o_vtsabb > 484) ||
> + (tp.degc_cali < 1 || tp.degc_cali > 63)) {
> + pr_err("bad thermal efuse data, disable mon mode\n");
> + mon_mode_support = false;
> + }
> + } else {
> + pr_err("no thermal efuse data, disable mon mode\n");
> + mon_mode_support = false;
> + }
> +
> + if (!mon_mode_support) {
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mode_support &= ~SVSB_MODE_MON;
> + }
> +
> + return true;
> + }
> +
> + tp.ge = ((tp.adc_ge_t - 512) * 10000) / 4096;
> + tp.oe = (tp.adc_oe_t - 512);
> + tp.gain = (10000 + tp.ge);
> +
> + format[0] = (tp.o_vtsmcu1 + 3350 - tp.oe);
> + format[1] = (tp.o_vtsmcu2 + 3350 - tp.oe);
> + format[2] = (tp.o_vtsmcu3 + 3350 - tp.oe);
> + format[3] = (tp.o_vtsmcu4 + 3350 - tp.oe);
> + format[4] = (tp.o_vtsmcu5 + 3350 - tp.oe);
> + format[5] = (tp.o_vtsabb + 3350 - tp.oe);
> +
> + for (i = 0; i < 6; i++)
> + x_roomt[i] = (((format[i] * 10000) / 4096) * 10000) / tp.gain;
> +
> + temp0 = (10000 * 100000 / tp.gain) * 15 / 18;
> +
> + if (!tp.o_slope_sign)
> + mts = (temp0 * 10) / (1534 + tp.o_slope * 10);
> + else
> + mts = (temp0 * 10) / (1534 - tp.o_slope * 10);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->mts = mts;
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + tb_roomt = x_roomt[3];
> + break;
> + case SVS_CPU_BIG:
> + tb_roomt = x_roomt[4];
> + break;
> + case SVS_CCI:
> + tb_roomt = x_roomt[3];
> + break;
> + case SVS_GPU:
> + tb_roomt = x_roomt[1];
> + break;
> + default:
> + break;
> + }
> +
> + temp0 = (tp.degc_cali * 10 / 2);
> + temp1 = ((10000 * 100000 / 4096 / tp.gain) *
> + tp.oe + tb_roomt * 10) * 15 / 18;
> +
> + if (!tp.o_slope_sign)
> + temp2 = temp1 * 100 / (1534 + tp.o_slope * 10);
> + else
> + temp2 = temp1 * 100 / (1534 - tp.o_slope * 10);
> +
> + svsb->bts = (temp0 + temp2 - 250) * 4 / 10;
> + }
> +
> + return true;
> +}
> +
> +static bool svs_is_supported(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct nvmem_cell *cell;
> + size_t len;
> + bool ret;
> +
> + if (svsp->fake_efuse) {
> + len = svsp->efuse_num * sizeof(u32);
> + svs->efuse = devm_kzalloc(svs->dev, len, GFP_KERNEL);
> + if (!svs->efuse) {
> + pr_err("no memory for allocating svs_efuse\n");
> + return false;
> + }
> +
> + len = svsp->thermal_efuse_num * sizeof(u32);
> + svs->thermal_efuse = devm_kzalloc(svs->dev, len, GFP_KERNEL);
> + if (!svs->thermal_efuse) {
> + pr_err("no memory for allocating svs_thermal_efuse\n");
> + return false;
> + }
> +
> + goto svsp_efuse_parsing;
> + }
> +
> + /* Get svs efuse by nvmem */
> + cell = nvmem_cell_get(svs->dev, "svs-calibration-data");
> + if (IS_ERR(cell)) {
> + pr_err("no \"svs-calibration-data\" from dts? disable svs\n");
> + return false;
> + }
> +
> + svs->efuse = (u32 *)nvmem_cell_read(cell, &len);
> + nvmem_cell_put(cell);
> +
> + if (!svs->efuse[svsp->efuse_check]) {
> + pr_err("svs_efuse[%u] = 0x%x?\n",
> + svsp->efuse_check, svs->efuse[svsp->efuse_check]);
> + return false;
> + }
> +
> +svsp_efuse_parsing:
> + ret = svsp->efuse_parsing(svs);
> +
> + return ret;
> +}
> +
> +static int svs_resource_setup(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct platform_device *pdev;
> + struct device_node *np = NULL;
> + struct dev_pm_opp *opp;
> + unsigned long freq;
> + int count, ret;
> + u32 idx, i;
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + switch (svsb->sw_id) {
> + case SVS_CPU_LITTLE:
> + svsb->name = "SVS_CPU_LITTLE";
> + break;
> + case SVS_CPU_BIG:
> + svsb->name = "SVS_CPU_BIG";
> + break;
> + case SVS_CCI:
> + svsb->name = "SVS_CCI";
> + break;
> + case SVS_GPU:
> + svsb->name = "SVS_GPU";
> + break;
> + default:
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + /* Add svs bank device for opp-table/mtcmos/buck control */
> + pdev = platform_device_alloc(svsb->name, 0);
> + if (!pdev) {
> + pr_err("%s: fail to alloc pdev for svs_bank\n",
> + svsb->name);
> + return -ENOMEM;
> + }
> +
> + for_each_child_of_node(svs->dev->of_node, np) {
> + if (of_device_is_compatible(np, svsb->of_compatible)) {
> + pdev->dev.of_node = np;
> + break;
> + }
> + }
> +
> + ret = platform_device_add(pdev);
> + if (ret) {
> + pr_err("%s: fail to add svs_bank device: %d\n",
> + svsb->name, ret);
> + return ret;
> + }
> +
> + svsb->dev = &pdev->dev;
> + dev_set_drvdata(svsb->dev, svs);
> + ret = dev_pm_opp_of_add_table(svsb->dev);
> + if (ret) {
> + pr_err("%s: fail to add opp table: %d\n",
> + svsb->name, ret);
> + return ret;
> + }
> +
> + mutex_init(&svsb->lock);
> +
> + svsb->buck = devm_regulator_get_optional(svsb->dev,
> + svsb->buck_name);
> + if (IS_ERR(svsb->buck)) {
> + pr_err("%s: cannot get regulator \"%s-supply\"\n",
> + svsb->name, svsb->buck_name);
> + return PTR_ERR(svsb->buck);
> + }
> +
> + count = dev_pm_opp_get_opp_count(svsb->dev);
> + if (svsb->opp_count != count) {
> + pr_err("%s: opp_count not \"%u\" but get \"%d\"?\n",
> + svsb->name, svsb->opp_count, count);
> + return count;
> + }
> +
> + for (i = 0, freq = U32_MAX; i < svsb->opp_count; i++, freq--) {
> + opp = dev_pm_opp_find_freq_floor(svsb->dev, &freq);
> + if (IS_ERR(opp)) {
> + pr_err("%s: error opp entry!!, err = %ld\n",
> + svsb->name, PTR_ERR(opp));
> + return PTR_ERR(opp);
> + }
> +
> + svsb->opp_freqs[i] = freq;
> + svsb->opp_volts[i] = dev_pm_opp_get_voltage(opp);
> + svsb->freqs_pct[i] = percent(svsb->opp_freqs[i],
> + svsb->freq_base);
> + dev_pm_opp_put(opp);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int svs_suspend(struct device *dev)
> +{
> + struct mtk_svs *svs = dev_get_drvdata(dev);
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long flags;
> + int ret;
> + u32 idx;
> +
> + /* Wait if there is processing svs_isr(). Suspend all banks. */
> + flags = claim_mtk_svs_lock();
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svs->bank = svsb;
> + svs_switch_bank(svs);
> + svs_writel(svs, SVSEN_OFF, SVSEN);
> + svs_writel(svs, INTSTS_CLEAN, INTSTS);
> + svsb->suspended = true;
> + }
> + release_mtk_svs_lock(flags);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + if (svsb->phase != SVSB_PHASE_INIT01) {
> + svsb->phase = SVSB_PHASE_ERROR;
> + svsb_set_volts(svsb, true);
> + }
> + }
> +
> + if (svsp->need_hw_reset) {
> + ret = device_reset(svs->dev);
> + if (ret) {
> + pr_err("%s: ret = %d\n", __func__, ret);
> + return ret;
> + }
> + }
> +
> + clk_disable_unprepare(svs->main_clk);
> +
> + return 0;
> +}
> +
> +static int svs_resume(struct device *dev)
> +{
> + struct mtk_svs *svs = dev_get_drvdata(dev);
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + int ret;
> + u32 idx;
> +
> + ret = clk_prepare_enable(svs->main_clk);
> + if (ret) {
> + pr_err("cannot enable main_clk, disable svs\n");
> + return ret;
> + }
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> + svsb->suspended = false;
> + }
> +
> + ret = svs_init02(svs);
> + if (ret)
> + return ret;
> +
> + svs_mon_mode(svs);
> +
> + return 0;
> +}
> +
> +static int svs_debug_proc_show(struct seq_file *m, void *v)
> +{
> + struct svs_bank *svsb = (struct svs_bank *)m->private;
> +
> + if (svsb->phase == SVSB_PHASE_INIT01)
> + seq_puts(m, "init1\n");
> + else if (svsb->phase == SVSB_PHASE_INIT02)
> + seq_puts(m, "init2\n");
> + else if (svsb->phase == SVSB_PHASE_MON)
> + seq_puts(m, "mon mode\n");
> + else if (svsb->phase == SVSB_PHASE_ERROR)
> + seq_puts(m, "disabled\n");
> + else
> + seq_puts(m, "unknown\n");
> +
> + return 0;
> +}
> +
> +static ssize_t svs_debug_proc_write(struct file *file,
> + const char __user *buffer,
> + size_t count, loff_t *pos)
> +{
> + struct svs_bank *svsb = (struct svs_bank *)PDE_DATA(file_inode(file));
> + struct mtk_svs *svs = dev_get_drvdata(svsb->dev);
> + unsigned long flags;
> + int enabled, ret;
> + char *buf = NULL;
> +
> + if (count >= PAGE_SIZE)
> + return -EINVAL;
> +
> + buf = (char *)memdup_user_nul(buffer, count);
> + if (IS_ERR(buf))
> + return PTR_ERR(buf);
> +
> + ret = kstrtoint(buf, 10, &enabled);
> + if (ret)
> + return ret;
> +
> + if (!enabled) {
> + flags = claim_mtk_svs_lock();
> + svs->bank = svsb;
> + svsb->mode_support = SVSB_MODE_ALL_DISABLE;
> + svs_switch_bank(svs);
> + svs_writel(svs, SVSEN_OFF, SVSEN);
> + svs_writel(svs, INTSTS_CLEAN, INTSTS);
> + release_mtk_svs_lock(flags);
> +
> + svsb->phase = SVSB_PHASE_ERROR;
> + svsb_set_volts(svsb, true);
> + }
> +
> + kfree(buf);
> +
> + return count;
> +}
> +
> +proc_fops_rw(svs_debug);
> +
> +static int svs_dump_proc_show(struct seq_file *m, void *v)
> +{
> + struct mtk_svs *svs = (struct mtk_svs *)m->private;
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + unsigned long svs_reg_addr;
> + u32 idx, i, j;
> +
> + for (i = 0; i < svsp->efuse_num; i++)
> + if (svs->efuse && svs->efuse[i])
> + seq_printf(m, "M_HW_RES%d = 0x%08x\n",
> + i, svs->efuse[i]);
> +
> + for (i = 0; i < svsp->thermal_efuse_num; i++)
> + if (svs->thermal_efuse)
> + seq_printf(m, "THERMAL_EFUSE%d = 0x%08x\n",
> + i, svs->thermal_efuse[i]);
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + for (i = SVSB_PHASE_INIT01; i <= SVSB_PHASE_MON; i++) {
> + seq_printf(m, "Bank_number = %u\n", svsb->bank_id);
> +
> + if (i < SVSB_PHASE_MON)
> + seq_printf(m, "mode = init%d\n", i + 1);
> + else
> + seq_puts(m, "mode = mon\n");
> +
> + for (j = TEMPMONCTL0; j < reg_num; j++) {
> + svs_reg_addr = (unsigned long)(svs->base +
> + svsp->regs[j]);
> + seq_printf(m, "0x%08lx = 0x%08x\n",
> + svs_reg_addr, svsb->reg_data[i][j]);
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +proc_fops_ro(svs_dump);
> +
> +static int svs_status_proc_show(struct seq_file *m, void *v)
> +{
> + struct svs_bank *svsb = (struct svs_bank *)m->private;
> + struct dev_pm_opp *opp;
> + unsigned long freq;
> + int tzone_temp, ret;
> + u32 i;
> +
> + ret = svsb_get_zone_temperature(svsb, &tzone_temp);
> + if (ret)
> + seq_printf(m, "%s: cannot get zone \"%s\" temperature\n",
> + svsb->name, svsb->tzone_name);
> + else
> + seq_printf(m, "%s: temperature = %d\n", svsb->name, tzone_temp);
> +
> + for (i = 0, freq = U32_MAX; i < svsb->opp_count; i++, freq--) {
> + opp = dev_pm_opp_find_freq_floor(svsb->dev, &freq);
> + if (IS_ERR(opp)) {
> + seq_printf(m, "%s: error opp entry!!, err = %ld\n",
> + svsb->name, PTR_ERR(opp));
> + return PTR_ERR(opp);
> + }
> +
> + seq_printf(m, "opp_freqs[%02u]: %lu, volts[%02u]: %lu, ",
> + i, freq, i, dev_pm_opp_get_voltage(opp));
> + seq_printf(m, "svsb_volts[%02u]: 0x%x, freqs_pct[%02u]: %u\n",
> + i, svsb->volts[i], i, svsb->freqs_pct[i]);
> + dev_pm_opp_put(opp);
> + }
> +
> + return 0;
> +}
> +
> +proc_fops_ro(svs_status);
> +
> +static int svs_volt_offset_proc_show(struct seq_file *m, void *v)
> +{
> + struct svs_bank *svsb = (struct svs_bank *)m->private;
> +
> + seq_printf(m, "%d\n", svsb->volt_offset);
> +
> + return 0;
> +}
> +
> +static ssize_t svs_volt_offset_proc_write(struct file *file,
> + const char __user *buffer,
> + size_t count, loff_t *pos)
> +{
> + struct svs_bank *svsb = (struct svs_bank *)PDE_DATA(file_inode(file));
> + char *buf = NULL;
> + s32 volt_offset;
> +
> + if (count >= PAGE_SIZE)
> + return -EINVAL;
> +
> + buf = (char *)memdup_user_nul(buffer, count);
> + if (IS_ERR(buf))
> + return PTR_ERR(buf);
> +
> + if (!kstrtoint(buf, 10, &volt_offset)) {
> + svsb->volt_offset = volt_offset;
> + svsb_set_volts(svsb, true);
> + }
> +
> + kfree(buf);
> +
> + return count;
> +}
> +
> +proc_fops_rw(svs_volt_offset);
> +
> +static int svs_create_svs_procfs(struct mtk_svs *svs)
> +{
> + const struct svs_platform *svsp = svs->platform;
> + struct svs_bank *svsb;
> + struct proc_dir_entry *svs_dir, *bank_dir;
> + u32 idx, i;
> +
> + struct pentry {
> + const char *name;
> + const struct proc_ops *fops;
> + };
> +
> + struct pentry svs_entries[] = {
> + proc_entry(svs_dump),
> + };
> +
> + struct pentry bank_entries[] = {
> + proc_entry(svs_debug),
> + proc_entry(svs_status),
> + proc_entry(svs_volt_offset),
> + };
> +
> + svs_dir = proc_mkdir("svs", NULL);
> + if (!svs_dir) {
> + pr_err("mkdir /proc/svs failed\n");
> + return -EPERM;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(svs_entries); i++) {
> + if (!proc_create_data(svs_entries[i].name, 0664,
> + svs_dir, svs_entries[i].fops, svs)) {
> + pr_err("create /proc/svs/%s failed\n",
> + svs_entries[i].name);
> + return -EPERM;
> + }
> + }
> +
> + for (idx = 0; idx < svsp->bank_num; idx++) {
> + svsb = &svsp->banks[idx];
> +
> + if (svsb->mode_support == SVSB_MODE_ALL_DISABLE)
> + continue;
> +
> + bank_dir = proc_mkdir(svsb->name, svs_dir);
> + if (!bank_dir) {
> + pr_err("mkdir /proc/svs/%s failed\n", svsb->name);
> + return -EPERM;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(bank_entries); i++) {
> + if (!proc_create_data(bank_entries[i].name, 0664,
> + bank_dir, bank_entries[i].fops,
> + svsb)) {
> + pr_err("create /proc/svs/%s/%s failed\n",
> + svsb->name, bank_entries[i].name);
> + return -EPERM;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct svs_bank_ops svs_mt8183_banks_ops = {
> + .set_freqs_pct = svs_set_freqs_pct_v2,
> + .get_vops = svs_get_vops_v2,
> +};
> +
> +static struct svs_bank svs_mt8183_banks[4] = {
> + {
> + .of_compatible = "mediatek,mt8183-svs-cpu-little",
> + .sw_id = SVS_CPU_LITTLE,
> + .bank_id = 0,
> + .ops = &svs_mt8183_banks_ops,
> + .tzone_name = "tzts4",
> + .buck_name = "vcpu-little",
> + .mtcmos_request = false,
> + .init01_volt_flag = SVSB_INIT01_VOLT_INC_ONLY,
> + .mode_support = SVSB_MODE_INIT01 | SVSB_MODE_INIT02,
> + .opp_count = 16,
> + .freq_base = 1989000000,
> + .vboot = 0x30,
> + .volt_step = 6250,
> + .volt_base = 500000,
> + .volt_offset = 0,
> + .vmax = 0x64,
> + .vmin = 0x18,
> + .dthi = 0x1,
> + .dtlo = 0xfe,
> + .det_window = 0xa28,
> + .det_max = 0xffff,
> + .age_config = 0x555555,
> + .agem = 0,
> + .dc_config = 0x555555,
> + .dvt_fixed = 0x7,
> + .vco = 0x10,
> + .chk_shift = 0x77,
> + .upper_temp_bound = 0x64,
> + .lower_temp_bound = 0xb2,
> + .high_temp_threashold = HIGH_TEMP_MAX,
> + .low_temp_threashold = 25000,
> + .low_temp_offset = 0,
> + .core_sel = 0x8fff0000,
> + .systemclk_en = BIT(31),
> + .int_st = BIT(0),
> + .ctl0 = 0x00010001,
> + },
> + {
> + .of_compatible = "mediatek,mt8183-svs-cpu-big",
> + .sw_id = SVS_CPU_BIG,
> + .bank_id = 1,
> + .ops = &svs_mt8183_banks_ops,
> + .tzone_name = "tzts5",
> + .buck_name = "vcpu-big",
> + .mtcmos_request = false,
> + .init01_volt_flag = SVSB_INIT01_VOLT_INC_ONLY,
> + .mode_support = SVSB_MODE_INIT01 | SVSB_MODE_INIT02,
> + .opp_count = 16,
> + .freq_base = 1989000000,
> + .vboot = 0x30,
> + .volt_step = 6250,
> + .volt_base = 500000,
> + .volt_offset = 0,
> + .vmax = 0x58,
> + .vmin = 0x10,
> + .dthi = 0x1,
> + .dtlo = 0xfe,
> + .det_window = 0xa28,
> + .det_max = 0xffff,
> + .age_config = 0x555555,
> + .agem = 0,
> + .dc_config = 0x555555,
> + .dvt_fixed = 0x7,
> + .vco = 0x10,
> + .chk_shift = 0x77,
> + .upper_temp_bound = 0x64,
> + .lower_temp_bound = 0xb2,
> + .high_temp_threashold = HIGH_TEMP_MAX,
> + .low_temp_threashold = 25000,
> + .low_temp_offset = 0,
> + .core_sel = 0x8fff0001,
> + .systemclk_en = BIT(31),
> + .int_st = BIT(1),
> + .ctl0 = 0x00000001,
> + },
> + {
> + .of_compatible = "mediatek,mt8183-svs-cci",
> + .sw_id = SVS_CCI,
> + .bank_id = 2,
> + .ops = &svs_mt8183_banks_ops,
> + .tzone_name = "tzts4",
> + .buck_name = "vcci",
> + .mtcmos_request = false,
> + .init01_volt_flag = SVSB_INIT01_VOLT_INC_ONLY,
> + .mode_support = SVSB_MODE_INIT01 | SVSB_MODE_INIT02,
> + .opp_count = 16,
> + .freq_base = 1196000000,
> + .vboot = 0x30,
> + .volt_step = 6250,
> + .volt_base = 500000,
> + .volt_offset = 0,
> + .vmax = 0x64,
> + .vmin = 0x18,
> + .dthi = 0x1,
> + .dtlo = 0xfe,
> + .det_window = 0xa28,
> + .det_max = 0xffff,
> + .age_config = 0x555555,
> + .agem = 0,
> + .dc_config = 0x555555,
> + .dvt_fixed = 0x7,
> + .vco = 0x10,
> + .chk_shift = 0x77,
> + .upper_temp_bound = 0x64,
> + .lower_temp_bound = 0xb2,
> + .high_temp_threashold = HIGH_TEMP_MAX,
> + .low_temp_threashold = 25000,
> + .low_temp_offset = 0,
> + .core_sel = 0x8fff0002,
> + .systemclk_en = BIT(31),
> + .int_st = BIT(2),
> + .ctl0 = 0x00100003,
> + },
> + {
> + .of_compatible = "mediatek,mt8183-svs-gpu",
> + .sw_id = SVS_GPU,
> + .bank_id = 3,
> + .ops = &svs_mt8183_banks_ops,
> + .tzone_name = "tzts2",
> + .buck_name = "vgpu",
> + .mtcmos_request = true,
> + .init01_volt_flag = SVSB_INIT01_VOLT_INC_ONLY,
> + .mode_support = SVSB_MODE_INIT01 | SVSB_MODE_INIT02 |
> + SVSB_MODE_MON,
> + .opp_count = 16,
> + .freq_base = 900000000,
> + .vboot = 0x30,
> + .volt_step = 6250,
> + .volt_base = 500000,
> + .volt_offset = 0,
> + .vmax = 0x40,
> + .vmin = 0x14,
> + .dthi = 0x1,
> + .dtlo = 0xfe,
> + .det_window = 0xa28,
> + .det_max = 0xffff,
> + .age_config = 0x555555,
> + .agem = 0,
> + .dc_config = 0x555555,
> + .dvt_fixed = 0x3,
> + .vco = 0x10,
> + .chk_shift = 0x77,
> + .upper_temp_bound = 0x64,
> + .lower_temp_bound = 0xb2,
> + .high_temp_threashold = HIGH_TEMP_MAX,
> + .low_temp_threashold = 25000,
> + .low_temp_offset = 3,
> + .core_sel = 0x8fff0003,
> + .systemclk_en = BIT(31),
> + .int_st = BIT(3),
> + .ctl0 = 0x00050001,
> + },
> +};
> +
> +static const struct svs_platform svs_mt8183_platform = {
> + .name = "mt8183-svs",
> + .banks = svs_mt8183_banks,
> + .efuse_parsing = svs_mt8183_efuse_parsing,
> + .regs = svs_regs_v2,
> + .irqflags = IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + .need_hw_reset = false,
> + .fake_efuse = false,
> + .bank_num = 4,
> + .efuse_num = 25,
> + .efuse_check = 2,
> + .thermal_efuse_num = 3,
> +};
> +
> +static const struct of_device_id mtk_svs_of_match[] = {
> + {
> + .compatible = "mediatek,mt8183-svs",
> + .data = &svs_mt8183_platform,
> + }, {
> + /* Sentinel */
> + },
> +};
> +
> +static int svs_add_thermal_device_link(struct platform_device *svs_pdev)
> +{
> + struct device_node *therm_node;
> + struct platform_device *therm_pdev;
> + struct device_link *sup_link;
> + char const *therm_node_name[] = {"thermal"};
> + u32 i;
> +
> + for (i = 0; i < ARRAY_SIZE(therm_node_name); i++) {
> + therm_node = of_find_node_by_name(NULL, therm_node_name[i]);
> + if (therm_node)
> + break;
> + }
> +
> + if (!therm_node) {
> + pr_err("no available thermal node? pass device link\n");
> + return 0;
> + }
> +
> + therm_pdev = of_find_device_by_node(therm_node);
> + if (!therm_pdev) {
> + pr_err("no \"%pOF\" platform device? pass device link\n",
> + therm_node);
> + of_node_put(therm_node);
> + return 0;
> + }
> +
> + of_node_put(therm_node);
> +
> + sup_link = device_link_add(&svs_pdev->dev, &therm_pdev->dev,
> + DL_FLAG_AUTOREMOVE_CONSUMER);
> +
> + if (sup_link->status == DL_STATE_DORMANT)
> + return -EPROBE_DEFER;
> +
> + return 0;
> +}
> +
> +static int svs_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *of_dev_id;
> + struct mtk_svs *svs;
> + int ret;
> + u32 svs_irq;
> +
> + ret = svs_add_thermal_device_link(pdev);
> + if (ret)
> + return ret;
> +
> + svs = devm_kzalloc(&pdev->dev, sizeof(*svs), GFP_KERNEL);
> + if (!svs)
> + return -ENOMEM;
> +
> + svs->dev = &pdev->dev;
> + if (!svs->dev->of_node) {
> + pr_err("cannot find device node\n");
> + return -ENODEV;
> + }
> +
> + svs->base = of_iomap(svs->dev->of_node, 0);
> + if (IS_ERR(svs->base)) {
> + pr_err("cannot find svs register base\n");
> + return PTR_ERR(svs->base);
> + }
> +
> + of_dev_id = of_match_node(mtk_svs_of_match, svs->dev->of_node);
> + if (!of_dev_id || !of_dev_id->data)
> + return -EINVAL;
> +
> + svs->platform = of_dev_id->data;
> + dev_set_drvdata(svs->dev, svs);
> +
> + svs_irq = irq_of_parse_and_map(svs->dev->of_node, 0);
> + ret = devm_request_threaded_irq(svs->dev, svs_irq, NULL, svs_isr,
> + svs->platform->irqflags, "mtk-svs",
> + svs);
> + if (ret) {
> + pr_err("register irq(%d) failed: %d\n", svs_irq, ret);
> + return ret;
> + }
> +
> + svs->main_clk = devm_clk_get(svs->dev, "main");
> + if (IS_ERR(svs->main_clk)) {
> + pr_err("failed to get clock: %ld\n", PTR_ERR(svs->main_clk));
> + return PTR_ERR(svs->main_clk);
> + }
> +
> + ret = clk_prepare_enable(svs->main_clk);
> + if (ret) {
> + pr_err("cannot enable main clk: %d\n", ret);
> + return ret;
> + }
> +
> + if (!svs_is_supported(svs)) {
> + pr_notice("svs is not supported\n");
> + ret = -EPERM;
> + goto svs_probe_fail;
> + }
> +
> + ret = svs_resource_setup(svs);
> + if (ret)
> + goto svs_probe_fail;
> +
> + ret = svs_start(svs);
> + if (ret)
> + goto svs_probe_fail;
> +
> + ret = svs_create_svs_procfs(svs);
> + if (ret)
> + goto svs_probe_fail;
> +
> + return 0;
> +
> +svs_probe_fail:
> + clk_disable_unprepare(svs->main_clk);
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops svs_pm_ops = {
> + .suspend = svs_suspend,
> + .resume = svs_resume,
> +};
Better use the PM macros to define that struct
> +
> +static struct platform_driver svs_driver = {
> + .probe = svs_probe,
> + .driver = {
> + .name = "mtk-svs",
> + .pm = &svs_pm_ops,
> + .of_match_table = of_match_ptr(mtk_svs_of_match),
> + },
> +};
> +
> +static int __init svs_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&svs_driver);
> + if (ret) {
> + pr_err("svs platform driver register failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +late_initcall_sync(svs_init);
Why? Can't be this a module_platform_driver?
> +
> +MODULE_DESCRIPTION("MediaTek SVS Driver v1.0");
Don't put the version in the description, there is a MODULE_VERSION
for that purpose.
Also would be good have the MODULE_AUTHOR
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/power/mtk_svs.h b/include/linux/power/mtk_svs.h
> new file mode 100644
> index 000000000000..5c03982e3576
> --- /dev/null
> +++ b/include/linux/power/mtk_svs.h
I don't think this file is needed just remove it.
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2020 MediaTek Inc.
> + */
> +
> +#ifndef __MTK_SVS_H__
> +#define __MTK_SVS_H__
> +
> +#if IS_ENABLED(CONFIG_MTK_SVS)
> +unsigned long claim_mtk_svs_lock(void);
> +void release_mtk_svs_lock(unsigned long flags);
> +#else
> +static inline unsigned long claim_mtk_svs_lock(void)
> +{
> + return 0;
> +}
> +
> +static inline void release_mtk_svs_lock(unsigned long flags)
> +{
> +}
> +#endif /* CONFIG_MTK_SVS */
> +
> +#endif /* __MTK_SVS_H__ */
> --
> 2.18.0
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v2] arm: dts: Move am33xx and am43xx mmc nodes to sdhci-omap driver
From: Tony Lindgren @ 2020-05-19 15:48 UTC (permalink / raw)
To: Faiz Abbas
Cc: Keerthy, linux-kernel, devicetree, linux-omap, robh+dt, bcousson,
Tomi Valkeinen
In-Reply-To: <53c815db-dd7d-e6e1-f81a-cf05ef340c71@ti.com>
* Faiz Abbas <faiz_abbas@ti.com> [200519 08:23]:
> Tony,
>
> On 15/05/20 3:04 pm, Faiz Abbas wrote:
> > Hi Tony,
> >
> > On 13/05/20 9:53 pm, Tony Lindgren wrote:
> >> * Faiz Abbas <faiz_abbas@ti.com> [200512 13:39]:
> >>> Move mmc nodes to be compatible with the sdhci-omap driver. The following
> >>> modifications are required for omap_hsmmc specific properties:
> >>>
> >>> ti,non-removable: convert to the generic mmc non-removable
> >>> ti,needs-special-reset: co-opted into the sdhci-omap driver
> >>> ti,dual-volt: removed. Legacy property not used in am335x or am43xx
> >>> ti,needs-special-hs-handling: removed. Legacy property not used in am335x
> >>> or am43xx
> >>>
> >>> Also since the sdhci-omap driver does not support runtime PM, explicitly
> >>> disable the mmc3 instance in the dtsi.
> >>>
> >>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> >>> ---
> >>>
> >>> v2: Rebased to latest mainline where all kernel dependancies have been merged.
> >>>
> >>> Suspend/Resume is now supported in the sdhci-omap driver.
> >>
> >> Great, thanks for updating it.
> >>
> >> Keerthy, care to test for am3 and am4?
> >>
> >
> > Suspend/resume on am43xx-gpevm is broken right now in mainline and the regression looks
> > like it is caused by the display subsystem. I have reported this to Tomi and
> > its being investigated.
> >
> > Meanwhile I have tested this patch with display configs disabled and Keerthy's
> > suspend/resume tests pass on both am3 and am4.
OK great thanks for checking it. Do you have the display subsystem
related commit that broke PM? I'm wondering if my recent DSS platform
data removal changes might have caused the regression.
> Can this patch be picked up? I would really like this to be merged by v5.8
Yeah me too :) Looks good to me, planning on applying these today.
Thanks,
Tony
^ permalink raw reply
* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Thomas Bogendoerfer @ 2020-05-19 15:50 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
linux-kernel
In-Reply-To: <20200518205752.txbylbjt2zkwdwwe@mobilestation>
On Mon, May 18, 2020 at 11:57:52PM +0300, Serge Semin wrote:
> On Mon, May 18, 2020 at 06:32:06PM +0200, Thomas Bogendoerfer wrote:
> > On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> > > On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > > > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > > > Thomas,
> > > > > Could you take a look at my comment below so I could proceed with the
> > > > > patchset v3 development?
> > > >
> > > > I can't help, but using r4k clocksource with changing frequency is
> > > > probaly only usefull as a random generator. So IMHO the only two
> > > > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> > > >
> > > > Thomas.
> > >
> > > Thomas, could you proceed with the rest of the patches review?
> > > ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> > > ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
> >
> > both are not my call, but look ok to me.
>
> Can I add your Reviewed-by tag there then?
only for 16/20. 15/20 looks ok to me, but I have not enough insides
on the hardware to say this is good.
> > > ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
> >
> > that's broken. A reg shift of 2 doesn't mean we could use 32bit access
> > to the registers on other platforms. As I don't think adding some ifdefery
> > makes things nicer, just implement the your prom_putchar in board code.
>
> I thought about that initially, but then I decided to alter the generic
> early_printk_8250 code instead. My version of prom_putchar() would be almost
> the same as one implemented in the early_printk_8250 module except minor
> modification of replacing readb/writeb methods with readl/writel. So I didn't
> want to duplicate the code, but wanted to provide a general way to fix the
> problem potentially also for another platforms.
>
> Since you don't like this fix alternatively I'd suggest to add the reg_width
> parameter passed to the setup_8250_early_printk_port() method like this:
> -setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> - unsigned int timeout)
> +setup_8250_early_printk_port(unsigned long base, unsigned int reg_shift,
> + unsigned int reg_width, unsigned int timeout)
>
> By reg_width parameter we could determine the actual width of the register:
> static inline u8 serial_in(int offset)
> {
> - return readb(serial8250_base + (offset << serial8250_reg_shift));
> + u8 ret = 0xFF;
> +
> + offset <<= serial8250_reg_shift;
> + switch (serial8250_reg_width) {
> + case 1:
> + ret = readb(serial8250_base + offset);
> + break;
> + case 2:
> + ret = readw(serial8250_base + offset);
> + break;
> + case 4:
> + ret = readl(serial8250_base + offset);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> }
>
> The similar modification will be implemented for serial_out(). I'll also modify
look at the lines of code you are adding. Doing your own prom_putchar will
probably have less lines.
> What do you think about this?
please do your own prom_putchar.
> >
> > > ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
> >
> > looks ok so far.
>
> Can I add your Reviewed-by tag there then?
As I'm the maintainer of the part, I've simply applied it.
> >
> > > ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
> >
> > that is fine
>
> Can I add your Reviewed-by tag there then?
As this didn't apply cleanly, I'll apply it after you've resent it.
IMHO no need for a Reviewed-by.
> > > └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
> >
> > this is IMHO a dangerous change. Enabling write merging for any
> > CPU supporting it might triggers bugs. Do it in your board bringup
> > code and at the moment I don't see a reason for the rest of that
> > patch.
>
> Let's at least leave the mm_config() implementation but without the write-merge
> enabling by default. Providing features availability macro
> cpu_has_mm_sysad/cpu_has_mm_full and c0 config fields
do you have a user of that ? I'm not introducing code nobody uses.
> I could use them to implement a code pattern like:
>
> + if (cpu_has_mm_full) {
> + unsigned int config0 = read_c0_config();
> + config0 = (config0 & ~MIPS_CONF_MM) | MIPS_CONF_MM_FULL;
> + write_c0_config(config0);
> + }
you know you are running on a R5 core, so you know you have MM_FULL.
No need to check this.
> By doing so I can manually enable/disable the MM feature in the
> cpu-feature-overrides.h. Without that I'd have to locally define these macro,
> which isn't good seeing they are in fact generic and can be useful for other
> platforms with SYSAD and FULL MM feature available. What do you think?
To me this is a hardware feature I expect to be done by firmware and
Linux shouldn't care about it, if it doesn't have any software
implications.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v2 12/20] mips: MAAR: Add XPA mode support
From: Thomas Bogendoerfer @ 2020-05-19 15:42 UTC (permalink / raw)
To: Sergey.Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-pm, devicetree,
Greg Kroah-Hartman, Allison Randal, Thomas Gleixner, Zhou Yanjie,
Paul Cercueil, Jiaxun Yang, Mike Rapoport, Andrew Morton,
linux-mips, linux-kernel
In-Reply-To: <20200506174238.15385-13-Sergey.Semin@baikalelectronics.ru>
On Wed, May 06, 2020 at 08:42:30PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> When XPA mode is enabled the normally 32-bits MAAR pair registers
> are extended to be of 64-bits width as in pure 64-bits MIPS
> architecture. In this case the MAAR registers can enable the
> speculative loads/stores for addresses of up to 39-bits width.
> But in this case the process of the MAAR initialization changes a bit.
> The upper 32-bits of the registers are supposed to be accessed by mean
> of the dedicated instructions mfhc0/mthc0 and there is a CP0.MAAR.VH
> bit which should be set together with CP0.MAAR.VL as indication
> of the boundary validity. All of these peculiarities were taken into
> account in this commit so the speculative loads/stores would work
> when XPA mode is enabled.
>
> Co-developed-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-pm@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
> arch/mips/include/asm/maar.h | 17 +++++++++++++++--
> arch/mips/include/asm/mipsregs.h | 10 ++++++++++
> arch/mips/mm/init.c | 8 +++++++-
> 3 files changed, 32 insertions(+), 3 deletions(-)
applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml
From: Andy Shevchenko @ 2020-05-19 16:01 UTC (permalink / raw)
To: Jonathan Albrieux
Cc: Stephan Gerhold, linux-kernel, ~postmarketos/upstreaming,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jilayne Lovejoy,
Jonathan Cameron, Kate Stewart, Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron, Rob Herring
In-Reply-To: <20200519140354.GB30573@ict14-OptiPlex-980>
On Tue, May 19, 2020 at 04:03:54PM +0200, Jonathan Albrieux wrote:
> On Tue, May 19, 2020 at 03:22:07PM +0200, Stephan Gerhold wrote:
> > On Tue, May 19, 2020 at 02:43:51PM +0200, Jonathan Albrieux wrote:
...
> > > +maintainers:
> > > + - can't find a mantainer, author is Laxman Dewangan <ldewangan@nvidia.com>
> >
> > Should probably add someone here, although I'm not sure who either.
> >
>
> Yep I couldn't find a maintainer for that driver..what to do in this case?
Volunteer yourself!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v8 1/6] MIPS: JZ4780: Introduce SMP support.
From: Paul Cercueil @ 2020-05-19 16:09 UTC (permalink / raw)
To: 周琰杰
Cc: linux-mips, linux-kernel, devicetree, tsbogend, paulburton,
jiaxun.yang, chenhc, tglx, robh+dt, daniel.lezcano, keescook,
krzk, hns, ebiederm, dongsheng.qiu, yanfei.li, rick.tyliu,
sernia.zhou, zhenwenjin
In-Reply-To: <1589898923-60048-3-git-send-email-zhouyanjie@wanyeetech.com>
Hi Zhou,
Le mar. 19 mai 2020 à 22:35, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> Forward port smp support from kernel 3.18.3 of CI20_linux
> to upstream kernel 5.6.
>
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>
> Notes:
> v1->v2:
> 1.Remove unnecessary "plat_irq_dispatch(void)" in irq-ingenic.c.
> 2.Add a timeout check for "jz4780_boot_secondary()" to avoid a
> dead loop.
> 3.Replace hard code in smp.c with macro.
>
> v2->v3:
> 1.Remove unnecessary "extern void (*r4k_blast_dcache)(void)" in
> smp.c.
> 2.Use "for_each_of_cpu_node" instead "for_each_compatible_node"
> in smp.c.
> 3.Use "of_cpu_node_to_id" instead "of_property_read_u32_index" in
> smp.c.
> 4.Move LCR related operations to jz4780-cgu.c.
>
> v3->v4:
> Rebase on top of kernel 5.6-rc1.
>
> v4->v5:
> 1.Splitting changes involving "jz4780-cgu.c" into separate commit.
> 2.Use "request_irq()" replace "setup_irq()".
>
> v5->v6:
> In order to have a kernel that works on multiple SoCs at the same
> time, use "IS_ENABLED()" replace "#ifdef".
>
> v6->v7:
> 1.SMP has be decoupled from the SoC version.
> 2.Add mailboxes 3 and 4 for XBurst.
> 3.Adjust code in "jz4780_smp_prepare_cpus()".
> 4."jz4780_smp_init()" has be marked "__init".
>
> v7->v8:
> No change.
>
> arch/mips/include/asm/mach-jz4740/smp.h | 87 +++++++++++
> arch/mips/jz4740/Kconfig | 2 +
> arch/mips/jz4740/Makefile | 5 +
> arch/mips/jz4740/prom.c | 4 +
> arch/mips/jz4740/smp-entry.S | 57 +++++++
> arch/mips/jz4740/smp.c | 258
> ++++++++++++++++++++++++++++++++
> arch/mips/kernel/idle.c | 35 ++++-
> 7 files changed, 447 insertions(+), 1 deletion(-)
> create mode 100644 arch/mips/include/asm/mach-jz4740/smp.h
> create mode 100644 arch/mips/jz4740/smp-entry.S
> create mode 100644 arch/mips/jz4740/smp.c
>
> diff --git a/arch/mips/include/asm/mach-jz4740/smp.h
> b/arch/mips/include/asm/mach-jz4740/smp.h
> new file mode 100644
> index 00000000..86f660f
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-jz4740/smp.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP definitions
> + */
> +
> +#ifndef __MIPS_ASM_MACH_JZ4740_SMP_H__
> +#define __MIPS_ASM_MACH_JZ4740_SMP_H__
> +
> +#define read_c0_corectrl() __read_32bit_c0_register($12, 2)
> +#define write_c0_corectrl(val) __write_32bit_c0_register($12, 2,
> val)
> +
> +#define read_c0_corestatus() __read_32bit_c0_register($12, 3)
> +#define write_c0_corestatus(val) __write_32bit_c0_register($12, 3,
> val)
> +
> +#define read_c0_reim() __read_32bit_c0_register($12, 4)
> +#define write_c0_reim(val) __write_32bit_c0_register($12, 4, val)
> +
> +#define read_c0_mailbox0() __read_32bit_c0_register($20, 0)
> +#define write_c0_mailbox0(val) __write_32bit_c0_register($20, 0,
> val)
> +
> +#define read_c0_mailbox1() __read_32bit_c0_register($20, 1)
> +#define write_c0_mailbox1(val) __write_32bit_c0_register($20, 1,
> val)
> +
> +#define read_c0_mailbox2() __read_32bit_c0_register($20, 2)
> +#define write_c0_mailbox2(val) __write_32bit_c0_register($20, 2,
> val)
> +
> +#define read_c0_mailbox3() __read_32bit_c0_register($20, 3)
> +#define write_c0_mailbox3(val) __write_32bit_c0_register($20, 3,
> val)
> +
> +#define smp_clr_pending(mask) do { \
> + unsigned int stat; \
> + stat = read_c0_corestatus(); \
> + stat &= ~((mask) & 0xff); \
> + write_c0_corestatus(stat); \
> + } while (0)
> +
> +/*
> + * Core Control register
> + */
> +#define CORECTRL_SLEEP1M_SHIFT 17
> +#define CORECTRL_SLEEP1M (_ULCAST_(0x1) << CORECTRL_SLEEP1M_SHIFT)
> +#define CORECTRL_SLEEP0M_SHIFT 16
> +#define CORECTRL_SLEEP0M (_ULCAST_(0x1) << CORECTRL_SLEEP0M_SHIFT)
> +#define CORECTRL_RPC1_SHIFT 9
> +#define CORECTRL_RPC1 (_ULCAST_(0x1) << CORECTRL_RPC1_SHIFT)
> +#define CORECTRL_RPC0_SHIFT 8
> +#define CORECTRL_RPC0 (_ULCAST_(0x1) << CORECTRL_RPC0_SHIFT)
> +#define CORECTRL_SWRST1_SHIFT 1
> +#define CORECTRL_SWRST1 (_ULCAST_(0x1) << CORECTRL_SWRST1_SHIFT)
> +#define CORECTRL_SWRST0_SHIFT 0
> +#define CORECTRL_SWRST0 (_ULCAST_(0x1) << CORECTRL_SWRST0_SHIFT)
> +
> +/*
> + * Core Status register
> + */
> +#define CORESTATUS_SLEEP1_SHIFT 17
> +#define CORESTATUS_SLEEP1 (_ULCAST_(0x1) << CORESTATUS_SLEEP1_SHIFT)
> +#define CORESTATUS_SLEEP0_SHIFT 16
> +#define CORESTATUS_SLEEP0 (_ULCAST_(0x1) << CORESTATUS_SLEEP0_SHIFT)
> +#define CORESTATUS_IRQ1P_SHIFT 9
> +#define CORESTATUS_IRQ1P (_ULCAST_(0x1) << CORESTATUS_IRQ1P_SHIFT)
> +#define CORESTATUS_IRQ0P_SHIFT 8
> +#define CORESTATUS_IRQ0P (_ULCAST_(0x1) << CORESTATUS_IRQ8P_SHIFT)
> +#define CORESTATUS_MIRQ1P_SHIFT 1
> +#define CORESTATUS_MIRQ1P (_ULCAST_(0x1) << CORESTATUS_MIRQ1P_SHIFT)
> +#define CORESTATUS_MIRQ0P_SHIFT 0
> +#define CORESTATUS_MIRQ0P (_ULCAST_(0x1) << CORESTATUS_MIRQ0P_SHIFT)
> +
> +/*
> + * Reset Entry & IRQ Mask register
> + */
> +#define REIM_ENTRY_SHIFT 16
> +#define REIM_ENTRY (_ULCAST_(0xffff) << REIM_ENTRY_SHIFT)
> +#define REIM_IRQ1M_SHIFT 9
> +#define REIM_IRQ1M (_ULCAST_(0x1) << REIM_IRQ1M_SHIFT)
> +#define REIM_IRQ0M_SHIFT 8
> +#define REIM_IRQ0M (_ULCAST_(0x1) << REIM_IRQ0M_SHIFT)
> +#define REIM_MBOXIRQ1M_SHIFT 1
> +#define REIM_MBOXIRQ1M (_ULCAST_(0x1) << REIM_MBOXIRQ1M_SHIFT)
> +#define REIM_MBOXIRQ0M_SHIFT 0
> +#define REIM_MBOXIRQ0M (_ULCAST_(0x1) << REIM_MBOXIRQ0M_SHIFT)
> +
> +extern void jz4780_smp_init(void);
> +extern void jz4780_secondary_cpu_entry(void);
> +
> +#endif /* __MIPS_ASM_MACH_JZ4740_SMP_H__ */
> diff --git a/arch/mips/jz4740/Kconfig b/arch/mips/jz4740/Kconfig
> index 412d2fa..2b88557 100644
> --- a/arch/mips/jz4740/Kconfig
> +++ b/arch/mips/jz4740/Kconfig
> @@ -34,9 +34,11 @@ config MACH_JZ4770
>
> config MACH_JZ4780
> bool
> + select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> select MIPS_CPU_SCACHE
> select SYS_HAS_CPU_MIPS32_R2
> select SYS_SUPPORTS_HIGHMEM
> + select SYS_SUPPORTS_SMP
>
> config MACH_X1000
> bool
> diff --git a/arch/mips/jz4740/Makefile b/arch/mips/jz4740/Makefile
> index 6de14c0..0a0f024 100644
> --- a/arch/mips/jz4740/Makefile
> +++ b/arch/mips/jz4740/Makefile
> @@ -12,3 +12,8 @@ CFLAGS_setup.o =
> -I$(src)/../../../scripts/dtc/libfdt
> # PM support
>
> obj-$(CONFIG_PM) += pm.o
> +
> +# SMP support
> +
> +obj-$(CONFIG_SMP) += smp.o
> +obj-$(CONFIG_SMP) += smp-entry.o
> diff --git a/arch/mips/jz4740/prom.c b/arch/mips/jz4740/prom.c
> index ff4555c..4acf5c2c 100644
> --- a/arch/mips/jz4740/prom.c
> +++ b/arch/mips/jz4740/prom.c
That file is gone in mips-next. You should rebase your patchset on top
of mips-next.
Cheers,
-Paul
> @@ -8,10 +8,14 @@
>
> #include <asm/bootinfo.h>
> #include <asm/fw/fw.h>
> +#include <asm/mach-jz4740/smp.h>
>
> void __init prom_init(void)
> {
> fw_init_cmdline();
> +
> + if (IS_ENABLED(CONFIG_SMP))
> + jz4780_smp_init();
> }
>
> void __init prom_free_prom_memory(void)
> diff --git a/arch/mips/jz4740/smp-entry.S
> b/arch/mips/jz4740/smp-entry.S
> new file mode 100644
> index 00000000..20049a3
> --- /dev/null
> +++ b/arch/mips/jz4740/smp-entry.S
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP entry point
> + */
> +
> +#include <asm/addrspace.h>
> +#include <asm/asm.h>
> +#include <asm/asmmacro.h>
> +#include <asm/cacheops.h>
> +#include <asm/mipsregs.h>
> +
> +#define CACHE_SIZE (32 * 1024)
> +#define CACHE_LINESIZE 32
> +
> +.extern jz4780_cpu_entry_sp
> +.extern jz4780_cpu_entry_gp
> +
> +.section .text.smp-entry
> +.balign 0x10000
> +.set noreorder
> +LEAF(jz4780_secondary_cpu_entry)
> + mtc0 zero, CP0_CAUSE
> +
> + li t0, ST0_CU0
> + mtc0 t0, CP0_STATUS
> +
> + /* cache setup */
> + li t0, KSEG0
> + ori t1, t0, CACHE_SIZE
> + mtc0 zero, CP0_TAGLO, 0
> +1: cache Index_Store_Tag_I, 0(t0)
> + cache Index_Store_Tag_D, 0(t0)
> + bne t0, t1, 1b
> + addiu t0, t0, CACHE_LINESIZE
> +
> + /* kseg0 cache attribute */
> + mfc0 t0, CP0_CONFIG, 0
> + ori t0, t0, CONF_CM_CACHABLE_NONCOHERENT
> + mtc0 t0, CP0_CONFIG, 0
> +
> + /* pagemask */
> + mtc0 zero, CP0_PAGEMASK, 0
> +
> + /* retrieve sp */
> + la t0, jz4780_cpu_entry_sp
> + lw sp, 0(t0)
> +
> + /* retrieve gp */
> + la t0, jz4780_cpu_entry_gp
> + lw gp, 0(t0)
> +
> + /* jump to the kernel in kseg0 */
> + la t0, smp_bootstrap
> + jr t0
> + nop
> + END(jz4780_secondary_cpu_entry)
> diff --git a/arch/mips/jz4740/smp.c b/arch/mips/jz4740/smp.c
> new file mode 100644
> index 00000000..d95d22a
> --- /dev/null
> +++ b/arch/mips/jz4740/smp.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2013, Paul Burton <paul.burton@imgtec.com>
> + * JZ4780 SMP
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/sched.h>
> +#include <linux/sched/task_stack.h>
> +#include <linux/smp.h>
> +#include <linux/tick.h>
> +#include <asm/mach-jz4740/smp.h>
> +#include <asm/smp-ops.h>
> +
> +static struct clk *cpu_clock_gates[CONFIG_NR_CPUS] = { NULL };
> +
> +u32 jz4780_cpu_entry_sp;
> +u32 jz4780_cpu_entry_gp;
> +
> +static struct cpumask cpu_running;
> +
> +static DEFINE_SPINLOCK(smp_lock);
> +
> +static irqreturn_t mbox_handler(int irq, void *dev_id)
> +{
> + int cpu = smp_processor_id();
> + u32 action, status;
> +
> + spin_lock(&smp_lock);
> +
> + switch (cpu) {
> + case 0:
> + action = read_c0_mailbox0();
> + write_c0_mailbox0(0);
> + break;
> + case 1:
> + action = read_c0_mailbox1();
> + write_c0_mailbox1(0);
> + break;
> + case 2:
> + action = read_c0_mailbox2();
> + write_c0_mailbox2(0);
> + break;
> + case 3:
> + action = read_c0_mailbox3();
> + write_c0_mailbox3(0);
> + break;
> + default:
> + panic("unhandled cpu %d!", cpu);
> + }
> +
> + /* clear pending mailbox interrupt */
> + status = read_c0_corestatus();
> + status &= ~(CORESTATUS_MIRQ0P << cpu);
> + write_c0_corestatus(status);
> +
> + spin_unlock(&smp_lock);
> +
> + if (action & SMP_RESCHEDULE_YOURSELF)
> + scheduler_ipi();
> + if (action & SMP_CALL_FUNCTION)
> + generic_smp_call_function_interrupt();
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void jz4780_smp_setup(void)
> +{
> + u32 addr, reim;
> + int cpu;
> +
> + reim = read_c0_reim();
> +
> + for (cpu = 0; cpu < NR_CPUS; cpu++) {
> + __cpu_number_map[cpu] = cpu;
> + __cpu_logical_map[cpu] = cpu;
> + set_cpu_possible(cpu, true);
> + }
> +
> + /* mask mailbox interrupts for this core */
> + reim &= ~REIM_MBOXIRQ0M;
> + write_c0_reim(reim);
> +
> + /* clear mailboxes & pending mailbox IRQs */
> + write_c0_mailbox0(0);
> + write_c0_mailbox1(0);
> + write_c0_corestatus(0);
> +
> + /* set reset entry point */
> + addr = KSEG1ADDR((u32)&jz4780_secondary_cpu_entry);
> + WARN_ON(addr & ~REIM_ENTRY);
> + reim &= ~REIM_ENTRY;
> + reim |= addr & REIM_ENTRY;
> +
> + /* unmask mailbox interrupts for this core */
> + reim |= REIM_MBOXIRQ0M;
> + write_c0_reim(reim);
> + set_c0_status(STATUSF_IP3);
> + irq_enable_hazard();
> +
> + cpumask_set_cpu(cpu, &cpu_running);
> +}
> +
> +static void jz4780_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + struct device_node *cpu_node;
> + unsigned cpu, ctrl;
> + int err;
> +
> + /* setup the mailbox IRQ */
> + err = request_irq(MIPS_CPU_IRQ_BASE + 3, mbox_handler,
> + IRQF_PERCPU | IRQF_NO_THREAD, "core mailbox", NULL);
> + if (err)
> + pr_err("request_irq() on core mailbox failed\n");
> +
> + ctrl = read_c0_corectrl();
> +
> + for_each_of_cpu_node(cpu_node) {
> + cpu = of_cpu_node_to_id(cpu_node);
> + if (cpu < 0) {
> + pr_err("Failed to read index of %s\n",
> + cpu_node->full_name);
> + continue;
> + }
> +
> + /* use reset entry point from REIM register */
> + ctrl |= CORECTRL_RPC0 << cpu;
> +
> + cpu_clock_gates[cpu] = of_clk_get(cpu_node, 0);
> + if (IS_ERR(cpu_clock_gates[cpu])) {
> + cpu_clock_gates[cpu] = NULL;
> + continue;
> + }
> +
> + err = clk_prepare(cpu_clock_gates[cpu]);
> + if (err)
> + pr_err("Failed to prepare CPU clock gate\n");
> + }
> +
> + write_c0_corectrl(ctrl);
> +}
> +
> +static int jz4780_boot_secondary(int cpu, struct task_struct *idle)
> +{
> + unsigned long flags;
> + u32 ctrl;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> +
> + /* ensure the core is in reset */
> + ctrl = read_c0_corectrl();
> + ctrl |= CORECTRL_SWRST0 << cpu;
> + write_c0_corectrl(ctrl);
> +
> + /* ungate core clock */
> + if (cpu_clock_gates[cpu])
> + clk_enable(cpu_clock_gates[cpu]);
> +
> + /* set entry sp/gp register values */
> + jz4780_cpu_entry_sp = __KSTK_TOS(idle);
> + jz4780_cpu_entry_gp = (u32)task_thread_info(idle);
> + smp_wmb();
> +
> + /* take the core out of reset */
> + ctrl &= ~(CORECTRL_SWRST0 << cpu);
> + write_c0_corectrl(ctrl);
> +
> + cpumask_set_cpu(cpu, &cpu_running);
> +
> + spin_unlock_irqrestore(&smp_lock, flags);
> +
> + return 0;
> +}
> +
> +static void jz4780_init_secondary(void)
> +{
> +}
> +
> +static void jz4780_smp_finish(void)
> +{
> + u32 reim;
> +
> + spin_lock(&smp_lock);
> +
> + /* unmask mailbox interrupts for this core */
> + reim = read_c0_reim();
> + reim |= REIM_MBOXIRQ0M << smp_processor_id();
> + write_c0_reim(reim);
> +
> + spin_unlock(&smp_lock);
> +
> + /* unmask interrupts for this core */
> + change_c0_status(ST0_IM, STATUSF_IP3 | STATUSF_IP2 |
> + STATUSF_IP1 | STATUSF_IP0);
> + irq_enable_hazard();
> +
> + /* force broadcast timer */
> + tick_broadcast_force();
> +}
> +
> +static void jz4780_send_ipi_single_locked(int cpu, unsigned int
> action)
> +{
> + u32 mbox;
> +
> + switch (cpu) {
> + case 0:
> + mbox = read_c0_mailbox0();
> + write_c0_mailbox0(mbox | action);
> + break;
> + case 1:
> + mbox = read_c0_mailbox1();
> + write_c0_mailbox1(mbox | action);
> + break;
> + default:
> + panic("unhandled cpu %d!", cpu);
> + }
> +}
> +
> +static void jz4780_send_ipi_single(int cpu, unsigned int action)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> + jz4780_send_ipi_single_locked(cpu, action);
> + spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static void jz4780_send_ipi_mask(const struct cpumask *mask,
> + unsigned int action)
> +{
> + unsigned long flags;
> + int cpu;
> +
> + spin_lock_irqsave(&smp_lock, flags);
> +
> + for_each_cpu(cpu, mask)
> + jz4780_send_ipi_single_locked(cpu, action);
> +
> + spin_unlock_irqrestore(&smp_lock, flags);
> +}
> +
> +static struct plat_smp_ops jz4780_smp_ops = {
> + .send_ipi_single = jz4780_send_ipi_single,
> + .send_ipi_mask = jz4780_send_ipi_mask,
> + .init_secondary = jz4780_init_secondary,
> + .smp_finish = jz4780_smp_finish,
> + .boot_secondary = jz4780_boot_secondary,
> + .smp_setup = jz4780_smp_setup,
> + .prepare_cpus = jz4780_smp_prepare_cpus,
> +};
> +
> +void __init jz4780_smp_init(void)
> +{
> + register_smp_ops(&jz4780_smp_ops);
> +}
> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
> index 37f8e78..d33f2d4 100644
> --- a/arch/mips/kernel/idle.c
> +++ b/arch/mips/kernel/idle.c
> @@ -18,6 +18,7 @@
> #include <asm/cpu-type.h>
> #include <asm/idle.h>
> #include <asm/mipsregs.h>
> +#include <asm/r4kcache.h>
>
> /*
> * Not all of the MIPS CPUs have the "wait" instruction available.
> Moreover,
> @@ -88,6 +89,34 @@ static void __cpuidle rm7k_wait_irqoff(void)
> }
>
> /*
> + * The Ingenic jz4780 SMP variant has to write back dirty cache
> lines before
> + * executing wait. The CPU & cache clock will be gated until we
> return from
> + * the wait, and if another core attempts to access data from our
> data cache
> + * during this time then it will lock up.
> + */
> +void jz4780_smp_wait_irqoff(void)
> +{
> + unsigned long pending = read_c0_cause() & read_c0_status() &
> CAUSEF_IP;
> +
> + /*
> + * Going to idle has a significant overhead due to the cache flush
> so
> + * try to avoid it if we'll immediately be woken again due to an
> IRQ.
> + */
> + if (!need_resched() && !pending) {
> + r4k_blast_dcache();
> +
> + __asm__(
> + " .set push \n"
> + " .set mips3 \n"
> + " sync \n"
> + " wait \n"
> + " .set pop \n");
> + }
> +
> + local_irq_enable();
> +}
> +
> +/*
> * Au1 'wait' is only useful when the 32kHz counter is used as timer,
> * since coreclock (and the cp0 counter) stops upon executing it.
> Only an
> * interrupt can wake it, so they must be enabled before entering
> idle modes.
> @@ -172,7 +201,6 @@ void __init check_wait(void)
> case CPU_CAVIUM_OCTEON_PLUS:
> case CPU_CAVIUM_OCTEON2:
> case CPU_CAVIUM_OCTEON3:
> - case CPU_XBURST:
> case CPU_LOONGSON32:
> case CPU_XLR:
> case CPU_XLP:
> @@ -246,6 +274,11 @@ void __init check_wait(void)
> cpu_wait = r4k_wait;
> */
> break;
> + case CPU_XBURST:
> + if (IS_ENABLED(CONFIG_SMP))
> + cpu_wait = jz4780_smp_wait_irqoff;
> + else
> + cpu_wait = r4k_wait;
> default:
> break;
> }
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: sram: add documentation for reserved-only flag
From: Stephen Warren @ 2020-05-19 16:16 UTC (permalink / raw)
To: Mian Yousaf Kaukab
Cc: robh+dt, robin.murphy, devicetree, talho, thierry.reding,
jonathanh, linux-tegra, linux-kernel, linux-arm-kernel, afaerber,
arnd, gregkh
In-Reply-To: <20200513104127.GA2309@suse.de>
On 5/13/20 4:41 AM, Mian Yousaf Kaukab wrote:
> On Tue, May 12, 2020 at 01:45:28PM -0600, Stephen Warren wrote:
>> On 5/12/20 8:48 AM, Mian Yousaf Kaukab wrote:
>>> Add documentation for the new optional flag added for SRAM driver.
>>
>>> diff --git a/Documentation/devicetree/bindings/sram/sram.yaml b/Documentation/devicetree/bindings/sram/sram.yaml
>>
>>> + reserved-only:
>>> + description:
>>> + The flag indicating, that only SRAM reserved regions have to be remapped.
>>> + remapping type is selected depending upon no-memory-wc as usual.
>>> + type: boolean
>>
>> This feels a bit like a SW flag rather than a HW description, so I'm not
>> sure it's appropriate to put it into DT.
>
> Reserved regions themselves are software descriptions, no? Then we have 'pool'
> flag which is again a software flag and so on. This flag falls into same
> category and nothing out of ordinary.
I suppose that's true to some extent. This is indeed a description of
the system environment presented to the SW that consumes the DT, which
is a bit more than pure HW description but still a description of
something imposed externally rather than describing something that's up
to the discretion of the consuming SW. So, go ahead.
>> Are there any cases where the SW should map all of the SRAM, i.e. where
>> we wouldn't expect to set reserved-only? [...]
>
> Yes, here are a few examples:
> arch/arm/boot/dts/aspeed-g*.dtsi
> arch/arm/boot/dts/at91*.dtsi
> arch/arm/boot/dts/bcm7445.dtsi
> Then arch/arm/boot/dts/dra7.dtsi is an example where we should map everything
> except the reserved region.
>
>> [...] I'd expect reserved-only to be
>> the default, and perhaps only, mode of operation for the SRAM driver.
>
> It will break compatibility with existing dtbs.
>
>> If we can't do that because some SW currently expects to be able to map
>> arbitrary portions of the SRAM, shouldn't that SW be fixed to tell the
>> SRAM driver which parts it's using, hence still allowing the driver to
>> only map in-use portions?
>
> User doesn’t need sram driver in that case. It can use genalloc api directly.
This sounds a bit odd. Without a driver for the reserved region, nothing
should be touching it, since otherwise there's no code that owns an
manages the region. If any code needs to consume the region, it should
obtain info about the region from some form of provider code that can
handle both the allocation and mapping. Anything else sounds like some
consumer code directly making use of DT nodes it doesn't own. But since
I'm not familiar enough with the SRAM driver and genalloc code that you
mention to fully understand the allocation paths I guess I won't object
for now, although it does still sound fishy.
^ permalink raw reply
* Re: [PATCH 4/4] sc16is7xx: Use threaded IRQ
From: Maarten Brock @ 2020-05-19 16:32 UTC (permalink / raw)
To: Daniel Mack
Cc: devicetree, linux-serial, gregkh, robh+dt, jslaby, pascal.huerst,
linux-serial-owner
In-Reply-To: <22116d56-9240-9bfe-1b6f-a94d57a085cf@zonque.org>
On 2020-05-18 18:57, Daniel Mack wrote:
> Hi Maarten,
>
> On 5/18/20 1:14 PM, Maarten Brock wrote:
>> On 2020-05-17 22:44, Daniel Mack wrote:
>
>> Summerizing:
>> - After switching to a threaded IRQ, the trigger could be switched to
>> IRQF_TRIGGER_LOW and with that interrupt sharing can be enabled for
>> this device with IRQF_SHARED.
>
> Yes, but we don't need that. As discussed, the UART driver can cope
> with
> edge IRQs just fine.
>
>> - Some (your) interrupt controllers do not support IRQF_TRIGGER_LOW.
>> For those only IRQF_TRIGGER_FALLING can be used for this device and
>> thus IRQF_SHARED cannot be used.
>
> True. Interrupts cannot be shared for this device then. That's a fair
> limitation, and it has always been like that.
It has always been like that for this driver. But that should be no
reason why the driver might not be improved. I wonder how the 8250
handles this. PC's have always shared interrupts for COM1/2/3/4 AFAIK.
>> - The driver for your interrupt controller should be improved to
>> support
>> level IRQs.
>
> It's a controller that sits behind another hardware bus itself, so
> polling is expensive. If the controller would need to check for level
> IRQs it would need to poll, and then we could as well just poll the
> UART
> directly, that's just as good :)
That depends on the IRQ coming out of the interrupt controller. If that
is
a level interrupt itself, then it is easy to see if all interrupts are
handled. Further polling zooms in on the devices that require attention.
> But again - the UART driver works perfectly fine with edge IRQs as long
> as the interrupt is not shared.
If you would require multiple sc16is7xx devices on I2C would you like to
connect multiple interrupt lines? Or just SCL,SDA and *one* IRQ?
OTOH for SPI you would require multiple CS already.
>> This makes me wonder if it would be better to let the device tree
>> specify
>> the interrupt configuration.
>
> There can be flags in the 2nd cell of the node, but their meaning is
> specific to the controller. Hence the SPI/I2C layers don't pass that
> information up.
>
> What many drivers do is try with one setting, and if that fails because
> the interrupt controller returns an error, they fall back to something
> else. We could do the same here of course, but it'd be another patch on
> top, as it's unrelated to the concrete change the patch we're
> commenting
> on is bringing in.
>
> So what I can add is logic that first tries with IRQF_LOW|IRQF_SHARED,
> and if that fails, we fall back to IRQF_FALLING and retry. WDYT?
That sounds like a decent plan.
>
> Thanks,
> Daniel
Kind regards,
Maarten
^ permalink raw reply
* [PATCH] dt-bindings: gpu: arm,mali-utgard: add additional properties
From: Johan Jonker @ 2020-05-19 16:44 UTC (permalink / raw)
To: heiko
Cc: robh+dt, maxime.ripard, airlied, daniel, dri-devel, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel
In the old txt situation we add/describe only properties that are used
by the driver/hardware itself. With yaml it also filters things in a
node that are used by other drivers like 'assigned-clocks' and
'assigned-clock-rates' for some older Rockchip SoCs in 'gpu' nodes,
so add them to 'arm,mali-utgard.yaml'.
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml
index 4869258da..2fc97c544 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.yaml
@@ -95,6 +95,12 @@ properties:
- const: bus
- const: core
+ assigned-clocks:
+ maxItems: 1
+
+ assigned-clock-rates:
+ maxItems: 1
+
memory-region: true
mali-supply: true
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v3 1/4] dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-19 16:44 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Stephan Gerhold, linux-kernel, ~postmarketos/upstreaming,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jilayne Lovejoy,
Jonathan Cameron, Kate Stewart, Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron, Rob Herring
In-Reply-To: <20200519160137.GJ1634618@smile.fi.intel.com>
On Tue, May 19, 2020 at 07:01:37PM +0300, Andy Shevchenko wrote:
> On Tue, May 19, 2020 at 04:03:54PM +0200, Jonathan Albrieux wrote:
> > On Tue, May 19, 2020 at 03:22:07PM +0200, Stephan Gerhold wrote:
> > > On Tue, May 19, 2020 at 02:43:51PM +0200, Jonathan Albrieux wrote:
>
> ...
>
> > > > +maintainers:
> > > > + - can't find a mantainer, author is Laxman Dewangan <ldewangan@nvidia.com>
> > >
> > > Should probably add someone here, although I'm not sure who either.
> > >
> >
> > Yep I couldn't find a maintainer for that driver..what to do in this case?
>
> Volunteer yourself!
>
While I'd really like to, I have to decline the offer as I currently don't have
enought knowledge to become a maintainer :-) but thank you! (Who knows, maybe in
a couple of year!) Now I'll make the final edits and will submit a new
patchset soon with all the changes
> --
> With Best Regards,
> Andy Shevchenko
>
>
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: phy: dp83869: Set opmode from straps
From: Jakub Kicinski @ 2020-05-19 16:58 UTC (permalink / raw)
To: Dan Murphy
Cc: andrew, f.fainelli, hkallweit1, davem, netdev, linux-kernel,
devicetree
In-Reply-To: <20200519141813.28167-3-dmurphy@ti.com>
On Tue, 19 May 2020 09:18:11 -0500 Dan Murphy wrote:
> If the op-mode for the device is not set in the device tree then set
> the strapped op-mode and store it for later configuration.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
../drivers/net/phy/dp83869.c: In function0 dp83869_set_strapped_mode:
../drivers/net/phy/dp83869.c:171:10: warning: comparison is always false due to limited range of data type [-Wtype-limits]
171 | if (val < 0)
| ^
^ permalink raw reply
* Re: [PATCH v2 4/6] dmaengine: dw: Print warning if multi-block is unsupported
From: Vinod Koul @ 2020-05-19 17:02 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Andy Shevchenko, Mark Brown, Viresh Kumar,
Dan Williams, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips, devicetree,
dmaengine, Linux Kernel Mailing List
In-Reply-To: <20200517192347.h3hiibsifwfyyr7z@mobilestation>
On 17-05-20, 22:23, Serge Semin wrote:
> On Fri, May 15, 2020 at 12:00:39PM +0530, Vinod Koul wrote:
> > Hi Serge,
> >
> > On 12-05-20, 15:42, Serge Semin wrote:
> > > Vinod,
> > >
> > > Could you join the discussion for a little bit?
> > >
> > > In order to properly fix the problem discussed in this topic, we need to
> > > introduce an additional capability exported by DMA channel handlers on per-channel
> > > basis. It must be a number, which would indicate an upper limitation of the SG list
> > > entries amount.
> > > Something like this would do it:
> > > struct dma_slave_caps {
> > > ...
> > > unsigned int max_sg_nents;
> > > ...
> >
> > Looking at the discussion, I agree we should can this up in the
> > interface. The max_dma_len suggests the length of a descriptor allowed,
> > it does not convey the sg_nents supported which in the case of nollp is
> > one.
> >
> > Btw is this is a real hardware issue, I have found that value of such
> > hardware is very less and people did fix it up in subsequent revs to add
> > llp support.
>
> Yes, it is. My DW DMAC doesn't support LLP and there isn't going to be new SoC
> version produced.(
Ouch
> > Also, another question is why this cannot be handled in driver, I agree
> > your hardware does not support llp but that does not stop you from
> > breaking a multi_sg list into N hardware descriptors and keep submitting
> > them (for this to work submission should be done in isr and not in bh,
> > unfortunately very few driver take that route).
>
> Current DW DMA driver does that, but this isn't enough. The problem is that
> in order to fix the issue in the DMA hardware driver we need to introduce
> an inter-dependent channels abstraction and synchronously feed both Tx and
> Rx DMA channels with hardware descriptors (LLP entries) one-by-one. This hardly
> needed by any slave device driver rather than SPI, which Tx and Rx buffers are
> inter-dependent. So Andy's idea was to move the fix to the SPI driver (feed
> the DMA engine channels with Tx and Rx data buffers synchronously), but DMA
> engine would provide an info whether such fix is required. This can be
> determined by the maximum SG entries capability.
Okay but having the sw limitation removed would also be a good idea, you
can handle any user, I will leave it upto you, either way is okay
>
> (Note max_sg_ents isn't a limitation on the number of SG entries supported by
> the DMA driver, but the number of SG entries handled by the DMA engine in a
> single DMA transaction.)
>
> > TBH the max_sg_nents or
> > max_dma_len are HW restrictions and SW *can* deal with then :-)
>
> Yes, it can, but it only works for the cases when individual DMA channels are
> utilized. DMA hardware driver doesn't know that the target and source slave
> device buffers (SPI Tx and Rx FIFOs) are inter-dependent, that writing to one
> you will implicitly push data to another. So due to the interrupts handling
> latency Tx DMA channel is restarted faster than Rx DMA channel is reinitialized.
> This causes the SPI Rx FIFO overflow and data loss.
>
> >
> > In an idea world, you should break the sw descriptor submitted into N hw
> > descriptors and submit to hardware and let user know when the sw
> > descriptor is completed. Of course we do not do that :(
>
> Well, the current Dw DMA driver does that. But due to the two slave device
> buffers inter-dependency this isn't enough to perform safe DMA transactions.
> Due to the interrupts handling latency Tx DMA channel pushes data to the slave
> device buffer faster than Rx DMA channel starts to handle incoming data. This
> causes the SPI Rx FIFO overflow.
>
> >
> > > };
> > > As Andy suggested it's value should be interpreted as:
> > > 0 - unlimited number of entries,
> > > 1:MAX_UINT - actual limit to the number of entries.
> >
>
> > Hmm why 0, why not MAX_UINT for unlimited?
>
> 0 is much better for many reasons. First of all MAX_UINT is a lot, but it's
> still a number. On x64 platform this might be actual limit if for instance
> the block-size register is 32-bits wide. Secondly interpreting 0 as unlimited
> number of entries would be more suitable since most of the drivers support
> LLP functionality and we wouldn't need to update their code to set MAX_UINT.
> Thirdly DMA engines, which don't support LLPs would need to set this parameter
> as 1. So if we do as you say and interpret unlimited number of LLPs as MAX_UINT,
> then 0 would left unused.
>
> To sum up I also think that using 0 as unlimited number SG entries supported is
> much better.
ok
> > > In addition to that seeing the dma_get_slave_caps() method provide the caps only
> > > by getting them from the DMA device descriptor, while we need to have an info on
> > > per-channel basis, it would be good to introduce a new DMA-device callback like:
> > > struct dma_device {
> > > ...
> > > int (*device_caps)(struct dma_chan *chan,
> > > struct dma_slave_caps *caps);
> >
>
> > Do you have a controller where channel caps are on per-channel basis?
>
> Yes, I do. Our DW DMA controller has got the maximum burst length non-uniformly
> distributed per DMA channels. There are eight channels our controller supports,
> among which first two channels can burst up to 32 transfer words, but the rest
> of the channels support bursting up to 4 transfer words.
>
> So having such device_caps() callback to customize the device capabilities on
> per-DMA-channel basis would be very useful! What do you think?
Okay looks like per-ch basis is the way forward!
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 5/6] dmaengine: dw: Introduce max burst length hw config
From: Vinod Koul @ 2020-05-19 17:07 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Andy Shevchenko, Viresh Kumar, Dan Williams,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200517193818.jaiwgzgz7tutj4mk@mobilestation>
On 17-05-20, 22:38, Serge Semin wrote:
> On Fri, May 15, 2020 at 12:09:50PM +0530, Vinod Koul wrote:
> > On 12-05-20, 22:12, Andy Shevchenko wrote:
> > > On Tue, May 12, 2020 at 05:08:20PM +0300, Serge Semin wrote:
> > > > On Fri, May 08, 2020 at 02:41:53PM +0300, Andy Shevchenko wrote:
> > > > > On Fri, May 08, 2020 at 01:53:03PM +0300, Serge Semin wrote:
> > > > > > IP core of the DW DMA controller may be synthesized with different
> > > > > > max burst length of the transfers per each channel. According to Synopsis
> > > > > > having the fixed maximum burst transactions length may provide some
> > > > > > performance gain. At the same time setting up the source and destination
> > > > > > multi size exceeding the max burst length limitation may cause a serious
> > > > > > problems. In our case the system just hangs up. In order to fix this
> > > > > > lets introduce the max burst length platform config of the DW DMA
> > > > > > controller device and don't let the DMA channels configuration code
> > > > > > exceed the burst length hardware limitation. Depending on the IP core
> > > > > > configuration the maximum value can vary from channel to channel.
> > > > > > It can be detected either in runtime from the DWC parameter registers
> > > > > > or from the dedicated dts property.
> > > > >
> > > > > I'm wondering what can be the scenario when your peripheral will ask something
> > > > > which is not supported by DMA controller?
> > > >
> > > > I may misunderstood your statement, because seeing your activity around my
> > > > patchsets including the SPI patchset and sometimes very helpful comments,
> > > > this question answer seems too obvious to see you asking it.
> > > >
> > > > No need to go far for an example. See the DW APB SSI driver. Its DMA module
> > > > specifies the burst length to be 16, while not all of ours channels supports it.
> > > > Yes, originally it has been developed for the Intel Midfield SPI, but since I
> > > > converted the driver into a generic code we can't use a fixed value. For instance
> > > > in our hardware only two DMA channels of total 16 are capable of bursting up to
> > > > 16 bytes (data items) at a time, the rest of them are limited with up to 4 bytes
> > > > burst length. While there are two SPI interfaces, each of which need to have two
> > > > DMA channels for communications. So I need four channels in total to allocate to
> > > > provide the DMA capability for all interfaces. In order to set the SPI controller
> > > > up with valid optimized parameters the max-burst-length is required. Otherwise we
> > > > can end up with buffers overrun/underrun.
> > >
> > > Right, and we come to the question which channel better to be used by SPI and
> > > the rest devices. Without specific filter function you can easily get into a
> > > case of inverted optimizations, when SPI got channels with burst = 4, while
> > > it's needed 16, and other hardware otherwise. Performance wise it's worse
> > > scenario which we may avoid in the first place, right?
> >
> > If one has channels which are different and described as such in DT,
> > then I think it does make sense to specify in your board-dt about the
> > specific channels you would require...
>
> Well, we do have such hardware. Our DW DMA controller has got different max
> burst lengths assigned to first two and the rest of the channels. But creating
> a functionality of the individual channels assignment is a matter of different
> patchset. Sorry. It's not one of my task at the moment.
>
> My primary task is to integrate the Baikal-T1 SoC support into the kernel. I've
> refactored a lot of code found in the Baikal-T1 SDK and currently under a pressure
> of a lot of review. Alas there is no time to create new functionality as you
> suggest. In future I may provide such, but not in the framework of this patchset.
Well you need to tell your folks that upstreaming does not work under
pressure and we can't put a timeline for upstreaming. It needs to do
what is deemed the right way. Reviews can take time, that needs to be
comprehended as well!
> > > > > Peripheral needs to supply a lot of configuration parameters specific to the
> > > > > DMA controller in use (that's why we have struct dw_dma_slave).
> > > > > So, seems to me the feasible approach is supply correct data in the first place.
> > > >
> > > > How to supply a valid data if clients don't know the DMA controller limitations
> > > > in general?
> > >
> > > This is a good question. DMA controllers are quite different and having unified
> > > capabilities structure for all is almost impossible task to fulfil. That's why
> > > custom filter function(s) can help here. Based on compatible string you can
> > > implement whatever customized quirks like two functions, for example, to try 16
> > > burst size first and fallback to 4 if none was previously found.
> > >
> > > > > If you have specific channels to acquire then you probably need to provide a
> > > > > custom xlate / filter functions. Because above seems a bit hackish workaround
> > > > > of dynamic channel allocation mechanism.
> > > >
> > > > No, I don't have a specific channel to acquire and in general you may use any
> > > > returned from the DMA subsystem (though some platforms may need a dedicated
> > > > channels to use, in this case xlate / filter is required). In our SoC any DW DMAC
> > > > channel can be used for any DMA-capable peripherals like SPI, I2C, UART. But the
> > > > their DMA settings must properly and optimally configured. It can be only done
> > > > if you know the DMA controller parameters like max burst length, max block-size,
> > > > etc.
> > > >
> > > > So no. The change proposed by this patch isn't workaround, but a useful feature,
> > > > moreover expected to be supported by the generic DMA subsystem.
> > >
> > > See above.
> > >
> > > > > But let's see what we can do better. Since maximum is defined on the slave side
> > > > > device, it probably needs to define minimum as well, otherwise it's possible
> > > > > that some hardware can't cope underrun bursts.
> > > >
> > > > There is no need to define minimum if such limit doesn't exists except a
> > > > natural 1. Moreover it doesn't exist for all DMA controllers seeing noone has
> > > > added such capability into the generic DMA subsystem so far.
> > >
> > > There is a contract between provider and consumer about DMA resource. That's
> > > why both sides should participate in fulfilling it. Theoretically it may be a
> > > hardware that doesn't support minimum burst available in DMA by a reason. For
> > > such we would need minimum to be provided as well.
> >
> > Agreed and if required caps should be extended to tell consumer the
> > minimum values supported.
>
> Sorry, it's not required by our hardware. Is there any, which actually has such
> limitation? (minimum burst length)
IIUC the idea is that you will tell maximum and minimum values supported
and client can pick the best value. Esp in case of slave transfers
things like burst, msize are governed by client capability and usage. So
exposing the set to pick from would make sense
--
~Vinod
^ permalink raw reply
* Re: [PATCH v3 4/4] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property
From: Rob Herring @ 2020-05-19 17:11 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Robin Murphy, Tom Joseph, PCI,
devicetree, linux-kernel@vger.kernel.org,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200508130646.23939-5-kishon@ti.com>
On Fri, May 8, 2020 at 7:07 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Cadence PCIe core driver (host mode) uses "cdns,no-bar-match-nbits"
> property to configure the number of bits passed through from PCIe
> address to internal address in Inbound Address Translation register.
> This only used the NO MATCH BAR.
>
> However standard PCI dt-binding already defines "dma-ranges" to
> describe the address ranges accessible by PCIe controller. Add support
> in Cadence PCIe host driver to parse dma-ranges and configure the
> inbound regions for BAR0, BAR1 and NO MATCH BAR. Cadence IP specifies
> maximum size for BAR0 as 256GB, maximum size for BAR1 as 2 GB, so if
> the dma-ranges specifies a size larger than the maximum allowed, the
> driver will split and configure the BARs.
Would be useful to know what your dma-ranges contains now.
> Legacy device tree binding compatibility is maintained by retaining
> support for "cdns,no-bar-match-nbits".
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> .../controller/cadence/pcie-cadence-host.c | 141 ++++++++++++++++--
> drivers/pci/controller/cadence/pcie-cadence.h | 17 ++-
> 2 files changed, 141 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 6ecebb79057a..2485ecd8434d 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -11,6 +11,12 @@
>
> #include "pcie-cadence.h"
>
> +static u64 cdns_rp_bar_max_size[] = {
> + [RP_BAR0] = _ULL(128 * SZ_2G),
> + [RP_BAR1] = SZ_2G,
> + [RP_NO_BAR] = SZ_64T,
> +};
> +
> void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
> int where)
> {
> @@ -106,6 +112,117 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> return 0;
> }
>
> +static void cdns_pcie_host_bar_ib_config(struct cdns_pcie_rc *rc,
> + enum cdns_pcie_rp_bar bar,
> + u64 cpu_addr, u32 aperture)
> +{
> + struct cdns_pcie *pcie = &rc->pcie;
> + u32 addr0, addr1;
> +
> + addr0 = CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(aperture) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR0(bar), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar), addr1);
> +}
> +
> +static int cdns_pcie_host_bar_config(struct cdns_pcie_rc *rc,
> + struct resource_entry *entry,
> + enum cdns_pcie_rp_bar *index)
> +{
> + u64 cpu_addr, pci_addr, size, winsize;
> + struct cdns_pcie *pcie = &rc->pcie;
> + struct device *dev = pcie->dev;
> + enum cdns_pcie_rp_bar bar;
> + unsigned long flags;
> + u32 aperture;
> + u32 value;
> +
> + cpu_addr = entry->res->start;
> + flags = entry->res->flags;
> + pci_addr = entry->res->start - entry->offset;
> + size = resource_size(entry->res);
> + bar = *index;
> +
> + if (entry->offset) {
> + dev_err(dev, "Cannot map PCI addr: %llx to CPU addr: %llx\n",
> + pci_addr, cpu_addr);
Would be a bit more clear to say PCI addr must equal CPU addr.
> + return -EINVAL;
> + }
> +
> + value = cdns_pcie_readl(pcie, CDNS_PCIE_LM_RC_BAR_CFG);
> + while (size > 0) {
> + if (bar > RP_NO_BAR) {
> + dev_err(dev, "Failed to map inbound regions!\n");
> + return -EINVAL;
> + }
> +
> + winsize = size;
> + if (size > cdns_rp_bar_max_size[bar])
> + winsize = cdns_rp_bar_max_size[bar];
> +
> + aperture = ilog2(winsize);
> +
> + cdns_pcie_host_bar_ib_config(rc, bar, cpu_addr, aperture);
> +
> + if (bar == RP_NO_BAR)
> + break;
> +
> + if (winsize + cpu_addr >= SZ_4G) {
> + if (!(flags & IORESOURCE_PREFETCH))
> + value |= LM_RC_BAR_CFG_CTRL_MEM_64BITS(bar);
> + value |= LM_RC_BAR_CFG_CTRL_PREF_MEM_64BITS(bar);
> + } else {
> + if (!(flags & IORESOURCE_PREFETCH))
> + value |= LM_RC_BAR_CFG_CTRL_MEM_32BITS(bar);
> + value |= LM_RC_BAR_CFG_CTRL_PREF_MEM_32BITS(bar);
> + }
> +
> + value |= LM_RC_BAR_CFG_APERTURE(bar, aperture);
> +
> + size -= winsize;
> + cpu_addr += winsize;
> + bar++;
> + }
> + cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value);
> + *index = bar;
> +
> + return 0;
> +}
> +
> +static int cdns_pcie_host_map_dma_ranges(struct cdns_pcie_rc *rc)
> +{
> + enum cdns_pcie_rp_bar bar = RP_BAR0;
> + struct cdns_pcie *pcie = &rc->pcie;
> + struct device *dev = pcie->dev;
> + struct device_node *np = dev->of_node;
> + struct pci_host_bridge *bridge;
> + struct resource_entry *entry;
> + u32 no_bar_nbits = 32;
> + int err;
> +
> + bridge = pci_host_bridge_from_priv(rc);
> + if (!bridge)
> + return -ENOMEM;
> +
> + if (list_empty(&bridge->dma_ranges)) {
> + of_property_read_u32(np, "cdns,no-bar-match-nbits",
> + &no_bar_nbits);
> + cdns_pcie_host_bar_ib_config(rc, RP_NO_BAR, 0x0, no_bar_nbits);
> + return 0;
> + }
> +
> + resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> + err = cdns_pcie_host_bar_config(rc, entry, &bar);
Seems like this should have some better logic to pick which BAR to
use. Something like find the biggest region and then find the smallest
BAR that it fits in. Then get the next biggest...
> + if (err) {
> + dev_err(dev, "Fail to configure IB using dma-ranges\n");
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Vinod Koul @ 2020-05-19 17:13 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Andy Shevchenko, Viresh Kumar, Rob Herring,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200517174739.uis3wfievdcmtsxj@mobilestation>
On 17-05-20, 20:47, Serge Semin wrote:
> On Fri, May 15, 2020 at 02:11:13PM +0300, Serge Semin wrote:
> > On Fri, May 15, 2020 at 04:26:58PM +0530, Vinod Koul wrote:
> > > On 15-05-20, 13:51, Andy Shevchenko wrote:
> > > > On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > > > > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > > > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> > > >
> > > > ...
> > > >
> > > > > > I leave it to Rob and Vinod.
> > > > > > It won't break our case, so, feel free with your approach.
> > > > >
> > > > > I agree the DT is about describing the hardware and looks like value of
> > > > > 1 is not allowed. If allowed it should be added..
> > > >
> > > > It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> > > > synthesizing the IP.
> > >
> > > Then it should be added ..
> >
> > Vinod, max-burst-len is "MAXimum" burst length not "run-time or current or any
> > other" burst length. It's a constant defined at the IP-core synthesis stage and
> > according to the Data Book, MAX burst length can't be 1. The allowed values are
> > exactly as I described in the binding [4, 8, 16, 32, ...]. MAX burst length
> > defines the upper limit of the run-time burst length. So setting it to 1 isn't
> > about describing a hardware, but using DT for the software convenience.
> >
> > -Sergey
>
> Vinod, to make this completely clear. According to the DW DMAC data book:
> - In general, run-time parameter of the DMA transaction burst length (set in
> the SRC_MSIZE/DST_MSIZE fields of the channel control register) may belong
> to the set [1, 4, 8, 16, 32, 64, 128, 256].
so 1 is valid value for msize
> - Actual upper limit of the burst length run-time parameter is limited by a
> constant defined at the IP-synthesize stage (it's called DMAH_CHx_MAX_MULT_SIZE)
> and this constant belongs to the set [4, 8, 16, 32, 64, 128, 256]. (See, no 1
> in this set).
maximum can be 4 onwards, but in my configuration I can choose 1 as
value for msize
> So the run-time burst length in a case of particular DW DMA controller belongs
> to the range:
> 1 <= SRC_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
> and
> 1 <= DST_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
>
> See. No mater which DW DMA controller we get each of them will at least support
> the burst length of 1 and 4 transfer words. This is determined by design of the
> DW DMA controller IP since DMAH_CHx_MAX_MULT_SIZE constant set starts with 4.
>
> In this patch I suggest to add the max-burst-len property, which specifies
> the upper limit for the run-time burst length. Since the maximum burst length
> capable to be set to the SRC_MSIZE/DST_MSIZE fields of the DMA channel control
> register is determined by the DMAH_CHx_MAX_MULT_SIZE constant (which can't be 1
> by the DW DMA IP design), max-burst-len property as being also responsible for
> the maximum burst length setting should be associated with DMAH_CHx_MAX_MULT_SIZE
> thus should belong to the same set [4, 8, 16, 32, 64, 128, 256].
>
> So 1 shouldn't be in the enum of the max-burst-len property constraint, because
> hardware doesn't support such limitation by design, while setting 1 as
> max-burst-len would mean incorrect description of the DMA controller.
>
> Vinod, could you take a look at the info I provided above and say your final word
> whether 1 should be really allowed to be in the max-burst-len enum constraints?
> I'll do as you say in the next version of the patchset.
You are specifying the parameter which will be used to pick, i think
starting with 4 makes sense as we are specifying maximum allowed values
for msize. Values lesser than or equal to this would be allowed, I guess
that should be added to documentation.
thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH v7 3/3] power: supply: bq25150 introduce the bq25150
From: Ricardo Rivera-Matos @ 2020-05-19 17:15 UTC (permalink / raw)
To: Andrew F. Davis, Dan Murphy, sre
Cc: sspatil, linux-pm, robh, linux-kernel, devicetree
In-Reply-To: <50b9ca3f-36e8-d13b-672c-5e72b6158869@ti.com>
On 5/19/20 9:46 AM, Andrew F. Davis wrote:
> On 5/19/20 10:19 AM, Dan Murphy wrote:
>> +static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
>> +{
>> + int i;
>> +
>> + if (val > BQ2515X_ILIM_MAX || val < BQ2515X_ILIM_MIN)
>> + return -EINVAL;
> Clamp to these limits, not reject. Or better, modify the below loop so
> it clamps to the highest or lowest value in bq2515x_ilim_lvl_values[],
> then drop these #defines.
>
>> +
>> + for (i = 0; i < ARRAY_SIZE(bq2515x_ilim_lvl_values); i++) {
>> + if (val == bq2515x_ilim_lvl_values[i])
>> + break;
>> +
>> + if (val > bq2515x_ilim_lvl_values[i - 1] &&
> Index out of bounds for the i = 0 case.
>
>> + val < bq2515x_ilim_lvl_values[i]) {
>> + if (val - bq2515x_ilim_lvl_values[i - 1] <
>> + bq2515x_ilim_lvl_values[i] - val) {
> You are still missing the case where the value is closer to the [i]
> element, you check that it is between [i-1] and [i], but only chose
> [i-1] when it is closer to that than [i] but equal and greater case is
> missing.
>
> Given this sets input current limits, would instead always rounding down
> be the safer option?
>
> Andrew
ACK. I am going to rewrite that altogether.
>
>> + i = i - 1;
>> + break;
>> + }
>> + }
>> + }
>> +
>> + return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
>> +}
^ permalink raw reply
* Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU
From: Paul Boddie @ 2020-05-19 17:10 UTC (permalink / raw)
To: dri-devel
Cc: Paul Cercueil, Emil Velikov, devicetree, od, Rafael J . Wysocki,
David Airlie, Greg Kroah-Hartman, linux-kernel, Rob Herring
In-Reply-To: <YGYIAQ.7N9YGTBML0143@crapouillou.net>
On Monday 18. May 2020 13.26.58 Paul Cercueil wrote:
> >>
> >> @@ -186,13 +186,16 @@ static void
> >>
> >> ingenic_drm_crtc_update_timings(struct ingenic_drm *priv,
> >>
> >> regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
> >> JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16,
> >> JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
> >>
> >> +
> >> + regmap_write(priv->map, JZ_REG_LCD_IPUR, JZ_LCD_IPUR_IPUREN
> >> + (ht * vpe / 3) << JZ_LCD_IPUR_IPUR_LSB);
> >
> > This hunk also indicates that it may be better to merge the IPU within
> > the existing driver.
>
> This writes the IPUR register of the CRTC, nothing wrong here.
Since I noticed it in the above patch details, I think the mask when updating
the burst setting in the LCD_CTRL register should - in general - involve
multiple bits, since the BST field is 3 bits wide on the JZ4780 and 2 bits
wide on earlier products. Just setting BURST_16 (0b10) could potentially
enable BURST_32 (0b11) or other field values that are not explicitly defined.
Hope this is useful!
Paul
^ permalink raw reply
* Re: [PATCH v9] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
From: Evan Green @ 2020-05-19 17:28 UTC (permalink / raw)
To: Rakesh Pillai
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-arm Mailing List, LKML, linux-arm-msm
In-Reply-To: <1589712411-26718-1-git-send-email-pillair@codeaurora.org>
On Sun, May 17, 2020 at 3:47 AM Rakesh Pillai <pillair@codeaurora.org> wrote:
>
> Add device node for the ath10k SNOC platform driver probe
> and add resources required for WCN3990 on sc7180 soc.
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
> Changes from v8:
> - Removed the qcom,msa-fixed-perm
> ---
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 27 +++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 4e9149d..38b102e 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -389,6 +389,13 @@
> };
> };
>
> +&wifi {
> + status = "okay";
> + wifi-firmware {
> + iommus = <&apps_smmu 0xc2 0x1>;
> + };
> +};
> +
> /* PINCTRL - additions to nodes defined in sc7180.dtsi */
>
> &qspi_clk {
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index f1280e0..dd4e095 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -63,6 +63,11 @@
> clock-frequency = <32764>;
> #clock-cells = <0>;
> };
> +
> + wlan_fw_mem: memory@94100000 {
> + reg = <0 0x94100000 0 0x200000>;
> + no-map;
> + };
This node is not in the right place. Up through v8, this lived inside
reserved-memory. Here it seems to apply into the clocks {} node, which
is not the right spot.
> };
>
> reserved_memory: reserved-memory {
> @@ -944,6 +949,28 @@
> };
> };
>
> + wifi: wifi@18800000 {
> + compatible = "qcom,wcn3990-wifi";
> + reg = <0 0x18800000 0 0x800000>;
> + reg-names = "membase";
> + iommus = <&apps_smmu 0xc0 0x1>;
> + interrupts =
> + <GIC_SPI 414 IRQ_TYPE_LEVEL_HIGH /* CE0 */ >,
> + <GIC_SPI 415 IRQ_TYPE_LEVEL_HIGH /* CE1 */ >,
> + <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH /* CE2 */ >,
> + <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH /* CE3 */ >,
> + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH /* CE4 */ >,
> + <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH /* CE5 */ >,
> + <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH /* CE6 */ >,
> + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH /* CE7 */ >,
> + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH /* CE8 */ >,
> + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH /* CE9 */ >,
> + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH /* CE10 */>,
> + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH /* CE11 */>;
> + memory-region = <&wlan_fw_mem>;
Should any of the *-supply regulators be specified? Or are they all
board specific? Or just not needed? The example has these:
vdd-0.8-cx-mx-supply = <&pm8998_l5>;
vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
> + status = "disabled";
> + };
> +
> config_noc: interconnect@1500000 {
> compatible = "qcom,sc7180-config-noc";
> reg = <0 0x01500000 0 0x28000>;
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: phy: dp83869: Set opmode from straps
From: kbuild test robot @ 2020-05-19 17:19 UTC (permalink / raw)
To: Dan Murphy, andrew, f.fainelli, hkallweit1, davem
Cc: kbuild-all, netdev, linux-kernel, devicetree, Dan Murphy
In-Reply-To: <20200519141813.28167-3-dmurphy@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]
Hi Dan,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on robh/for-next sparc-next/master net/master linus/master v5.7-rc6 next-20200518]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Dan-Murphy/DP83869-Enhancements/20200519-222047
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 5cdfe8306631b2224e3f81fc5a1e2721c7a1948b
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
drivers/net/phy/dp83869.c: In function 'dp83869_set_strapped_mode':
>> drivers/net/phy/dp83869.c:171:10: warning: comparison is always false due to limited range of data type [-Wtype-limits]
171 | if (val < 0)
| ^
vim +171 drivers/net/phy/dp83869.c
164
165 static int dp83869_set_strapped_mode(struct phy_device *phydev)
166 {
167 struct dp83869_private *dp83869 = phydev->priv;
168 u16 val;
169
170 val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_STRAP_STS1);
> 171 if (val < 0)
172 return val;
173
174 dp83869->mode = val & DP83869_STRAP_OP_MODE_MASK;
175
176 return 0;
177 }
178
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54690 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: memory: document Renesas RPC-IF bindings
From: Rob Herring @ 2020-05-19 17:36 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Mason Yang, Philipp Zabel, Chris Brandt, linux-spi, devicetree,
linux-mtd@lists.infradead.org, Rob Herring
In-Reply-To: <f998fe6c-c586-17cb-9343-30460ce0bf9d@cogentembedded.com>
On Mon, 18 May 2020 23:33:15 +0300, Sergei Shtylyov wrote:
> Renesas Reduced Pin Count Interface (RPC-IF) allows a SPI flash or
> HyperFlash connected to the SoC to be accessed via the external address
> space read mode or the manual mode.
>
> Document the device tree bindings for the Renesas RPC-IF found in the R-Car
> gen3 SoCs.
>
> Based on the original patch by Mason Yang <masonccyang@mxic.com.tw>.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> Changes in version 3:
> - fixed up the whitespace and added hyphens in the "compatible" prop text, also
> removed the comments;
> - specified the subnode name as "flash", and used "enum" in the subnode text;
> - fixed up the SPDX-License-Identifier: tag.
>
> Changes in version 2:
> - rewrote the bindings in YAML.
>
> Documentation/devicetree/bindings/memory-controllers/renesas,rpc-if.yaml | 88 ++++++++++
> 1 file changed, 88 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/renesas,rpc-if.yaml: patternProperties:flash@[0-9a-f]+$:properties:compatible: [{'enum': ['cfi-flash', 'jedec,spi-nor']}] is not of type 'object', 'boolean'
Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/memory-controllers/renesas,rpc-if.example.dts' failed
make[1]: *** [Documentation/devicetree/bindings/memory-controllers/renesas,rpc-if.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1292811
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox