Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/7] iommu/arm-smmu: Add a SMMU variant for the Adreno GPU
From: Jordan Crouse @ 2019-08-20 19:06 UTC (permalink / raw)
  To: freedreno
  Cc: linux-arm-msm, Joerg Roedel, linux-kernel, iommu, Will Deacon,
	linux-arm-kernel, Robin Murphy
In-Reply-To: <1566327992-362-1-git-send-email-jcrouse@codeaurora.org>

Add a SMMU model for the Adreno GPU and use it to enable split
pagetable support if the conditions are right.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/iommu/arm-smmu-impl.c | 15 +++++++++++++++
 drivers/iommu/arm-smmu.c      |  2 ++
 drivers/iommu/arm-smmu.h      |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index 3f88cd0..5d197dd 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -147,6 +147,18 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
 	.reset = arm_mmu500_reset,
 };
 
+static int qcom_adreno_init_context(struct arm_smmu_domain *smmu_domain)
+{
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
+		smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64)
+		smmu_domain->split_pagetables = true;
+
+	return 0;
+}
+
+static const struct arm_smmu_impl qcom_adreno_impl = {
+	.init_context = qcom_adreno_init_context,
+};
 
 struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 {
@@ -162,6 +174,9 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 		break;
 	case CAVIUM_SMMUV2:
 		return cavium_smmu_impl_init(smmu);
+	case QCOM_ADRENO_SMMUV2:
+		smmu->impl = &qcom_adreno_impl;
+		break;
 	default:
 		break;
 	}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 39e81ef..3f41cf7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1858,6 +1858,7 @@ ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
 ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
 ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
 ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
+ARM_SMMU_MATCH_DATA(qcom_adreno_smmuv2, ARM_SMMU_V2, QCOM_ADRENO_SMMUV2);
 
 static const struct of_device_id arm_smmu_of_match[] = {
 	{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -1867,6 +1868,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
 	{ .compatible = "arm,mmu-500", .data = &arm_mmu500 },
 	{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
 	{ .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
+	{ .compatible = "qcom,adreno-smmu-v2", .data = &qcom_adreno_smmuv2 },
 	{ },
 };
 
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 91a4eb8..e5a2cc8 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -222,6 +222,7 @@ enum arm_smmu_implementation {
 	ARM_MMU500,
 	CAVIUM_SMMUV2,
 	QCOM_SMMUV2,
+	QCOM_ADRENO_SMMUV2,
 };
 
 struct arm_smmu_device {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 5/7] iommu/arm-smmu: Support DOMAIN_ATTR_SPLIT_TABLES
From: Jordan Crouse @ 2019-08-20 19:06 UTC (permalink / raw)
  To: freedreno
  Cc: linux-arm-msm, Joerg Roedel, linux-kernel, iommu, Will Deacon,
	linux-arm-kernel, Robin Murphy
In-Reply-To: <1566327992-362-1-git-send-email-jcrouse@codeaurora.org>

Support the DOMAIN_ATTR_SPLIT_TABLES attribute to let the leaf driver
know if split pagetables are enabled for the domain.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/iommu/arm-smmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 3f41cf7..6a512ff 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1442,6 +1442,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 		case DOMAIN_ATTR_NESTING:
 			*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
 			return 0;
+		case DOMAIN_ATTR_SPLIT_TABLES:
+			*(int *)data = !!(smmu_domain->split_pagetables);
+			return 0;
 		default:
 			return -ENODEV;
 		}
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [RFC 04/11] soc: amlogic: Add support for SM1 power controller
From: Kevin Hilman @ 2019-08-20 19:19 UTC (permalink / raw)
  To: Neil Armstrong, jbrunet; +Cc: linux-amlogic, linux-kernel, linux-arm-kernel
In-Reply-To: <98bda35e-1b4c-404c-fdbd-eaef9ecf38a6@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> On 20/08/2019 01:56, Kevin Hilman wrote:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>> 
>>> Add support for the General Purpose Amlogic SM1 Power controller,
>>> dedicated to the PCIe, USB, NNA and GE2D Power Domains.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> 
>> I like this driver in general, but as I look at all the EE power domains
>> for GX, G12 and SM1 they are really very similar.  I had started to
>> generalize the gx-pwrc-vpu driver and it ends up looking just like this.
>
> Yes I developed it to be generic, but when starting to fill up the GXBB/GXL/G12A
> domains, except the VPU, they only need the PD parts.
>
>> 
>> I think this driver could be generalized just a little bit more and then
>> replace the the GX-specific VPU one, and AFAICT, then be used across all
>> the 64-bit SoCs, and be called "meson-pwrc-ee" or something like that...
>> 
>>> ---
>>>  drivers/soc/amlogic/Kconfig          |  11 ++
>>>  drivers/soc/amlogic/Makefile         |   1 +
>>>  drivers/soc/amlogic/meson-sm1-pwrc.c | 245 +++++++++++++++++++++++++++
>>>  3 files changed, 257 insertions(+)
>>>  create mode 100644 drivers/soc/amlogic/meson-sm1-pwrc.c
>>>
>>> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
>>> index 5501ad5650b2..596f1afef1a7 100644
>>> --- a/drivers/soc/amlogic/Kconfig
>>> +++ b/drivers/soc/amlogic/Kconfig
>>> @@ -36,6 +36,17 @@ config MESON_GX_PM_DOMAINS
>>>  	  Say yes to expose Amlogic Meson GX Power Domains as
>>>  	  Generic Power Domains.
>>>  
>>> +config MESON_SM1_PM_DOMAINS
>>> +	bool "Amlogic Meson SM1 Power Domains driver"
>>> +	depends on ARCH_MESON || COMPILE_TEST
>>> +	depends on PM && OF
>>> +	default ARCH_MESON
>>> +	select PM_GENERIC_DOMAINS
>>> +	select PM_GENERIC_DOMAINS_OF
>>> +	help
>>> +	  Say yes to expose Amlogic Meson SM1 Power Domains as
>>> +	  Generic Power Domains.
>>> +
>>>  config MESON_MX_SOCINFO
>>>  	bool "Amlogic Meson MX SoC Information driver"
>>>  	depends on ARCH_MESON || COMPILE_TEST
>>> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
>>> index bf2d109f61e9..f99935499ee6 100644
>>> --- a/drivers/soc/amlogic/Makefile
>>> +++ b/drivers/soc/amlogic/Makefile
>>> @@ -3,3 +3,4 @@ obj-$(CONFIG_MESON_CLK_MEASURE) += meson-clk-measure.o
>>>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>>>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>>>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
>>> +obj-$(CONFIG_MESON_SM1_PM_DOMAINS) += meson-sm1-pwrc.o
>>> diff --git a/drivers/soc/amlogic/meson-sm1-pwrc.c b/drivers/soc/amlogic/meson-sm1-pwrc.c
>>> new file mode 100644
>>> index 000000000000..9ece1d06f417
>>> --- /dev/null
>>> +++ b/drivers/soc/amlogic/meson-sm1-pwrc.c
>>> @@ -0,0 +1,245 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (c) 2017 BayLibre, SAS
>>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>>> + */
>>> +
>>> +#include <linux/of_address.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_domain.h>
>>> +#include <linux/bitfield.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/mfd/syscon.h>
>>> +#include <linux/of_device.h>
>>> +#include <dt-bindings/power/meson-sm1-power.h>
>>> +
>>> +/* AO Offsets */
>>> +
>>> +#define AO_RTI_GEN_PWR_SLEEP0		(0x3a << 2)
>>> +#define AO_RTI_GEN_PWR_ISO0		(0x3b << 2)
>>> +
>>> +/* HHI Offsets */
>>> +
>>> +#define HHI_MEM_PD_REG0			(0x40 << 2)
>>> +#define HHI_NANOQ_MEM_PD_REG0		(0x46 << 2)
>>> +#define HHI_NANOQ_MEM_PD_REG1		(0x47 << 2)
>>> +
>>> +struct meson_sm1_pwrc;
>>> +
>>> +struct meson_sm1_pwrc_mem_domain {
>>> +	unsigned int reg;
>>> +	unsigned int mask;
>>> +};
>>> +
>>> +struct meson_sm1_pwrc_domain_desc {
>>> +	char *name;
>>> +	unsigned int sleep_reg;
>>> +	unsigned int sleep_bit;
>>> +	unsigned int iso_reg;
>>> +	unsigned int iso_bit;
>>> +	unsigned int mem_pd_count;
>>> +	struct meson_sm1_pwrc_mem_domain *mem_pd;
>>> +};
>> 
>> If you add resets and clocks (using clk bulk like my other proposed
>> patch to gx-pwrc-vpu) then this could be used for VPU also.  We could
>> ignore my clk bulk patch and then just deprecate the old driver and use
>> this one for everything.
>> 
>> We would just need SoC-specific tables selected by compatible-string to
>> select the memory pds, and the clocks and resets could (optionaly) come
>> from the DT.
>
> Could you elaborate ?
>
> Do you mean I should slit out the memory PDs as different compatible ?

You currently create all these SoC-specific `mem_domain` tables.  We'll
need more of those for the other SoCs, so my suggestion was that, in
order to use this across multiple SoCs, you select the set of mem_domain
tables based on compatible string.

That was just my first idea.  If you have a better idea, I'm open to
that too.

> Let me try to fit the VPU stuff in it.

Great, thanks!

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
From: Jon Hunter @ 2019-08-20 19:24 UTC (permalink / raw)
  To: Sylwester Nawrocki, krzk
  Cc: devicetree, linux-samsung-soc, linux-pm, pankaj.dubey,
	b.zolnierkie, linux-kernel, robh+dt, kgene, vireshk, linux-tegra,
	linux-arm-kernel, m.szyprowski
In-Reply-To: <20190813150827.31972-3-s.nawrocki@samsung.com>


On 13/08/2019 16:08, Sylwester Nawrocki wrote:
> Convert the driver to use regmap API in order to allow other
> drivers, like ASV, to access the CHIPID registers.
> 
> This patch adds definition of selected CHIPID register offsets
> and register bit fields for Exynos5422 SoC.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v2:
>  - s/_EXYNOS_ASV_H/__LINU_SOC_EXYNOS_ASV_H,
>  - removed __func__ from error log,
>  - removed unneeded <linux/of_address.h> header inclusion.
> 
> Changes since v1 (RFC):
>  - new patch
> ---
>  drivers/soc/samsung/exynos-chipid.c       | 34 ++++++---------
>  include/linux/soc/samsung/exynos-chipid.h | 52 +++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 21 deletions(-)
>  create mode 100644 include/linux/soc/samsung/exynos-chipid.h
> 
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index bcf691f2b650..006a95feb618 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -9,16 +9,13 @@
>   */
>  
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
> -#include <linux/of_address.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/soc/samsung/exynos-chipid.h>
>  #include <linux/sys_soc.h>
>  
> -#define EXYNOS_SUBREV_MASK	(0xF << 4)
> -#define EXYNOS_MAINREV_MASK	(0xF << 0)
> -#define EXYNOS_REV_MASK		(EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> -#define EXYNOS_MASK		0xFFFFF000
> -
>  static const struct exynos_soc_id {
>  	const char *name;
>  	unsigned int id;
> @@ -51,29 +48,24 @@ static const char * __init product_id_to_soc_id(unsigned int product_id)
>  int __init exynos_chipid_early_init(void)
>  {
>  	struct soc_device_attribute *soc_dev_attr;
> -	void __iomem *exynos_chipid_base;
>  	struct soc_device *soc_dev;
>  	struct device_node *root;
> -	struct device_node *np;
> +	struct regmap *regmap;
>  	u32 product_id;
>  	u32 revision;
> +	int ret;
>  
> -	/* look up for chipid node */
> -	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
> -	if (!np)
> -		return -ENODEV;
> -
> -	exynos_chipid_base = of_iomap(np, 0);
> -	of_node_put(np);
> -
> -	if (!exynos_chipid_base) {
> -		pr_err("Failed to map SoC chipid\n");
> -		return -ENXIO;
> +	regmap = syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid");
> +	if (IS_ERR(regmap)) {
> +		pr_err("Failed to get CHIPID regmap\n");
> +		return PTR_ERR(regmap);
>  	}

Following this change, I am now seeing the above error on our Tegra
boards where this driver is enabled. This is triggering a kernel
warnings test we have to fail. Hence, I don't think that you can remove
the compatible node test here, unless you have a better way to determine
if this is a samsung device.

Cheers
Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 0/6] arm64: Add support for Amlogic SM1 SoC Family
From: Kevin Hilman @ 2019-08-20 19:25 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-amlogic, linux-kernel, linux-arm-kernel, Neil Armstrong
In-Reply-To: <20190820144052.18269-1-narmstrong@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> The new Amlogic SM1 SoC Family is a derivative of the Amlogic G12A
> SoC Family, with the following changes :
> - Cortex-A55 cores instead of A53
> - more power domains, including USB & PCIe
> - a neural network co-processor (NNA)
> - a CSI input and image processor
> - some changes in the audio complex, thus not yet enabled
> - new clocks, for NNA, CSI and a clock tree for each CPU Core
>
> This serie does not add support for NNA, CSI, Audio, USB, Display
> or DVFS, it only aligns with the current G12A Support.
>
> With this serie, the SEI610 Board has supported :
> - Default-boot CPU frequency
> - Ethernet
> - LEDs
> - IR
> - GPIO Buttons
> - eMMC
> - SDCard
> - SDIO WiFi
> - UART Bluetooth
>
> Audio (HDMI, Embedded HP, MIcs), USB, HDMI, IR Output, & RGB Led
> would be supported in following patchsets.
>
> Dependencies:
> - g12-common.dtsi from the DVFS patchset at [1]
>
> Changes from rfc at [2]:
> - Removed Power domain patches & display/USB support, will resend separately
> - Removed applied AO-CEC patches
> - Fixed clk-measure IDs
> - Collected reviewed-by tags
>
> [1] https://patchwork.kernel.org/cover/11025309/
> [2] https://patchwork.kernel.org/cover/11025511/

Series queued for v5.4...
> Neil Armstrong (6):
>   soc: amlogic: meson-gx-socinfo: Add SM1 and S905X3 IDs
>   dt-bindings: soc: amlogic: clk-measure: Add SM1 compatible
>   soc: amlogic: clk-measure: Add support for SM1

... these 3 in v5.4/drivers ...

>   dt-bindings: arm: amlogic: add SM1 bindings
>   dt-bindings: arm: amlogic: add SEI Robotics SEI610 bindings
>   arm64: dts: add support for SM1 based SEI Robotics SEI610

... and these 3 in v5.4/dt64 with Rob's tag.

Thanks,

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 11/17] perf cs-etm: Support sample flags 'insn' and 'insnlen'
From: Arnaldo Carvalho de Melo @ 2019-08-20 19:27 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner
  Cc: Arnaldo Carvalho de Melo, Mathieu Poirier, Suzuki Poulouse,
	Clark Williams, coresight, linux-kernel, linux-perf-users,
	Alexander Shishkin, Jiri Olsa, Leo Yan, Namhyung Kim,
	Robert Walker, Jiri Olsa, linux-arm-kernel, Mike Leach
In-Reply-To: <20190820192733.19180-1-acme@kernel.org>

From: Leo Yan <leo.yan@linaro.org>

The synthetic branch and instruction samples are missed to set
instruction related info, thus the perf tool fails to display samples
with flags '-F,+insn,+insnlen'.

The CoreSight trace decoder provides sufficient information to decide
the instruction size based on the ISA type: A64/A32 instructions are
32-bit size, but one exception is the T32 instruction size, which might
be 32-bit or 16-bit.

This patch handles these cases and it reads the instruction values from
DSO file; thus can support the flags '-F,+insn,+insnlen'.

Before:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0
     ffff97174938 _dl_start ilen: 0

  [...]

After:

  # perf script -F,insn,insnlen,ip,sym
                0 [unknown] ilen: 0
     ffff97174044 _start ilen: 4 insn: 2f 02 00 94
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54
     ffff97174938 _dl_start ilen: 4 insn: c1 ff ff 54

  [...]

Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Robert Walker <robert.walker@arm.com>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20190815082854.18191-1-leo.yan@linaro.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index ed6f7fd5b90b..b3a5daaf1a8f 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1076,6 +1076,35 @@ bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq)
 	return !!etmq->etm->timeless_decoding;
 }
 
+static void cs_etm__copy_insn(struct cs_etm_queue *etmq,
+			      u64 trace_chan_id,
+			      const struct cs_etm_packet *packet,
+			      struct perf_sample *sample)
+{
+	/*
+	 * It's pointless to read instructions for the CS_ETM_DISCONTINUITY
+	 * packet, so directly bail out with 'insn_len' = 0.
+	 */
+	if (packet->sample_type == CS_ETM_DISCONTINUITY) {
+		sample->insn_len = 0;
+		return;
+	}
+
+	/*
+	 * T32 instruction size might be 32-bit or 16-bit, decide by calling
+	 * cs_etm__t32_instr_size().
+	 */
+	if (packet->isa == CS_ETM_ISA_T32)
+		sample->insn_len = cs_etm__t32_instr_size(etmq, trace_chan_id,
+							  sample->ip);
+	/* Otherwise, A64 and A32 instruction size are always 32-bit. */
+	else
+		sample->insn_len = 4;
+
+	cs_etm__mem_access(etmq, trace_chan_id, sample->ip,
+			   sample->insn_len, (void *)sample->insn);
+}
+
 static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 					    struct cs_etm_traceid_queue *tidq,
 					    u64 addr, u64 period)
