LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6/6] ASoC: fsl_micfil: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24  9:58 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>

When there is power domain bind with ipg_clk clock,

The call flow:
devm_regmap_init_mmio_clk
   - clk_prepare()
      - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Reviewed-by: Viorel Suman <viorel.suman@nxp.com>
---
 sound/soc/fsl/fsl_micfil.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 2b9edd4bb94d..3cf789ed6cbe 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -31,6 +31,7 @@ struct fsl_micfil {
 	struct platform_device *pdev;
 	struct regmap *regmap;
 	const struct fsl_micfil_soc_data *soc;
+	struct clk *busclk;
 	struct clk *mclk;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
 	unsigned int dataline;
@@ -660,16 +661,22 @@ static int fsl_micfil_probe(struct platform_device *pdev)
 		return PTR_ERR(micfil->mclk);
 	}
 
+	micfil->busclk = devm_clk_get(&pdev->dev, "ipg_clk");
+	if (IS_ERR(micfil->busclk)) {
+		dev_err(&pdev->dev, "failed to get ipg clock: %ld\n",
+			PTR_ERR(micfil->busclk));
+		return PTR_ERR(micfil->busclk);
+	}
+
 	/* init regmap */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	micfil->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
-						   "ipg_clk",
-						   regs,
-						   &fsl_micfil_regmap_config);
+	micfil->regmap = devm_regmap_init_mmio(&pdev->dev,
+					       regs,
+					       &fsl_micfil_regmap_config);
 	if (IS_ERR(micfil->regmap)) {
 		dev_err(&pdev->dev, "failed to init MICFIL regmap: %ld\n",
 			PTR_ERR(micfil->regmap));
@@ -729,6 +736,7 @@ static int fsl_micfil_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, micfil);
 
 	pm_runtime_enable(&pdev->dev);
+	regcache_cache_only(micfil->regmap, true);
 
 	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_micfil_component,
 					      &fsl_micfil_dai, 1);
@@ -752,6 +760,7 @@ static int __maybe_unused fsl_micfil_runtime_suspend(struct device *dev)
 	regcache_cache_only(micfil->regmap, true);
 
 	clk_disable_unprepare(micfil->mclk);
+	clk_disable_unprepare(micfil->busclk);
 
 	return 0;
 }
@@ -761,10 +770,16 @@ static int __maybe_unused fsl_micfil_runtime_resume(struct device *dev)
 	struct fsl_micfil *micfil = dev_get_drvdata(dev);
 	int ret;
 
-	ret = clk_prepare_enable(micfil->mclk);
+	ret = clk_prepare_enable(micfil->busclk);
 	if (ret < 0)
 		return ret;
 
+	ret = clk_prepare_enable(micfil->mclk);
+	if (ret < 0) {
+		clk_disable_unprepare(micfil->busclk);
+		return ret;
+	}
+
 	regcache_cache_only(micfil->regmap, false);
 	regcache_mark_dirty(micfil->regmap);
 	regcache_sync(micfil->regmap);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 4/6] ASoC: fsl_easrc: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24  9:58 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>

When there is power domain bind with mem clock,

The call flow:
devm_regmap_init_mmio_clk
  - clk_prepare()
      - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_easrc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 600e0d670ca6..5e33afe87c4a 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -1896,8 +1896,7 @@ static int fsl_easrc_probe(struct platform_device *pdev)
 
 	easrc->paddr = res->start;
 
-	easrc->regmap = devm_regmap_init_mmio_clk(dev, "mem", regs,
-						  &fsl_easrc_regmap_config);
+	easrc->regmap = devm_regmap_init_mmio(dev, regs, &fsl_easrc_regmap_config);
 	if (IS_ERR(easrc->regmap)) {
 		dev_err(dev, "failed to init regmap");
 		return PTR_ERR(easrc->regmap);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 2/6] ASoC: fsl_spdif: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24  9:58 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>

When there is power domain bind with core clock,

The call flow:
devm_regmap_init_mmio_clk
   - clk_prepare()
       - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_spdif.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 1cd3441d1c03..c631de325a6e 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1294,8 +1294,7 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	spdif_priv->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
-			"core", regs, &fsl_spdif_regmap_config);
+	spdif_priv->regmap = devm_regmap_init_mmio(&pdev->dev, regs, &fsl_spdif_regmap_config);
 	if (IS_ERR(spdif_priv->regmap)) {
 		dev_err(&pdev->dev, "regmap init failed\n");
 		return PTR_ERR(spdif_priv->regmap);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 5/6] ASoC: fsl_audmix: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24  9:58 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1616579928-22428-1-git-send-email-shengjiu.wang@nxp.com>

When there is power domain bind with ipg clock,

The call flow:
devm_regmap_init_mmio_clk
   - clk_prepare()
       - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk,but explicitly enable
clock when it is used.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_audmix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index 8dc44dec7956..f931288e256c 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -476,8 +476,7 @@ static int fsl_audmix_probe(struct platform_device *pdev)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
-	priv->regmap = devm_regmap_init_mmio_clk(dev, "ipg", regs,
-						 &fsl_audmix_regmap_config);
+	priv->regmap = devm_regmap_init_mmio(dev, regs, &fsl_audmix_regmap_config);
 	if (IS_ERR(priv->regmap)) {
 		dev_err(dev, "failed to init regmap\n");
 		return PTR_ERR(priv->regmap);
-- 
2.27.0


^ permalink raw reply related

* [PATCH 0/6] ASoC: fsl: Don't use devm_regmap_init_mmio_clk
From: Shengjiu Wang @ 2021-03-24  9:58 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
	alsa-devel
  Cc: linuxppc-dev, linux-kernel

When there is power domain bind with ipg clock,

The call flow:
devm_regmap_init_mmio_clk
    - clk_prepare()
        - clk_pm_runtime_get()

cause the power domain of clock always be enabled after
regmap_init(). which impact the power consumption.

So use devm_regmap_init_mmio instead of
devm_regmap_init_mmio_clk.

Shengjiu Wang (6):
  ASoC: fsl_esai: Don't use devm_regmap_init_mmio_clk
  ASoC: fsl_spdif: Don't use devm_regmap_init_mmio_clk
  ASoC: fsl_asrc: Don't use devm_regmap_init_mmio_clk
  ASoC: fsl_easrc: Don't use devm_regmap_init_mmio_clk
  ASoC: fsl_audmix: Don't use devm_regmap_init_mmio_clk
  ASoC: fsl_micfil: Don't use devm_regmap_init_mmio_clk

 sound/soc/fsl/fsl_asrc.c   | 57 +++++++++++++++++++++++++++++---------
 sound/soc/fsl/fsl_audmix.c |  3 +-
 sound/soc/fsl/fsl_easrc.c  |  3 +-
 sound/soc/fsl/fsl_esai.c   | 48 ++++++++++++++++++++++++--------
 sound/soc/fsl/fsl_micfil.c | 25 +++++++++++++----
 sound/soc/fsl/fsl_spdif.c  |  3 +-
 6 files changed, 103 insertions(+), 36 deletions(-)

-- 
2.27.0


^ permalink raw reply

* Re: [PATCH 08/10] MIPS: disable CONFIG_IDE in malta*_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
	linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-9-hch@lst.de>

On Thu, Mar 18, 2021 at 05:57:04AM +0100, Christoph Hellwig wrote:
> Various malta defconfigs enable CONFIG_IDE for the tc86c001 ide driver,
> hich is a Toshiba plug in card that does not make much sense to use on
> bigsur platforms.  For all other ATA cards libata support is already
> enabled.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/configs/malta_kvm_defconfig       | 3 ---
>  arch/mips/configs/malta_kvm_guest_defconfig | 3 ---
>  arch/mips/configs/maltaup_xpa_defconfig     | 3 ---
>  3 files changed, 9 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 04/10] MIPS: disable CONFIG_IDE in sb1250_swarm_defconfig
From: Thomas Bogendoerfer @ 2021-03-24  9:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
	linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-5-hch@lst.de>

