lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 26 May 2020 17:12:08 +0800
From:   Roger Lu <roger.lu@...iatek.com>
To:     Matthias Brugger <matthias.bgg@...il.com>
CC:     Enric Balletbo Serra <eballetbo@...il.com>,
        Kevin Hilman <khilman@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Nicolas Boichat <drinkcat@...gle.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Nishanth Menon <nm@...com>, Angus Lin <Angus.Lin@...iatek.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Xiaoqing Liu <Xiaoqing.Liu@...iatek.com>,
        YT Lee <yt.lee@...iatek.com>, Fan Chen <fan.chen@...iatek.com>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        HenryC Chen <HenryC.Chen@...iatek.com>,
        Charles Yang <Charles.Yang@...iatek.com>,
        "Linux ARM" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine

Hi Matthias,

Thanks for the feedback.

On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote:
> 
> On 22/05/2020 11:40, Roger Lu wrote:
> > 
> > Hi Enric,
> > 
> > On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote:
> >> 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.
> > 
> > Thanks for the advices. I'll submit SVS v9 with basic functionality
> > patch + step by step functionalities' patches. 
> > 
> >>
> >> Missatge de Roger Lu <roger.lu@...iatek.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@...iatek.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
> > 
> > Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide
> > module option in config. If, somehow, SVS isn't needed, we suggest
> > CONFIG_MTK_SVS=n to be set.
> > 
> 
> The question here is if it needs to be probed before we probe the modules. If
> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS.

Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that
will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used
with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC
to enable SVS power feature. Anyway, back to Enric's question, I'll make
MTK_SVS become a tristate feature in the next patch. Thanks.

> 
> >>
> >>> +       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.
> > 
> > MTK_SVS is designed for supporting different MTK SoCs.Therefore, the answer is second
> > question and thanks for the heads-up.
> > 
> >>
> >>> 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
> > 
> > OK. I'll use GPL-2.0-only.Thanks.
> > 
> >>
> >>> +/*
> >>> + * 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.
> > 
> > Ok. I will remove it. Thanks.
> > 
> >>
> >>> +
> >>> +#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_?
> > 
> > Oh. If we add bank wording like SVS_Bxxx, it might cause some confusion when B combines
> > with other words. So, I'll keep SVSB for SVS Bank representation.
> > E.g: SVS_BDC_SIGNED_BIT might lead to be explained differently ("SVS bank + DC_SIGNED_BIT" or "SVS + BDC_SIGNED_BIT")
> >      - "SVS bank + DC_SIGNED_BIT" is what we want for naming SVS_BDC_SIGNED_BIT but it might be misunderstood.
> > 
> >>
> >>> +#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_*
> > 
> > ok. I will add SVS or SVSB when it refers to SVS BANK.
> > 
> >>
> >>> +#define RUNCONFIG_DEFAULT              (0x80000000)
> >>
> >> Btw, there is any public datasheet where I can see those addresses and
> >> registers and bit fields?
> > 
> > Excuse us, there is no public datasheet. We can reply it on patchwork. Thanks.
> > 
> >>
> >>> +#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.
> > 
> > Ok. I'll change it to debugfs and could you give us an example about entries in documentation?
> > We can follow them. Thanks.
> > 
> >>
> >>> +static DEFINE_SPINLOCK(mtk_svs_lock);
> >>> +struct mtk_svs;
> >>> +
> >>> +enum svsb_phase {
> >>
> >> nit: mtk_svs_bphase?
> > 
> > ditto
> > 
> >>
> >>> +       SVSB_PHASE_INIT01 = 0,
> >>
> >> nit: SVS_BPHASE_?
> > 
> > ditto
> > 
> >>
> >>> +       SVSB_PHASE_INIT02,
> >>> +       SVSB_PHASE_MON,
> >>> +       SVSB_PHASE_ERROR,
> >>> +};
> >>> +
> >>> +enum reg_index {
> >>
> >> nit: svs_reg_index?
> > 
> > OK. Thanks.
> > 
> >>
> >>> +       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?
> > 
> > Shared between SoCs. Some SVS in MTK SoCs use v2 register map.
> > 
> 
> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep?

1. MT8173 IC uses v1 register map. 
2. Yes, I'll keep v2 postfix.

> 
> >>
> >>> +       [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.
> > 
> > Ok. Could you share a documentation example to us? We'll share the
> > information as much as we can. Thanks a lot.
> > 
> 
> you should find that in all drivers, eg:
> https://elixir.bootlin.com/linux/latest/source/drivers/soc/mediatek/mtk-scpsys.c#L111

No problem Sir. Thanks for showing a direction to me. I'll take a look
at it.

> 
> Regards,
> Matthias

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