@@ -1097,9 +1126,10 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq,
 	sample.period = period;
 	sample.cpu = tidq->packet->cpu;
 	sample.flags = tidq->prev_packet->flags;
-	sample.insn_len = 1;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->packet, &sample);
+
 	if (etm->synth_opts.last_branch) {
 		cs_etm__copy_last_branch_rb(etmq, tidq);
 		sample.branch_stack = tidq->last_branch;
@@ -1159,6 +1189,9 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq,
 	sample.flags = tidq->prev_packet->flags;
 	sample.cpumode = event->sample.header.misc;
 
+	cs_etm__copy_insn(etmq, tidq->trace_chan_id, tidq->prev_packet,
+			  &sample);
+
 	/*
 	 * perf report cannot handle events without a branch stack
 	 */
-- 
2.21.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
From: Krzysztof Kozlowski @ 2019-08-20 19:37 UTC (permalink / raw)
  To: Jon Hunter, Sylwester Nawrocki
  Cc: devicetree, linux-samsung-soc@vger.kernel.org, Arnd Bergmann,
	linux-pm, pankaj.dubey, Bartłomiej Żołnierkiewicz,
	linux-kernel@vger.kernel.org, robh+dt, kgene, vireshk,
	linux-tegra, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <b5359603-b337-dcd8-b025-ca7dff5f4a06@nvidia.com>

On Tue, 20 Aug 2019 at 21:24, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 13/08/2019 16:08, Sylwester Nawrocki wrote:
> > Convert the driver to use regmap API in order to allow other
> > drivers, like ASV, to access the CHIPID registers.
> >
> > This patch adds definition of selected CHIPID register offsets
> > and register bit fields for Exynos5422 SoC.
> >
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > ---
> > Changes since v2:
> >  - s/_EXYNOS_ASV_H/__LINU_SOC_EXYNOS_ASV_H,
> >  - removed __func__ from error log,
> >  - removed unneeded <linux/of_address.h> header inclusion.
> >
> > Changes since v1 (RFC):
> >  - new patch
> > ---
> >  drivers/soc/samsung/exynos-chipid.c       | 34 ++++++---------
> >  include/linux/soc/samsung/exynos-chipid.h | 52 +++++++++++++++++++++++
> >  2 files changed, 65 insertions(+), 21 deletions(-)
> >  create mode 100644 include/linux/soc/samsung/exynos-chipid.h
> >
> > diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> > index bcf691f2b650..006a95feb618 100644
> > --- a/drivers/soc/samsung/exynos-chipid.c
> > +++ b/drivers/soc/samsung/exynos-chipid.c
> > @@ -9,16 +9,13 @@
> >   */
> >
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of.h>
> > -#include <linux/of_address.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> > +#include <linux/soc/samsung/exynos-chipid.h>
> >  #include <linux/sys_soc.h>
> >
> > -#define EXYNOS_SUBREV_MASK   (0xF << 4)
> > -#define EXYNOS_MAINREV_MASK  (0xF << 0)
> > -#define EXYNOS_REV_MASK              (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> > -#define EXYNOS_MASK          0xFFFFF000
> > -
> >  static const struct exynos_soc_id {
> >       const char *name;
> >       unsigned int id;
> > @@ -51,29 +48,24 @@ static const char * __init product_id_to_soc_id(unsigned int product_id)
> >  int __init exynos_chipid_early_init(void)
> >  {
> >       struct soc_device_attribute *soc_dev_attr;
> > -     void __iomem *exynos_chipid_base;
> >       struct soc_device *soc_dev;
> >       struct device_node *root;
> > -     struct device_node *np;
> > +     struct regmap *regmap;
> >       u32 product_id;
> >       u32 revision;
> > +     int ret;
> >
> > -     /* look up for chipid node */
> > -     np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
> > -     if (!np)
> > -             return -ENODEV;
> > -
> > -     exynos_chipid_base = of_iomap(np, 0);
> > -     of_node_put(np);
> > -
> > -     if (!exynos_chipid_base) {
> > -             pr_err("Failed to map SoC chipid\n");
> > -             return -ENXIO;
> > +     regmap = syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid");
> > +     if (IS_ERR(regmap)) {
> > +             pr_err("Failed to get CHIPID regmap\n");
> > +             return PTR_ERR(regmap);
> >       }
>
> Following this change, I am now seeing the above error on our Tegra
> boards where this driver is enabled. This is triggering a kernel
> warnings test we have to fail. Hence, I don't think that you can remove
> the compatible node test here, unless you have a better way to determine
> if this is a samsung device.

Right, this is really wrong... I missed that it is not a probe but
early init. And this init will be called on every board... Probably it
should be converted to a regular driver.

This is very old patchset, revived recently. I see that in v6 it was a
platform driver:
https://patchwork.kernel.org/patch/9134949/
Pankaj, apparently based on these comments, made it initcall... but why?

Another point is that Arnd complained there about exposing global
header and no change here - we still expose the global header, but not
with soc revisions but register internals... although it has its
purpose - other Exynos-specific drivers need to access through regmap.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 6/7] ARM: dts: at91: at91sam9x5: switch to new clock bindings
From: Alexandre Belloni @ 2019-08-20 19:40 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-kernel, Ludovic Desroches, linux-arm-kernel,
	Thomas Gleixner, info
In-Reply-To: <20190728205615.lgxyfwzqcftb6e77@pengutronix.de>

Hi,

On 28/07/2019 22:56:15+0200, Uwe Kleine-König wrote:
> Hello,
> 
> [added tglx to Cc because Alexandre said in irc: 4 or 5 years ago, there
> was some discussion (with tglx) to make the driver not sleep when early
> in the boot process I'll try to dig that up]
> 

This is the discussion I was referring to:
https://lore.kernel.org/lkml/6120818.MyeJZ74hYa@wuerfel/

Incidentally, I think this is the patch you need to fix your issue but I
doesn't apply because the code moved to sckc.c

Afzal then points to tglx's mail:
https://lore.kernel.org/linux-arm-kernel/alpine.DEB.2.11.1606061448010.28031@nanos/

The whole series took 3 years to get in and this patch slipped through
the cracks. I'll resend it to restart the discussion.

However, could you dump the clock tree before and after the switch to
the new clock binding because it should actually change the behaviour
(it should have crashed before the change)  and I fear something else
is selected the wrong slow clock parent.

> On Sat, Jul 27, 2019 at 05:39:43PM +0200, Uwe Kleine-König wrote:
> > Hello Alexandre,
> > 
> > On Mon, Nov 12, 2018 at 02:31:07PM +0100, Alexandre Belloni wrote:
> > > Switch at91sam9x5 boards to the new PMC clock bindings.
> > 
> > This patch results in a broken dtb for my AriettaG25[1]. It compiles
> > fine, but booting results in:
> > 
> > 	SD/MMC: dt blob: Read file acme-arietta.dtb to 0x21000000
> > 	SD: Card Capacity: Standard
> > 	SD: Specification Version 3.0X
> > 
> > 	Booting zImage ......
> > 	zImage magic: 0x16f2818 is found
> > 
> > 	Using device tree in place at 0x21000000
> > 
> > 	Starting linux kernel ..., machid: 0xffffffff
> > 
> > 	Uncompressing Linux... done, booting the kernel.
> > 	Booting Linux on physical CPU 0x0
> > 	Linux version 5.3.0-rc1+ (uwe@taurus) (gcc version 7.3.1 20180201 (OSELAS.Toolchain-2018.02.0 7-20180201)) #4 Sat Jul 27 15:47:15 CEST 2019
> > 	CPU: ARM926EJ-S [41069265] revision 5 (ARMv5TEJ), cr=0005317f
> > 	CPU: VIVT data cache, VIVT instruction cache
> > 	OF: fdt: Machine model: Acme Systems Arietta G25
> > 	printk: bootconsole [earlycon0] enabled
> > 	printk: debug: ignoring loglevel setting.
> > 	Memory policy: Data cache writeback
> > 	On node 0 totalpages: 32768
> > 	  Normal zone: 256 pages used for memmap
> > 	  Normal zone: 0 pages reserved
> > 	  Normal zone: 32768 pages, LIFO batch:7
> > 	pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
> > 	pcpu-alloc: [0] 0 
> > 	Built 1 zonelists, mobility grouping on.  Total pages: 32512
> > 	Kernel command line: earlyprintk ignore_loglevel  mem=128M console=ttyS0,115200 root=/dev/mmcblk0p2 rootfstype=ext4 rw rootwait
> > 	Dentry cache hash table entries: 16384 (order: 4, 65536 bytes, linear)
> > 	Inode-cache hash table entries: 8192 (order: 3, 32768 bytes, linear)
> > 	mem auto-init: stack:off, heap alloc:off, heap free:off
> > 	Memory: 121352K/131072K available (5826K kernel code, 199K rwdata, 1892K rodata, 232K init, 125K bss, 9720K reserved, 0K cma-reserved)
> > 	NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
> > 	random: get_random_bytes called from start_kernel+0x294/0x474 with crng_init=0
> > 	bad: scheduling from the idle thread!
> > 	CPU: 0 PID: 0 Comm: swapper Not tainted 5.3.0-rc1+ #4
> > 	Hardware name: Atmel AT91SAM9
> > 	[<c000fc4c>] (unwind_backtrace) from [<c000d654>] (show_stack+0x10/0x18)
> > 	[<c000d654>] (show_stack) from [<c059db74>] (dump_stack+0x18/0x24)
> > 	[<c059db74>] (dump_stack) from [<c00386e8>] (dequeue_task_idle+0x1c/0x34)
> > 	[<c00386e8>] (dequeue_task_idle) from [<c05b5818>] (__schedule+0x250/0x38c)
> > 	[<c05b5818>] (__schedule) from [<c05b59e8>] (schedule+0x94/0xc8)
> > 	[<c05b59e8>] (schedule) from [<c05b876c>] (schedule_hrtimeout_range_clock+0xf4/0x13c)
> > 	[<c05b876c>] (schedule_hrtimeout_range_clock) from [<c05b87cc>] (schedule_hrtimeout_range+0x18/0x24)
> > 	[<c05b87cc>] (schedule_hrtimeout_range) from [<c05b8240>] (usleep_range+0x70/0x98)
> > 	[<c05b8240>] (usleep_range) from [<c02a18a0>] (clk_slow_rc_osc_prepare+0x28/0x34)
> > 	[<c02a18a0>] (clk_slow_rc_osc_prepare) from [<c029af78>] (clk_core_prepare+0x84/0xa0)
> > 	[<c029af78>] (clk_core_prepare) from [<c029af54>] (clk_core_prepare+0x60/0xa0)
> > 	[<c029af54>] (clk_core_prepare) from [<c029b790>] (clk_prepare+0x1c/0x30)
> > 	[<c029b790>] (clk_prepare) from [<c02f1310>] (regmap_mmio_attach_clk+0x1c/0x24)
> > 	[<c02f1310>] (regmap_mmio_attach_clk) from [<c02f7a98>] (of_syscon_register+0x218/0x268)
> > 	[<c02f7a98>] (of_syscon_register) from [<c02f7b18>] (syscon_node_to_regmap+0x30/0x58)
> > 	[<c02f7b18>] (syscon_node_to_regmap) from [<c07d2f7c>] (at91sam9x5_pmc_setup+0x78/0x430)
> > 	[<c07d2f7c>] (at91sam9x5_pmc_setup) from [<c07cfe88>] (of_clk_init+0x188/0x21c)
> > 	[<c07cfe88>] (of_clk_init) from [<c07c062c>] (time_init+0x1c/0x2c)
> > 	[<c07c062c>] (time_init) from [<c07bcbec>] (start_kernel+0x2c4/0x474)
> > 	[<c07bcbec>] (start_kernel) from [<00000000>] (0x0)
> > 	bad: scheduling from the idle thread!
> > 	CPU: 0 PID: 0 Comm: swapper Not tainted 5.3.0-rc1+ #4
> > 	Hardware name: Atmel AT91SAM9
> > 	[<c000fc4c>] (unwind_backtrace) from [<c000d654>] (show_stack+0x10/0x18)
> > 	[<c000d654>] (show_stack) from [<c059db74>] (dump_stack+0x18/0x24)
> > 	[<c059db74>] (dump_stack) from [<c00386e8>] (dequeue_task_idle+0x1c/0x34)
> > 	[<c00386e8>] (dequeue_task_idle) from [<c05b5818>] (__schedule+0x250/0x38c)
> > 	[<c05b5818>] (__schedule) from [<c05b59e8>] (schedule+0x94/0xc8)
> > 	[<c05b59e8>] (schedule) from [<c05b876c>] (schedule_hrtimeout_range_clock+0xf4/0x13c)
> > 	[<c05b876c>] (schedule_hrtimeout_range_clock) from [<c05b87cc>] (schedule_hrtimeout_range+0x18/0x24)
> > 	[<c05b87cc>] (schedule_hrtimeout_range) from [<c05b8240>] (usleep_range+0x70/0x98)
> > 	[<c05b8240>] (usleep_range) from [<c02a18a0>] (clk_slow_rc_osc_prepare+0x28/0x34)
> > 	[<c02a18a0>] (clk_slow_rc_osc_prepare) from [<c029af78>] (clk_core_prepare+0x84/0xa0)
> > 	[<c029af78>] (clk_core_prepare) from [<c029af54>] (clk_core_prepare+0x60/0xa0)
> > 	[<c029af54>] (clk_core_prepare) from [<c029b790>] (clk_prepare+0x1c/0x30)
> > 	[<c029b790>] (clk_prepare) from [<c02f1310>] (regmap_mmio_attach_clk+0x1c/0x24)
> > 	[<c02f1310>] (regmap_mmio_attach_clk) from [<c02f7a98>] (of_syscon_register+0x218/0x268)
> > 	[<c02f7a98>] (of_syscon_register) from [<c02f7b18>] (syscon_node_to_regmap+0x30/0x58)
> > 	[<c02f7b18>] (syscon_node_to_regmap) from [<c07d2f7c>] (at91sam9x5_pmc_setup+0x78/0x430)
> > 	[<c07d2f7c>] (at91sam9x5_pmc_setup) from [<c07cfe88>] (of_clk_init+0x188/0x21c)
> > 	[<c07cfe88>] (of_clk_init) from [<c07c062c>] (time_init+0x1c/0x2c)
> > 	[<c07c062c>] (time_init) from [<c07bcbec>] (start_kernel+0x2c4/0x474)
> > 	[<c07bcbec>] (start_kernel) from [<00000000>] (0x0)
> > 	bad: scheduling from the idle thread!
> > 	CPU: 0 PID: 0 Comm: swapper Not tainted 5.3.0-rc1+ #4
> > 	Hardware name: Atmel AT91SAM9
> > 	...
> > 
> > The error message repeats without end. (I obviously didn't test until
> > the end :-), but after 10 minutes the machine still prints this message.)
> > 
> > With the old dtb Linux 5.3-rc1 boots just fine. I assume this is not
> > related to my dts or .config so I'm not attaching these here. If you're
> > interested I can provide them of course.
> 
> With the following diff applied, the machine boots just fine:
> 
> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
> index 9bfe9a28294a..4d2be45be916 100644
> --- a/drivers/clk/at91/sckc.c
> +++ b/drivers/clk/at91/sckc.c
> @@ -187,7 +187,7 @@ static int clk_slow_rc_osc_prepare(struct clk_hw *hw)
>  
>  	writel(readl(sckcr) | osc->bits->cr_rcen, sckcr);
>  
> -	usleep_range(osc->startup_usec, osc->startup_usec + 1);
> +	udelay(osc->startup_usec);
>  
>  	return 0;
>  }
> 
> Best regards
> Uwe

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 0/2] Coresight: Fix for v5.3-rc5
From: Mathieu Poirier @ 2019-08-20 19:41 UTC (permalink / raw)
  To: gregkh; +Cc: linux-arm-kernel

Good day,

Please see if you can add the following fix to the current cycle.  If
you think it is a little late I'll simply lump them with the set I
have for v5.4.

Applies cleanly to your char-misc-linus (d1abaeb3be7b) branch.

Thanks,
Mathieu 


Yabin Cui (2):
  coresight: tmc-etr: Fix updating buffer in not-snapshot mode
  coresight: tmc-etr: Fix perf_data check

 .../hwtracing/coresight/coresight-tmc-etr.c   | 26 +++++++++++--------
 drivers/hwtracing/coresight/coresight-tmc.h   |  6 ++---
 2 files changed, 18 insertions(+), 14 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2] coresight: tmc-etr: Fix perf_data check