On Thu, Mar 18, 2021 at 05:57:00AM +0100, Christoph Hellwig wrote:
> sb1250_swarm_defconfig enables CONFIG_IDE but no actual host controller
> driver, so just drop CONFIG_IDE, CONFIG_BLK_DEV_IDECD and
> CONFIG_BLK_DEV_IDETAPE as they are useless.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/configs/sb1250_swarm_defconfig | 3 ---
>  1 file changed, 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 06/10] MIPS: disable CONFIG_IDE in rbtx49xx_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
	linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-7-hch@lst.de>

On Thu, Mar 18, 2021 at 05:57:02AM +0100, Christoph Hellwig wrote:
> rbtx49xx_defconfig enables CONFIG_IDE for the tx4938 and tx4939 ide
> drivers, but those aren't actually used by the last known remaining user:
> 
> https://lore.kernel.org/lkml/20210107.101729.1936921832901251107.anemo@mba.ocn.ne.jp/
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/configs/rbtx49xx_defconfig | 3 ---
>  1 file changed, 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 05/10] MIPS: switch workpad_defconfig from legacy IDE to libata
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
	linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-6-hch@lst.de>

On Thu, Mar 18, 2021 at 05:57:01AM +0100, Christoph Hellwig wrote:
> Use libata instead of the deprecated legacy ide driver in
> workpad_defconfig.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/configs/workpad_defconfig | 9 ++++++---
>  1 file changed, 6 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 07/10] MIPS: disable CONFIG_IDE in bigsur_defconfig
From: Thomas Bogendoerfer @ 2021-03-24 10:00 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-doc, Russell King, linux-kernel, linux-ide,
	linux-m68k, Ivan Kokshaysky, linux-alpha, Geert Uytterhoeven,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210318045706.200458-8-hch@lst.de>

On Thu, Mar 18, 2021 at 05:57:03AM +0100, Christoph Hellwig wrote:
> bigsur_defconfig enables CONFIG_IDE for the tc86c001 ide driver, which
> is a Toshiba plug in card that does not make much sense to use on bigsur
> platforms.  For all other ATA cards libata support is already enabled.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/mips/configs/bigsur_defconfig | 4 ----
>  1 file changed, 4 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 v2] powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall
From: Vaibhav Jain @ 2021-03-24  9:53 UTC (permalink / raw)
  To: Shivaprasad G Bhat, linuxppc-dev, kvm-ppc, linux-nvdimm,
	aneesh.kumar, ellerman
  Cc: sbhat, linux-doc
In-Reply-To: <161651910115.13873.14215644994307713797.stgit@6532096d84d3>


Hi Shiva,

Thanks for the patch. Few minor review comments:

Shivaprasad G Bhat <sbhat@linux.ibm.com> writes:

> Add support for ND_REGION_ASYNC capability if the device tree
> indicates 'ibm,hcall-flush-required' property in the NVDIMM node.
> Flush is done by issuing H_SCM_FLUSH hcall to the hypervisor.
>
> If the flush request failed, the hypervisor is expected to
> to reflect the problem in the subsequent dimm health request call.
s/dimm/nvdimm
s/health request call/H_SCM_HEALTH hcall/

>
> This patch prevents mmap of namespaces with MAP_SYNC flag if the
> nvdimm requires explicit flush[1].
s/explicit/an explicit/

>
> References:
> [1] https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c
>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> v1 - https://www.spinics.net/lists/kvm-ppc/msg18272.html
> Changes from v1:
>        - Hcall semantics finalized, all changes are to accomodate them.
>
>  Documentation/powerpc/papr_hcalls.rst     |   14 ++++++++++
>  arch/powerpc/include/asm/hvcall.h         |    3 +-
>  arch/powerpc/platforms/pseries/papr_scm.c |   39 +++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/powerpc/papr_hcalls.rst b/Documentation/powerpc/papr_hcalls.rst
> index 48fcf1255a33..648f278eea8f 100644
> --- a/Documentation/powerpc/papr_hcalls.rst
> +++ b/Documentation/powerpc/papr_hcalls.rst
> @@ -275,6 +275,20 @@ Health Bitmap Flags:
>  Given a DRC Index collect the performance statistics for NVDIMM and copy them
>  to the resultBuffer.
>  
> +**H_SCM_FLUSH**
> +
> +| Input: *drcIndex, continue-token*
> +| Out: *continue-token*
> +| Return Value: *H_SUCCESS, H_Parameter, H_P2, H_BUSY*
> +
> +Given a DRC Index Flush the data to backend NVDIMM device.
> +
> +The hcall returns H_BUSY when the flush takes longer time and the hcall needs
> +to be issued multiple times in order to be completely serviced. The
> +*continue-token* from the output to be passed in the argument list of
> +subsequent hcalls to the hypervisor until the hcall is completely serviced
> +at which point H_SUCCESS or other error is returned by the hypervisor.
> +
>  References
>  ==========
>  .. [1] "Power Architecture Platform Reference"
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index ed6086d57b22..9f7729a97ebd 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -315,7 +315,8 @@
>  #define H_SCM_HEALTH            0x400
>  #define H_SCM_PERFORMANCE_STATS 0x418
>  #define H_RPT_INVALIDATE	0x448
> -#define MAX_HCALL_OPCODE	H_RPT_INVALIDATE
> +#define H_SCM_FLUSH		0x44C
> +#define MAX_HCALL_OPCODE	H_SCM_FLUSH
>  
>  /* Scope args for H_SCM_UNBIND_ALL */
>  #define H_UNBIND_SCOPE_ALL (0x1)
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index 835163f54244..f0407e135410 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -93,6 +93,7 @@ struct papr_scm_priv {
>  	uint64_t block_size;
>  	int metadata_size;
>  	bool is_volatile;
> +	bool hcall_flush_required;
>  
>  	uint64_t bound_addr;
>  
> @@ -117,6 +118,38 @@ struct papr_scm_priv {
>  	size_t stat_buffer_len;
>  };
>  
> +static int papr_scm_pmem_flush(struct nd_region *nd_region,
> +			       struct bio *bio __maybe_unused)
> +{
> +	struct papr_scm_priv *p = nd_region_provider_data(nd_region);
> +	unsigned long ret_buf[PLPAR_HCALL_BUFSIZE];
> +	uint64_t token = 0;
> +	int64_t rc;
> +
Suggest adding a dev_dbg to to indicate a flush request to a drc. That
way if the loop below gets stuck the issue can be debugged with kernel
logs.
> +	do {
> +		rc = plpar_hcall(H_SCM_FLUSH, ret_buf, p->drc_index, token);
> +		token = ret_buf[0];
> +
> +		/* Check if we are stalled for some time */
> +		if (H_IS_LONG_BUSY(rc)) {
> +			msleep(get_longbusy_msecs(rc));
> +			rc = H_BUSY;
> +		} else if (rc == H_BUSY) {
> +			cond_resched();
> +		}
> +
> +	} while (rc == H_BUSY);
> +
> +	if (rc) {
> +		dev_err(&p->pdev->dev, "flush error: %lld", rc);
> +		rc = -EIO;
> +	} else {
> +		dev_dbg(&p->pdev->dev, "flush drc 0x%x complete", p->drc_index);
> +	}
> +
> +	return rc;
> +}
> +
>  static LIST_HEAD(papr_nd_regions);
>  static DEFINE_MUTEX(papr_ndr_lock);
>  
> @@ -943,6 +976,11 @@ static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
>  	ndr_desc.num_mappings = 1;
>  	ndr_desc.nd_set = &p->nd_set;
>  
> +	if (p->hcall_flush_required) {
> +		set_bit(ND_REGION_ASYNC, &ndr_desc.flags);
> +		ndr_desc.flush = papr_scm_pmem_flush;
> +	}
> +
>  	if (p->is_volatile)
>  		p->region = nvdimm_volatile_region_create(p->bus, &ndr_desc);
>  	else {
> @@ -1088,6 +1126,7 @@ static int papr_scm_probe(struct platform_device *pdev)
>  	p->block_size = block_size;
>  	p->blocks = blocks;
>  	p->is_volatile = !of_property_read_bool(dn, "ibm,cache-flush-required");
> +	p->hcall_flush_required = of_property_read_bool(dn, "ibm,hcall-flush-required");
>  
>  	/* We just need to ensure that set cookies are unique across */
>  	uuid_parse(uuid_str, (uuid_t *) uuid);
>
>

-- 
Cheers
~ Vaibhav

^ permalink raw reply

* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24  9:46 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, kernelfans, paulus, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFsGYgdNH5HrlqDJ@piout.net>

Dear Alexandre,


在 2021/3/24 17:29, Alexandre Belloni 写道:
> On 24/03/2021 05:09:39-0400, He Ying wrote:
>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>> warning: symbol 'decrementer_max' was not declared. Should it be static?
>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>
>> Declare 'decrementer_max' in powerpc asm/time.h.
>> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
>> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
>> is declared there.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>> V2:
>> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>>    rtc_lock in powerpc asm/time.h.
>> V3:
>> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>>    kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>>    platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>>
>>   arch/powerpc/include/asm/time.h    | 1 +
>>   arch/powerpc/kernel/time.c         | 9 ++++-----
>>   arch/powerpc/platforms/chrp/time.c | 2 --
>>   3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>> index 8dd3cdb25338..2cd2b50bedda 100644
>> --- a/arch/powerpc/include/asm/time.h
>> +++ b/arch/powerpc/include/asm/time.h
>> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>>   extern unsigned long tb_ticks_per_usec;
>>   extern unsigned long tb_ticks_per_sec;
>>   extern struct clock_event_device decrementer_clockevent;
>> +extern u64 decrementer_max;
>>   
>>   
>>   extern void generic_calibrate_decr(void);
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index b67d93a609a2..ac81f043bf49 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -55,8 +55,9 @@
>>   #include <linux/sched/cputime.h>
>>   #include <linux/sched/clock.h>
>>   #include <linux/processor.h>
>> -#include <asm/trace.h>
>> +#include <linux/mc146818rtc.h>
> I'm fine with that but I really think my suggestion to make the rtc_lock
> local to the platforms was better because it is only used to synchronize
> between concurrent invocations of chrp_set_rtc_time or
> maple_set_rtc_time. The rtc core will never do that and the only case
> would be concurrent calls to rtc_ops.set_time and
> update_persistent_clock64 (which should also be removed at some point).

Many thanks for your suggestion. As you suggest, rtc_lock should be 
local to platforms.

Does it mean not only powerpc but also all other platforms should adapt 
this change?

It might be a big change. I have no idea if that's OK. What are other 
maintainers' opinions?


Thanks.



^ permalink raw reply

* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24  9:43 UTC (permalink / raw)
  To: Alexandre Belloni, He Ying
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, paulus, kernelfans, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFsGYgdNH5HrlqDJ@piout.net>



Le 24/03/2021 à 10:29, Alexandre Belloni a écrit :
> On 24/03/2021 05:09:39-0400, He Ying wrote:
>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>> warning: symbol 'decrementer_max' was not declared. Should it be static?
>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>
>> Declare 'decrementer_max' in powerpc asm/time.h.
>> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
>> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
>> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
>> is declared there.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>> V2:
>> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>>    rtc_lock in powerpc asm/time.h.
>> V3:
>> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>>    kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>>    platforms/chrp/time.c because it has included linux/mc146818rtc.h.
>>
>>   arch/powerpc/include/asm/time.h    | 1 +
>>   arch/powerpc/kernel/time.c         | 9 ++++-----
>>   arch/powerpc/platforms/chrp/time.c | 2 --
>>   3 files changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
>> index 8dd3cdb25338..2cd2b50bedda 100644
>> --- a/arch/powerpc/include/asm/time.h
>> +++ b/arch/powerpc/include/asm/time.h
>> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>>   extern unsigned long tb_ticks_per_usec;
>>   extern unsigned long tb_ticks_per_sec;
>>   extern struct clock_event_device decrementer_clockevent;
>> +extern u64 decrementer_max;
>>   
>>   
>>   extern void generic_calibrate_decr(void);
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index b67d93a609a2..ac81f043bf49 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -55,8 +55,9 @@
>>   #include <linux/sched/cputime.h>
>>   #include <linux/sched/clock.h>
>>   #include <linux/processor.h>
>> -#include <asm/trace.h>
>> +#include <linux/mc146818rtc.h>
> 
> I'm fine with that but I really think my suggestion to make the rtc_lock
> local to the platforms was better because it is only used to synchronize
> between concurrent invocations of chrp_set_rtc_time or
> maple_set_rtc_time. The rtc core will never do that and the only case
> would be concurrent calls to rtc_ops.set_time and
> update_persistent_clock64 (which should also be removed at some point).

I agree but I think it must be done carefully. If the lock is local to the driver really and without 
a link with the RTC core, then the lock var should probably be static. But then we'll have name 
conflict with the global rtc_lock which is declared in <linux/mc146818rtc.h>

All this is not easy, and I like your idea in the other mail to really clean up the rtc core and 
remove this global rtc_lock completely.

For the time being I guess the fix provided by this patch is just semantic and is just fine as is, 
as there is no real bug behind the messages from sparse.

Christophe

^ permalink raw reply

* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24  9:29 UTC (permalink / raw)
  To: He Ying, mpe, benh, paulus, a.zummo, alexandre.belloni, npiggin,
	msuchanek, tglx, peterz, geert, geert+renesas, kernelfans,
	frederic
  Cc: linux-rtc, linuxppc-dev, linux-kernel
In-Reply-To: <20210324090939.143477-1-heying24@huawei.com>



Le 24/03/2021 à 10:09, He Ying a écrit :
> We found these warnings in arch/powerpc/kernel/time.c as follows:
> warning: symbol 'decrementer_max' was not declared. Should it be static?
> warning: symbol 'rtc_lock' was not declared. Should it be static?
> warning: symbol 'dtl_consumer' was not declared. Should it be static?
> 
> Declare 'decrementer_max' in powerpc asm/time.h.
> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
> is declared there.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
> V2:
> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>    rtc_lock in powerpc asm/time.h.
> V3:
> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>    kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>    platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> 
>   arch/powerpc/include/asm/time.h    | 1 +
>   arch/powerpc/kernel/time.c         | 9 ++++-----
>   arch/powerpc/platforms/chrp/time.c | 2 --
>   3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>   extern unsigned long tb_ticks_per_usec;
>   extern unsigned long tb_ticks_per_sec;
>   extern struct clock_event_device decrementer_clockevent;
> +extern u64 decrementer_max;
>   
>   
>   extern void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..ac81f043bf49 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,8 +55,9 @@
>   #include <linux/sched/cputime.h>
>   #include <linux/sched/clock.h>
>   #include <linux/processor.h>
> -#include <asm/trace.h>
> +#include <linux/mc146818rtc.h>
>   
> +#include <asm/trace.h>
>   #include <asm/interrupt.h>
>   #include <asm/io.h>
>   #include <asm/nvram.h>
> @@ -150,10 +151,6 @@ bool tb_invalid;
>   u64 __cputime_usec_factor;
>   EXPORT_SYMBOL(__cputime_usec_factor);
>   
> -#ifdef CONFIG_PPC_SPLPAR
> -void (*dtl_consumer)(struct dtl_entry *, u64);
> -#endif
> -
>   static void calc_cputime_factors(void)
>   {
>   	struct div_result res;
> @@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>   
>   #include <asm/dtl.h>
>   
> +void (*dtl_consumer)(struct dtl_entry *, u64);
> +
>   /*
>    * Scan the dispatch trace log and count up the stolen time.
>    * Should be called with interrupts disabled.
> diff --git a/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
> index acde7bbe0716..b94dfd5090d8 100644
> --- a/arch/powerpc/platforms/chrp/time.c
> +++ b/arch/powerpc/platforms/chrp/time.c
> @@ -30,8 +30,6 @@
>   
>   #include <platforms/chrp/chrp.h>
>   
> -extern spinlock_t rtc_lock;
> -
>   #define NVRAM_AS0  0x74
>   #define NVRAM_AS1  0x75
>   #define NVRAM_DATA 0x77
> 

^ permalink raw reply

* Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: Alexandre Belloni @ 2021-03-24  9:29 UTC (permalink / raw)
  To: He Ying
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, kernelfans, paulus, geert, tglx, msuchanek, linuxppc-dev
In-Reply-To: <20210324090939.143477-1-heying24@huawei.com>

On 24/03/2021 05:09:39-0400, He Ying wrote:
> We found these warnings in arch/powerpc/kernel/time.c as follows:
> warning: symbol 'decrementer_max' was not declared. Should it be static?
> warning: symbol 'rtc_lock' was not declared. Should it be static?
> warning: symbol 'dtl_consumer' was not declared. Should it be static?
> 
> Declare 'decrementer_max' in powerpc asm/time.h.
> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
> is declared there.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: He Ying <heying24@huawei.com>
> ---
> V2:
> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>   rtc_lock in powerpc asm/time.h.
> V3:
> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>   kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>   platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> 
>  arch/powerpc/include/asm/time.h    | 1 +
>  arch/powerpc/kernel/time.c         | 9 ++++-----
>  arch/powerpc/platforms/chrp/time.c | 2 --
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>  extern unsigned long tb_ticks_per_usec;
>  extern unsigned long tb_ticks_per_sec;
>  extern struct clock_event_device decrementer_clockevent;
> +extern u64 decrementer_max;
>  
>  
>  extern void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..ac81f043bf49 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,8 +55,9 @@
>  #include <linux/sched/cputime.h>
>  #include <linux/sched/clock.h>
>  #include <linux/processor.h>
> -#include <asm/trace.h>
> +#include <linux/mc146818rtc.h>

I'm fine with that but I really think my suggestion to make the rtc_lock
local to the platforms was better because it is only used to synchronize
between concurrent invocations of chrp_set_rtc_time or
maple_set_rtc_time. The rtc core will never do that and the only case
would be concurrent calls to rtc_ops.set_time and
update_persistent_clock64 (which should also be removed at some point).

>  
> +#include <asm/trace.h>
>  #include <asm/interrupt.h>
>  #include <asm/io.h>
>  #include <asm/nvram.h>
> @@ -150,10 +151,6 @@ bool tb_invalid;
>  u64 __cputime_usec_factor;
>  EXPORT_SYMBOL(__cputime_usec_factor);
>  
> -#ifdef CONFIG_PPC_SPLPAR
> -void (*dtl_consumer)(struct dtl_entry *, u64);
> -#endif
> -
>  static void calc_cputime_factors(void)
>  {
>  	struct div_result res;
> @@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>  
>  #include <asm/dtl.h>
>  
> +void (*dtl_consumer)(struct dtl_entry *, u64);
> +
>  /*
>   * Scan the dispatch trace log and count up the stolen time.
>   * Should be called with interrupts disabled.
> diff --git a/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
> index acde7bbe0716..b94dfd5090d8 100644
> --- a/arch/powerpc/platforms/chrp/time.c
> +++ b/arch/powerpc/platforms/chrp/time.c
> @@ -30,8 +30,6 @@
>  
>  #include <platforms/chrp/chrp.h>
>  
> -extern spinlock_t rtc_lock;
> -
>  #define NVRAM_AS0  0x74
>  #define NVRAM_AS1  0x75
>  #define NVRAM_DATA 0x77
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings
From: He Ying @ 2021-03-24  9:09 UTC (permalink / raw)
  To: mpe, benh, paulus, a.zummo, alexandre.belloni, christophe.leroy,
	npiggin, msuchanek, heying24, tglx, peterz, geert, geert+renesas,
	kernelfans, frederic
  Cc: linux-rtc, linuxppc-dev, linux-kernel

We found these warnings in arch/powerpc/kernel/time.c as follows:
warning: symbol 'decrementer_max' was not declared. Should it be static?
warning: symbol 'rtc_lock' was not declared. Should it be static?
warning: symbol 'dtl_consumer' was not declared. Should it be static?

Declare 'decrementer_max' in powerpc asm/time.h.
Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
platforms/chrp/time.c because it has included linux/mc146818rtc.h.
Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
is declared there.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: He Ying <heying24@huawei.com>
---
V2:
- Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
  rtc_lock in powerpc asm/time.h.
V3:
- Recover to V1, that is including linux/mc146818rtc.h in powerpc
  kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
  platforms/chrp/time.c because it has included linux/mc146818rtc.h.

 arch/powerpc/include/asm/time.h    | 1 +
 arch/powerpc/kernel/time.c         | 9 ++++-----
 arch/powerpc/platforms/chrp/time.c | 2 --
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 8dd3cdb25338..2cd2b50bedda 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
 extern unsigned long tb_ticks_per_usec;
 extern unsigned long tb_ticks_per_sec;
 extern struct clock_event_device decrementer_clockevent;
+extern u64 decrementer_max;
 
 
 extern void generic_calibrate_decr(void);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b67d93a609a2..ac81f043bf49 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -55,8 +55,9 @@
 #include <linux/sched/cputime.h>
 #include <linux/sched/clock.h>
 #include <linux/processor.h>
-#include <asm/trace.h>
+#include <linux/mc146818rtc.h>
 
+#include <asm/trace.h>
 #include <asm/interrupt.h>
 #include <asm/io.h>
 #include <asm/nvram.h>
@@ -150,10 +151,6 @@ bool tb_invalid;
 u64 __cputime_usec_factor;
 EXPORT_SYMBOL(__cputime_usec_factor);
 
-#ifdef CONFIG_PPC_SPLPAR
-void (*dtl_consumer)(struct dtl_entry *, u64);
-#endif
-
 static void calc_cputime_factors(void)
 {
 	struct div_result res;
@@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
 
 #include <asm/dtl.h>
 
+void (*dtl_consumer)(struct dtl_entry *, u64);
+
 /*
  * Scan the dispatch trace log and count up the stolen time.
  * Should be called with interrupts disabled.
diff --git a/arch/powerpc/platforms/chrp/time.c b/arch/powerpc/platforms/chrp/time.c
index acde7bbe0716..b94dfd5090d8 100644
--- a/arch/powerpc/platforms/chrp/time.c
+++ b/arch/powerpc/platforms/chrp/time.c
@@ -30,8 +30,6 @@
 
 #include <platforms/chrp/chrp.h>
 
-extern spinlock_t rtc_lock;
-
 #define NVRAM_AS0  0x74
 #define NVRAM_AS1  0x75
 #define NVRAM_DATA 0x77
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Alexandre Belloni @ 2021-03-24  9:09 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-rtc, Alessandro Zummo, Geert Uytterhoeven, Peter Zijlstra,
	frederic, Linux Kernel Mailing List, Paul Mackerras,
	Nicholas Piggin, kernelfans, Thomas Gleixner, Michal Suchanek,
	linuxppc-dev, He Ying
In-Reply-To: <CAMuHMdWfFtJOQQf0b-2QJRd1EMLSW7rDsjNYzjjZhg6=JNZ0AA@mail.gmail.com>

On 24/03/2021 09:19:58+0100, Geert Uytterhoeven wrote:
> Hi Alexandre,
> 
> On Tue, Mar 23, 2021 at 11:18 PM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > On 23/03/2021 05:12:57-0400, He Ying wrote:
> > > We found these warnings in arch/powerpc/kernel/time.c as follows:
> > > warning: symbol 'decrementer_max' was not declared. Should it be static?
> > > warning: symbol 'rtc_lock' was not declared. Should it be static?
> > > warning: symbol 'dtl_consumer' was not declared. Should it be static?
> > >
> > > Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
> > > Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
> > > avoid the conflict with the variable in powerpc asm/time.h.
> > > Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
> > > is declared there.
> > >
> > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > Signed-off-by: He Ying <heying24@huawei.com>
> > > ---
> > > v2:
> > > - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
> > >   rtc_lock in powerpc asm/time.h.
> > >
> >
> > V1 was actually the correct thing to do. rtc_lock is there exactly
> > because chrp and maple are using mc146818 compatible RTCs. This is then
> > useful because then drivers/char/nvram.c is enabled. The proper fix
> > would be to scrap all of that and use rtc-cmos for those platforms as
> > this drives the RTC properly and exposes the NVRAM for the mc146818.
> >
> > Or at least, if there are no users for the char/nvram driver on those
> > two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
> > more likely rename the symbol as it seems to be abused by both chrp and
> > powermac.
> 
> IIRC, on CHRP LongTrail, NVRAM was inherited from CHRP's Mac ancestry,
> not from CHRP's PC ancestry, and thus NVRAM is not the one in the
> mc146818-compatible RTC.
> 
> http://users.telenet.be/geertu/Linux/PPC/DeviceTree.html confirms that,
> showing that nvram is a different device node than rtc.
> 

Yes, what I missed was the ifdefery in drivers/char/nvram.c that makes
it a completely different driver on both platforms. I tend to forget
about that as reading this driver is not a pleasant experience. I would
really like to get rid of the x86 part which would in turn allow to
remove the global rtc_lock spinlock on all architectures.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24  8:29 UTC (permalink / raw)
  To: Christophe Leroy, Alexandre Belloni
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <a30a94af-f551-c2a4-18df-a9488ba11d53@csgroup.eu>

Dear,


在 2021/3/24 14:22, Christophe Leroy 写道:
>
>
> Le 24/03/2021 à 07:14, Christophe Leroy a écrit :
>>
>>
>> Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
>>> On 23/03/2021 23:18:17+0100, Alexandre Belloni wrote:
>>>> Hello,
>>>>
>>>> On 23/03/2021 05:12:57-0400, He Ying wrote:
>>>>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>>>>> warning: symbol 'decrementer_max' was not declared. Should it be 
>>>>> static?
>>>>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>>>>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>>>>
>>>>> Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
>>>>> Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
>>>>> avoid the conflict with the variable in powerpc asm/time.h.
>>>>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" 
>>>>> because it
>>>>> is declared there.
>>>>>
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Signed-off-by: He Ying <heying24@huawei.com>
>>>>> ---
>>>>> v2:
>>>>> - Instead of including linux/mc146818rtc.h in powerpc 
>>>>> kernel/time.c, declare
>>>>>    rtc_lock in powerpc asm/time.h.
>>>>>
>>>>
>>>> V1 was actually the correct thing to do. rtc_lock is there exactly
>>>> because chrp and maple are using mc146818 compatible RTCs. This is 
>>>> then
>>>> useful because then drivers/char/nvram.c is enabled. The proper fix
>>>> would be to scrap all of that and use rtc-cmos for those platforms as
>>>> this drives the RTC properly and exposes the NVRAM for the mc146818.
>>>>
>>>> Or at least, if there are no users for the char/nvram driver on those
>>>> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
>>>> more likely rename the symbol as it seems to be abused by both chrp 
>>>> and
>>>> powermac.
>>>>
>>>
>>> Ok so rtc_lock is not even used by the char/nvram.c driver as it is
>>> completely compiled out.
>>>
>>> I guess it is fine having it move to the individual platform as looking
>>> very quickly at the Kconfig, it is not possible to select both
>>> simultaneously. Tentative patch:
>>>
>>
>> Looking at it once more, it looks like including linux/mc146818rtc.h 
>> is the thing to do, at least for now. Several platforms are defining 
>> the rtc_lock exactly the same way as powerpc does, and including 
>> mc146818rtc.h
>>
>> I think that to get it clean, this change should go in a dedicated 
>> patch and do a bit more and explain exactly what is being do and why. 
>> I'll try to draft something for it.
>>
>> He Y., can you make a version v3 of your patch excluding the rtc_lock 
>> change ?
>>
>
> Finally, I think there is not enough changes to justify a separate patch.
>
> So you can send a V3 based on your V1. In addition to the changes you 
> had in V1, please remove the declaration of rfc_lock in 
> arch/powerpc/platforms/chrp/chrp.h

So glad to hear that. I'll do that and send my V3.


Thanks.


^ permalink raw reply

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Geert Uytterhoeven @ 2021-03-24  8:19 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-rtc, Alessandro Zummo, Geert Uytterhoeven, Peter Zijlstra,
	frederic, Linux Kernel Mailing List, Paul Mackerras,
	Nicholas Piggin, kernelfans, Thomas Gleixner, Michal Suchanek,
	linuxppc-dev, He Ying
In-Reply-To: <YFppJkpZRHMJFay0@piout.net>

Hi Alexandre,

On Tue, Mar 23, 2021 at 11:18 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 23/03/2021 05:12:57-0400, He Ying wrote:
> > We found these warnings in arch/powerpc/kernel/time.c as follows:
> > warning: symbol 'decrementer_max' was not declared. Should it be static?
> > warning: symbol 'rtc_lock' was not declared. Should it be static?
> > warning: symbol 'dtl_consumer' was not declared. Should it be static?
> >
> > Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
> > Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
> > avoid the conflict with the variable in powerpc asm/time.h.
> > Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
> > is declared there.
> >
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: He Ying <heying24@huawei.com>
> > ---
> > v2:
> > - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
> >   rtc_lock in powerpc asm/time.h.
> >
>
> V1 was actually the correct thing to do. rtc_lock is there exactly
> because chrp and maple are using mc146818 compatible RTCs. This is then
> useful because then drivers/char/nvram.c is enabled. The proper fix
> would be to scrap all of that and use rtc-cmos for those platforms as
> this drives the RTC properly and exposes the NVRAM for the mc146818.
>
> Or at least, if there are no users for the char/nvram driver on those
> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
> more likely rename the symbol as it seems to be abused by both chrp and
> powermac.

IIRC, on CHRP LongTrail, NVRAM was inherited from CHRP's Mac ancestry,
not from CHRP's PC ancestry, and thus NVRAM is not the one in the
mc146818-compatible RTC.

http://users.telenet.be/geertu/Linux/PPC/DeviceTree.html confirms that,
showing that nvram is a different device node than rtc.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24  6:22 UTC (permalink / raw)
  To: Alexandre Belloni, He Ying
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <95cd80c5-40ff-1316-9c89-2e8e7836fb6a@csgroup.eu>



Le 24/03/2021 à 07:14, Christophe Leroy a écrit :
> 
> 
> Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
>> On 23/03/2021 23:18:17+0100, Alexandre Belloni wrote:
>>> Hello,
>>>
>>> On 23/03/2021 05:12:57-0400, He Ying wrote:
>>>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>>>> warning: symbol 'decrementer_max' was not declared. Should it be static?
>>>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>>>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>>>
>>>> Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
>>>> Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
>>>> avoid the conflict with the variable in powerpc asm/time.h.
>>>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
>>>> is declared there.
>>>>
>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>> Signed-off-by: He Ying <heying24@huawei.com>
>>>> ---
>>>> v2:
>>>> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>>>>    rtc_lock in powerpc asm/time.h.
>>>>
>>>
>>> V1 was actually the correct thing to do. rtc_lock is there exactly
>>> because chrp and maple are using mc146818 compatible RTCs. This is then
>>> useful because then drivers/char/nvram.c is enabled. The proper fix
>>> would be to scrap all of that and use rtc-cmos for those platforms as
>>> this drives the RTC properly and exposes the NVRAM for the mc146818.
>>>
>>> Or at least, if there are no users for the char/nvram driver on those
>>> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
>>> more likely rename the symbol as it seems to be abused by both chrp and
>>> powermac.
>>>
>>
>> Ok so rtc_lock is not even used by the char/nvram.c driver as it is
>> completely compiled out.
>>
>> I guess it is fine having it move to the individual platform as looking
>> very quickly at the Kconfig, it is not possible to select both
>> simultaneously. Tentative patch:
>>
> 
> Looking at it once more, it looks like including linux/mc146818rtc.h is the thing to do, at least 
> for now. Several platforms are defining the rtc_lock exactly the same way as powerpc does, and 
> including mc146818rtc.h
> 
> I think that to get it clean, this change should go in a dedicated patch and do a bit more and 
> explain exactly what is being do and why. I'll try to draft something for it.
> 
> He Y., can you make a version v3 of your patch excluding the rtc_lock change ?
> 

Finally, I think there is not enough changes to justify a separate patch.

So you can send a V3 based on your V1. In addition to the changes you had in V1, please remove the 
declaration of rfc_lock in arch/powerpc/platforms/chrp/chrp.h

Christophe

^ permalink raw reply

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: Christophe Leroy @ 2021-03-24  6:14 UTC (permalink / raw)
  To: Alexandre Belloni, He Ying
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFp0Qc2P61V+3bm0@piout.net>



Le 24/03/2021 à 00:05, Alexandre Belloni a écrit :
> On 23/03/2021 23:18:17+0100, Alexandre Belloni wrote:
>> Hello,
>>
>> On 23/03/2021 05:12:57-0400, He Ying wrote:
>>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>>> warning: symbol 'decrementer_max' was not declared. Should it be static?
>>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>>
>>> Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
>>> Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
>>> avoid the conflict with the variable in powerpc asm/time.h.
>>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
>>> is declared there.
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Signed-off-by: He Ying <heying24@huawei.com>
>>> ---
>>> v2:
>>> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>>>    rtc_lock in powerpc asm/time.h.
>>>
>>
>> V1 was actually the correct thing to do. rtc_lock is there exactly
>> because chrp and maple are using mc146818 compatible RTCs. This is then
>> useful because then drivers/char/nvram.c is enabled. The proper fix
>> would be to scrap all of that and use rtc-cmos for those platforms as
>> this drives the RTC properly and exposes the NVRAM for the mc146818.
>>
>> Or at least, if there are no users for the char/nvram driver on those
>> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
>> more likely rename the symbol as it seems to be abused by both chrp and
>> powermac.
>>
> 
> Ok so rtc_lock is not even used by the char/nvram.c driver as it is
> completely compiled out.
> 
> I guess it is fine having it move to the individual platform as looking
> very quickly at the Kconfig, it is not possible to select both
> simultaneously. Tentative patch:
> 

Looking at it once more, it looks like including linux/mc146818rtc.h is the thing to do, at least 
for now. Several platforms are defining the rtc_lock exactly the same way as powerpc does, and 
including mc146818rtc.h

I think that to get it clean, this change should go in a dedicated patch and do a bit more and 
explain exactly what is being do and why. I'll try to draft something for it.

He Y., can you make a version v3 of your patch excluding the rtc_lock change ?

Christophe

^ permalink raw reply

* Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
From: Cye Borg @ 2021-03-24  5:47 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Jens Axboe, Thomas Bogendoerfer, linux-doc, linux-alpha,
	linux-kernel, Christoph Hellwig, linux-ide, linux-m68k,
	Geert Uytterhoeven, John Paul Adrian Glaubitz, Ivan Kokshaysky,
	Matt Turner, linux-mips, linuxppc-dev, David S. Miller,
	linux-arm-kernel, Richard Henderson
In-Reply-To: <20210323184321.GE1463@shell.armlinux.org.uk>

Sure, here it is:
snow / # lspci -vxxx -s 7.0
00:07.0 ISA bridge: Contaq Microsystems 82c693
        Flags: bus master, medium devsel, latency 0
        Kernel modules: pata_cypress
00: 80 10 93 c6 47 00 80 02 00 00 01 06 00 00 80 00
10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 03 02 00 00 26 60 00 01 f0 60 00 80 80 71 00 00
50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Best regards,
Barnabas

ps.: let me know, if anything else I can do.

On Tue, Mar 23, 2021 at 7:43 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Mon, Mar 22, 2021 at 06:10:01PM +0100, Cye Borg wrote:
> > PWS 500au:
> >
> > snow / # lspci -vvx -s 7.1
> > 00:07.1 IDE interface: Contaq Microsystems 82c693 (prog-if 80 [ISA
> > Compatibility mode-only controller, supports bus mastering])
> >         Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop-
> > ParErr+ Stepping- SERR- FastB2B- DisINTx-
> >         Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> >         Latency: 0
> >         Interrupt: pin A routed to IRQ 0
> >         Region 0: I/O ports at 01f0 [size=8]
> >         Region 1: I/O ports at 03f4
> >         Region 4: I/O ports at 9080 [size=16]
> >         Kernel driver in use: pata_cypress
> >         Kernel modules: pata_cypress
> > 00: 80 10 93 c6 45 00 80 02 00 80 01 01 00 00 80 00
> > 10: f1 01 00 00 f5 03 00 00 00 00 00 00 00 00 00 00
> > 20: 81 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00
> >
> > snow / # lspci -vvx -s 7.2
> > 00:07.2 IDE interface: Contaq Microsystems 82c693 (prog-if 00 [ISA
> > Compatibility mode-only controller])
> >         Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop-
> > ParErr+ Stepping- SERR- FastB2B- DisINTx-
> >         Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium
> > >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> >         Latency: 0
> >         Interrupt: pin B routed to IRQ 0
> >         Region 0: I/O ports at 0170 [size=8]
> >         Region 1: I/O ports at 0374
> >         Region 4: Memory at 0c240000 (32-bit, non-prefetchable)
> > [disabled] [size=64K]
> >         Kernel modules: pata_cypress
> > 00: 80 10 93 c6 45 00 80 02 00 00 01 01 00 00 80 00
> > 10: 71 01 00 00 75 03 00 00 00 00 00 00 00 00 00 00
> > 20: 00 00 24 0c 00 00 00 00 00 00 00 00 00 00 00 00
> > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00
>
> Thanks very much.
>
> Could I also ask for the output of:
>
> # lspci -vxxx -s 7.0
>
> as well please - this will dump all 256 bytes for the ISA bridge, which
> contains a bunch of configuration registers. Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Madhavan Srinivasan @ 2021-03-24  4:35 UTC (permalink / raw)
  To: Athira Rajeev, linuxppc-dev, linux-kernel, linux-perf-users, mpe,
	acme, jolsa
  Cc: peterz, ravi.bangoria, kan.liang, kjain
In-Reply-To: <1616425047-1666-2-git-send-email-atrajeev@linux.vnet.ibm.com>


On 3/22/21 8:27 PM, Athira Rajeev wrote:
> Performance Monitoring Unit (PMU) registers in powerpc provides
> information on cycles elapsed between different stages in the
> pipeline. This can be used for application tuning. On ISA v3.1
> platform, this information is exposed by sampling registers.
> Patch adds kernel support to capture two of the cycle counters
> as part of perf sample using the sample type:
> PERF_SAMPLE_WEIGHT_STRUCT.
>
> The power PMU function 'get_mem_weight' currently uses 64 bit weight
> field of perf_sample_data to capture memory latency. But following the
> introduction of PERF_SAMPLE_WEIGHT_TYPE, weight field could contain
> 64-bit or 32-bit value depending on the architexture support for
> PERF_SAMPLE_WEIGHT_STRUCT. Patches uses WEIGHT_STRUCT to expose the
> pipeline stage cycles info. Hence update the ppmu functions to work for
> 64-bit and 32-bit weight values.
>
> If the sample type is PERF_SAMPLE_WEIGHT, use the 64-bit weight field.
> if the sample type is PERF_SAMPLE_WEIGHT_STRUCT, memory subsystem
> latency is stored in the low 32bits of perf_sample_weight structure.
> Also for CPU_FTR_ARCH_31, capture the two cycle counter information in
> two 16 bit fields of perf_sample_weight structure.

Changes looks fine to me.

Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>


> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
>   arch/powerpc/include/asm/perf_event_server.h |  2 +-
>   arch/powerpc/perf/core-book3s.c              |  4 ++--
>   arch/powerpc/perf/isa207-common.c            | 29 +++++++++++++++++++++++++---
>   arch/powerpc/perf/isa207-common.h            |  6 +++++-
>   4 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 00e7e671bb4b..112cf092d7b3 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -43,7 +43,7 @@ struct power_pmu {
>   				u64 alt[]);
>   	void		(*get_mem_data_src)(union perf_mem_data_src *dsrc,
>   				u32 flags, struct pt_regs *regs);
> -	void		(*get_mem_weight)(u64 *weight);
> +	void		(*get_mem_weight)(u64 *weight, u64 type);
>   	unsigned long	group_constraint_mask;
>   	unsigned long	group_constraint_val;
>   	u64             (*bhrb_filter_map)(u64 branch_sample_type);
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 766f064f00fb..6936763246bd 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2206,9 +2206,9 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
>   						ppmu->get_mem_data_src)
>   			ppmu->get_mem_data_src(&data.data_src, ppmu->flags, regs);
>   
> -		if (event->attr.sample_type & PERF_SAMPLE_WEIGHT &&
> +		if (event->attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE &&
>   						ppmu->get_mem_weight)
> -			ppmu->get_mem_weight(&data.weight.full);
> +			ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
>   
>   		if (perf_event_overflow(event, &data, regs))
>   			power_pmu_stop(event, 0);
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index e4f577da33d8..5dcbdbd54598 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -284,8 +284,10 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
>   	}
>   }
>   
> -void isa207_get_mem_weight(u64 *weight)
> +void isa207_get_mem_weight(u64 *weight, u64 type)
>   {
> +	union perf_sample_weight *weight_fields;
> +	u64 weight_lat;
>   	u64 mmcra = mfspr(SPRN_MMCRA);
>   	u64 exp = MMCRA_THR_CTR_EXP(mmcra);
>   	u64 mantissa = MMCRA_THR_CTR_MANT(mmcra);
> @@ -296,9 +298,30 @@ void isa207_get_mem_weight(u64 *weight)
>   		mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
>   
>   	if (val == 0 || val == 7)
> -		*weight = 0;
> +		weight_lat = 0;
>   	else
> -		*weight = mantissa << (2 * exp);
> +		weight_lat = mantissa << (2 * exp);
> +
> +	/*
> +	 * Use 64 bit weight field (full) if sample type is
> +	 * WEIGHT.
> +	 *
> +	 * if sample type is WEIGHT_STRUCT:
> +	 * - store memory latency in the lower 32 bits.
> +	 * - For ISA v3.1, use remaining two 16 bit fields of
> +	 *   perf_sample_weight to store cycle counter values
> +	 *   from sier2.
> +	 */
> +	weight_fields = (union perf_sample_weight *)weight;
> +	if (type & PERF_SAMPLE_WEIGHT)
> +		weight_fields->full = weight_lat;
> +	else {
> +		weight_fields->var1_dw = (u32)weight_lat;
> +		if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> +			weight_fields->var2_w = P10_SIER2_FINISH_CYC(mfspr(SPRN_SIER2));
> +			weight_fields->var3_w = P10_SIER2_DISPATCH_CYC(mfspr(SPRN_SIER2));
> +		}
> +	}
>   }
>   
>   int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp, u64 event_config1)
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 1af0e8c97ac7..fc30d43c4d0c 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -265,6 +265,10 @@
>   #define ISA207_SIER_DATA_SRC_SHIFT	53
>   #define ISA207_SIER_DATA_SRC_MASK	(0x7ull << ISA207_SIER_DATA_SRC_SHIFT)
>   
> +/* Bits in SIER2/SIER3 for Power10 */
> +#define P10_SIER2_FINISH_CYC(sier2)	(((sier2) >> (63 - 37)) & 0x7fful)
> +#define P10_SIER2_DISPATCH_CYC(sier2)	(((sier2) >> (63 - 13)) & 0x7fful)
> +
>   #define P(a, b)				PERF_MEM_S(a, b)
>   #define PH(a, b)			(P(LVL, HIT) | P(a, b))
>   #define PM(a, b)			(P(LVL, MISS) | P(a, b))
> @@ -278,6 +282,6 @@ int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
>   					const unsigned int ev_alt[][MAX_ALT]);
>   void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
>   							struct pt_regs *regs);
> -void isa207_get_mem_weight(u64 *weight);
> +void isa207_get_mem_weight(u64 *weight, u64 type);
>   
>   #endif