From: Mathieu Poirier @ 2019-08-20 19:41 UTC (permalink / raw)
  To: gregkh; +Cc: linux-arm-kernel
In-Reply-To: <20190820194155.28992-1-mathieu.poirier@linaro.org>

From: Yabin Cui <yabinc@google.com>

When tracing etm data of multiple threads on multiple cpus through
perf interface, each cpu has a unique etr_perf_buffer while sharing
the same etr device. There is no guarantee that the last cpu starts
etm tracing also stops last. This makes perf_data check fail.

Fix it by checking etr_buf instead of etr_perf_buffer.
Also move the code setting and clearing perf_buf to more suitable
places.

Fixes: 3147da92a8a8 ("coresight: tmc-etr: Allocate and free ETR memory buffers for CPU-wide scenarios")
Signed-off-by: Yabin Cui <yabinc@google.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 ++++----
 drivers/hwtracing/coresight/coresight-tmc.h     | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 676dcb4cf0e2..ba644f444d4c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1484,7 +1484,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
 		goto out;
 	}
 
-	if (WARN_ON(drvdata->perf_data != etr_perf)) {
+	if (WARN_ON(drvdata->perf_buf != etr_buf)) {
 		lost = true;
 		spin_unlock_irqrestore(&drvdata->spinlock, flags);
 		goto out;
@@ -1496,8 +1496,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
 	tmc_sync_etr_buf(drvdata);
 
 	CS_LOCK(drvdata->base);
-	/* Reset perf specific data */
-	drvdata->perf_data = NULL;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	size = etr_buf->len;
@@ -1560,7 +1558,6 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
 	}
 
 	etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
-	drvdata->perf_data = etr_perf;
 
 	/*
 	 * No HW configuration is needed if the sink is already in
@@ -1576,6 +1573,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
 		/* Associate with monitored process. */
 		drvdata->pid = pid;
 		drvdata->mode = CS_MODE_PERF;
+		drvdata->perf_buf = etr_perf->etr_buf;
 		atomic_inc(csdev->refcnt);
 	}
 
@@ -1621,6 +1619,8 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
 	/* Dissociate from monitored process. */
 	drvdata->pid = -1;
 	drvdata->mode = CS_MODE_DISABLED;
+	/* Reset perf specific data */
+	drvdata->perf_buf = NULL;
 
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 1ed50411cc3c..f9a0c95e9ba2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -178,8 +178,8 @@ struct etr_buf {
  *		device configuration register (DEVID)
  * @idr:	Holds etr_bufs allocated for this ETR.
  * @idr_mutex:	Access serialisation for idr.
- * @perf_data:	PERF buffer for ETR.
- * @sysfs_data:	SYSFS buffer for ETR.
+ * @sysfs_buf:	SYSFS buffer for ETR.
+ * @perf_buf:	PERF buffer for ETR.
  */
 struct tmc_drvdata {
 	void __iomem		*base;
@@ -202,7 +202,7 @@ struct tmc_drvdata {
 	struct idr		idr;
 	struct mutex		idr_mutex;
 	struct etr_buf		*sysfs_buf;
-	void			*perf_data;
+	struct etr_buf		*perf_buf;
 };
 
 struct etr_buf_operations {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 1/2] coresight: tmc-etr: Fix updating buffer in not-snapshot mode
From: Mathieu Poirier @ 2019-08-20 19:41 UTC (permalink / raw)
  To: gregkh; +Cc: linux-arm-kernel
In-Reply-To: <20190820194155.28992-1-mathieu.poirier@linaro.org>

From: Yabin Cui <yabinc@google.com>

TMC etr always copies all available data to perf aux buffer, which
may exceed the available space in perf aux buffer. It isn't suitable
for not-snapshot mode, because:
1) It may overwrite previously written data.
2) It may make the perf_event_mmap_page->aux_head report having more
or less data than the reality.

So change to only copy the latest data fitting the available space in
perf aux buffer.

Signed-off-by: Yabin Cui <yabinc@google.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 .../hwtracing/coresight/coresight-tmc-etr.c    | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 17006705287a..676dcb4cf0e2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1410,9 +1410,10 @@ static void tmc_free_etr_buffer(void *config)
  * tmc_etr_sync_perf_buffer: Copy the actual trace data from the hardware
  * buffer to the perf ring buffer.
  */
-static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf)
+static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf,
+				     unsigned long to_copy)
 {
-	long bytes, to_copy;
+	long bytes;
 	long pg_idx, pg_offset, src_offset;
 	unsigned long head = etr_perf->head;
 	char **dst_pages, *src_buf;
@@ -1422,8 +1423,7 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf)
 	pg_idx = head >> PAGE_SHIFT;
 	pg_offset = head & (PAGE_SIZE - 1);
 	dst_pages = (char **)etr_perf->pages;
-	src_offset = etr_buf->offset;
-	to_copy = etr_buf->len;
+	src_offset = etr_buf->offset + etr_buf->len - to_copy;
 
 	while (to_copy > 0) {
 		/*
@@ -1434,6 +1434,8 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf)
 		 *  3) what is available in the destination page.
 		 * in one iteration.
 		 */
+		if (src_offset >= etr_buf->size)
+			src_offset -= etr_buf->size;
 		bytes = tmc_etr_buf_get_data(etr_buf, src_offset, to_copy,
 					     &src_buf);
 		if (WARN_ON_ONCE(bytes <= 0))
@@ -1454,8 +1456,6 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf)
 
 		/* Move source pointers */
 		src_offset += bytes;
-		if (src_offset >= etr_buf->size)
-			src_offset -= etr_buf->size;
 	}
 }
 
@@ -1501,7 +1501,11 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	size = etr_buf->len;
-	tmc_etr_sync_perf_buffer(etr_perf);
+	if (!etr_perf->snapshot && size > handle->size) {
+		size = handle->size;
+		lost = true;
+	}
+	tmc_etr_sync_perf_buffer(etr_perf, size);
 
 	/*
 	 * In snapshot mode we simply increment the head by the number of byte
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] ARM: UNWINDER_FRAME_POINTER implementation for Clang
From: Nathan Huckleberry @ 2019-08-20 19:43 UTC (permalink / raw)
  To: linux
  Cc: Tri Vo, ndesaulniers, linux-kernel, Nathan Huckleberry,
	clang-built-linux, miles.chen, linux-arm-kernel

The stackframe setup when compiled with clang is different.
Since the stack unwinder expects the gcc stackframe setup it
fails to print backtraces. This patch adds support for the
clang stackframe setup.

Link: https://github.com/ClangBuiltLinux/linux/issues/35
Cc: clang-built-linux@googlegroups.com
Suggested-by: Tri Vo <trong@google.com>
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
Changes from RFC
* Push extra register to satisfy 8 byte alignment requirement
* Add clarifying comments
* Separate code into its own file

 arch/arm/Kconfig.debug         |   4 +-
 arch/arm/Makefile              |   5 +-
 arch/arm/lib/Makefile          |   8 +-
 arch/arm/lib/backtrace-clang.S | 224 +++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/lib/backtrace-clang.S

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 85710e078afb..92fca7463e21 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -56,7 +56,7 @@ choice
 
 config UNWINDER_FRAME_POINTER
 	bool "Frame pointer unwinder"
-	depends on !THUMB2_KERNEL && !CC_IS_CLANG
+	depends on !THUMB2_KERNEL
 	select ARCH_WANT_FRAME_POINTERS
 	select FRAME_POINTER
 	help
@@ -1872,7 +1872,7 @@ config DEBUG_UNCOMPRESS
 	  When this option is set, the selected DEBUG_LL output method
 	  will be re-used for normal decompressor output on multiplatform
 	  kernels.
-	  
+
 
 config UNCOMPRESS_INCLUDE
 	string
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index c3624ca6c0bc..729e223b83fe 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -36,7 +36,10 @@ KBUILD_CFLAGS	+= $(call cc-option,-mno-unaligned-access)
 endif
 
 ifeq ($(CONFIG_FRAME_POINTER),y)
-KBUILD_CFLAGS	+=-fno-omit-frame-pointer -mapcs -mno-sched-prolog
+KBUILD_CFLAGS	+=-fno-omit-frame-pointer
+  ifeq ($(CONFIG_CC_IS_GCC),y)
+  KBUILD_CFLAGS += -mapcs -mno-sched-prolog
+  endif
 endif
 
 ifeq ($(CONFIG_CPU_BIG_ENDIAN),y)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index b25c54585048..e10a769c72ec 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -5,7 +5,7 @@
 # Copyright (C) 1995-2000 Russell King
 #
 
-lib-y		:= backtrace.o changebit.o csumipv6.o csumpartial.o   \
+lib-y		:= changebit.o csumipv6.o csumpartial.o               \
 		   csumpartialcopy.o csumpartialcopyuser.o clearbit.o \
 		   delay.o delay-loop.o findbit.o memchr.o memcpy.o   \
 		   memmove.o memset.o setbit.o                        \
@@ -19,6 +19,12 @@ lib-y		:= backtrace.o changebit.o csumipv6.o csumpartial.o   \
 mmu-y		:= clear_user.o copy_page.o getuser.o putuser.o       \
 		   copy_from_user.o copy_to_user.o
 
+ifdef CONFIG_CC_IS_CLANG
+  lib-y += backtrace-clang.o
+else
+  lib-y += backtrace.o
+endif
+
 # using lib_ here won't override already available weak symbols
 obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o
 
diff --git a/arch/arm/lib/backtrace-clang.S b/arch/arm/lib/backtrace-clang.S
new file mode 100644
index 000000000000..2b02014dbdd1
--- /dev/null
+++ b/arch/arm/lib/backtrace-clang.S
@@ -0,0 +1,224 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ *  linux/arch/arm/lib/backtrace-clang.S
+ *
+ *  Copyright (C) 2019 Nathan Huckleberry
+ *
+ */
+#include <linux/kern_levels.h>
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+		.text
+
+/* fp is 0 or stack frame */
+
+#define frame	r4
+#define sv_fp	r5
+#define sv_pc	r6
+#define mask	r7
+#define sv_lr   r8
+
+ENTRY(c_backtrace)
+
+#if !defined(CONFIG_FRAME_POINTER) || !defined(CONFIG_PRINTK)
+		ret	lr
+ENDPROC(c_backtrace)
+#else
+
+
+/*
+ * Clang does not store pc or sp in function prologues
+ * 		so we don't know exactly where the function
+ * 		starts.
+ * We can treat the current frame's lr as the saved pc and the
+ * 		preceding frame's lr as the lr, but we can't
+ * 		trace the most recent call.
+ * Inserting a false stack frame allows us to reference the
+ * 		function called last in the stacktrace.
+ * If the call instruction was a bl we can look at the callers
+ * 		branch instruction to calculate the saved pc.
+ * We can recover the pc in most cases, but in cases such as
+ * 		calling function pointers we cannot. In this
+ * 		case, default to using the lr. This will be
+ * 		some address in the function, but will not
+ * 		be the function start.
+ * Unfortunately due to the stack frame layout we can't dump
+ *              r0 - r3, but these are less frequently saved.
+ *
+ * Stack frame layout:
+ *             <larger addresses>
+ *             saved lr
+ *    frame => saved fp
+ *             optionally saved caller registers (r4 - r10)
+ *             optionally saved arguments (r0 - r3)
+ *             <top of stack frame>
+ *             <smaller addresses>
+ *
+ * Functions start with the following code sequence:
+ * corrected pc =>  stmfd sp!, {..., fp, lr}
+ *		    add fp, sp, #x
+ *		    stmfd sp!, {r0 - r3} (optional)
+ *
+ *
+ *
+ *
+ *
+ *
+ * The diagram below shows an example stack setup
+ * 	for dump_stack.
+ *
+ * The frame for c_backtrace has pointers to the
+ * 	code of dump_stack. This is why the frame of
+ * 	c_backtrace is used to for the pc calculation
+ * 	of dump_stack. This is why we must move back
+ * 	a frame to print dump_stack.
+ *
+ * The stored locals for dump_stack are in dump_stack's
+ * 	frame. This means that to fully print dump_stack's frame
+ * 	we need the both the frame for dump_stack (for locals) and the
+ * 	frame that was called by dump_stack (for pc).
+ *
+ * To print locals we must know where the function start is. If
+ * 	we read the function prologue opcodes we can determine
+ * 	which variables are stored in the stack frame.
+ *
+ * To find the function start of dump_stack we can look at the
+ * 	stored LR of show_stack. It points at the instruction
+ * 	directly after the bl dump_stack. We can then read the
+ * 	offset from the bl opcode to determine where the branch takes us.
+ * 	The address calculated must be the start of dump_stack.
+ *
+ * c_backtrace frame           dump_stack:
+ * {[LR]    }  ============|   ...
+ * {[FP]    }  =======|    |   bl c_backtrace
+ *                    |    |=> ...
+ * {[R4-R10]}         |
+ * {[R0-R3] }         |        show_stack:
+ * dump_stack frame   |        ...
+ * {[LR]    } =============|   bl dump_stack
+ * {[FP]    } <=======|    |=> ...
+ * {[R4-R10]}
+ * {[R0-R3] }
+ */
+
+stmfd   sp!, {r4 - r9, fp, lr}	@ Save an extra register
+				@ to ensure 8 byte alignment
+movs	frame, r0		@ if frame pointer is zero
+beq	no_frame		@ we have no stack frames
+
+tst	r1, #0x10		@ 26 or 32-bit mode?
+moveq	mask, #0xfc000003
+movne	mask, #0		@ mask for 32-bit
+
+/*
+ * Switches the current frame to be the frame for dump_stack.
+ */
+		add     frame, sp, #24          @ switch to false frame
+for_each_frame:	tst	frame, mask		@ Check for address exceptions
+		bne	no_frame
+
+/*
+ * sv_fp is the stack frame with the locals for the current considered
+ * 	function.
+ * sv_pc is the saved lr frame the frame above. This is a pointer to a
+ * 	code address within the current considered function, but
+ * 	it is not the function start. This value gets updated to be
+ * 	the function start later if it is possible.
+ */
+1001:		ldr	sv_pc, [frame, #4]	@ get saved 'pc'
+1002:		ldr	sv_fp, [frame, #0]	@ get saved fp
+
+		teq     sv_fp, #0               @ make sure next frame exists
+		beq     no_frame
+
+/*
+ * sv_lr is the lr from the function that called the current function. This
+ * 	is a pointer to a code address in the current function's caller.
+ * 	sv_lr-4 is the instruction used to call the current function.
+ * This sv_lr can be used to calculate the function start if the function
+ * 	was called using a bl instruction. If the function start
+ * 	can be recovered sv_pc is overwritten with the function start.
+ * If the current function was called using a function pointer we cannot
+ * 	recover the function start and instead continue with sv_pc as
+ * 	an arbitrary value within the current function. If this is the case
+ * 	we cannot print registers for the current function, but the stacktrace
+ * 	is still printed properly.
+ */
+1003:		ldr     sv_lr, [sv_fp, #4]      @ get saved lr from next frame
+
+		ldr     r0, [sv_lr, #-4]        @ get call instruction
+		ldr     r3, .Ldsi+8
+		and     r2, r3, r0              @ is this a bl call
+		teq     r2, r3
+		bne     finished_setup          @ give up if it's not
+		and     r0, #0xffffff           @ get call offset 24-bit int
+		lsl     r0, r0, #8              @ sign extend offset
+		asr     r0, r0, #8
+		ldr     sv_pc, [sv_fp, #4]      @ get lr address
+		add     sv_pc, sv_pc, #-4	@ get call instruction address
+		add     sv_pc, sv_pc, #8        @ take care of prefetch
+		add     sv_pc, sv_pc, r0, lsl #2 @ find function start
+
+finished_setup:
+
+		bic	sv_pc, sv_pc, mask	@ mask PC/LR for the mode
+
+/*
+ * Print the function (sv_pc) and where it was called
+ * 	from (sv_lr).
+ */
+1004:		mov     r0, sv_pc
+
+		mov     r1, sv_lr
+		mov	r2, frame
+		bic	r1, r1, mask		@ mask PC/LR for the mode
+		bl	dump_backtrace_entry
+
+/*
+ * Test if the function start is a stmfd instruction
+ * 	to determine which registers were stored in the function
+ * 	prologue.
+ * If we could not recover the sv_pc because we were called through
+ * 	a function pointer the comparison will fail and no registers
+ * 	will print.
+ */
+1005:		ldr	r1, [sv_pc, #0]		@ if stmfd sp!, {..., fp, lr}
+		ldr	r3, .Ldsi		@ instruction exists,
+		teq	r3, r1, lsr #11
+		ldr     r0, [frame]             @ locals are stored in
+						@ the preceding frame
+		subeq	r0, r0, #4
+		bleq	dump_backtrace_stm	@ dump saved registers
+
+/*
+ * If we are out of frames or if the next frame
+ * 	is invalid.
+ */
+		teq	sv_fp, #0		@ zero saved fp means
+		beq	no_frame		@ no further frames
+
+		cmp	sv_fp, frame		@ next frame must be
+		mov	frame, sv_fp		@ above the current frame
+		bhi	for_each_frame
+
+1006:		adr	r0, .Lbad
+		mov	r1, frame
+		bl	printk
+no_frame:	ldmfd	sp!, {r4 - r9, fp, pc}
+ENDPROC(c_backtrace)
+		.pushsection __ex_table,"a"
+		.align	3
+		.long	1001b, 1006b
+		.long	1002b, 1006b
+		.long	1003b, 1006b
+		.long	1004b, 1006b
+		.long   1005b, 1006b
+		.popsection
+
+.Lbad:		.asciz	"Backtrace aborted due to bad frame pointer <%p>\n"
+		.align
+.Ldsi:		.word	0xe92d4800 >> 11	@ stmfd sp!, {... fp, lr}
+		.word	0xe92d0000 >> 11	@ stmfd sp!, {}
+		.word   0x0b000000              @ bl if these bits are set
+
+#endif
-- 
2.23.0.rc1.153.gdeed80330f-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v6 05/13] rtc: mt6397: move some common definitions into rtc.h
From: Alexandre Belloni @ 2019-08-20 19:48 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-rtc, devicetree, Josef Friedl, Sean Wang,
	linux-arm-kernel @ lists . infradead . org Alessandro Zummo,
	linux-pm, linux-kernel, Sebastian Reichel, Tianping Fang,
	Rob Herring, linux-mediatek, Matthias Brugger, Mark Rutland,
	Eddie Huang, Lee Jones, linux-arm-kernel
In-Reply-To: <20190818135611.7776-6-frank-w@public-files.de>

On 18/08/2019 15:56:03+0200, Frank Wunderlich wrote:
> From: Josef Friedl <josef.friedl@speed.at>
> 
> move code to separate header-file to reuse definitions later
> in poweroff-driver (drivers/power/reset/mt6323-poweroff.c)
> 
> Suggested-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Josef Friedl <josef.friedl@speed.at>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> changes since v5: none
> changes since v4: none
> changes since v3: none
> changes since v2: add missing commit-message
> ---
>  drivers/rtc/rtc-mt6397.c       | 55 +-------------------------
>  include/linux/mfd/mt6397/rtc.h | 71 ++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 54 deletions(-)
>  create mode 100644 include/linux/mfd/mt6397/rtc.h
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index b46ed4dc7015..c08ee5edf865 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -9,60 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  #include <linux/rtc.h>
> -#include <linux/irqdomain.h>
> -#include <linux/platform_device.h>
> -#include <linux/of_address.h>
> -#include <linux/of_irq.h>
> -#include <linux/io.h>
> -#include <linux/mfd/mt6397/core.h>
> -
> -#define RTC_BBPU		0x0000
> -#define RTC_BBPU_CBUSY		BIT(6)
> -
> -#define RTC_WRTGR		0x003c
> -
> -#define RTC_IRQ_STA		0x0002
> -#define RTC_IRQ_STA_AL		BIT(0)
> -#define RTC_IRQ_STA_LP		BIT(3)
> -
> -#define RTC_IRQ_EN		0x0004
> -#define RTC_IRQ_EN_AL		BIT(0)
> -#define RTC_IRQ_EN_ONESHOT	BIT(2)
> -#define RTC_IRQ_EN_LP		BIT(3)
> -#define RTC_IRQ_EN_ONESHOT_AL	(RTC_IRQ_EN_ONESHOT | RTC_IRQ_EN_AL)
> -
> -#define RTC_AL_MASK		0x0008
> -#define RTC_AL_MASK_DOW		BIT(4)
> -
> -#define RTC_TC_SEC		0x000a
> -/* Min, Hour, Dom... register offset to RTC_TC_SEC */
> -#define RTC_OFFSET_SEC		0
> -#define RTC_OFFSET_MIN		1
> -#define RTC_OFFSET_HOUR		2
> -#define RTC_OFFSET_DOM		3
> -#define RTC_OFFSET_DOW		4
> -#define RTC_OFFSET_MTH		5
> -#define RTC_OFFSET_YEAR		6
> -#define RTC_OFFSET_COUNT	7
> -
> -#define RTC_AL_SEC		0x0018
> -
> -#define RTC_PDN2		0x002e
> -#define RTC_PDN2_PWRON_ALARM	BIT(4)
> -
> -#define RTC_MIN_YEAR		1968
> -#define RTC_BASE_YEAR		1900
> -#define RTC_NUM_YEARS		128
> -#define RTC_MIN_YEAR_OFFSET	(RTC_MIN_YEAR - RTC_BASE_YEAR)
> -
> -struct mt6397_rtc {
> -	struct device		*dev;
> -	struct rtc_device	*rtc_dev;
> -	struct mutex		lock;
> -	struct regmap		*regmap;
> -	int			irq;
> -	u32			addr_base;
> -};
> +#include <linux/mfd/mt6397/rtc.h>
>  
>  static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
>  {
> diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc.h
> new file mode 100644
> index 000000000000..b702c29e8c74
> --- /dev/null
> +++ b/include/linux/mfd/mt6397/rtc.h
> @@ -0,0 +1,71 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2014-2018 MediaTek Inc.
> + *
> + * Author: Tianping.Fang <tianping.fang@mediatek.com>
> + *        Sean Wang <sean.wang@mediatek.com>
> + */
> +
> +#ifndef _LINUX_MFD_MT6397_RTC_H_
> +#define _LINUX_MFD_MT6397_RTC_H_
> +
> +#include <linux/jiffies.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/rtc.h>
> +
> +#define RTC_BBPU               0x0000
> +#define RTC_BBPU_CBUSY         BIT(6)
> +#define RTC_BBPU_KEY            (0x43 << 8)
> +
> +#define RTC_WRTGR              0x003c
> +
> +#define RTC_IRQ_STA            0x0002
> +#define RTC_IRQ_STA_AL         BIT(0)
> +#define RTC_IRQ_STA_LP         BIT(3)
> +
> +#define RTC_IRQ_EN             0x0004
> +#define RTC_IRQ_EN_AL          BIT(0)
> +#define RTC_IRQ_EN_ONESHOT     BIT(2)
> +#define RTC_IRQ_EN_LP          BIT(3)
> +#define RTC_IRQ_EN_ONESHOT_AL  (RTC_IRQ_EN_ONESHOT | RTC_IRQ_EN_AL)
> +
> +#define RTC_AL_MASK            0x0008
> +#define RTC_AL_MASK_DOW                BIT(4)
> +
> +#define RTC_TC_SEC             0x000a
> +/* Min, Hour, Dom... register offset to RTC_TC_SEC */
> +#define RTC_OFFSET_SEC         0
> +#define RTC_OFFSET_MIN         1
> +#define RTC_OFFSET_HOUR                2
> +#define RTC_OFFSET_DOM         3
> +#define RTC_OFFSET_DOW         4
> +#define RTC_OFFSET_MTH         5
> +#define RTC_OFFSET_YEAR                6
> +#define RTC_OFFSET_COUNT       7
> +
> +#define RTC_AL_SEC             0x0018
> +
> +#define RTC_PDN2               0x002e
> +#define RTC_PDN2_PWRON_ALARM   BIT(4)
> +
> +#define RTC_MIN_YEAR           1968
> +#define RTC_BASE_YEAR          1900
> +#define RTC_NUM_YEARS          128
> +#define RTC_MIN_YEAR_OFFSET    (RTC_MIN_YEAR - RTC_BASE_YEAR)
> +
> +#define MTK_RTC_POLL_DELAY_US  10
> +#define MTK_RTC_POLL_TIMEOUT   (jiffies_to_usecs(HZ))
> +
> +struct mt6397_rtc {
> +	struct device           *dev;
> +	struct rtc_device       *rtc_dev;
> +
> +	/* Protect register access from multiple tasks */
> +	struct mutex            lock;
> +	struct regmap           *regmap;
> +	int                     irq;
> +	u32                     addr_base;
> +};
> +
> +#endif /* _LINUX_MFD_MT6397_RTC_H_ */
> -- 
> 2.17.1
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 09/13] rtc: mt6397: add compatible for mt6323
From: Alexandre Belloni @ 2019-08-20 19:49 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-rtc, devicetree, Josef Friedl, Sean Wang,
	linux-arm-kernel @ lists . infradead . org Alessandro Zummo,
	linux-pm, linux-kernel, Sebastian Reichel, Tianping Fang,
	Rob Herring, linux-mediatek, Matthias Brugger, Mark Rutland,
	Eddie Huang, Lee Jones, linux-arm-kernel
In-Reply-To: <20190818135611.7776-10-frank-w@public-files.de>

On 18/08/2019 15:56:07+0200, Frank Wunderlich wrote:
> From: Josef Friedl <josef.friedl@speed.at>
> 
> use mt6397 rtc driver also for mt6323 but with different
> base/size see "mfd: mt6323: add mt6323 rtc+pwrc"
> 
> Signed-off-by: Josef Friedl <josef.friedl@speed.at>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> changes since v5: none
> changes since v4: none
> changes since v3: moved (was part 5)
> changes since v2: splitted this from v2.3 suggested-by Alexandre Belloni
> ---
>  drivers/rtc/rtc-mt6397.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 9370b7fc9f81..21cd9cc8b4c7 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -325,6 +325,7 @@ static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend,
>  			mt6397_rtc_resume);
>  
>  static const struct of_device_id mt6397_rtc_of_match[] = {
> +	{ .compatible = "mediatek,mt6323-rtc", },
>  	{ .compatible = "mediatek,mt6397-rtc", },
>  	{ }
>  };
> -- 
> 2.17.1
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 07/13] rtc: mt6397: improvements of rtc driver
From: Alexandre Belloni @ 2019-08-20 19:49 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: linux-rtc, devicetree, Josef Friedl, Sean Wang,
	linux-arm-kernel @ lists . infradead . org Alessandro Zummo,
	linux-pm, linux-kernel, Sebastian Reichel, Tianping Fang,
	Rob Herring, linux-mediatek, Matthias Brugger, Mark Rutland,
	Eddie Huang, Lee Jones, linux-arm-kernel
In-Reply-To: <20190818135611.7776-8-frank-w@public-files.de>

On 18/08/2019 15:56:05+0200, Frank Wunderlich wrote:
> From: Josef Friedl <josef.friedl@speed.at>
> 
> - use regmap_read_poll_timeout to drop while-loop
> - use devm-api to drop remove-callback
> 
> Suggested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Josef Friedl <josef.friedl@speed.at>
> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> changes since v5: none
> changes since v4: none
> changes since v3: none
> changes since v2:
> - fix allocation after irq-request
> - compatible for mt6323 in separate commit => part 5
> ---
>  drivers/rtc/rtc-mt6397.c | 51 +++++++++++++++-------------------------
>  1 file changed, 19 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index c08ee5edf865..9370b7fc9f81 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -4,16 +4,19 @@
>  * Author: Tianping.Fang <tianping.fang@mediatek.com>
>  */
>  
> -#include <linux/delay.h>
> -#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/mt6397/core.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/rtc.h>
>  #include <linux/mfd/mt6397/rtc.h>
> +#include <linux/mod_devicetable.h>
>  
>  static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
>  {
> -	unsigned long timeout = jiffies + HZ;
>  	int ret;
>  	u32 data;
>  
> @@ -21,19 +24,13 @@ static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc)
>  	if (ret < 0)
>  		return ret;
>  
> -	while (1) {
> -		ret = regmap_read(rtc->regmap, rtc->addr_base + RTC_BBPU,
> -				  &data);
> -		if (ret < 0)
> -			break;
> -		if (!(data & RTC_BBPU_CBUSY))
> -			break;
> -		if (time_after(jiffies, timeout)) {
> -			ret = -ETIMEDOUT;
> -			break;
> -		}
> -		cpu_relax();
> -	}
> +	ret = regmap_read_poll_timeout(rtc->regmap,
> +					rtc->addr_base + RTC_BBPU, data,
> +					!(data & RTC_BBPU_CBUSY),
> +					MTK_RTC_POLL_DELAY_US,
> +					MTK_RTC_POLL_TIMEOUT);
> +	if (ret < 0)
> +		dev_err(rtc->dev, "failed to write WRTGE: %d\n", ret);
>  
>  	return ret;
>  }
> @@ -266,19 +263,19 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>  		return rtc->irq;
>  
>  	rtc->regmap = mt6397_chip->regmap;
> -	rtc->dev = &pdev->dev;
>  	mutex_init(&rtc->lock);
>  
>  	platform_set_drvdata(pdev, rtc);
>  
> -	rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev);
> +	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
>  	if (IS_ERR(rtc->rtc_dev))
>  		return PTR_ERR(rtc->rtc_dev);
>  
> -	ret = request_threaded_irq(rtc->irq, NULL,
> -				   mtk_rtc_irq_handler_thread,
> -				   IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> -				   "mt6397-rtc", rtc);
> +	ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
> +					mtk_rtc_irq_handler_thread,
> +					IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> +					"mt6397-rtc", rtc);
> +
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
>  			rtc->irq, ret);
> @@ -302,15 +299,6 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int mtk_rtc_remove(struct platform_device *pdev)
> -{
> -	struct mt6397_rtc *rtc = platform_get_drvdata(pdev);
> -
> -	free_irq(rtc->irq, rtc);
> -
> -	return 0;
> -}
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int mt6397_rtc_suspend(struct device *dev)
>  {
> @@ -349,7 +337,6 @@ static struct platform_driver mtk_rtc_driver = {
>  		.pm = &mt6397_pm_ops,
>  	},
>  	.probe	= mtk_rtc_probe,
> -	.remove = mtk_rtc_remove,
>  };
>  
>  module_platform_driver(mtk_rtc_driver);
> -- 
> 2.17.1
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 3/4] dt-bindings: arm: fsl: Add Kontron i.MX6UL N6310 compatibles
From: Rob Herring @ 2019-08-20 20:04 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Mark Rutland, devicetree, Shawn Guo, Sascha Hauer,
	linux-kernel@vger.kernel.org, Schrempf Frieder, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAJKOXPfkNcWw9sunwXGRz42jOL0cdRC-iiHLtWCYvo5oxCMwFQ@mail.gmail.com>

On Tue, Aug 20, 2019 at 1:36 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, 20 Aug 2019 at 18:59, Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Aug 20, 2019 at 10:35 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > Add the compatibles for Kontron i.MX6UL N6310 SoM and boards.
> > >
> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > >
> > > ---
> > >
> > > Changes since v5:
> > > New patch
> > > ---
> > >  Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > index 7294ac36f4c0..d07b3c06d7cf 100644
> > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > @@ -161,6 +161,9 @@ properties:
> > >          items:
> > >            - enum:
> > >                - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
> > > +              - kontron,imx6ul-n6310-som  # Kontron N6310 SOM
> > > +              - kontron,imx6ul-n6310-s    # Kontron N6310 S Board
> > > +              - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board
> >
> > This doesn't match what is in your dts files. Run 'make dtbs_check' and see.
>
> You mean the name does not match? I thought that '#' is a comment in YAML...

No, the number of compatible strings is the problem.

> The dtbs_check fail on missing dt-mk-schema. Any reason why it is not
> in the scripts?

Because it is not just that script, but the whole project of scripts,
schemas and meta-schemas. Read the instructions in
Documentation/devicetree/writing-schema.md(.rst in next).

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 6/6] arm64: dts: add support for SM1 based SEI Robotics SEI610
From: Martin Blumenstingl @ 2019-08-20 20:11 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: khilman, linux-kernel, linux-arm-kernel, linux-amlogic
In-Reply-To: <20190820144052.18269-7-narmstrong@baylibre.com>

Hi Neil,

On Tue, Aug 20, 2019 at 4:43 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Add support for the Amlogic SM1 Based SEI610 board.
>
> The SM1 SoC is a derivative of the G12A SoC Family with :
> - Cortex-A55 core instead of A53
> - more power domains, including USB & PCIe
> - a neural network co-processor (NNA)
> - a CSI input and image processor
> - some changes in the audio complex, thus not yet enabled
>
> The SEI610 board is a derivative of the SEI510 board with :
> - removed ADC based touch button, replaced with 3x GPIO buttons
> - physical switch disabling on-board MICs
> - USB-C port for USB 2.0 OTG
> - On-board FTDI USB2SERIAL port for Linux console
>
> Audio, Display and USB support will be added later when support of the
> corresponding power domains will be added, for now they are kept disabled.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
I don't have any details about this board but overall this looks sane, so:
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[...]
> +       /* Used by Tuner, RGB Led & IR Emitter LED array */
> +       vddao_3v3_t: regultor-vddao_3v3_t {
that should be regulator-vddao_3v3_t - maybe Kevin can fix this up, if
not then we can still fix it with a follow-up patch


Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/4] rtc: Add support for the MediaTek MT2712 RTC
From: Alexandre Belloni @ 2019-08-20 20:17 UTC (permalink / raw)
  To: Ran Bi
  Cc: Mark Rutland, Alessandro Zummo, YT Shen, Flora Fu, srv_heupstream,
	devicetree, Greg Kroah-Hartman, Linus Walleij, Sean Wang,
	linux-kernel, Mauro Carvalho Chehab, Rob Herring, linux-mediatek,
	Jonathan Cameron, Matthias Brugger, Yingjoe Chen, Eddie Huang,
	David S . Miller, linux-arm-kernel, linux-rtc
In-Reply-To: <20190801110122.26834-3-ran.bi@mediatek.com>

Hi,

On 01/08/2019 19:01:20+0800, Ran Bi wrote:
> diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c
> new file mode 100644
> index 000000000000..1eb71ca64c2c
> --- /dev/null
> +++ b/drivers/rtc/rtc-mt2712.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Author: Ran Bi <ran.bi@mediatek.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +
> +#define MTK_RTC_DEV		KBUILD_MODNAME

You probably shouldn't do that and have a static string for the driver
name. I probably doesn't matter much though because DT is used to probe
the driver.

> +#define MT2712_WRTGR		0x0078
> +
> +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
> +#define MT2712_MIN_YEAR		2000
> +#define MT2712_BASE_YEAR	1900
> +#define MT2712_MIN_YEAR_OFFSET	(MT2712_MIN_YEAR - MT2712_BASE_YEAR)
> +#define MT2712_MAX_YEAR_OFFSET	(MT2712_MIN_YEAR_OFFSET + 127)
> +

All those defines are unecessary, see below.


> +struct mt2712_rtc {
> +	struct device		*dev;

Looking at the code closely, it seems this is only used for debug and
error messages. Maybe you could use rtc_dev->dev instead.

> +	struct rtc_device	*rtc_dev;
> +	void __iomem		*base;
> +	int			irq;
> +	u8			irq_wake_enabled;
> +};
> +
> +static inline u32 mt2712_readl(struct mt2712_rtc *rtc, u32 reg)
> +{
> +	return readl(rtc->base + reg);
> +}
> +
> +static inline void mt2712_writel(struct mt2712_rtc *rtc, u32 reg, u32 val)
> +{
> +	writel(val, rtc->base + reg);
> +}
> +
> +static void mt2712_rtc_write_trigger(struct mt2712_rtc *rtc)
> +{
> +	unsigned long timeout = jiffies + HZ/10;
> +
> +	mt2712_writel(rtc, MT2712_WRTGR, 1);
> +	while (1) {
> +		if (!(mt2712_readl(rtc, MT2712_BBPU) & MT2712_BBPU_CBUSY))
> +			break;
> +
> +		if (time_after(jiffies, timeout)) {
> +			dev_err(rtc->dev, "%s time out!\n", __func__);
> +			break;
> +		}
> +		cpu_relax();
> +	}
> +}
> +
> +static void mt2712_rtc_writeif_unlock(struct mt2712_rtc *rtc)
> +{
> +	mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK1);
> +	mt2712_rtc_write_trigger(rtc);
> +	mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK2);
> +	mt2712_rtc_write_trigger(rtc);
> +}
> +
> +static irqreturn_t rtc_irq_handler_thread(int irq, void *data)
> +{
> +	struct mt2712_rtc *rtc = data;
> +	u16 irqsta, irqen;
> +
> +	mutex_lock(&rtc->rtc_dev->ops_lock);
> +
> +	irqsta = mt2712_readl(rtc, MT2712_IRQ_STA);

Do you have to lock that read? Is the register cleared on read?

> +	if (irqsta & MT2712_IRQ_STA_AL) {
> +		rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
> +		irqen = irqsta & ~MT2712_IRQ_EN_AL;
> +
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +		mt2712_rtc_write_trigger(rtc);
> +
> +		mutex_unlock(&rtc->rtc_dev->ops_lock);
> +		return IRQ_HANDLED;
> +	}
> +
> +	mutex_unlock(&rtc->rtc_dev->ops_lock);
> +	return IRQ_NONE;
> +}
> +
> +static void __mt2712_rtc_read_time(struct mt2712_rtc *rtc,
> +				   struct rtc_time *tm, int *sec)
> +{
> +	tm->tm_sec  = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> +	tm->tm_min  = mt2712_readl(rtc, MT2712_TC_MIN) & MT2712_MIN_MASK;
> +	tm->tm_hour = mt2712_readl(rtc, MT2712_TC_HOU) & MT2712_HOU_MASK;
> +	tm->tm_mday = mt2712_readl(rtc, MT2712_TC_DOM) & MT2712_DOM_MASK;
> +	tm->tm_mon  = mt2712_readl(rtc, MT2712_TC_MTH) & MT2712_MTH_MASK;
> +	tm->tm_year = mt2712_readl(rtc, MT2712_TC_YEA) & MT2712_YEA_MASK;
> +
> +	*sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> +}
> +
> +static int mt2712_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	int sec;
> +
> +	do {
> +		__mt2712_rtc_read_time(rtc, tm, &sec);
> +	} while (sec < tm->tm_sec);	/* SEC has carried */

Shouldn't that be while (tm->tm_sec < sec)?

> +
> +	/* HW register use 7 bits to store year data, minus
> +	 * MT2712_MIN_YEAR_OFFSET brfore write year data to register, and plus
> +	 * MT2712_MIN_YEAR_OFFSET back after read year from register
> +	 */
> +	tm->tm_year += MT2712_MIN_YEAR_OFFSET;

Simply add 100 in __mt2712_rtc_read_time

> +
> +	/* HW register start mon from one, but tm_mon start from zero. */
> +	tm->tm_mon--;
> +

You can also do that in __mt2712_rtc_read_time.

> +	if (rtc_valid_tm(tm)) {

This check is unnecessary, the validity is always checked by the core.

> +		dev_dbg(rtc->dev, "%s: invalid time %ptR\n", __func__, tm);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> +		dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> +			1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> +			1900 + MT2712_MAX_YEAR_OFFSET);
> +		return -EINVAL;
> +	}

This check is unnecessary, see below.

> +
> +	tm->tm_year -= MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon++;

You should probably avoid modifying tm, move the substraction and
addition in the mt2712_writel calls.

> +
> +	mt2712_writel(rtc, MT2712_TC_SEC, tm->tm_sec  & MT2712_SEC_MASK);
> +	mt2712_writel(rtc, MT2712_TC_MIN, tm->tm_min  & MT2712_MIN_MASK);
> +	mt2712_writel(rtc, MT2712_TC_HOU, tm->tm_hour & MT2712_HOU_MASK);
> +	mt2712_writel(rtc, MT2712_TC_DOM, tm->tm_mday & MT2712_DOM_MASK);
> +	mt2712_writel(rtc, MT2712_TC_MTH, tm->tm_mon  & MT2712_MTH_MASK);
> +	mt2712_writel(rtc, MT2712_TC_YEA, tm->tm_year & MT2712_YEA_MASK);
> +
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alm->time;
> +	u16 irqen;
> +
> +	irqen = mt2712_readl(rtc, MT2712_IRQ_EN);
> +	alm->enabled = !!(irqen & MT2712_IRQ_EN_AL);
> +
> +	tm->tm_sec  = mt2712_readl(rtc, MT2712_AL_SEC) & MT2712_SEC_MASK;
> +	tm->tm_min  = mt2712_readl(rtc, MT2712_AL_MIN) & MT2712_MIN_MASK;
> +	tm->tm_hour = mt2712_readl(rtc, MT2712_AL_HOU) & MT2712_HOU_MASK;
> +	tm->tm_mday = mt2712_readl(rtc, MT2712_AL_DOM) & MT2712_DOM_MASK;
> +	tm->tm_mon  = mt2712_readl(rtc, MT2712_AL_MTH) & MT2712_MTH_MASK;
> +	tm->tm_year = mt2712_readl(rtc, MT2712_AL_YEA) & MT2712_YEA_MASK;
> +
> +	tm->tm_year += MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon--;
> +
> +	if (rtc_valid_tm(tm)) {
> +		dev_dbg(rtc->dev, "%s: invalid alarm %ptR\n", __func__, tm);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alm->time;
> +	u16 irqen;
> +
> +	if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> +		dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> +			1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> +			1900 + MT2712_MAX_YEAR_OFFSET);
> +		return -EINVAL;
> +	}
> +

Unnecessary check.

> +	dev_dbg(rtc->dev, "set al time: %ptR, alm en: %d\n", tm, alm->enabled);
> +
> +	tm->tm_year -= MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon++;
> +
> +	irqen = mt2712_readl(rtc, MT2712_IRQ_EN) & ~(MT2712_IRQ_EN_ONESHOT_AL);
> +	mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_writel(rtc, MT2712_AL_SEC,
> +		      (mt2712_readl(rtc, MT2712_AL_SEC) & ~(MT2712_SEC_MASK)) |
> +		      (tm->tm_sec  & MT2712_SEC_MASK));
> +	mt2712_writel(rtc, MT2712_AL_MIN,
> +		      (mt2712_readl(rtc, MT2712_AL_MIN) & ~(MT2712_MIN_MASK)) |
> +		      (tm->tm_min  & MT2712_MIN_MASK));
> +	mt2712_writel(rtc, MT2712_AL_HOU,
> +		      (mt2712_readl(rtc, MT2712_AL_HOU) & ~(MT2712_HOU_MASK)) |
> +		      (tm->tm_hour & MT2712_HOU_MASK));
> +	mt2712_writel(rtc, MT2712_AL_DOM,
> +		      (mt2712_readl(rtc, MT2712_AL_DOM) & ~(MT2712_DOM_MASK)) |
> +		      (tm->tm_mday & MT2712_DOM_MASK));
> +	mt2712_writel(rtc, MT2712_AL_MTH,
> +		      (mt2712_readl(rtc, MT2712_AL_MTH) & ~(MT2712_MTH_MASK)) |
> +		      (tm->tm_mon  & MT2712_MTH_MASK));
> +	mt2712_writel(rtc, MT2712_AL_YEA,
> +		      (mt2712_readl(rtc, MT2712_AL_YEA) & ~(MT2712_YEA_MASK)) |
> +		      (tm->tm_year & MT2712_YEA_MASK));
> +
> +	mt2712_writel(rtc, MT2712_AL_MASK, MT2712_AL_MASK_DOW);	/* mask DOW */
> +
> +	if (alm->enabled) {
> +		irqen = mt2712_readl(rtc, MT2712_IRQ_EN) |
> +				     MT2712_IRQ_EN_ONESHOT_AL;
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	} else {
> +		irqen = mt2712_readl(rtc, MT2712_IRQ_EN) &
> +				     ~(MT2712_IRQ_EN_ONESHOT_AL);
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	}
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	return 0;
> +}
> +
> +/* Init RTC register */
> +static void mt2712_rtc_hw_init(struct mt2712_rtc *rtc)
> +{
> +	u32 p1, p2;
> +
> +	mt2712_writel(rtc, MT2712_BBPU, MT2712_BBPU_KEY | MT2712_BBPU_RELOAD);
> +
> +	mt2712_writel(rtc, MT2712_CII_EN, 0);
> +	mt2712_writel(rtc, MT2712_AL_MASK, 0);
> +	/* necessary before set MT2712_POWERKEY */
> +	mt2712_writel(rtc, MT2712_CON0, 0x4848);
> +	mt2712_writel(rtc, MT2712_CON1, 0x0048);
> +
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_readl(rtc, MT2712_IRQ_STA);	/* read clear */
> +
> +	p1 = mt2712_readl(rtc, MT2712_POWERKEY1);
> +	p2 = mt2712_readl(rtc, MT2712_POWERKEY2);
> +	if (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)
> +		dev_dbg(rtc->dev, "powerkey not set (lost power)\n");
> +

This info is valuable, you should check that when reading the time and
return -EINVAL if power was lost.


> +	/* RTC need POWERKEY1/2 match, then goto normal work mode */
> +	mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY);
> +	mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY);

This should be written when setting the time after power was lost.

> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_rtc_writeif_unlock(rtc);
> +}
> +
> +static const struct rtc_class_ops mt2712_rtc_ops = {
> +	.read_time	= mt2712_rtc_read_time,
> +	.set_time	= mt2712_rtc_set_time,
> +	.read_alarm	= mt2712_rtc_read_alarm,
> +	.set_alarm	= mt2712_rtc_set_alarm,

For proper operations, you should also provide the .alarm_irq_enable
callback.

> +};
> +
> +static int mt2712_rtc_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct mt2712_rtc *rtc;
> +	int ret;
> +
> +	rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt2712_rtc), GFP_KERNEL);
> +	if (!rtc)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	rtc->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(rtc->base))
> +		return PTR_ERR(rtc->base);
> +
> +	rtc->irq = platform_get_irq(pdev, 0);
> +	if (rtc->irq < 0) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		return rtc->irq;
> +	}
> +
> +	rtc->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, rtc);
> +
> +	rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev);
> +	if (IS_ERR(rtc->rtc_dev))
> +		return PTR_ERR(rtc->rtc_dev);
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
> +					rtc_irq_handler_thread,
> +					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> +					dev_name(rtc->dev), rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
> +			rtc->irq, ret);
> +		return ret;
> +	}
> +
> +	/* rtc hw init */
> +	mt2712_rtc_hw_init(rtc);
> +
> +	device_init_wakeup(&pdev->dev, true);
> +
> +	rtc->rtc_dev->ops = &mt2712_rtc_ops;

If you set the range properly here using rtc_dev->range_min and
rtc_dev->range_max, then the core will be able to do range checking and
will also take care of the year offset/windowing calculations instead of
having to hardcode that in the driver.

> +
> +	ret = rtc_register_device(rtc->rtc_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "register rtc device failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mt2712_rtc_suspend(struct device *dev)
> +{
> +	int wake_status = 0;
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev)) {
> +		wake_status = enable_irq_wake(rtc->irq);
> +		if (!wake_status)
> +			rtc->irq_wake_enabled = true;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_resume(struct device *dev)
> +{
> +	int wake_status = 0;
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev) && rtc->irq_wake_enabled) {
> +		wake_status = disable_irq_wake(rtc->irq);
> +		if (!wake_status)
> +			rtc->irq_wake_enabled = false;
> +	}
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mt2712_pm_ops, mt2712_rtc_suspend,
> +			 mt2712_rtc_resume);
> +#endif
> +
> +static const struct of_device_id mt2712_rtc_of_match[] = {
> +	{ .compatible = "mediatek,mt2712-rtc", },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, mt2712_rtc_of_match)
> +
> +static struct platform_driver mt2712_rtc_driver = {
> +	.driver = {
> +		.name = MTK_RTC_DEV,
> +		.of_match_table = mt2712_rtc_of_match,
> +		.pm = &mt2712_pm_ops,
> +	},
> +	.probe  = mt2712_rtc_probe,
> +};
> +
> +module_platform_driver(mt2712_rtc_driver);
> +
> +MODULE_DESCRIPTION("MediaTek MT2712 SoC based RTC Driver");
> +MODULE_AUTHOR("Ran Bi <ran.bi@mediatek.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.21.0
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH net-next v4 1/2] dt-bindings: net: snps, dwmac: update reg minItems maxItems
From: Martin Blumenstingl @ 2019-08-20 20:17 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, Rob Herring, Maxime Ripard, netdev, linux-kernel,
	robh+dt, linux-amlogic, davem, linux-arm-kernel
In-Reply-To: <20190820075742.14857-2-narmstrong@baylibre.com>

On Tue, Aug 20, 2019 at 9:57 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> The Amlogic Meson DWMAC glue bindings needs a second reg cells for the
> glue registers, thus update the reg minItems/maxItems to allow more
> than a single reg cell.
>
> Also update the allwinner,sun7i-a20-gmac.yaml derivative schema to specify
> maxItems to 1.
this looks good to me because:
- allwinner,sun7i-a20-gmac.yaml now restricts reg to maxItems 1
- allwinner,sun8i-a83t-emac.yaml already restricts reg to maxItems 1
- amlogic,meson-dwmac.yaml (introduced in patch 2 from this series)
will need maxItems 2
- (these are all yaml based schemas for DWMAC IP)

> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 3/4] dt-bindings: arm: fsl: Add Kontron i.MX6UL N6310 compatibles
From: Krzysztof Kozlowski @ 2019-08-20 20:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Shawn Guo, Sascha Hauer,
	linux-kernel@vger.kernel.org, Schrempf Frieder, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqKAH6n1sMoWOhfiHKxgREr-EN1tw0QtC1H8Fm=a7PNzOA@mail.gmail.com>

On Tue, Aug 20, 2019 at 03:04:57PM -0500, Rob Herring wrote:
> On Tue, Aug 20, 2019 at 1:36 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Tue, 20 Aug 2019 at 18:59, Rob Herring <robh+dt@kernel.org> wrote:
> > >
> > > On Tue, Aug 20, 2019 at 10:35 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > Add the compatibles for Kontron i.MX6UL N6310 SoM and boards.
> > > >
> > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > >
> > > > ---
> > > >
> > > > Changes since v5:
> > > > New patch
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/fsl.yaml | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > > index 7294ac36f4c0..d07b3c06d7cf 100644
> > > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > > @@ -161,6 +161,9 @@ properties:
> > > >          items:
> > > >            - enum:
> > > >                - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
> > > > +              - kontron,imx6ul-n6310-som  # Kontron N6310 SOM
> > > > +              - kontron,imx6ul-n6310-s    # Kontron N6310 S Board
> > > > +              - kontron,imx6ul-n6310-s-43 # Kontron N6310 S 43 Board
> > >
> > > This doesn't match what is in your dts files. Run 'make dtbs_check' and see.
> >
> > You mean the name does not match? I thought that '#' is a comment in YAML...
> 
> No, the number of compatible strings is the problem.

I see. If I understand the schema correctly, this should look like:

diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 7294ac36f4c0..eb263d1ccf13 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -161,6 +161,22 @@ properties:
         items:
           - enum:
               - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
+              - kontron,imx6ul-n6310-som  # Kontron N6310 SOM
+          - const: fsl,imx6ul
+
+      - description: Kontron N6310 S Board
+        items:
+          - enum:
+              - kontron,imx6ul-n6310-s
+          - const: kontron,imx6ul-n6310-som
+          - const: fsl,imx6ul
+
+      - description: Kontron N6310 S 43 Board
+        items:
+          - enum:
+              - kontron,imx6ul-n6310-s-43
+          - const: kontron,imx6ul-n6310-s
+          - const: kontron,imx6ul-n6310-som
           - const: fsl,imx6ul

       - description: i.MX6ULL based Boards


It passes the dtbs_check. Is it correct?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 01/14] arm64: dts: meson: fix ethernet mac reg format
From: Martin Blumenstingl @ 2019-08-20 20:21 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190814142918.11636-2-narmstrong@baylibre.com>

On Wed, Aug 14, 2019 at 4:30 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-axg-s400.dt.yaml: soc: ethernet@ff3f0000:reg:0: [0, 4282318848, 0, 65536, 0, 4284695872, 0, 8] is too long
> meson-axg-s400.dt.yaml: ethernet@ff3f0000: reg: [[0, 4282318848, 0, 65536, 0, 4284695872, 0, 8]] is too short
> meson-g12a-u200.dt.yaml: soc: ethernet@ff3f0000:reg:0: [0, 4282318848, 0, 65536, 0, 4284695872, 0, 8] is too long
> meson-g12a-u200.dt.yaml: ethernet@ff3f0000: reg: [[0, 4282318848, 0, 65536, 0, 4284695872, 0, 8]] is too short
> meson-gxbb-nanopi-k2.dt.yaml: soc: ethernet@c9410000:reg:0: [0, 3376480256, 0, 65536, 0, 3364046144, 0, 4] is too long
> meson-gxl-s805x-libretech-ac.dt.yaml: soc: ethernet@c9410000:reg:0: [0, 3376480256, 0, 65536, 0, 3364046144, 0, 4] is too lon
if you have to re-send it for whatever reason I would appreciate if
you could add:
"while here, also drop the redundant reg property from meson-gxl.dtsi
because it had the same value as meson-gx.dtsi from which it inherits"

> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 03/14] arm64: dts: meson-gx: fix reset controller compatible
From: Martin Blumenstingl @ 2019-08-20 20:21 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190814142918.11636-4-narmstrong@baylibre.com>

On Wed, Aug 14, 2019 at 4:29 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxbb-nanopi-k2.dt.yaml: reset-controller@4404: compatible:0: 'amlogic,meson-gx-reset' is not one of ['amlogic,meson8b-reset', 'amlogic,meson-gxbb-reset', 'amlogic,meson-axg-reset']
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 05/14] arm64: dts: meson-gx: fix watchdog compatible
From: Martin Blumenstingl @ 2019-08-20 20:22 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190814142918.11636-6-narmstrong@baylibre.com>

On Wed, Aug 14, 2019 at 4:30 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxbb-nanopi-k2.dt.yaml: watchdog@98d0: compatible:0: 'amlogic,meson-gx-wdt' is not one of ['amlogic,meson-gxbb-wdt']
> meson-gxl-s805x-libretech-ac.dt.yaml: watchdog@98d0: compatible:0: 'amlogic,meson-gx-wdt' is not one of ['amlogic,meson-gxbb-wdt']
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 13/14] arm64: dts: meson-gxbb-p201: fix snps, reset-delays-us format
From: Martin Blumenstingl @ 2019-08-20 20:23 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190814142918.11636-14-narmstrong@baylibre.com>

On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxbb-p201.dt.yaml: ethernet@c9410000: snps,reset-delays-us: [[0, 10000, 1000000]] is too short
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 12/14] arm64: dts: meson-gxbb-nanopi-k2: add missing model
From: Martin Blumenstingl @ 2019-08-20 20:25 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: khilman, devicetree, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <20190814142918.11636-13-narmstrong@baylibre.com>

On Wed, Aug 14, 2019 at 4:33 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This fixes the following DT schemas check errors:
> meson-gxbb-nanopi-k2.dt.yaml: /: 'model' is a required property
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
> index c34c1c90ccb6..1a36d2bd2d21 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
> @@ -10,6 +10,7 @@
>
>  / {
>         compatible = "friendlyarm,nanopi-k2", "amlogic,meson-gxbb";
> +       model = "Nanopi K2";
this should be "FriendlyARM NanoPi K2" to be consistent with other
boards (for example meson-gxbb-odroidc2.dts)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ 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