^ permalink raw reply

* Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings
From: heying (H) @ 2021-03-24  1:58 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-rtc, a.zummo, geert+renesas, peterz, frederic, linux-kernel,
	npiggin, paulus, kernelfans, tglx, msuchanek, linuxppc-dev
In-Reply-To: <YFppJkpZRHMJFay0@piout.net>

Dear,


在 2021/3/24 6:18, Alexandre Belloni 写道:
> Hello,
>
> On 23/03/2021 05:12:57-0400, He Ying wrote:
>> We found these warnings in arch/powerpc/kernel/time.c as follows:
>> warning: symbol 'decrementer_max' was not declared. Should it be static?
>> warning: symbol 'rtc_lock' was not declared. Should it be static?
>> warning: symbol 'dtl_consumer' was not declared. Should it be static?
>>
>> Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
>> Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
>> avoid the conflict with the variable in powerpc asm/time.h.
>> Move 'dtl_consumer' definition behind "include <asm/dtl.h>" because it
>> is declared there.
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: He Ying <heying24@huawei.com>
>> ---
>> v2:
>> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>>    rtc_lock in powerpc asm/time.h.
>>
> V1 was actually the correct thing to do. rtc_lock is there exactly
> because chrp and maple are using mc146818 compatible RTCs. This is then
> useful because then drivers/char/nvram.c is enabled. The proper fix
> would be to scrap all of that and use rtc-cmos for those platforms as
> this drives the RTC properly and exposes the NVRAM for the mc146818.

Do you mean that 'rtc_lock' declared in linux/mc146818rtc.h points to

same thing as that defined in powerpc kernel/time.c? And you think V1

was correct? Oh, I should have added you to my patch V1 senders:)

>
> Or at least, if there are no users for the char/nvram driver on those
> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
> more likely rename the symbol as it seems to be abused by both chrp and
> powermac.
>
> I'm not completely against the rename in vr41xxx but the fix for the
> warnings can and should be contained in arch/powerpc.

Yes, I agree with you. But I have no choice because there is a compiling 
error.

Maybe there's a better way.

So, what about my patch V1? Should I resend it and add you to senders?


Thanks.


^ permalink raw reply

* Re: [PATCH v4 44/46] KVM: PPC: Book3S HV P9: implement hash guest support
From: Nicholas Piggin @ 2021-03-24  1:34 UTC (permalink / raw)
  To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87tup1kgtb.fsf@linux.ibm.com>

Excerpts from Fabiano Rosas's message of March 24, 2021 1:53 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> Guest entry/exit has to restore and save/clear the SLB, plus several
>> other bits to accommodate hash guests in the P9 path.
>>
>> Radix host, hash guest support is removed from the P7/8 path.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
> 
> <snip>
> 
>> diff --git a/arch/powerpc/kvm/book3s_hv_interrupt.c b/arch/powerpc/kvm/book3s_hv_interrupt.c
>> index cd84d2c37632..03fbfef708a8 100644
>> --- a/arch/powerpc/kvm/book3s_hv_interrupt.c
>> +++ b/arch/powerpc/kvm/book3s_hv_interrupt.c
>> @@ -55,6 +55,50 @@ static void __accumulate_time(struct kvm_vcpu *vcpu, struct kvmhv_tb_accumulator
>>  #define accumulate_time(vcpu, next) do {} while (0)
>>  #endif
>>
>> +static inline void mfslb(unsigned int idx, u64 *slbee, u64 *slbev)
>> +{
>> +	asm volatile("slbmfev  %0,%1" : "=r" (*slbev) : "r" (idx));
>> +	asm volatile("slbmfee  %0,%1" : "=r" (*slbee) : "r" (idx));
>> +}
>> +
>> +static inline void __mtslb(u64 slbee, u64 slbev)
>> +{
>> +	asm volatile("slbmte %0,%1" :: "r" (slbev), "r" (slbee));
>> +}
>> +
>> +static inline void mtslb(unsigned int idx, u64 slbee, u64 slbev)
>> +{
>> +	BUG_ON((slbee & 0xfff) != idx);
>> +
>> +	__mtslb(slbee, slbev);
>> +}
>> +
>> +static inline void slb_invalidate(unsigned int ih)
>> +{
>> +	asm volatile("slbia %0" :: "i"(ih));
>> +}
> 
> Fyi, in my environment the assembler complains:
> 
> {standard input}: Assembler messages:                                    
> {standard input}:1293: Error: junk at end of line: `6'                             
> {standard input}:2138: Error: junk at end of line: `6'                    
> make[3]: *** [../scripts/Makefile.build:271:
> arch/powerpc/kvm/book3s_hv_interrupt.o] Error 1
> 
> This works:
> 
> -       asm volatile("slbia %0" :: "i"(ih));
> +       asm volatile(PPC_SLBIA(%0) :: "i"(ih));
> 
> But I don't know what is going on.

Ah yes, we still need to use PPC_SLBIA. IH parameter to slbia was only 
added in binutils 2.27 and we support down to 2.23.

Thanks for the fix I'll add it.

Thanks,
Nick

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox