Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4] of/pci: Fix memory leak in of_pci_get_host_bridge_resources
From: kbuild test robot @ 2017-04-05 17:36 UTC (permalink / raw)
  Cc: kbuild-all, linux-pci, robh, toshi.kani, shawn.lin, briannorris,
	linux-kernel, dianders, bhelgaas, dtor, Jeffy Chen, devicetree,
	Rob Herring, Frank Rowand
In-Reply-To: <1491358521-17834-2-git-send-email-jeffy.chen@rock-chips.com>

[-- Attachment #1: Type: text/plain, Size: 4036 bytes --]

Hi Jeffy,

[auto build test ERROR on pci/next]
[also build test ERROR on v4.11-rc5 next-20170405]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jeffy-Chen/of-pci-Fix-memory-leak-in-of_pci_get_host_bridge_resources/20170406-005941
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-n0-04051821 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers//of/of_pci.c: In function 'of_pci_get_host_bridge_resources':
>> drivers//of/of_pci.c:215:30: error: incompatible type for argument 2 of 'pci_add_resource'
     pci_add_resource(resources, res);
                                 ^~~
   In file included from include/linux/of_pci.h:4:0,
                    from drivers//of/of_pci.c:8:
   include/linux/pci.h:1152:6: note: expected 'struct resource *' but argument is of type 'struct resource'
    void pci_add_resource(struct list_head *resources, struct resource *res);
         ^~~~~~~~~~~~~~~~
>> drivers//of/of_pci.c:261:9: error: invalid type argument of '->' (have 'struct resource')
         res->start - range.pci_addr);
            ^~
>> drivers//of/of_pci.c:260:38: error: incompatible type for argument 2 of 'pci_add_resource_offset'
      pci_add_resource_offset(resources, res,
                                         ^~~
   In file included from include/linux/of_pci.h:4:0,
                    from drivers//of/of_pci.c:8:
   include/linux/pci.h:1153:6: note: expected 'struct resource *' but argument is of type 'struct resource'
    void pci_add_resource_offset(struct list_head *resources, struct resource *res,
         ^~~~~~~~~~~~~~~~~~~~~~~

vim +/pci_add_resource +215 drivers//of/of_pci.c

   209		} else {
   210			if (res.end > res.start + bus_max)
   211				res.end = res.start + bus_max;
   212		}
   213	
   214		res.flags |= IORESOURCE_AUTO;
 > 215		pci_add_resource(resources, res);
   216	
   217		/* Check for ranges property */
   218		err = of_pci_range_parser_init(&parser, dev);
   219		if (err)
   220			goto parse_failed;
   221	
   222		pr_debug("Parsing ranges property...\n");
   223		for_each_of_pci_range(&parser, &range) {
   224			/* Read next ranges element */
   225			if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
   226				snprintf(range_type, 4, " IO");
   227			else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
   228				snprintf(range_type, 4, "MEM");
   229			else
   230				snprintf(range_type, 4, "err");
   231			pr_info("  %s %#010llx..%#010llx -> %#010llx\n", range_type,
   232				range.cpu_addr, range.cpu_addr + range.size - 1,
   233				range.pci_addr);
   234	
   235			/*
   236			 * If we failed translation or got a zero-sized region
   237			 * then skip this range
   238			 */
   239			if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
   240				continue;
   241	
   242			err = of_pci_range_to_resource(&range, dev, &res);
   243			if (err)
   244				continue;
   245	
   246			if (resource_type(&res) == IORESOURCE_IO) {
   247				if (!io_base) {
   248					pr_err("I/O range found for %s. Please provide an io_base pointer to save CPU base address\n",
   249						dev->full_name);
   250					err = -EINVAL;
   251					goto parse_failed;
   252				}
   253				if (*io_base != (resource_size_t)OF_BAD_ADDR)
   254					pr_warn("More than one I/O resource converted for %s. CPU base address for old range lost!\n",
   255						dev->full_name);
   256				*io_base = range.cpu_addr;
   257			}
   258	
   259			res.flags |= IORESOURCE_AUTO;
 > 260			pci_add_resource_offset(resources, res,
 > 261						res->start - range.pci_addr);
   262		}
   263	
   264		return 0;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25202 bytes --]

^ permalink raw reply

* Applied "regulator: DT: Add settling time property for non-linear voltage change" to the regulator tree
From: Mark Brown @ 2017-04-05 17:31 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: Rob Herring, Mark Brown
In-Reply-To: <1479898960-14166-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

The patch

   regulator: DT: Add settling time property for non-linear voltage change

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From cfd2cedb482ae29ce13a3db46cc24f0c082ca9cf Mon Sep 17 00:00:00 2001
From: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Date: Tue, 4 Apr 2017 18:59:49 +0530
Subject: [PATCH] regulator: DT: Add settling time property for non-linear
 voltage change

Some regulators (some PWM regulators) have the voltage transition
exponentially. On such cases, the settling time for voltage change
is treated as constant time.

Add DT property for providing the settling time for any level of
voltage change for non-linear voltage change.

signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 6ab5aef619d9..d18edb075e1c 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,9 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-settling-time-us: Settling time, in microseconds, for voltage
+  change if regulator have the constant time for any level voltage change.
+  This is useful when regulator have exponential voltage change.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Applied "regulator: Add settling time for non-linear voltage transition" to the regulator tree
From: Mark Brown @ 2017-04-05 17:31 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: Mark Brown
In-Reply-To: <1491312590-20887-2-git-send-email-ldewangan@nvidia.com>

The patch

   regulator: Add settling time for non-linear voltage transition

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d6c1dc3f52e3a65f35c58433ba57d14c0bad902f Mon Sep 17 00:00:00 2001
From: Laxman Dewangan <ldewangan@nvidia.com>
Date: Tue, 4 Apr 2017 18:59:50 +0530
Subject: [PATCH] regulator: Add settling time for non-linear voltage
 transition

Some regulators (some PWM regulators) have the voltage transition
non-linear i.e. exponentially. On such cases, the settling time
for voltage transition can not be presented in the voltage-ramp-delay.

Add new property for non-linear voltage transition and handle this
in getting the voltage settling time.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c          | 2 ++
 drivers/regulator/of_regulator.c  | 4 ++++
 include/linux/regulator/machine.h | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 04baac9a165b..3a641d64f8e1 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2753,6 +2753,8 @@ static int _regulator_set_voltage_time(struct regulator_dev *rdev,
 		ramp_delay = rdev->constraints->ramp_delay;
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
+	else if (rdev->constraints->settling_time)
+		return rdev->constraints->settling_time;
 
 	if (ramp_delay == 0) {
 		rdev_dbg(rdev, "ramp_delay not set\n");
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec99500..09d677d5d3f0 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -86,6 +86,10 @@ static void of_get_regulation_constraints(struct device_node *np,
 			constraints->ramp_disable = true;
 	}
 
+	ret = of_property_read_u32(np, "regulator-settling-time-us", &pval);
+	if (!ret)
+		constraints->settling_time = pval;
+
 	ret = of_property_read_u32(np, "regulator-enable-ramp-delay", &pval);
 	if (!ret)
 		constraints->enable_time = pval;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ad3e5158e586..598a493b3927 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -108,6 +108,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @settling_time: Time to settle down after voltage change when voltage
+ *		   change is non-linear (unit: microseconds).
  * @active_discharge: Enable/disable active discharge. The enum
  *		      regulator_active_discharge values are used for
  *		      initialisation.
@@ -149,6 +151,7 @@ struct regulation_constraints {
 	unsigned int initial_mode;
 
 	unsigned int ramp_delay;
+	unsigned int settling_time;
 	unsigned int enable_time;
 
 	unsigned int active_discharge;
-- 
2.11.0

^ permalink raw reply related

* Applied "ASoC: wm_adsp: Add support for ADSP2V2" to the asoc tree
From: Mark Brown @ 2017-04-05 17:31 UTC (permalink / raw)
  To: Richard Fitzgerald
  Cc: Charles Keepax, Mark Brown, linus.walleij, gnurou, robh+dt, tglx,
	jason, lee.jones
In-Reply-To: <1491386884-30689-12-git-send-email-rf@opensource.wolfsonmicro.com>

The patch

   ASoC: wm_adsp: Add support for ADSP2V2

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From e1ea1879f2889a26370bd0bc29c1e95caf9d36f2 Mon Sep 17 00:00:00 2001
From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
Date: Wed, 5 Apr 2017 11:07:59 +0100
Subject: [PATCH] ASoC: wm_adsp: Add support for ADSP2V2

Adds support for ADSP2V2 cores. Primary differences are that
they use a 32-bit register map compared to the 16-bit register
map of ADSP2V1, and there are some changes to clocking control.

Signed-off-by: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/wm_adsp.c | 187 ++++++++++++++++++++++++++++++++++-----------
 sound/soc/codecs/wm_adsp.h |   1 +
 2 files changed, 145 insertions(+), 43 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index bbdb72f73df1..a9acf222b502 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -112,17 +112,22 @@
 #define ADSP1_CLK_SEL_SHIFT                    0  /* CLK_SEL_ENA */
 #define ADSP1_CLK_SEL_WIDTH                    3  /* CLK_SEL_ENA */
 
-#define ADSP2_CONTROL        0x0
-#define ADSP2_CLOCKING       0x1
-#define ADSP2_STATUS1        0x4
-#define ADSP2_WDMA_CONFIG_1 0x30
-#define ADSP2_WDMA_CONFIG_2 0x31
-#define ADSP2_RDMA_CONFIG_1 0x34
-
-#define ADSP2_SCRATCH0        0x40
-#define ADSP2_SCRATCH1        0x41
-#define ADSP2_SCRATCH2        0x42
-#define ADSP2_SCRATCH3        0x43
+#define ADSP2_CONTROL                     0x0
+#define ADSP2_CLOCKING                    0x1
+#define ADSP2V2_CLOCKING                  0x2
+#define ADSP2_STATUS1                     0x4
+#define ADSP2_WDMA_CONFIG_1               0x30
+#define ADSP2_WDMA_CONFIG_2               0x31
+#define ADSP2V2_WDMA_CONFIG_2             0x32
+#define ADSP2_RDMA_CONFIG_1               0x34
+
+#define ADSP2_SCRATCH0                    0x40
+#define ADSP2_SCRATCH1                    0x41
+#define ADSP2_SCRATCH2                    0x42
+#define ADSP2_SCRATCH3                    0x43
+
+#define ADSP2V2_SCRATCH0_1                0x40
+#define ADSP2V2_SCRATCH2_3                0x42
 
 /*
  * ADSP2 Control
@@ -153,6 +158,17 @@
 #define ADSP2_CLK_SEL_WIDTH                    3  /* CLK_SEL_ENA */
 
 /*
+ * ADSP2V2 clocking
+ */
+#define ADSP2V2_CLK_SEL_MASK             0x70000  /* CLK_SEL_ENA */
+#define ADSP2V2_CLK_SEL_SHIFT                 16  /* CLK_SEL_ENA */
+#define ADSP2V2_CLK_SEL_WIDTH                  3  /* CLK_SEL_ENA */
+
+#define ADSP2V2_RATE_MASK                 0x7800  /* DSP_RATE */
+#define ADSP2V2_RATE_SHIFT                    11  /* DSP_RATE */
+#define ADSP2V2_RATE_WIDTH                     4  /* DSP_RATE */
+
+/*
  * ADSP2 Status 1
  */
 #define ADSP2_RAM_RDY                     0x0001
@@ -683,6 +699,9 @@ static const struct soc_enum wm_adsp_fw_enum[] = {
 	SOC_ENUM_SINGLE(0, 1, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
 	SOC_ENUM_SINGLE(0, 2, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
 	SOC_ENUM_SINGLE(0, 3, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
+	SOC_ENUM_SINGLE(0, 4, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
+	SOC_ENUM_SINGLE(0, 5, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
+	SOC_ENUM_SINGLE(0, 6, ARRAY_SIZE(wm_adsp_fw_text), wm_adsp_fw_text),
 };
 
 const struct snd_kcontrol_new wm_adsp_fw_controls[] = {
@@ -694,6 +713,12 @@ const struct snd_kcontrol_new wm_adsp_fw_controls[] = {
 		     wm_adsp_fw_get, wm_adsp_fw_put),
 	SOC_ENUM_EXT("DSP4 Firmware", wm_adsp_fw_enum[3],
 		     wm_adsp_fw_get, wm_adsp_fw_put),
+	SOC_ENUM_EXT("DSP5 Firmware", wm_adsp_fw_enum[4],
+		     wm_adsp_fw_get, wm_adsp_fw_put),
+	SOC_ENUM_EXT("DSP6 Firmware", wm_adsp_fw_enum[5],
+		     wm_adsp_fw_get, wm_adsp_fw_put),
+	SOC_ENUM_EXT("DSP7 Firmware", wm_adsp_fw_enum[6],
+		     wm_adsp_fw_get, wm_adsp_fw_put),
 };
 EXPORT_SYMBOL_GPL(wm_adsp_fw_controls);
 
@@ -750,6 +775,29 @@ static void wm_adsp2_show_fw_status(struct wm_adsp *dsp)
 		 be16_to_cpu(scratch[3]));
 }
 
+static void wm_adsp2v2_show_fw_status(struct wm_adsp *dsp)
+{
+	u32 scratch[2];
+	int ret;
+
+	ret = regmap_raw_read(dsp->regmap, dsp->base + ADSP2V2_SCRATCH0_1,
+			      scratch, sizeof(scratch));
+
+	if (ret) {
+		adsp_err(dsp, "Failed to read SCRATCH regs: %d\n", ret);
+		return;
+	}
+
+	scratch[0] = be32_to_cpu(scratch[0]);
+	scratch[1] = be32_to_cpu(scratch[1]);
+
+	adsp_dbg(dsp, "FW SCRATCH 0:0x%x 1:0x%x 2:0x%x 3:0x%x\n",
+		 scratch[0] & 0xFFFF,
+		 scratch[0] >> 16,
+		 scratch[1] & 0xFFFF,
+		 scratch[1] >> 16);
+}
+
 static inline struct wm_coeff_ctl *bytes_ext_to_ctl(struct soc_bytes_ext *ext)
 {
 	return container_of(ext, struct wm_coeff_ctl, bytes_ext);
@@ -2435,10 +2483,17 @@ static int wm_adsp2_ena(struct wm_adsp *dsp)
 	unsigned int val;
 	int ret, count;
 
-	ret = regmap_update_bits_async(dsp->regmap, dsp->base + ADSP2_CONTROL,
-				       ADSP2_SYS_ENA, ADSP2_SYS_ENA);
-	if (ret != 0)
-		return ret;
+	switch (dsp->rev) {
+	case 0:
+		ret = regmap_update_bits_async(dsp->regmap,
+					       dsp->base + ADSP2_CONTROL,
+					       ADSP2_SYS_ENA, ADSP2_SYS_ENA);
+		if (ret != 0)
+			return ret;
+		break;
+	default:
+		break;
+	}
 
 	/* Wait for the RAM to start, should be near instantaneous */
 	for (count = 0; count < 10; ++count) {
@@ -2497,11 +2552,17 @@ static void wm_adsp2_boot_work(struct work_struct *work)
 	if (ret != 0)
 		goto err_ena;
 
-	/* Turn DSP back off until we are ready to run */
-	ret = regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
-				 ADSP2_SYS_ENA, 0);
-	if (ret != 0)
-		goto err_ena;
+	switch (dsp->rev) {
+	case 0:
+		/* Turn DSP back off until we are ready to run */
+		ret = regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
+					 ADSP2_SYS_ENA, 0);
+		if (ret != 0)
+			goto err_ena;
+		break;
+	default:
+		break;
+	}
 
 	dsp->booted = true;
 
@@ -2523,12 +2584,21 @@ static void wm_adsp2_set_dspclk(struct wm_adsp *dsp, unsigned int freq)
 {
 	int ret;
 
-	ret = regmap_update_bits_async(dsp->regmap,
-				       dsp->base + ADSP2_CLOCKING,
-				       ADSP2_CLK_SEL_MASK,
-				       freq << ADSP2_CLK_SEL_SHIFT);
-	if (ret != 0)
-		adsp_err(dsp, "Failed to set clock rate: %d\n", ret);
+	switch (dsp->rev) {
+	case 0:
+		ret = regmap_update_bits_async(dsp->regmap,
+					       dsp->base + ADSP2_CLOCKING,
+					       ADSP2_CLK_SEL_MASK,
+					       freq << ADSP2_CLK_SEL_SHIFT);
+		if (ret) {
+			adsp_err(dsp, "Failed to set clock rate: %d\n", ret);
+			return;
+		}
+		break;
+	default:
+		/* clock is handled by parent codec driver */
+		break;
+	}
 }
 
 int wm_adsp2_preloader_get(struct snd_kcontrol *kcontrol,
@@ -2664,22 +2734,46 @@ int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 		wm_adsp_signal_event_controls(dsp, WM_ADSP_FW_EVENT_SHUTDOWN);
 
 		/* Log firmware state, it can be useful for analysis */
-		wm_adsp2_show_fw_status(dsp);
+		switch (dsp->rev) {
+		case 0:
+			wm_adsp2_show_fw_status(dsp);
+			break;
+		default:
+			wm_adsp2v2_show_fw_status(dsp);
+			break;
+		}
 
 		mutex_lock(&dsp->pwr_lock);
 
 		dsp->running = false;
 
-		regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
+		regmap_update_bits(dsp->regmap,
+				   dsp->base + ADSP2_CONTROL,
 				   ADSP2_CORE_ENA | ADSP2_START, 0);
 
 		/* Make sure DMAs are quiesced */
-		regmap_write(dsp->regmap, dsp->base + ADSP2_RDMA_CONFIG_1, 0);
-		regmap_write(dsp->regmap, dsp->base + ADSP2_WDMA_CONFIG_1, 0);
-		regmap_write(dsp->regmap, dsp->base + ADSP2_WDMA_CONFIG_2, 0);
-
-		regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
-				   ADSP2_SYS_ENA, 0);
+		switch (dsp->rev) {
+		case 0:
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2_RDMA_CONFIG_1, 0);
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2_WDMA_CONFIG_1, 0);
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2_WDMA_CONFIG_2, 0);
+
+			regmap_update_bits(dsp->regmap,
+					   dsp->base + ADSP2_CONTROL,
+					   ADSP2_SYS_ENA, 0);
+			break;
+		default:
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2_RDMA_CONFIG_1, 0);
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2_WDMA_CONFIG_1, 0);
+			regmap_write(dsp->regmap,
+				     dsp->base + ADSP2V2_WDMA_CONFIG_2, 0);
+			break;
+		}
 
 		if (wm_adsp_fw[dsp->fw].num_caps != 0)
 			wm_adsp_buffer_free(dsp);
@@ -2732,15 +2826,22 @@ int wm_adsp2_init(struct wm_adsp *dsp)
 {
 	int ret;
 
-	/*
-	 * Disable the DSP memory by default when in reset for a small
-	 * power saving.
-	 */
-	ret = regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
-				 ADSP2_MEM_ENA, 0);
-	if (ret != 0) {
-		adsp_err(dsp, "Failed to clear memory retention: %d\n", ret);
-		return ret;
+	switch (dsp->rev) {
+	case 0:
+		/*
+		 * Disable the DSP memory by default when in reset for a small
+		 * power saving.
+		 */
+		ret = regmap_update_bits(dsp->regmap, dsp->base + ADSP2_CONTROL,
+					 ADSP2_MEM_ENA, 0);
+		if (ret) {
+			adsp_err(dsp,
+				 "Failed to clear memory retention: %d\n", ret);
+			return ret;
+		}
+		break;
+	default:
+		break;
 	}
 
 	INIT_LIST_HEAD(&dsp->alg_regions);
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 3706b11053a3..997227f4d404 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -40,6 +40,7 @@ struct wm_adsp_compr_buf;
 
 struct wm_adsp {
 	const char *part;
+	int rev;
 	int num;
 	int type;
 	struct device *dev;
-- 
2.11.0


^ permalink raw reply related

* Applied "ASoC: wm_adsp: add support for DSP region lock" to the asoc tree
From: Mark Brown @ 2017-04-05 17:31 UTC (permalink / raw)
  To: Mayuresh Kulkarni
  Cc: Nikesh Oswal, Charles Keepax, Richard Fitzgerald, Mark Brown,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	gnurou-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, jason-NLaQJdtUoK4Be96aLqz0jA,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1491386884-30689-13-git-send-email-rf-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

The patch

   ASoC: wm_adsp: add support for DSP region lock

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 51a2c944ead91171495ef338689da057bbcaab0c Mon Sep 17 00:00:00 2001
From: Mayuresh Kulkarni <mkulkarni-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Date: Wed, 5 Apr 2017 11:08:00 +0100
Subject: [PATCH] ASoC: wm_adsp: add support for DSP region lock

Newer ADSP2V2 codecs include a memory protection unit that can
be set to trap illegal accesses. When enabling an ADSPV2 core we
must configure the memory region traps so that the firmware can
access its own memory.

Signed-off-by: Mayuresh Kulkarni <mkulkarni-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Signed-off-by: Nikesh Oswal <Nikesh.Oswal-UVNVL95qEvAciDkP5Hr2oA@public.gmane.org>
Signed-off-by: Charles Keepax <ckeepax-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Signed-off-by: Richard Fitzgerald <rf-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 sound/soc/codecs/wm_adsp.c | 137 +++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/wm_adsp.h |  23 ++++++++
 2 files changed, 160 insertions(+)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index a9acf222b502..20695b691aff 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -176,6 +176,37 @@
 #define ADSP2_RAM_RDY_SHIFT                    0
 #define ADSP2_RAM_RDY_WIDTH                    1
 
+/*
+ * ADSP2 Lock support
+ */
+#define ADSP2_LOCK_CODE_0                    0x5555
+#define ADSP2_LOCK_CODE_1                    0xAAAA
+
+#define ADSP2_WATCHDOG                       0x0A
+#define ADSP2_BUS_ERR_ADDR                   0x52
+#define ADSP2_REGION_LOCK_STATUS             0x64
+#define ADSP2_LOCK_REGION_1_LOCK_REGION_0    0x66
+#define ADSP2_LOCK_REGION_3_LOCK_REGION_2    0x68
+#define ADSP2_LOCK_REGION_5_LOCK_REGION_4    0x6A
+#define ADSP2_LOCK_REGION_7_LOCK_REGION_6    0x6C
+#define ADSP2_LOCK_REGION_9_LOCK_REGION_8    0x6E
+#define ADSP2_LOCK_REGION_CTRL               0x7A
+#define ADSP2_PMEM_ERR_ADDR_XMEM_ERR_ADDR    0x7C
+
+#define ADSP2_REGION_LOCK_ERR_MASK           0x8000
+#define ADSP2_SLAVE_ERR_MASK                 0x4000
+#define ADSP2_WDT_TIMEOUT_STS_MASK           0x2000
+#define ADSP2_CTRL_ERR_PAUSE_ENA             0x0002
+#define ADSP2_CTRL_ERR_EINT                  0x0001
+
+#define ADSP2_BUS_ERR_ADDR_MASK              0x00FFFFFF
+#define ADSP2_XMEM_ERR_ADDR_MASK             0x0000FFFF
+#define ADSP2_PMEM_ERR_ADDR_MASK             0x7FFF0000
+#define ADSP2_PMEM_ERR_ADDR_SHIFT            16
+#define ADSP2_WDT_ENA_MASK                   0xFFFFFFFD
+
+#define ADSP2_LOCK_REGION_SHIFT              16
+
 #define ADSP_MAX_STD_CTRL_SIZE               512
 
 #define WM_ADSP_ACKED_CTL_TIMEOUT_MS         100
@@ -2638,6 +2669,18 @@ int wm_adsp2_preloader_put(struct snd_kcontrol *kcontrol,
 }
 EXPORT_SYMBOL_GPL(wm_adsp2_preloader_put);
 
+static void wm_adsp_stop_watchdog(struct wm_adsp *dsp)
+{
+	switch (dsp->rev) {
+	case 0:
+	case 1:
+		return;
+	default:
+		regmap_update_bits(dsp->regmap, dsp->base + ADSP2_WATCHDOG,
+				   ADSP2_WDT_ENA_MASK, 0);
+	}
+}
+
 int wm_adsp2_early_event(struct snd_soc_dapm_widget *w,
 			 struct snd_kcontrol *kcontrol, int event,
 			 unsigned int freq)
@@ -2710,6 +2753,8 @@ int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 		if (ret != 0)
 			goto err;
 
+		wm_adsp2_lock(dsp, dsp->lock_regions);
+
 		ret = regmap_update_bits(dsp->regmap,
 					 dsp->base + ADSP2_CONTROL,
 					 ADSP2_CORE_ENA | ADSP2_START,
@@ -2733,6 +2778,8 @@ int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 		/* Tell the firmware to cleanup */
 		wm_adsp_signal_event_controls(dsp, WM_ADSP_FW_EVENT_SHUTDOWN);
 
+		wm_adsp_stop_watchdog(dsp);
+
 		/* Log firmware state, it can be useful for analysis */
 		switch (dsp->rev) {
 		case 0:
@@ -3624,4 +3671,94 @@ int wm_adsp_compr_copy(struct snd_compr_stream *stream, char __user *buf,
 }
 EXPORT_SYMBOL_GPL(wm_adsp_compr_copy);
 
+int wm_adsp2_lock(struct wm_adsp *dsp, unsigned int lock_regions)
+{
+	struct regmap *regmap = dsp->regmap;
+	unsigned int code0, code1, lock_reg;
+
+	if (!(lock_regions & WM_ADSP2_REGION_ALL))
+		return 0;
+
+	lock_regions &= WM_ADSP2_REGION_ALL;
+	lock_reg = dsp->base + ADSP2_LOCK_REGION_1_LOCK_REGION_0;
+
+	while (lock_regions) {
+		code0 = code1 = 0;
+		if (lock_regions & BIT(0)) {
+			code0 = ADSP2_LOCK_CODE_0;
+			code1 = ADSP2_LOCK_CODE_1;
+		}
+		if (lock_regions & BIT(1)) {
+			code0 |= ADSP2_LOCK_CODE_0 << ADSP2_LOCK_REGION_SHIFT;
+			code1 |= ADSP2_LOCK_CODE_1 << ADSP2_LOCK_REGION_SHIFT;
+		}
+		regmap_write(regmap, lock_reg, code0);
+		regmap_write(regmap, lock_reg, code1);
+		lock_regions >>= 2;
+		lock_reg += 2;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wm_adsp2_lock);
+
+irqreturn_t wm_adsp2_bus_error(struct wm_adsp *dsp)
+{
+	unsigned int val;
+	struct regmap *regmap = dsp->regmap;
+	int ret = 0;
+
+	ret = regmap_read(regmap, dsp->base + ADSP2_LOCK_REGION_CTRL, &val);
+	if (ret) {
+		adsp_err(dsp,
+			"Failed to read Region Lock Ctrl register: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	if (val & ADSP2_WDT_TIMEOUT_STS_MASK) {
+		adsp_err(dsp, "watchdog timeout error\n");
+		wm_adsp_stop_watchdog(dsp);
+	}
+
+	if (val & (ADSP2_SLAVE_ERR_MASK | ADSP2_REGION_LOCK_ERR_MASK)) {
+		if (val & ADSP2_SLAVE_ERR_MASK)
+			adsp_err(dsp, "bus error: slave error\n");
+		else
+			adsp_err(dsp, "bus error: region lock error\n");
+
+		ret = regmap_read(regmap, dsp->base + ADSP2_BUS_ERR_ADDR, &val);
+		if (ret) {
+			adsp_err(dsp,
+				 "Failed to read Bus Err Addr register: %d\n",
+				 ret);
+			return IRQ_HANDLED;
+		}
+
+		adsp_err(dsp, "bus error address = 0x%x\n",
+			 val & ADSP2_BUS_ERR_ADDR_MASK);
+
+		ret = regmap_read(regmap,
+				  dsp->base + ADSP2_PMEM_ERR_ADDR_XMEM_ERR_ADDR,
+				  &val);
+		if (ret) {
+			adsp_err(dsp,
+				 "Failed to read Pmem Xmem Err Addr register: %d\n",
+				 ret);
+			return IRQ_HANDLED;
+		}
+
+		adsp_err(dsp, "xmem error address = 0x%x\n",
+			 val & ADSP2_XMEM_ERR_ADDR_MASK);
+		adsp_err(dsp, "pmem error address = 0x%x\n",
+			 (val & ADSP2_PMEM_ERR_ADDR_MASK) >>
+			 ADSP2_PMEM_ERR_ADDR_SHIFT);
+	}
+
+	regmap_update_bits(regmap, dsp->base + ADSP2_LOCK_REGION_CTRL,
+			   ADSP2_CTRL_ERR_EINT, ADSP2_CTRL_ERR_EINT);
+
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL_GPL(wm_adsp2_bus_error);
+
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 997227f4d404..41cc11c19b83 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -23,6 +23,23 @@
 #define WM_ADSP_COMPR_OK                 0
 #define WM_ADSP_COMPR_VOICE_TRIGGER      1
 
+#define WM_ADSP2_REGION_0 BIT(0)
+#define WM_ADSP2_REGION_1 BIT(1)
+#define WM_ADSP2_REGION_2 BIT(2)
+#define WM_ADSP2_REGION_3 BIT(3)
+#define WM_ADSP2_REGION_4 BIT(4)
+#define WM_ADSP2_REGION_5 BIT(5)
+#define WM_ADSP2_REGION_6 BIT(6)
+#define WM_ADSP2_REGION_7 BIT(7)
+#define WM_ADSP2_REGION_8 BIT(8)
+#define WM_ADSP2_REGION_9 BIT(9)
+#define WM_ADSP2_REGION_1_9 (WM_ADSP2_REGION_1 | \
+		WM_ADSP2_REGION_2 | WM_ADSP2_REGION_3 | \
+		WM_ADSP2_REGION_4 | WM_ADSP2_REGION_5 | \
+		WM_ADSP2_REGION_6 | WM_ADSP2_REGION_7 | \
+		WM_ADSP2_REGION_8 | WM_ADSP2_REGION_9)
+#define WM_ADSP2_REGION_ALL (WM_ADSP2_REGION_0 | WM_ADSP2_REGION_1_9)
+
 struct wm_adsp_region {
 	int type;
 	unsigned int base;
@@ -76,6 +93,8 @@ struct wm_adsp {
 
 	struct mutex pwr_lock;
 
+	unsigned int lock_regions;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs_root;
 	char *wmfw_file_name;
@@ -114,6 +133,10 @@ int wm_adsp1_event(struct snd_soc_dapm_widget *w,
 int wm_adsp2_early_event(struct snd_soc_dapm_widget *w,
 			 struct snd_kcontrol *kcontrol, int event,
 			 unsigned int freq);
+
+int wm_adsp2_lock(struct wm_adsp *adsp, unsigned int regions);
+irqreturn_t wm_adsp2_bus_error(struct wm_adsp *adsp);
+
 int wm_adsp2_event(struct snd_soc_dapm_widget *w,
 		   struct snd_kcontrol *kcontrol, int event);
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Mauro Carvalho Chehab @ 2017-04-05 17:16 UTC (permalink / raw)
  To: Devin Heitmueller
  Cc: mark.rutland, andrew-ct.chen, minghsiu.tsai,
	sakari.ailus@linux.intel.com, nick, songjun.wu, Hans Verkuil,
	Steve Longerbeam, Pavel Machek, robert.jarzmik, devel,
	markus.heiser, laurent.pinchart+renesas, shuah,
	Russell King - ARM Linux, geert, Steve Longerbeam,
	Linux Media Mailing List, devicetree, Philipp Zabel,
	Arnd Bergmann, Mauro Carvalho Chehab, bparrot, robh+dt,
	horms+renesas, tiffany.lin, linux-arm-kernel
In-Reply-To: <CAGoCfiyfXc2bcTR72XwL3Vv8ny-dQUjEUk2OUuy_s4nedNJqxA@mail.gmail.com>

Em Wed, 5 Apr 2017 13:02:52 -0400
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:

> > I remember I looked on this when I wrote the driver, but I was
> > unable to find a way for em28xx to parse (or forward) such
> > data packets.  
> 
> I'm pretty sure it's possible, but I haven't looked at the datasheets
> in a number of years and don't recall the details.
> 
> Hardware VBI splicing is supported by a number of decoders but it's
> rarely used on commodity PCs (the Conexant and NXP decoders support it
> as well).  That said, I won't argue there might be some value on
> really low end platforms.  All I would ask is that if you do introduce
> any such functionality into the tvp5150 driver for some embedded
> application that you please not break support for devices such as the
> em28xx.

Yeah, sure. If I write such patchset[1], it will be optional, and
will depend on a DT variable (or platform_data) setup that would
tell what GPIO pin should be used for interrupt.

Not providing it should either disable such feature or enable it
via polling.

[1] Please notice that I don't have any demand of doing it. Yet,
I may do it for fun on my spare time:-)

I added in the past an initial support for sliced VBI API on em28xx, 
with got reverted on changeset 1d179eeedc8cb48712bc236ec82ec6c63af42008, 
mainly due to the lack of such feature on tvp5150. So, such code was never
tested (and likely need fixes/updates), but if I end by adding sliced VBI
support on tvp5150 and on OMAP3, I may add support for it on em28xx too,
using I2C poll. On such case, I'll likely add a modprobe parameter to
enable such feature.

Thanks,
Mauro

^ permalink raw reply

* Re: Re: [PATCH v3 04/11] drm/sun4i: abstract the layer type
From: icenowy-h8G6r0blFSE @ 2017-04-05 17:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: devicetree, Jernej Skrabec, linux-sunxi, linux-kernel, dri-devel,
	Rob Herring, Sean Paul, Maxime Ripard, linux-clk,
	linux-arm-kernel
In-Reply-To: <CAGb2v66vxP0c3qMTAUCseTCbpJ6gxnKfNGNcC41jPHA_Ye4ggw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

在 2017-04-05 10:27,Chen-Yu Tsai 写道:
> On Wed, Apr 5, 2017 at 3:53 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>> 
>> 
>> 在 2017年04月05日 03:28, Sean Paul 写道:
>>> 
>>> On Thu, Mar 30, 2017 at 03:46:06AM +0800, Icenowy Zheng wrote:
>>>> 
>>>> As we are going to add support for the Allwinner DE2 Mixer in 
>>>> sun4i-drm
>>>> driver, we will finally have two types of layer.
>>>> 
>>>> Abstract the layer type to void * and a ops struct, which contains 
>>>> the
>>>> only function used by crtc -- get the drm_plane struct of the layer.
>>>> 
>>>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>>>> ---
>>>> Refactored patch in v3.
>>>> 
>>>>  drivers/gpu/drm/sun4i/sun4i_crtc.c  | 19 +++++++++++--------
>>>>  drivers/gpu/drm/sun4i/sun4i_crtc.h  |  3 ++-
>>>>  drivers/gpu/drm/sun4i/sun4i_layer.c | 19 ++++++++++++++++++-
>>>>  drivers/gpu/drm/sun4i/sun4i_layer.h |  2 +-
>>>>  drivers/gpu/drm/sun4i/sunxi_layer.h | 17 +++++++++++++++++
>>>>  5 files changed, 49 insertions(+), 11 deletions(-)
>>>>  create mode 100644 drivers/gpu/drm/sun4i/sunxi_layer.h
>>>> 
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c
>>>> b/drivers/gpu/drm/sun4i/sun4i_crtc.c
>>>> index 3c876c3a356a..33854ee7f636 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
>>>> @@ -29,6 +29,7 @@
>>>>  #include "sun4i_crtc.h"
>>>>  #include "sun4i_drv.h"
>>>>  #include "sun4i_layer.h"
>>>> +#include "sunxi_layer.h"
>>>>  #include "sun4i_tcon.h"
>>>> 
>>>>  static void sun4i_crtc_atomic_begin(struct drm_crtc *crtc,
>>>> @@ -149,7 +150,7 @@ struct sun4i_crtc *sun4i_crtc_init(struct 
>>>> drm_device
>>>> *drm,
>>>>         scrtc->tcon = tcon;
>>>> 
>>>>         /* Create our layers */
>>>> -       scrtc->layers = sun4i_layers_init(drm, scrtc->backend);
>>>> +       scrtc->layers = (void **)sun4i_layers_init(drm, scrtc);
>>>>         if (IS_ERR(scrtc->layers)) {
>>>>                 dev_err(drm->dev, "Couldn't create the planes\n");
>>>>                 return NULL;
>>>> @@ -157,14 +158,15 @@ struct sun4i_crtc *sun4i_crtc_init(struct
>>>> drm_device *drm,
>>>> 
>>>>         /* find primary and cursor planes for 
>>>> drm_crtc_init_with_planes
>>>> */
>>>>         for (i = 0; scrtc->layers[i]; i++) {
>>>> -               struct sun4i_layer *layer = scrtc->layers[i];
>>>> +               void *layer = scrtc->layers[i];
>>>> +               struct drm_plane *plane =
>>>> scrtc->layer_ops->get_plane(layer);
>>>> 
>>>> -               switch (layer->plane.type) {
>>>> +               switch (plane->type) {
>>>>                 case DRM_PLANE_TYPE_PRIMARY:
>>>> -                       primary = &layer->plane;
>>>> +                       primary = plane;
>>>>                         break;
>>>>                 case DRM_PLANE_TYPE_CURSOR:
>>>> -                       cursor = &layer->plane;
>>>> +                       cursor = plane;
>>>>                         break;
>>>>                 default:
>>>>                         break;
>>>> @@ -190,10 +192,11 @@ struct sun4i_crtc *sun4i_crtc_init(struct
>>>> drm_device *drm,
>>>>         /* Set possible_crtcs to this crtc for overlay planes */
>>>>         for (i = 0; scrtc->layers[i]; i++) {
>>>>                 uint32_t possible_crtcs =
>>>> BIT(drm_crtc_index(&scrtc->crtc));
>>>> -               struct sun4i_layer *layer = scrtc->layers[i];
>>>> +               void *layer = scrtc->layers[i];
>>>> +               struct drm_plane *plane =
>>>> scrtc->layer_ops->get_plane(layer);
>>>> 
>>>> -               if (layer->plane.type == DRM_PLANE_TYPE_OVERLAY)
>>>> -                       layer->plane.possible_crtcs = 
>>>> possible_crtcs;
>>>> +               if (plane->type == DRM_PLANE_TYPE_OVERLAY)
>>>> +                       plane->possible_crtcs = possible_crtcs;
>>>>         }
>>>> 
>>>>         return scrtc;
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.h
>>>> b/drivers/gpu/drm/sun4i/sun4i_crtc.h
>>>> index 230cb8f0d601..a4036ee44cf8 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.h
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.h
>>>> @@ -19,7 +19,8 @@ struct sun4i_crtc {
>>>> 
>>>>         struct sun4i_backend            *backend;
>>>>         struct sun4i_tcon               *tcon;
>>>> -       struct sun4i_layer              **layers;
>>>> +       void                            **layers;
>>>> +       const struct sunxi_layer_ops    *layer_ops;
>>> 
>>> 
>>> I think you should probably take a different approach to abstract the
>>> layer
>>> type. How about creating
>>> 
>>> struct sunxi_layer {
>>>         struct drm_plane plane;
>>> }
>>> 
>>> base and then subclassing that for sun4i and sun8i? By doing this you 
>>> can
>>> avoid
>>> the nasty casting and you can also get rid of the get_plane() hook 
>>> and
>>> layer_ops.
>> 
>> 
>> For the situation that using ** things are easily to get weird.
> 
> That code could be reworked, by initializing the layers directly within
> the crtc init code. If you look at rockchip's drm driver, you'll see
> they do this. There is a good reason to do it this way, as you need
> to first create the primary and cursor layers, pass them in when you
> create the crtc, then initialize any additional layers with the
> possible_crtcs bitmap.

I feel that it's still more proper to offload plane creation code
to *_layers_init function, as:

1. We cannot assume the cursor layer's
existance. In fact currently no code in sun4i-drm (including this
patchset) create a cursor layer.

2. The format of planes heavily depend on the engine type (
sun4i-backend or sun8i-mixer).

3. We should create planes according to the type of engine.
Currently the *_layers_init function is part of engine code (See my
Makefile change).

4. If we do so we will have two codes for plane creating -- one for
primary in sun4i_crtc_init, another for overlays in *_layers_init.

> 
> In our driver we are currently initializing all layers, then going
> back and filling in possible_crtcs for the extra layers.
> 
> And as Maxime and I mentioned in the other thread, we don't really
> need to keep a reference to **layers.

It's correct, layers doesn't need to be kept.

And the struct sunxi_layer refactor also makes sense.

> 
> Regards
> ChenYu
> 
>> 
>>> 
>>> Sean
>>> 
>>> 
>>> 
>>>>  };
>>>> 
>>>>  static inline struct sun4i_crtc *drm_crtc_to_sun4i_crtc(struct 
>>>> drm_crtc
>>>> *crtc)
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c
>>>> b/drivers/gpu/drm/sun4i/sun4i_layer.c
>>>> index f26bde5b9117..bc4a70d6968b 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
>>>> @@ -16,7 +16,9 @@
>>>>  #include <drm/drmP.h>
>>>> 
>>>>  #include "sun4i_backend.h"
>>>> +#include "sun4i_crtc.h"
>>>>  #include "sun4i_layer.h"
>>>> +#include "sunxi_layer.h"
>>>> 
>>>>  struct sun4i_plane_desc {
>>>>                enum drm_plane_type     type;
>>>> @@ -100,6 +102,17 @@ static const struct sun4i_plane_desc
>>>> sun4i_backend_planes[] = {
>>>>         },
>>>>  };
>>>> 
>>>> +static struct drm_plane *sun4i_layer_get_plane(void *layer)
>>>> +{
>>>> +       struct sun4i_layer *sun4i_layer = layer;
>>>> +
>>>> +       return &sun4i_layer->plane;
>>>> +}
>>>> +
>>>> +static const struct sunxi_layer_ops layer_ops = {
>>>> +       .get_plane = sun4i_layer_get_plane,
>>>> +};
>>>> +
>>>>  static struct sun4i_layer *sun4i_layer_init_one(struct drm_device 
>>>> *drm,
>>>>                                                 struct sun4i_backend
>>>> *backend,
>>>>                                                 const struct
>>>> sun4i_plane_desc *plane)
>>>> @@ -129,9 +142,10 @@ static struct sun4i_layer
>>>> *sun4i_layer_init_one(struct drm_device *drm,
>>>>  }
>>>> 
>>>>  struct sun4i_layer **sun4i_layers_init(struct drm_device *drm,
>>>> -                                      struct sun4i_backend 
>>>> *backend)
>>>> +                                      struct sun4i_crtc *crtc)
>>>>  {
>>>>         struct sun4i_layer **layers;
>>>> +       struct sun4i_backend *backend = crtc->backend;
>>>>         int i;
>>>> 
>>>>         layers = devm_kcalloc(drm->dev, 
>>>> ARRAY_SIZE(sun4i_backend_planes)
>>>> + 1,
>>>> @@ -181,5 +195,8 @@ struct sun4i_layer **sun4i_layers_init(struct
>>>> drm_device *drm,
>>>>                 layers[i] = layer;
>>>>         };
>>>> 
>>>> +       /* Assign layer ops to the CRTC */
>>>> +       crtc->layer_ops = &layer_ops;
>>>> +
>>>>         return layers;
>>>>  }
>>>> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h
>>>> b/drivers/gpu/drm/sun4i/sun4i_layer.h
>>>> index 4be1f0919df2..425eea7b9e3b 100644
>>>> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
>>>> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
>>>> @@ -27,6 +27,6 @@ plane_to_sun4i_layer(struct drm_plane *plane)
>>>>  }
>>>> 
>>>>  struct sun4i_layer **sun4i_layers_init(struct drm_device *drm,
>>>> -                                      struct sun4i_backend 
>>>> *backend);
>>>> +                                      struct sun4i_crtc *crtc);
>>>> 
>>>>  #endif /* _SUN4I_LAYER_H_ */
>>>> diff --git a/drivers/gpu/drm/sun4i/sunxi_layer.h
>>>> b/drivers/gpu/drm/sun4i/sunxi_layer.h
>>>> new file mode 100644
>>>> index 000000000000..d8838ec39299
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/sun4i/sunxi_layer.h
>>>> @@ -0,0 +1,17 @@
>>>> +/*
>>>> + * Copyright (C) 2017 Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License as
>>>> + * published by the Free Software Foundation; either version 2 of
>>>> + * the License, or (at your option) any later version.
>>>> + */
>>>> +
>>>> +#ifndef _SUNXI_LAYER_H_
>>>> +#define _SUNXI_LAYER_H_
>>>> +
>>>> +struct sunxi_layer_ops {
>>>> +       struct drm_plane *(*get_plane)(void *layer);
>>>> +};
>>>> +
>>>> +#endif /* _SUNXI_LAYER_H_ */
>>>> --
>>>> 2.12.0
>>>> 
>>>> 
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>> 
>>> 
>> 
>> --
>> You received this message because you are subscribed to the Google 
>> Groups
>> "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send 
>> an
>> email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
>> For more options, visit https://groups.google.com/d/optout.
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Devin Heitmueller @ 2017-04-05 17:02 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Philipp Zabel, Russell King - ARM Linux, Mauro Carvalho Chehab,
	Steve Longerbeam, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
	Hans Verkuil, nick-gcszYUEDH4VrovVCs/uTlw,
	markus.heiser-m1Uo1GnMJf0b1SvskN2V4Q,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
	bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g, Arnd Bergmann,
	sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
	minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
	tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
	jean-christophe.trotin-qxv4g6HH51o,
	horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
	niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
	robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
	andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w, Greg Kroah-Hartman
In-Reply-To: <20170405131725.22c13a1d-ch4gOOMV7nf/PtFMR13I2A@public.gmane.org>

>> For what it's worth, I doubt most of the em28xx designs have the
>> tvp5150 interrupt request line connected in any way.
>
> True. But, on embedded hardware, such line may be connected into the
> SoC. Actually, from the IGEPv3 expansion diagram:
>
>         https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics
>
> The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin
> at OMAP3.
>
> So, on a first glance, it seems possible to use it, instead of polling.

To be clear, I wasn't suggesting that the IRQ request line on the
tvp5150 couldn't be supported in general (for example, for those
embedded targets which have it wired up to a host processor).  I'm
just saying you shouldn't expect it to work on most (perhaps all)
em28xx designs which have the tvp5150.  In fact on some em28xx designs
the pin is used as a GPIO output tied to a mux to control input
selection.  Hence blindly enabling the interrupt request line by
default would do all sorts of bad things.

>> You would likely
>> have to poll the FIFO status register via I2C,
>
> Yes, I considered this option when I wrote the driver. It could work,
> although it would likely have some performance drawback, as the driver
> would need to poll it at least 60 times per second.
>
>> or use the feature to
>> embed the sliced data into as VANC data in the 656 output (as
>> described in sec 3.9 of the tvp5150am1 spec).
>
> True, but the bridge driver would need to handle such data.

Correct.

> I remember I looked on this when I wrote the driver, but I was
> unable to find a way for em28xx to parse (or forward) such
> data packets.

I'm pretty sure it's possible, but I haven't looked at the datasheets
in a number of years and don't recall the details.

Hardware VBI splicing is supported by a number of decoders but it's
rarely used on commodity PCs (the Conexant and NXP decoders support it
as well).  That said, I won't argue there might be some value on
really low end platforms.  All I would ask is that if you do introduce
any such functionality into the tvp5150 driver for some embedded
application that you please not break support for devices such as the
em28xx.

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 01/23] PCI: endpoint: Add EP core layer to enable EP controller and EP functions
From: Bjorn Helgaas @ 2017-04-05 16:52 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: devicetree, Joao Pinto, linux-doc, linux-pci, nsekhar,
	linux-kernel, hch, Bjorn Helgaas, linux-omap, linux-arm-kernel
In-Reply-To: <20170405085243.18123-2-kishon@ti.com>

On Wed, Apr 05, 2017 at 02:22:21PM +0530, Kishon Vijay Abraham I wrote:
> Introduce a new EP core layer in order to support endpoint functions in
> linux kernel. This comprises the EPC library (Endpoint Controller Library)
> and EPF library (Endpoint Function Library). EPC library implements
> functions specific to an endpoint controller and EPF library implements
> functions specific to an endpoint function.
> ...

> +/**
> + * pci_epf_linkup() - Notify the function driver that EPC device has
> + *		      established a connection with the Root Complex.
> + * @epf: the EPF device bound to the EPC device which has established
> + *	 the connection with the host
> + *
> + * Invoke to notify the function driver that EPC device has established
> + * a connection with the Root Complex.
> + */
> +void pci_epf_linkup(struct pci_epf *epf)
> +{
> +	if (!epf->driver)
> +		dev_WARN(&epf->dev, "epf device not bound to driver\n");
> +
> +	epf->driver->ops->linkup(epf);

I don't understand what's going on here.  We warn if epf->driver is
NULL, but the next thing we do is dereference it.

For NULL pointers that are symptoms of Linux defects, I usually prefer
not to check at all so that a dereference generates an oops and we can
debug the problem.  For NULL pointers caused by user error, we would
generally return an error that percolates up to the user.

I haven't competely wrapped my head around this endpoint support, but
I assume a NULL pointer here would be caused by user error, not
necessarily a Linux defect.  So why would we dereference a NULL
pointer?  And what happens when we do?  Is this just going to oops an
embedded Linux running inside the endpoint?  Is that the correct
behavior?

Bjorn

^ permalink raw reply

* Re: [PATCH 4/4] iio: dac: stm32: add support for waveform generator
From: Fabrice Gasnier @ 2017-04-05 16:46 UTC (permalink / raw)
  To: Jonathan Cameron, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: mark.rutland, benjamin.gaignard, lars, alexandre.torgue,
	Hennerich, Michael, linux-iio, pmeerw, amelie.delaunay,
	mcoquelin.stm32, knaack.h, linus.walleij, benjamin.gaignard
In-Reply-To: <e6a02296-fa72-50e8-d5e8-25bb5d496085@kernel.org>

JonathanOn 04/02/2017 02:19 PM, Jonathan Cameron wrote:
> On 31/03/17 12:45, Fabrice Gasnier wrote:
>> STM32 DAC has built-in noise or triangle waveform generator.
>> Waveform generator requires trigger to be configured.
>> - "wave" extended attribute selects noise or triangle.
>> - "mamp" extended attribute selects either LFSR (linear feedback
>>   shift register) mask for noise waveform, OR triangle amplitude.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> 
> Looks like AN3126 is the relevant doc.
> (a quick note from this relevant to earlier patches- doc says
> 1-3 channels - perhaps build that from the start with that
> possibility in mind).
Hi Jonathan,

Just to clarify this, some products like STM32F334xx have 3 channels,
yes. Several STM32 DAC IPs (& so registers) are instantiated: DAC1 have
two outputs (dac1_out1 & dac1_out2), DAC2 have one output (e.g.
dac2_out1). Driver can be instantiated several times. Is it ok ?

> 
> As you probably know, this wanders into a large chunk of 'poorly'
> defined ABI within IIO as it stands.
> 
> Note there are a number of waveform generators still in staging.
> Not a lot of movement on getting them out of staging unfortunately
> (so far!)
> 
> However, let us keep those drivers in mind as we work on ABI and
> I definitely want some input from someone at Analog. 
> Lars, who is best for this? I see at least some of these were
> originally Michael's work.
> 
> They do have partial docs under
> drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> I'll highlight thoughts from there as I look through this...

Thanks for pointing this out.

> 
> 
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 |  32 ++++++
>>  drivers/iio/dac/stm32-dac.c                       | 124 ++++++++++++++++++++++
>>  2 files changed, 156 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> new file mode 100644
>> index 0000000..c2432e1
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-stm32
>> @@ -0,0 +1,32 @@
>> +What:		/sys/bus/iio/devices/iio:deviceX/wave
>> +What:		/sys/bus/iio/devices/iio:deviceX/wave_available
> Needs to be channel associated. Whilst in your case you have basically
> a pair of single channel devices, in more general case, it's not usual
> to have multiple parallel waveform generators clocked together.
> 
> Old ABI is:
> What:		/sys/bus/iio/devices/.../out_altvoltageX_outY_wavetype etc
> 
I'll rework this in V2.
> 
>> +KernelVersion:	4.12
>> +Contact:	fabrice.gasnier@st.com
>> +Description:
>> +		List and/or select waveform generation provided by STM32 DAC:
>> +		- "none": (default) means normal DAC operations
> none kind of hints at nothing coming out.  Perhaps 'flat' would be closer?
> i.e. only changes when someone tells it to.
> 
>> +		- "noise": select noise waveform
>> +		- "triangle": select triangle waveform
>> +		Note: when waveform generator is used, writing _raw sysfs entry
>> +		adds a DC offset to generated waveform. Reading it reports
>> +		current output value.
> Interesting.  This gets fiddly but one option would be to describe the whole
> device as a dds.
> 
> Then we have flat type above, combined with an _offset.

I'll update from 'none' to 'flat' in V2, and use _offset.
> 
>> +
>> +What:		/sys/bus/iio/devices/iio:deviceX/mamp
>> +What:		/sys/bus/iio/devices/iio:deviceX/mamp_available
>> +KernelVersion:	4.12
>> +Contact:	fabrice.gasnier@st.com
>> +Description:
>> +		List and select mask/amplitude used for noise/triangle waveform
>> +		generator, which are:
>> +		- "0": unmask bit 0 of LFSR / triangle amplitude equal to 1
>> +		- "1": unmask bit [1:0] of LFSR / triangle amplitude equal to 3
>> +		- "2": unmask bit [2:0] of LFSR / triangle amplitude equal to 7
>> +		- "3": unmask bit [3:0] of LFSR / triangle amplitude equal to 15
>> +		- "4": unmask bit [4:0] of LFSR / triangle amplitude equal to 31
>> +		- "5": unmask bit [5:0] of LFSR / triangle amplitude equal to 63
>> +		- "6": unmask bit [6:0] of LFSR / triangle amplitude equal to 127
>> +		- "7": unmask bit [7:0] of LFSR / triangle amplitude equal to 255
>> +		- "8": unmask bit [8:0] of LFSR / triangle amplitude equal to 511
>> +		- "9": unmask bit [9:0] of LFSR / triangle amplitude equal to 1023
>> +		- "10": unmask bit [10:0] of LFSR / triangle amplitude equal to 2047
>> +		- "11": unmask bit [11:0] of LFSR / triangle amplitude equal to 4095
> I don't fully understand what is going on here - so I'm guessing somewhat.
Sorry for this, this is basically amplitude.

I think best is to rename above to something like 'amplitude' and
'amplitude_available'.

I'll rework this in V2.
> 
> 
> Let us try describing these generically.  If we define standard 'forms' of each
> waveform type.  Say a 0 to 1 V peak to peak, then we could use _scale to control
> this nicely.
> 
>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>> index 62e43e9..d7dda78 100644
>> --- a/drivers/iio/dac/stm32-dac.c
>> +++ b/drivers/iio/dac/stm32-dac.c
>> @@ -41,10 +41,14 @@
>>  /**
>>   * struct stm32_dac - private data of DAC driver
>>   * @common:		reference to DAC common data
>> + * @wave:		waveform generator
>> + * @mamp:		waveform mask/amplitude
>>   * @swtrig:		Using software trigger
>>   */
>>  struct stm32_dac {
>>  	struct stm32_dac_common *common;
>> +	u32 wave;
>> +	u32 mamp;
>>  	bool swtrig;
>>  };
>>  
>> @@ -157,6 +161,24 @@ static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
>>  	return !!en;
>>  }
>>  
>> +static int stm32_dac_wavegen(struct stm32_dac *dac, int channel)
>> +{
>> +	struct regmap *regmap = dac->common->regmap;
>> +	u32 mask, val;
>> +
>> +	if (channel == STM32_DAC_CHANNEL_1) {
>> +		val = FIELD_PREP(STM32_DAC_CR_WAVE1, dac->wave) |
>> +			FIELD_PREP(STM32_DAC_CR_MAMP1, dac->mamp);
>> +		mask = STM32_DAC_CR_WAVE1 | STM32_DAC_CR_MAMP1;
>> +	} else {
>> +		val = FIELD_PREP(STM32_DAC_CR_WAVE2, dac->wave) |
>> +			FIELD_PREP(STM32_DAC_CR_MAMP2, dac->mamp);
>> +		mask = STM32_DAC_CR_WAVE2 | STM32_DAC_CR_MAMP2;
>> +	}
>> +
>> +	return regmap_update_bits(regmap, STM32_DAC_CR, mask, val);
>> +}
>> +
>>  static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
>>  {
>>  	struct stm32_dac *dac = iio_priv(indio_dev);
>> @@ -164,6 +186,17 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
>>  		STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
>>  	int ret;
>>  
>> +	if (dac->wave && !indio_dev->trig) {
>> +		dev_err(&indio_dev->dev, "Wavegen requires a trigger\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = stm32_dac_wavegen(dac, channel);
>> +	if (ret < 0) {
>> +		dev_err(&indio_dev->dev, "Wavegen setup failed\n");
>> +		return ret;
>> +	}
>> +
>>  	ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
>>  	if (ret < 0) {
>>  		dev_err(&indio_dev->dev, "Trigger setup failed\n");
>> @@ -291,6 +324,96 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +/* waveform generator wave selection */
>> +static const char * const stm32_dac_wave_desc[] = {
>> +	"none",
>> +	"noise",
>> +	"triangle",
>> +};
>> +
>> +static int stm32_dac_set_wave(struct iio_dev *indio_dev,
>> +			      const struct iio_chan_spec *chan,
>> +			      unsigned int type)
>> +{
>> +	struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +	if (stm32_dac_is_enabled(dac, chan->channel))
>> +		return -EBUSY;
>> +	dac->wave = type;
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_dac_get_wave(struct iio_dev *indio_dev,
>> +			      const struct iio_chan_spec *chan)
>> +{
>> +	struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +	return dac->wave;
>> +}
>> +
>> +static const struct iio_enum stm32_dac_wave_enum = {
>> +	.items = stm32_dac_wave_desc,
>> +	.num_items = ARRAY_SIZE(stm32_dac_wave_desc),
>> +	.get = stm32_dac_get_wave,
>> +	.set = stm32_dac_set_wave,
>> +};
>> +
>> +/*
>> + * waveform generator mask/amplitude selection:
>> + * - noise: LFSR mask (linear feedback shift register, umasks bit 0, [1:0]...)
>> + * - triangle: amplitude (equal to 1, 3, 5, 7... 4095)
>> + */
>> +static const char * const stm32_dac_mamp_desc[] = {
>> +	"0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11",
>> +};
>> +
>> +static int stm32_dac_set_mamp(struct iio_dev *indio_dev,
>> +			      const struct iio_chan_spec *chan,
>> +			      unsigned int type)
>> +{
>> +	struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +	if (stm32_dac_is_enabled(dac, chan->channel))
>> +		return -EBUSY;
>> +	dac->mamp = type;
>> +
>> +	return 0;
>> +}
>> +
>> +static int  stm32_dac_get_mamp(struct iio_dev *indio_dev,
>> +			       const struct iio_chan_spec *chan)
>> +{
>> +	struct stm32_dac *dac = iio_priv(indio_dev);
>> +
>> +	return dac->mamp;
>> +}
>> +
>> +static const struct iio_enum stm32_dac_mamp_enum = {
>> +	.items = stm32_dac_mamp_desc,
>> +	.num_items = ARRAY_SIZE(stm32_dac_mamp_desc),
>> +	.get = stm32_dac_get_mamp,
>> +	.set = stm32_dac_set_mamp,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info stm32_dac_ext_info[] = {
>> +	IIO_ENUM("wave", IIO_SHARED_BY_ALL, &stm32_dac_wave_enum),
>> +	{
>> +		.name = "wave_available",
>> +		.shared = IIO_SHARED_BY_ALL,
>> +		.read = iio_enum_available_read,
>> +		.private = (uintptr_t)&stm32_dac_wave_enum,
>> +	},
>> +	IIO_ENUM("mamp", IIO_SHARED_BY_ALL, &stm32_dac_mamp_enum),
>> +	{
>> +		.name = "mamp_available",
>> +		.shared = IIO_SHARED_BY_ALL,
>> +		.read = iio_enum_available_read,
>> +		.private = (uintptr_t)&stm32_dac_mamp_enum,
>> +	},
>> +	{},
>> +};
>> +
>>  #define STM32_DAC_CHANNEL(chan, name) {		\
>>  	.type = IIO_VOLTAGE,			\
>>  	.indexed = 1,				\
>> @@ -306,6 +429,7 @@ static int stm32_dac_debugfs_reg_access(struct iio_dev *indio_dev,
>>  		.storagebits = 16,		\
>>  	},					\
>>  	.datasheet_name = name,			\
>> +	.ext_info = stm32_dac_ext_info		\
>>  }
>>  
>>  static const struct iio_chan_spec stm32_dac_channels[] = {
>>
> 

^ permalink raw reply

* Re: [PATCH 3/4] iio: dac: stm32: add support for trigger events
From: Fabrice Gasnier @ 2017-04-05 16:44 UTC (permalink / raw)
  To: Jonathan Cameron, linux, robh+dt, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: mark.rutland, benjamin.gaignard, lars, alexandre.torgue,
	linux-iio, pmeerw, amelie.delaunay, mcoquelin.stm32, knaack.h,
	linus.walleij, benjamin.gaignard
In-Reply-To: <7dba5b17-5f82-e68d-3c06-afe4a4e478e7@kernel.org>

On 04/02/2017 02:21 PM, Jonathan Cameron wrote:
> On 02/04/17 12:45, Jonathan Cameron wrote:
>> On 31/03/17 12:45, Fabrice Gasnier wrote:
>>> STM32 DAC supports triggers to synchronize conversions. When trigger
>>> occurs, data is transferred from DHR (data holding register) to DOR
>>> (data output register) so output voltage is updated.
>>> Both hardware and software triggers are supported.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Hmm. This is a somewhat different use of triggered event from normal...
>>
Waveform generator in STM32 DAC requires a trigger to increment /
decrement internal counter in case of triangle generator. Noise
generator is a bit different, but same trigger usage applies. I agree
this is unusual.
Is it acceptable to use event trigger for this use ?

>> What you have here is rather closer to the output buffers stuff that Analog
>> have in their tree which hasn't made it upstream yet.
>> To that end I'll want Lars to have a look at this...  I've completely
>> lost track of where they are with this.
>> Perhaps Lars can give us a quick update?
>>
>> If that was in place (or what I have in my head was true anyway),
>> it would look like the reverse of the triggered buffer input devices.
>> You'd be able to write to a software buffer and it would clock them
>> out as the trigger fires (here I think it would have to keep updating
>> the DHR whenever the trigger occurs).

Hmm.. for waveform generator mode, there is no need for data buffer. DAC
generate samples itself, using trigger. But, i agree it would be nice
for playing data samples (write DHR registers, or dma), yes.

>>
>> Even if it's not there, we aren't necessarily looking at terribly big job
>> to implement it in the core and that would make this handling a lot more
>> 'standard' and consistent.
> 
> Having tracked down some limited docs (AN3126 - Audio and waveform
> generation using the DAC in STM32 microcontrollers) the fact this
> can also be driven by DMA definitely argues in favour of working with
> Analog on getting the output buffers support upstream.
> 
> *crosses fingers people have the time!*

Hopefully this can happen.

For the time being, I'll propose a similar patch in V2. I found out this
patch is missing a clear path to (re-)assign trigger, once set by
userland. Also, driver never gets informed in case trigger gets changed
or removed, without re-enabling it:
e.g. like echo "" > trigger/current_trigger
I'll propose a small change. Hope you agree with this approach.

Thanks,
Fabrice

>>
>> Jonathan
>>
>>> ---
>>>  drivers/iio/dac/Kconfig          |   3 +
>>>  drivers/iio/dac/stm32-dac-core.h |  12 ++++
>>>  drivers/iio/dac/stm32-dac.c      | 124 ++++++++++++++++++++++++++++++++++++++-
>>>  3 files changed, 136 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
>>> index 7198648..786c38b 100644
>>> --- a/drivers/iio/dac/Kconfig
>>> +++ b/drivers/iio/dac/Kconfig
>>> @@ -278,6 +278,9 @@ config STM32_DAC
>>>  	tristate "STMicroelectronics STM32 DAC"
>>>  	depends on (ARCH_STM32 && OF) || COMPILE_TEST
>>>  	depends on REGULATOR
>>> +	select IIO_TRIGGERED_EVENT
>>> +	select IIO_STM32_TIMER_TRIGGER
>>> +	select MFD_STM32_TIMERS
>>>  	select STM32_DAC_CORE
>>>  	help
>>>  	  Say yes here to build support for STMicroelectronics STM32 Digital
>>> diff --git a/drivers/iio/dac/stm32-dac-core.h b/drivers/iio/dac/stm32-dac-core.h
>>> index d3099f7..3bf211c 100644
>>> --- a/drivers/iio/dac/stm32-dac-core.h
>>> +++ b/drivers/iio/dac/stm32-dac-core.h
>>> @@ -26,6 +26,7 @@
>>>  
>>>  /* STM32 DAC registers */
>>>  #define STM32_DAC_CR		0x00
>>> +#define STM32_DAC_SWTRIGR	0x04
>>>  #define STM32_DAC_DHR12R1	0x08
>>>  #define STM32_DAC_DHR12R2	0x14
>>>  #define STM32_DAC_DOR1		0x2C
>>> @@ -33,8 +34,19 @@
>>>  
>>>  /* STM32_DAC_CR bit fields */
>>>  #define STM32_DAC_CR_EN1		BIT(0)
>>> +#define STM32H7_DAC_CR_TEN1		BIT(1)
>>> +#define STM32H7_DAC_CR_TSEL1_SHIFT	2
>>> +#define STM32H7_DAC_CR_TSEL1		GENMASK(5, 2)
>>> +#define STM32_DAC_CR_WAVE1		GENMASK(7, 6)
>>> +#define STM32_DAC_CR_MAMP1		GENMASK(11, 8)
>>>  #define STM32H7_DAC_CR_HFSEL		BIT(15)
>>>  #define STM32_DAC_CR_EN2		BIT(16)
>>> +#define STM32_DAC_CR_WAVE2		GENMASK(23, 22)
>>> +#define STM32_DAC_CR_MAMP2		GENMASK(27, 24)
>>> +
>>> +/* STM32_DAC_SWTRIGR bit fields */
>>> +#define STM32_DAC_SWTRIGR_SWTRIG1	BIT(0)
>>> +#define STM32_DAC_SWTRIGR_SWTRIG2	BIT(1)
>>>  
>>>  /**
>>>   * struct stm32_dac_common - stm32 DAC driver common data (for all instances)
>>> diff --git a/drivers/iio/dac/stm32-dac.c b/drivers/iio/dac/stm32-dac.c
>>> index ee9711d..62e43e9 100644
>>> --- a/drivers/iio/dac/stm32-dac.c
>>> +++ b/drivers/iio/dac/stm32-dac.c
>>> @@ -23,6 +23,10 @@
>>>  #include <linux/bitfield.h>
>>>  #include <linux/delay.h>
>>>  #include <linux/iio/iio.h>
>>> +#include <linux/iio/timer/stm32-timer-trigger.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_event.h>
>>>  #include <linux/kernel.h>
>>>  #include <linux/module.h>
>>>  #include <linux/platform_device.h>
>>> @@ -31,15 +35,112 @@
>>>  
>>>  #define STM32_DAC_CHANNEL_1		1
>>>  #define STM32_DAC_CHANNEL_2		2
>>> +/* channel2 shift */
>>> +#define STM32_DAC_CHAN2_SHIFT		16
>>>  
>>>  /**
>>>   * struct stm32_dac - private data of DAC driver
>>>   * @common:		reference to DAC common data
>>> + * @swtrig:		Using software trigger
>>>   */
>>>  struct stm32_dac {
>>>  	struct stm32_dac_common *common;
>>> +	bool swtrig;
>>>  };
>>>  
>>> +/**
>>> + * struct stm32_dac_trig_info - DAC trigger info
>>> + * @name: name of the trigger, corresponding to its source
>>> + * @tsel: trigger selection, value to be configured in DAC_CR.TSELx
>>> + */
>>> +struct stm32_dac_trig_info {
>>> +	const char *name;
>>> +	u32 tsel;
>>> +};
>>> +
>>> +static const struct stm32_dac_trig_info stm32h7_dac_trinfo[] = {
>>> +	{ "swtrig", 0 },
>>> +	{ TIM1_TRGO, 1 },
>>> +	{ TIM2_TRGO, 2 },
>>> +	{ TIM4_TRGO, 3 },
>>> +	{ TIM5_TRGO, 4 },
>>> +	{ TIM6_TRGO, 5 },
>>> +	{ TIM7_TRGO, 6 },
>>> +	{ TIM8_TRGO, 7 },
>>> +	{},
>>> +};
>>> +
>>> +static irqreturn_t stm32_dac_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *indio_dev = pf->indio_dev;
>>> +	struct stm32_dac *dac = iio_priv(indio_dev);
>>> +	int channel = indio_dev->channels[0].channel;
>>> +
>>> +	/* Using software trigger? Then, trigger it now */
>>> +	if (dac->swtrig) {
>>> +		u32 swtrig;
>>> +
>>> +		if (channel == STM32_DAC_CHANNEL_1)
>>> +			swtrig = STM32_DAC_SWTRIGR_SWTRIG1;
>>> +		else
>>> +			swtrig = STM32_DAC_SWTRIGR_SWTRIG2;
>>> +		regmap_update_bits(dac->common->regmap, STM32_DAC_SWTRIGR,
>>> +				   swtrig, swtrig);
>>> +	}
>>> +
>>> +	iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>> +static unsigned int stm32_dac_get_trig_tsel(struct stm32_dac *dac,
>>> +					    struct iio_trigger *trig)
>>> +{
>>> +	unsigned int i;
>>> +
>>> +	/* skip 1st trigger that should be swtrig */
>>> +	for (i = 1; stm32h7_dac_trinfo[i].name; i++) {
>>> +		/*
>>> +		 * Checking both stm32 timer trigger type and trig name
>>> +		 * should be safe against arbitrary trigger names.
>>> +		 */
>>> +		if (is_stm32_timer_trigger(trig) &&
>>> +		    !strcmp(stm32h7_dac_trinfo[i].name, trig->name)) {
>>> +			return stm32h7_dac_trinfo[i].tsel;
>>> +		}
>>> +	}
>>> +
>>> +	/* When no trigger has been found, default to software trigger */
>>> +	dac->swtrig = true;
>>> +
>>> +	return stm32h7_dac_trinfo[0].tsel;
>>> +}
>>> +
>>> +static int stm32_dac_set_trig(struct stm32_dac *dac, struct iio_trigger *trig,
>>> +			      int channel)
>>> +{
>>> +	struct iio_dev *indio_dev = iio_priv_to_dev(dac);
>>> +	u32 shift = channel == STM32_DAC_CHANNEL_1 ? 0 : STM32_DAC_CHAN2_SHIFT;
>>> +	u32 val = 0, tsel;
>>> +	u32 msk = (STM32H7_DAC_CR_TEN1 | STM32H7_DAC_CR_TSEL1) << shift;
>>> +
>>> +	dac->swtrig = false;
>>> +	if (trig) {
>>> +		/* select & enable trigger (tsel / ten) */
>>> +		tsel = stm32_dac_get_trig_tsel(dac, trig);
>>> +		val = tsel << STM32H7_DAC_CR_TSEL1_SHIFT;
>>> +		val = (val | STM32H7_DAC_CR_TEN1) << shift;
>>> +	}
>>> +
>>> +	if (trig)
>>> +		dev_dbg(&indio_dev->dev, "enable trigger: %s\n", trig->name);
>>> +	else
>>> +		dev_dbg(&indio_dev->dev, "disable trigger\n");
>>> +
>>> +	return regmap_update_bits(dac->common->regmap, STM32_DAC_CR, msk, val);
>>> +}
>>> +
>>>  static int stm32_dac_is_enabled(struct stm32_dac *dac, int channel)
>>>  {
>>>  	u32 en, val;
>>> @@ -63,9 +164,16 @@ static int stm32_dac_enable(struct iio_dev *indio_dev, int channel)
>>>  		STM32_DAC_CR_EN1 : STM32_DAC_CR_EN2;
>>>  	int ret;
>>>  
>>> +	ret = stm32_dac_set_trig(dac, indio_dev->trig, channel);
>>> +	if (ret < 0) {
>>> +		dev_err(&indio_dev->dev, "Trigger setup failed\n");
>>> +		return ret;
>>> +	}
>>> +
>>>  	ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, en);
>>>  	if (ret < 0) {
>>>  		dev_err(&indio_dev->dev, "Enable failed\n");
>>> +		stm32_dac_set_trig(dac, NULL, channel);
>>>  		return ret;
>>>  	}
>>>  
>>> @@ -88,10 +196,12 @@ static int stm32_dac_disable(struct iio_dev *indio_dev, int channel)
>>>  	int ret;
>>>  
>>>  	ret = regmap_update_bits(dac->common->regmap, STM32_DAC_CR, en, 0);
>>> -	if (ret)
>>> +	if (ret) {
>>>  		dev_err(&indio_dev->dev, "Disable failed\n");
>>> +		return ret;
>>> +	}
>>>  
>>> -	return ret;
>>> +	return stm32_dac_set_trig(dac, NULL, channel);
>>>  }
>>>  
>>>  static int stm32_dac_get_value(struct stm32_dac *dac, int channel, int *val)
>>> @@ -258,10 +368,17 @@ static int stm32_dac_probe(struct platform_device *pdev)
>>>  	if (ret < 0)
>>>  		return ret;
>>>  
>>> -	ret = iio_device_register(indio_dev);
>>> +	ret = iio_triggered_event_setup(indio_dev, NULL,
>>> +					stm32_dac_trigger_handler);
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret) {
>>> +		iio_triggered_event_cleanup(indio_dev);
>>> +		return ret;
>>> +	}
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -269,6 +386,7 @@ static int stm32_dac_remove(struct platform_device *pdev)
>>>  {
>>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>>  
>>> +	iio_triggered_event_cleanup(indio_dev);
>>>  	iio_device_unregister(indio_dev);
>>>  
>>>  	return 0;
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

^ permalink raw reply

* Re: [PATCH v5 22/23] drivers/fsi: Add hub master support
From: Randy Dunlap @ 2017-04-05 16:36 UTC (permalink / raw)
  To: Christopher Bostic, robh+dt, mark.rutland, linux, rostedt, mingo,
	gregkh, devicetree, linux-arm-kernel
  Cc: andrew, alistair, linux-kernel, joel, benh, Jeremy Kerr
In-Reply-To: <20170405020607.79939-23-cbostic@linux.vnet.ibm.com>

On 04/04/17 19:06, Christopher Bostic wrote:
> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
> 
> Add an engine driver to expose a "hub" FSI master - which has a set of
> control registers in the engine address space, and uses a chunk of the
> slave address space for actual FSI communication.
> 
> Additional changes from Jeremy Kerr <jk@ozlabs.org>.
> 
> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/fsi/Kconfig          |   9 ++
>  drivers/fsi/Makefile         |   1 +
>  drivers/fsi/fsi-master-hub.c | 327 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 337 insertions(+)
>  create mode 100644 drivers/fsi/fsi-master-hub.c
> 
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 0fa265c..e1156b4 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -18,6 +18,15 @@ config FSI_MASTER_GPIO
>  	---help---
>  	This option enables a FSI master driver using GPIO lines.
>  
> +config FSI_MASTER_HUB
> +	tristate "FSI hub master"
> +	depends on FSI

redundant again.

> +	---help---
> +	This option enables a FSI hub master driver.  Hub is a type of FSI
> +	master that is connected to the upstream master via a slave.  Hubs
> +	allow chaining of FSI links to an arbitrary depth.  This allows for
> +	a high target device fanout.
> +
>  config FSI_SCOM
>  	tristate "SCOM FSI client device driver"
>  	depends on FSI


-- 
~Randy

^ permalink raw reply

* Re: [PATCH v5 21/23] drivers/fsi: Add SCOM FSI client device driver
From: Randy Dunlap @ 2017-04-05 16:35 UTC (permalink / raw)
  To: Christopher Bostic, robh+dt, mark.rutland, linux, rostedt, mingo,
	gregkh, devicetree, linux-arm-kernel
  Cc: andrew, alistair, linux-kernel, joel, Edward A . James, benh,
	Jeremy Kerr
In-Reply-To: <20170405020607.79939-22-cbostic@linux.vnet.ibm.com>

On 04/04/17 19:06, Christopher Bostic wrote:
> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
> 
> Create a simple SCOM engine device driver that reads and writes
> its control registers via an FSI bus.
> 
> Includes changes from Edward A. James <eajames@us.ibm.com>.
> 
> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> ---
>  drivers/fsi/Kconfig    |   6 ++
>  drivers/fsi/Makefile   |   1 +
>  drivers/fsi/fsi-scom.c | 263 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 270 insertions(+)
>  create mode 100644 drivers/fsi/fsi-scom.c
> 
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 9cf8345..0fa265c 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -18,6 +18,12 @@ config FSI_MASTER_GPIO
>  	---help---
>  	This option enables a FSI master driver using GPIO lines.
>  
> +config FSI_SCOM
> +	tristate "SCOM FSI client device driver"
> +	depends on FSI

depends on FSI is redundant.

> +	---help---
> +	This option enables an FSI based SCOM device driver.
> +
>  endif
>  


-- 
~Randy

^ permalink raw reply

* Re: [PATCH v5 19/23] drivers/fsi: Add GPIO based FSI master
From: Randy Dunlap @ 2017-04-05 16:35 UTC (permalink / raw)
  To: Christopher Bostic, robh+dt, mark.rutland, linux, rostedt, mingo,
	gregkh, devicetree, linux-arm-kernel
  Cc: andrew, alistair, linux-kernel, joel, Edward A . James, benh,
	Jeremy Kerr
In-Reply-To: <20170405020607.79939-20-cbostic@linux.vnet.ibm.com>

On 04/04/17 19:06, Christopher Bostic wrote:
> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
> 
> Implement a FSI master using GPIO.  Will generate FSI protocol for
> read and write commands to particular addresses.  Sends master command
> and waits for and decodes a slave response.
> 
> Includes changes from Edward A. James <eajames@us.ibm.com> and Jeremy
> Kerr <jk@ozlabs.org>.
> 
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  drivers/fsi/Kconfig           |  11 +
>  drivers/fsi/Makefile          |   1 +
>  drivers/fsi/fsi-master-gpio.c | 610 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 622 insertions(+)
>  create mode 100644 drivers/fsi/fsi-master-gpio.c
> 
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 04c1a0e..9cf8345 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -9,4 +9,15 @@ config FSI
>  	---help---
>  	  FSI - the FRU Support Interface - is a simple bus for low-level
>  	  access to POWER-based hardware.
> +
> +if FSI
> +
> +config FSI_MASTER_GPIO
> +	tristate "GPIO-based FSI master"
> +	depends on FSI && GPIOLIB

depends on FSI is redundant since "if FSI" does the same thing.

> +	---help---
> +	This option enables a FSI master driver using GPIO lines.
> +
> +endif
> +


-- 
~Randy

^ permalink raw reply

* Re: [PATCH 0/2] mtd: spi-nor: add stm32 qspi driver
From: Ludovic BARRE @ 2017-04-05 16:20 UTC (permalink / raw)
  To: Marek Vasut, Cyrille Pitchen, Cyrille Pitchen
  Cc: Boris Brezillon, Alexandre Torgue, devicetree, Richard Weinberger,
	linux-kernel, Rob Herring, linux-mtd, Brian Norris,
	David Woodhouse
In-Reply-To: <e04fa293-63d4-7a7e-a998-37aab31167e5@gmail.com>

hi Cyrille, Marek

I've re-based and tested my patchset onto

"mtd: spi-nor: introduce more SPI protocols and the Dual Transfer Mode"

So I can deliver my patchset before or after Cyrille patchset

How do you wish process? what version do you want for the v3?


BR

Ludo

On 03/30/2017 12:15 PM, Marek Vasut wrote:
> On 03/30/2017 09:31 AM, Ludovic BARRE wrote:
>> hi Cyrille
>>
>> I see your patch series
>>
>> [PATCH v5 0/6] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories
>>
>> No problem, I rebase my V2 onto your patch
> I still didn't review that, so it might take a bit until it hits
> mainline. I think the stm32 stuff looks pretty OK, so we can take that
> before the SFDP stuff, no?
>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply

* Re: [RFC] [media] imx: assume MEDIA_ENT_F_ATV_DECODER entities output video on pad 1
From: Mauro Carvalho Chehab @ 2017-04-05 16:17 UTC (permalink / raw)
  To: Devin Heitmueller
  Cc: mark.rutland, andrew-ct.chen, minghsiu.tsai,
	sakari.ailus@linux.intel.com, nick, songjun.wu, Hans Verkuil,
	Steve Longerbeam, Pavel Machek, robert.jarzmik, devel,
	markus.heiser, laurent.pinchart+renesas, shuah,
	Russell King - ARM Linux, geert, Steve Longerbeam,
	Linux Media Mailing List, devicetree, Philipp Zabel,
	Arnd Bergmann, Mauro Carvalho Chehab, bparrot, robh+dt,
	horms+renesas, tiffany.lin, linux-arm-kernel
In-Reply-To: <CAGoCfizXdDV_Eo1NSOAb+-wrC7F47iFQKyP8-wiJMpb-nsYArA@mail.gmail.com>

Em Wed, 5 Apr 2017 11:39:06 -0400
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:

> > Currently, the driver doesn't support (2), because, at the time
> > I wrote the driver, I didn't find a way to read the interrupts generated
> > by tvp5150 at em28xx[1], due to the lack of em28xx documentation,
> > but adding support for it shoudn't be hard. I may eventually do it
> > when I have some time to play with my ISEE hardware.  
> 
> For what it's worth, I doubt most of the em28xx designs have the
> tvp5150 interrupt request line connected in any way.

True. But, on embedded hardware, such line may be connected into the
SoC. Actually, from the IGEPv3 expansion diagram:

	https://www.isee.biz/support/downloads/item/igepv2-expansion-rc-schematics

The INT line is connected to CAM_IRQ. That's connected to GPIO_154 pin
at OMAP3.

So, on a first glance, it seems possible to use it, instead of polling.

> You would likely
> have to poll the FIFO status register via I2C,

Yes, I considered this option when I wrote the driver. It could work, 
although it would likely have some performance drawback, as the driver
would need to poll it at least 60 times per second.

> or use the feature to
> embed the sliced data into as VANC data in the 656 output (as
> described in sec 3.9 of the tvp5150am1 spec).

True, but the bridge driver would need to handle such data. 

I remember I looked on this when I wrote the driver, but I was
unable to find a way for em28xx to parse (or forward) such
data packets.

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH 1/3] dt: bindings: add pmu0 regs for USB PHYs on Allwinner H3/V3s/A64
From: Chen-Yu Tsai @ 2017-04-05 16:13 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Icenowy Zheng, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi,
	Icenowy Zheng
In-Reply-To: <1524732c-b72f-61ce-8ae7-1d764f85c903-l0cyMroinI0@public.gmane.org>

On Wed, Apr 5, 2017 at 8:58 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
>
>
> On Wednesday 05 April 2017 06:20 PM, Icenowy Zheng wrote:
>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>
>> Allwinner H3/V3s/A64 SoCs have a special USB PHY0 that can route to two
>> controllers: one is MUSB and the other is a EHCI/OHCI pair.
>>
>> When it's routed to EHCI/OHCI pair, it will needs a "pmu0" regs to
>> tweak, like other EHCI/OHCI pairs in Allwinner SoCs.
>>
>> Add this to the binding of USB PHYs on Allwinner H3/V3s/A64.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>> ---
>>
>> Kishon, could you push this to 4.11?
>
> Is this for the patch titled "phy: sun4i-usb: add support for V3s USB PHY" that
> was added during the last merge window.

Yes. In fact, as the description suggests, this also applies retroactively to
H3 and A64, which were added in the following commits:

4.9  732e35da7b4a ("dt: bindings: add bindings for Allwinner A64 usb phy")
4.5  626a630e003c ("phy-sun4i-usb: Add support for the host usb-phys found
                    on the H3 SoC")

Though this was missed at the time the binding was added.

ChenYu

> this patch looks simpler enough to be merged in this -rc cycle. However it
> depends on Greg KH.
>
> Thanks
> Kishon
>
>>
>>  Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> index e42334258185..005bc22938ff 100644
>> --- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
>> @@ -15,6 +15,7 @@ Required properties:
>>  - reg : a list of offset + length pairs
>>  - reg-names :
>>    * "phy_ctrl"
>> +  * "pmu0" for H3, V3s and A64
>>    * "pmu1"
>>    * "pmu2" for sun4i, sun6i or sun7i
>>  - #phy-cells : from the generic phy bindings, must be 1
>>

^ permalink raw reply

* Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port
From: Ralph Sennhauser @ 2017-04-05 16:08 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Mark Rutland, devicetree, Jason Cooper, linux-kernel,
	Russell King, Rob Herring, Gregory Clement, linux-arm-kernel,
	Sebastian Hesselbarth
In-Reply-To: <20170405155032.GL13449@lunn.ch>

Hi Andrew,

On Wed, 5 Apr 2017 17:50:32 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > In fact "internet" label on the case is uppercase, would this matter
> > for you for new bindings, or would you still use the lowercase
> > version?  
> 
> I would use lower case, just to fix with the general convention that
> interface names are lower case.

Thanks

> 
> > > I would say, this is now too late. Changing an interface name will
> > > break configuration scripts. We are stuck with it.
> > >   
> > 
> > If it weren't for commit cb4f71c42988 that would have been obvious
> > for me as well.  
> 
> Yes, that was not nice. But it was also very earlier in the life of
> 38x, so it just affected a few developers with reference boards, not
> real products out in the wild, as far as i remember.
> 
>      Andrew

The mentioned commit bit me amongst others, see
https://lkml.org/lkml/2016/8/21/62

In fact I expect the impact to be much lower this time around, before
4.10 DSA was no contender for swconfig (out of tree switch driver as
used by OpenWrt and others). On Mamba ~200Mbit was top speed. Now they
perform roughly the same.


Thanks
Ralph

^ permalink raw reply

* Re: [PATCH v2] clk/axs10x: introduce AXS10X pll driver
From: Vlad Zakharov @ 2017-04-05 16:06 UTC (permalink / raw)
  To: sboyd@codeaurora.org
  Cc: mark.rutland@arm.com, linux-kernel@vger.kernel.org,
	Jose.Abreu@synopsys.com, mturquette@baylibre.com,
	devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-snps-arc@lists.infradead.org
In-Reply-To: <20170405013525.GJ18246@codeaurora.org>

Hi Stephen,

On Tue, 2017-04-04 at 18:35 -0700, Stephen Boyd wrote:
> > +     .pll_table = (struct pll_of_table []){
> > +             {
> > +                     .prate = 27000000,
> 
> Can this be another clk in the framework instead of hardcoding
> the parent rate?

In fact there is another clk in the framework that represents this parent clock. But this field is needed to get
appropriate pll_cfg_table as it depends on parent clock frequency. Below in pll_cfg_get function we are searching for
the correct table comparing .parent_node field with real hardware parent clock frequency:
---------------------------------->8------------------------------------
for (i = 0; pll_table[i].prate != 0; i++)
    if (pll_table[i].prate == prate)
        return pll_table[i].pll_cfg_table;
---------------------------------->8------------------------------------

> 
> > +                     .pll_cfg_table = (struct pll_cfg []){
> > +                             { 25200000, 1, 84, 90 },
> > +                             { 50000000, 1, 100, 54 },
> > +                             { 74250000, 1, 44, 16 },
> > +                             { },
> > +                     },
> > +             },
> > +             /* Used as list limiter */
> > +             { },
> 
> There's only ever one, so I'm confused why we're making a list.

By this patch we only add support of core arc pll and pgu pll and today they are clocked by the only parent clocks
introduced here. But other plls on axs10x may be driven by different or configurable clocks, so in such cases we will
have more than one entry in this list. And we are going to add more supported plls to this driver in the nearest future.

> > +
> > +     clk = clk_register(NULL, &pll_clk->hw);
> > +     if (IS_ERR(clk)) {
> > +             pr_err("failed to register %s clock (%ld)\n",
> > +                             node->name, PTR_ERR(clk));
> > +             kfree(pll_clk);
> > +             return;
> > +     }
> > +
> > +     of_clk_add_provider(node, of_clk_src_simple_get, clk);
> 
> Can you please use the clk_hw based provider and clk registration
> functions?

Sure. Could you be so kind to explain what is the difference between hw and non-hw based provider and clk registration
functions please? In which cases they are preferred? 

> 
> > +}
> > +
> > +CLK_OF_DECLARE(axs10x_pll_clock, "snps,axs10x-arc-pll-clock", of_pll_clk_setup);
> 
> Does this need to be CLK_OF_DECLARE_DRIVER? I mean does the
> driver need to probe and also have this of declare happen? Is the
> PLL special and needs to be used for the timers?

It is special and is used for the timers, so we have to CLK_OF_DECLARE it. On the other hand similar pll is used to
drive PGU clock frequency and other subsystems and so we add usual probe func.

-- 
Best regards,
Vlad Zakharov <vzakhar@synopsys.com>

^ permalink raw reply

* Re: [PATCH v4 5/7] pinctrl: aramda-37xx: Add irqchip support
From: Gregory CLEMENT @ 2017-04-05 16:03 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thomas Petazzoni, Andrew Lunn, Jason Cooper, devicetree,
	linux-kernel, Nadav Haklai, linux-gpio, Rob Herring,
	Neta Zur Hershkovits, Victor Gu, Hua Jing, Marcin Wojtas,
	Wilson Ding, linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <70ffe3343c13d01737bf74e5de4898d0c0be07a0.1491405475.git-series.gregory.clement@free-electrons.com>


Argh, I sill have the typo in the title of this patch! :(

If you are going to apply it could you fix it, else it will be fixed in
the next version.

Sorry,

Gregory

 
 On mer., avril 05 2017, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> The Armada 37xx SoCs can handle interrupt through GPIO. However it can
> only manage the edge ones.
>
> The way the interrupt are managed are classical so we can use the generic
> interrupt chip model.
>
> The only unusual "feature" is that many interrupts are connected to the
> parent interrupt controller. But we do not take advantage of this and use
> the chained irq with all of them.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 221 +++++++++++++++++++++-
>  1 file changed, 221 insertions(+)
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 5c96f5558310..7356516e0921 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -13,7 +13,9 @@
>  #include <linux/gpio/driver.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/pinctrl/pinconf-generic.h>
>  #include <linux/pinctrl/pinconf.h>
>  #include <linux/pinctrl/pinctrl.h>
> @@ -30,6 +32,11 @@
>  #define OUTPUT_CTL	0x20
>  #define SELECTION	0x30
>  
> +#define IRQ_EN		0x0
> +#define IRQ_POL		0x08
> +#define IRQ_STATUS	0x10
> +#define IRQ_WKUP	0x18
> +
>  #define NB_FUNCS 2
>  #define GPIO_PER_REG	32
>  
> @@ -75,9 +82,12 @@ struct armada_37xx_pmx_func {
>  
>  struct armada_37xx_pinctrl {
>  	struct regmap			*regmap;
> +	void __iomem			*base;
>  	const struct armada_37xx_pin_data	*data;
>  	struct device			*dev;
>  	struct gpio_chip		gpio_chip;
> +	struct irq_chip			irq_chip;
> +	spinlock_t			irq_lock;
>  	struct pinctrl_desc		pctl;
>  	struct pinctrl_dev		*pctl_dev;
>  	struct armada_37xx_pin_group	*groups;
> @@ -346,6 +356,14 @@ static int armada_37xx_pmx_set(struct pinctrl_dev *pctldev,
>  	return armada_37xx_pmx_set_by_name(pctldev, name, grp);
>  }
>  
> +static inline void armada_37xx_irq_update_reg(unsigned int *reg,
> +					  struct irq_data *d)
> +{
> +	int offset = irqd_to_hwirq(d);
> +
> +	armada_37xx_update_reg(reg, offset);
> +}
> +
>  static int armada_37xx_gpio_direction_input(struct gpio_chip *chip,
>  					    unsigned int offset)
>  {
> @@ -468,6 +486,206 @@ static const struct gpio_chip armada_37xx_gpiolib_chip = {
>  	.owner = THIS_MODULE,
>  };
>  
> +void armada_37xx_irq_ack(struct irq_data *d)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> +	u32 reg = IRQ_STATUS, mask = d->mask;
> +	unsigned long flags;
> +
> +	armada_37xx_irq_update_reg(&reg, d);
> +	spin_lock_irqsave(&info->irq_lock, flags);
> +	writel(mask, info->base + reg);
> +	spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +void armada_37xx_irq_mask(struct irq_data *d)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> +	u32 val, reg = IRQ_EN, mask = d->mask;
> +	unsigned long flags;
> +
> +	armada_37xx_irq_update_reg(&reg, d);
> +	spin_lock_irqsave(&info->irq_lock, flags);
> +	val = readl(info->base + reg);
> +	writel(val & ~mask, info->base + reg);
> +	spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +void armada_37xx_irq_unmask(struct irq_data *d)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> +	u32 val, reg = IRQ_EN, mask = d->mask;
> +	unsigned long flags;
> +
> +	armada_37xx_irq_update_reg(&reg, d);
> +	spin_lock_irqsave(&info->irq_lock, flags);
> +	val = readl(info->base + reg);
> +	writel(val | mask, info->base + reg);
> +	spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +static int armada_37xx_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> +	u32 val, reg = IRQ_WKUP, mask = d->mask;
> +	unsigned long flags;
> +
> +	armada_37xx_irq_update_reg(&reg, d);
> +	spin_lock_irqsave(&info->irq_lock, flags);
> +	val = readl(info->base + reg);
> +	if (on)
> +		val |= mask;
> +	else
> +		val &= ~mask;
> +	writel(val, info->base + reg);
> +	spin_unlock_irqrestore(&info->irq_lock, flags);
> +
> +	return 0;
> +}
> +
> +static int armada_37xx_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> +	u32 val, reg = IRQ_POL, mask = d->mask;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&info->irq_lock, flags);
> +	armada_37xx_irq_update_reg(&reg, d);
> +	val = readl(info->base + reg);
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		val &= ~mask;
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		val |= mask;
> +		break;
> +	default:
> +		spin_unlock_irqrestore(&info->irq_lock, flags);
> +		return -EINVAL;
> +	}
> +	writel(val, info->base + reg);
> +	spin_unlock_irqrestore(&info->irq_lock, flags);
> +
> +	return 0;
> +}
> +
> +
> +static void armada_37xx_irq_handler(struct irq_desc *desc)
> +{
> +	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct armada_37xx_pinctrl *info = gpiochip_get_data(gc);
> +	struct irq_domain *d = gc->irqdomain;
> +	int i;
> +
> +	chained_irq_enter(chip, desc);
> +	for (i = 0; i <= d->revmap_size / GPIO_PER_REG; i++) {
> +		u32 status;
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&info->irq_lock, flags);
> +		status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
> +		/* Manage only the interrupt that was enabled */
> +		status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
> +		spin_unlock_irqrestore(&info->irq_lock, flags);
> +		while (status) {
> +			u32 hwirq = ffs(status) - 1;
> +			u32 virq = irq_find_mapping(d, hwirq +
> +						     i * GPIO_PER_REG);
> +
> +			generic_handle_irq(virq);
> +			status &= ~BIT(hwirq);
> +		}
> +	}
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static int armada_37xx_irqchip_register(struct platform_device *pdev,
> +					struct armada_37xx_pinctrl *info)
> +{
> +	struct device_node *np = info->dev->of_node;
> +	int nrirqs = info->data->nr_pins;
> +	struct gpio_chip *gc = &info->gpio_chip;
> +	struct irq_chip *irqchip = &info->irq_chip;
> +	struct resource res;
> +	int ret = -ENODEV, i, nr_irq_parent;
> +
> +	for_each_child_of_node(info->dev->of_node, np) {
> +		if (of_find_property(np, "gpio-controller", NULL)) {
> +			ret = 0;
> +			break;
> +		}
> +	};
> +	if (ret)
> +		return ret;
> +
> +	nr_irq_parent = of_irq_count(np);
> +	spin_lock_init(&info->irq_lock);
> +
> +	if (!nr_irq_parent) {
> +		dev_err(&pdev->dev, "Invalid or no IRQ\n");
> +		return 0;
> +	}
> +
> +	if (of_address_to_resource(info->dev->of_node, 1, &res)) {
> +		dev_err(info->dev, "cannot find IO resource\n");
> +		return -ENOENT;
> +	}
> +
> +	info->base = devm_ioremap_resource(info->dev, &res);
> +	if (IS_ERR(info->base))
> +		return PTR_ERR(info->base);
> +
> +	irqchip->irq_ack = armada_37xx_irq_ack;
> +	irqchip->irq_mask = armada_37xx_irq_mask;
> +	irqchip->irq_unmask = armada_37xx_irq_unmask;
> +	irqchip->irq_set_wake = armada_37xx_irq_set_wake;
> +	irqchip->irq_set_type = armada_37xx_irq_set_type;
> +	irqchip->name = info->data->name;
> +
> +	ret = gpiochip_irqchip_add(gc, irqchip, 0,
> +				   handle_edge_irq, IRQ_TYPE_NONE);
> +	if (ret) {
> +		dev_info(&pdev->dev, "could not add irqchip\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * Many interrupts are connected to the parent interrupt
> +	 * controller. But we do not take advantage of this and use
> +	 * the chained irq with all of them.
> +	 */
> +	for (i = 0; i < nrirqs; i++) {
> +		struct irq_data *d = irq_get_irq_data(gc->irq_base + i);
> +
> +		/*
> +		 * The mask field is a "precomputed bitmask for
> +		 * accessing the chip registers" which was introduced
> +		 * for the generic irqchip framework. As we don't use
> +		 * this framework, we can reuse this field for our own
> +		 * usage.
> +		 */
> +		d->mask = BIT(i % GPIO_PER_REG);
> +	}
> +
> +	for (i = 0; i < nr_irq_parent; i++) {
> +		int irq = irq_of_parse_and_map(np, i);
> +
> +		if (irq < 0)
> +			continue;
> +
> +		gpiochip_set_chained_irqchip(gc, irqchip, irq,
> +					     armada_37xx_irq_handler);
> +	}
> +
> +	return 0;
> +}
> +
>  static int armada_37xx_gpiochip_register(struct platform_device *pdev,
>  					struct armada_37xx_pinctrl *info)
>  {
> @@ -496,6 +714,9 @@ static int armada_37xx_gpiochip_register(struct platform_device *pdev,
>  	ret = devm_gpiochip_add_data(&pdev->dev, gc, info);
>  	if (ret)
>  		return ret;
> +	ret = armada_37xx_irqchip_register(pdev, info);
> +	if (ret)
> +		return ret;
>  
>  	return 0;
>  }
> -- 
> git-series 0.9.1

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 07/11] regulator: axp20x-regulator: add support for AXP803
From: Chen-Yu Tsai @ 2017-04-05 16:01 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Lee Jones, Rob Herring, Chen-Yu Tsai, Maxime Ripard,
	Liam Girdwood, devicetree, linux-sunxi, linux-kernel,
	linux-arm-kernel
In-Reply-To: <20170404180145.12897-8-icenowy-h8G6r0blFSE@public.gmane.org>

Hi,

On Wed, Apr 5, 2017 at 2:01 AM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> AXP803 PMIC also have a series of regulators (DCDCs and LDOs)
> controllable via I2C/RSB bus.
>
> Add support for them.
>
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/regulator/axp20x-regulator.c | 135 +++++++++++++++++++++++++++++++----
>  include/linux/mfd/axp20x.h           |  37 ++++++++++
>  2 files changed, 158 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 0b9d4e3e52c7..219a731df392 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -355,6 +355,81 @@ static const struct regulator_desc axp809_regulators[] = {
>         AXP_DESC_SW(AXP809, SW, "sw", "swin", AXP22X_PWR_OUT_CTRL2, BIT(6)),
>  };
>
> +static const struct regulator_linear_range axp803_dcdc234_ranges[] = {
> +       REGULATOR_LINEAR_RANGE(500000, 0x0, 0x46, 10000),
> +       REGULATOR_LINEAR_RANGE(1200000, 0x47, 0x4b, 20000),
> +};

Can you keep the ranges the datasheet uses? It makes it easier to compare.

> +
> +static const struct regulator_linear_range axp803_dcdc5_ranges[] = {
> +       REGULATOR_LINEAR_RANGE(800000, 0x0, 0x20, 10000),
> +       REGULATOR_LINEAR_RANGE(1140000, 0x21, 0x43, 20000),

1.14 ~ 1.84 / 0.02 = 36 steps. This would end at 0x44.

> +};
> +
> +static const struct regulator_linear_range axp803_dcdc6_ranges[] = {
> +       REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
> +       REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
> +};
> +
> +static const struct regulator_linear_range axp803_dldo2_ranges[] = {
> +       REGULATOR_LINEAR_RANGE(700000, 0x0, 0x1a, 100000),
> +       REGULATOR_LINEAR_RANGE(3400000, 0x1b, 0x1f, 200000),
> +};

This is the same as axp806_cldo2_ranges. To keep the variants sorted
in ascending order, you could keep this version but replace references
to axp806_cldo2_ranges with this one.

> +
> +static const struct regulator_desc axp803_regulators[] = {
> +       AXP_DESC(AXP803, DCDC1, "dcdc1", "vin1", 1600, 3400, 100,
> +                AXP803_DCDC1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL1, BIT(0)),
> +       AXP_DESC_RANGES(AXP803, DCDC2, "dcdc2", "vin2", axp803_dcdc234_ranges,
> +                       76, AXP803_DCDC2_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
> +                       BIT(1)),
> +       AXP_DESC_RANGES(AXP803, DCDC3, "dcdc3", "vin3", axp803_dcdc234_ranges,
> +                       76, AXP803_DCDC3_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
> +                       BIT(2)),
> +       AXP_DESC_RANGES(AXP803, DCDC4, "dcdc4", "vin4", axp803_dcdc234_ranges,
> +                       76, AXP803_DCDC4_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
> +                       BIT(3)),
> +       AXP_DESC_RANGES(AXP803, DCDC5, "dcdc5", "vin5", axp803_dcdc5_ranges,
> +                       68, AXP803_DCDC5_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
> +                       BIT(4)),
> +       AXP_DESC_RANGES(AXP803, DCDC6, "dcdc6", "vin6", axp803_dcdc6_ranges,
> +                       72, AXP803_DCDC6_V_OUT, 0x7f, AXP22X_PWR_OUT_CTRL1,
> +                       BIT(5)),
> +       /* secondary switchable output of DCDC1 */
> +       AXP_DESC_SW(AXP803, DC1SW, "dc1sw", NULL, AXP22X_PWR_OUT_CTRL2,
> +                   BIT(7)),
> +       AXP_DESC(AXP803, ALDO1, "aldo1", "aldoin", 700, 3300, 100,
> +                AXP22X_ALDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(5)),
> +       AXP_DESC(AXP803, ALDO2, "aldo2", "aldoin", 700, 3300, 100,
> +                AXP22X_ALDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(6)),
> +       AXP_DESC(AXP803, ALDO3, "aldo3", "aldoin", 700, 3300, 100,
> +                AXP22X_ALDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(7)),
> +       AXP_DESC(AXP803, DLDO1, "dldo1", "dldoin", 700, 3300, 100,
> +                AXP22X_DLDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(3)),
> +       AXP_DESC_RANGES(AXP803, DLDO2, "dldo2", "dldoin", axp803_dldo2_ranges,
> +                       32, AXP22X_DLDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2,
> +                       BIT(4)),
> +       AXP_DESC(AXP803, DLDO3, "dldo3", "dldoin", 700, 3300, 100,
> +                AXP22X_DLDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(5)),
> +       AXP_DESC(AXP803, DLDO4, "dldo4", "dldoin", 700, 3300, 100,
> +                AXP22X_DLDO4_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(6)),

dldo3 and dldo4 are controlled from AXP22X_PWR_OUT_CTRL2.

> +       AXP_DESC(AXP803, ELDO1, "eldo1", "eldoin", 700, 1900, 50,
> +                AXP22X_ELDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(0)),
> +       AXP_DESC(AXP803, ELDO2, "eldo2", "eldoin", 700, 1900, 50,
> +                AXP22X_ELDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(1)),
> +       AXP_DESC(AXP803, ELDO3, "eldo3", "eldoin", 700, 1900, 50,
> +                AXP22X_ELDO3_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL2, BIT(2)),
> +       AXP_DESC(AXP803, FLDO1, "fldo1", "fldoin", 700, 1450, 50,
> +                AXP803_FLDO1_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
> +       AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
> +                AXP803_FLDO2_V_OUT, 0x1f, AXP22X_PWR_OUT_CTRL3, BIT(3)),

The fields for fldo* are only four bits wide.

> +       AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
> +                   AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
> +                   AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
> +       AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
> +                   AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
> +                   AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
> +       AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
> +};
> +
>  static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>  {
>         struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -377,6 +452,13 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>                  * (See include/linux/mfd/axp20x.h)
>                  */
>                 reg = AXP806_DCDC_FREQ_CTRL;
> +       case AXP803_ID:

AXP803 should come before AXP806.

> +               /*
> +                * AXP803 also have DCDC work frequency setting register at a
> +                * different position.
> +                */
> +               if (axp20x->variant == AXP803_ID)
> +                       reg = AXP803_DCDC_FREQ_CTRL;
>         case AXP221_ID:
>         case AXP223_ID:
>         case AXP809_ID:
> @@ -474,7 +556,13 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
>                 mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP22X_DCDC1);
>                 workmode <<= id - AXP22X_DCDC1;
>                 break;
> +       case AXP803_ID:

Same here. Also this switch block has empty lines separating each case.

> +               if (id < AXP803_DCDC1 || id > AXP803_DCDC6)
> +                       return -EINVAL;
>
> +               mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP803_DCDC1);
> +               workmode <<= id - AXP803_DCDC1;
> +               break;
>         default:
>                 /* should not happen */
>                 WARN_ON(1);
> @@ -492,20 +580,34 @@ static bool axp20x_is_polyphase_slave(struct axp20x_dev *axp20x, int id)
>  {
>         u32 reg = 0;
>
> -       /* Only AXP806 has poly-phase outputs */
> -       if (axp20x->variant != AXP806_ID)
> +       /*
> +        * Currently in our supported AXP variants, only AXP806 and AXP803
> +        * have polyphase regulators.
> +        */
> +       switch (axp20x->variant) {
> +       case AXP806_ID:
> +               regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
> +
> +               switch (id) {
> +               case AXP806_DCDCB:
> +                       return (((reg & GENMASK(7, 6)) == BIT(6)) ||
> +                               ((reg & GENMASK(7, 6)) == BIT(7)));
> +               case AXP806_DCDCC:
> +                       return ((reg & GENMASK(7, 6)) == BIT(7));
> +               case AXP806_DCDCE:
> +                       return !!(reg & BIT(5));
> +               }
> +       case AXP803_ID:

803 before 806.

> +               regmap_read(axp20x->regmap, AXP803_POLYPHASE_CTRL, &reg);
> +
> +               switch (id) {
> +               case AXP803_DCDC3:
> +                       return !!(reg & BIT(6));
> +               case AXP803_DCDC6:
> +                       return !!(reg & BIT(7));
> +               }
> +       default:
>                 return false;
> -
> -       regmap_read(axp20x->regmap, AXP806_DCDC_MODE_CTRL2, &reg);
> -
> -       switch (id) {
> -       case AXP806_DCDCB:
> -               return (((reg & GENMASK(7, 6)) == BIT(6)) ||
> -                       ((reg & GENMASK(7, 6)) == BIT(7)));
> -       case AXP806_DCDCC:
> -               return ((reg & GENMASK(7, 6)) == BIT(7));
> -       case AXP806_DCDCE:
> -               return !!(reg & BIT(5));
>         }
>
>         return false;
> @@ -548,6 +650,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>                 regulators = axp809_regulators;
>                 nregulators = AXP809_REG_ID_MAX;
>                 break;
> +       case AXP803_ID:
> +               regulators = axp803_regulators;
> +               nregulators = AXP803_REG_ID_MAX;
> +               break;

Ascending order.

>         default:
>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
>                         axp20x->variant);
> @@ -579,7 +685,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>                  * name.
>                  */
>                 if ((regulators == axp22x_regulators && i == AXP22X_DC1SW) ||
> -                   (regulators == axp809_regulators && i == AXP809_DC1SW)) {
> +                   (regulators == axp809_regulators && i == AXP809_DC1SW) ||
> +                   (regulators == axp803_regulators && i == AXP803_DC1SW)) {

803 before 809.

So, mostly ordering issues, and a few minor errors.

Regards
ChenYu

>                         new_desc = devm_kzalloc(&pdev->dev, sizeof(*desc),
>                                                 GFP_KERNEL);
>                         *new_desc = regulators[i];
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index b3220ef374d3..858d5b1b43b9 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -119,6 +119,17 @@ enum axp20x_variants {
>  #define AXP806_BUS_ADDR_EXT            0xfe
>  #define AXP806_REG_ADDR_EXT            0xff
>
> +#define AXP803_POLYPHASE_CTRL          0x14
> +#define AXP803_FLDO1_V_OUT             0x1c
> +#define AXP803_FLDO2_V_OUT             0x1d
> +#define AXP803_DCDC1_V_OUT             0x20
> +#define AXP803_DCDC2_V_OUT             0x21
> +#define AXP803_DCDC3_V_OUT             0x22
> +#define AXP803_DCDC4_V_OUT             0x23
> +#define AXP803_DCDC5_V_OUT             0x24
> +#define AXP803_DCDC6_V_OUT             0x25
> +#define AXP803_DCDC_FREQ_CTRL          0x3b
> +
>  /* Interrupt */
>  #define AXP152_IRQ1_EN                 0x40
>  #define AXP152_IRQ2_EN                 0x41
> @@ -350,6 +361,32 @@ enum {
>         AXP809_REG_ID_MAX,
>  };
>
> +enum {
> +       AXP803_DCDC1 = 0,
> +       AXP803_DCDC2,
> +       AXP803_DCDC3,
> +       AXP803_DCDC4,
> +       AXP803_DCDC5,
> +       AXP803_DCDC6,
> +       AXP803_DC1SW,
> +       AXP803_ALDO1,
> +       AXP803_ALDO2,
> +       AXP803_ALDO3,
> +       AXP803_DLDO1,
> +       AXP803_DLDO2,
> +       AXP803_DLDO3,
> +       AXP803_DLDO4,
> +       AXP803_ELDO1,
> +       AXP803_ELDO2,
> +       AXP803_ELDO3,
> +       AXP803_FLDO1,
> +       AXP803_FLDO2,
> +       AXP803_RTC_LDO,
> +       AXP803_LDO_IO0,
> +       AXP803_LDO_IO1,
> +       AXP803_REG_ID_MAX,
> +};
> +
>  /* IRQs */
>  enum {
>         AXP152_IRQ_LDO0IN_CONNECT = 1,
> --
> 2.12.2
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings
From: Ludovic BARRE @ 2017-04-05 16:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: Cyrille Pitchen, Marek Vasut, David Woodhouse, Brian Norris,
	Boris Brezillon, Richard Weinberger, Alexandre Torgue,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <CAL_JsqK5mZojW1fVRrg=TfO3sXd7Eqa6gS9NS9aDjXwn4_26_w@mail.gmail.com>


On 04/04/2017 02:20 PM, Rob Herring wrote:
> On Tue, Apr 4, 2017 at 2:28 AM, Ludovic BARRE <ludovic.barre@st.com> wrote:
>> Hi Rob
>>
>> thanks for review
>> my comments below
>>
>> br
>> Ludo
>>
>> On 04/03/2017 06:57 PM, Rob Herring wrote:
>>> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch adds documentation of device tree bindings for the STM32
>>>> QSPI controller.
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>>    .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45
>>>> ++++++++++++++++++++++
>>>>    1 file changed, 45 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> new file mode 100644
>>>> index 0000000..95a8ebd
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> @@ -0,0 +1,45 @@
>>>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>>>> +
>>>> +Required properties:
>>>> +- compatible: should be "st,stm32f469-qspi"
>>>> +- reg: contains the register location and length.
>>>> +  (optional) the memory mapping address and length
>>> Why optional? Either the h/w has it or doesn't. If some chips don't,
>>> they should have a different compatible string.
>> in fact, the stm32 qspi controller can operate in any of the following
>> modes:
>> -indirect mode: all the operations are performed using the qspi registers
>> with read/write.
>> -read memory-mapped mode: the external Flash memory is mapped to the
>>   microcontroller address space and is seen by the system as if it was
>>   an internal memory (use memcpy_fromio). this mode improve read throughput
>>
>> if qspi_mm is defined the qspi controller use read memory-mapped mode
>> else the controller transfers in indirect mode.
> You should always have the memory region defined because that's what
> the h/w has. If you want another property to select the mode, then
> perhaps that's fine. But why? Can't the OS figure out which to use?
> Why would you ever not use memory mapped mode unless the driver
> doesn't yet support it?
ok, I always map the memory region (qspi_mm is now required).
if the nor-flash is more bigger than "qspi memory region", I force to use
the indirect mode.
> Rob

^ permalink raw reply

* Re: [PATCH 2/3] of/fdt: introduce of_scan_flat_dt_subnodes and of_get_flat_dt_phandle
From: Rob Herring @ 2017-04-05 15:58 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Michael Ellerman,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev,
	Benjamin Herrenschmidt, Frank Rowand
In-Reply-To: <20170406003251.533e2845-a5aMA/AkCkgK5Ils6ZIQy0EOCMrvLtNR@public.gmane.org>

On Wed, Apr 5, 2017 at 9:32 AM, Nicholas Piggin <npiggin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, 5 Apr 2017 08:35:06 -0500
> Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> On Wed, Apr 5, 2017 at 7:37 AM, Nicholas Piggin <npiggin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > Introduce primitives for FDT parsing. These will be used for powerpc
>> > cpufeatures node scanning, which has quite complex structure but should
>> > be processed early.
>>
>> Have you looked at unflattening the FDT earlier?
>
> Hi, thanks for taking a look. Did you mean to trim the cc list?

Ugg, no. I've added everyone back.

> It may be possible but I'd like to avoid it if we can. There might
> turn out to be some errata or feature that requires early setup. And
> the current cpu feature parsing code does it with flat dt.

Well, I'd like to avoid expanding usage of flat DT parsing in the
kernel. But you could just put this function into arch/powerpc and I'd
never see it, but I like that even less. Mainly, I just wanted to
raise the point.

Your argument works until you need that setup in assembly code, then
you are in the situation that you need to either handle the setup in
bootloader/firmware or have an simple way to determine that condition.

Rob

>
>> > Signed-off-by: Nicholas Piggin <npiggin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > ---
>> >  drivers/of/fdt.c       | 39 +++++++++++++++++++++++++++++++++++++++
>> >  include/linux/of_fdt.h |  6 ++++++
>> >  2 files changed, 45 insertions(+)
>> >
>> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> > index e5ce4b59e162..a45854fe5156 100644
>> > --- a/drivers/of/fdt.c
>> > +++ b/drivers/of/fdt.c
>> > @@ -754,6 +754,37 @@ int __init of_scan_flat_dt(int (*it)(unsigned long node,
>> >  }
>> >
>> >  /**
>> > + * of_scan_flat_dt_subnodes - scan sub-nodes of a node call callback on each.
>> > + * @it: callback function
>> > + * @data: context data pointer
>> > + *
>> > + * This function is used to scan sub-nodes of a node.
>> > + */
>> > +int __init of_scan_flat_dt_subnodes(unsigned long node,
>> > +                                   int (*it)(unsigned long node,
>> > +                                             const char *uname,
>> > +                                             void *data),
>> > +                                   void *data)
>> > +{
>> > +       const void *blob = initial_boot_params;
>> > +       const char *pathp;
>> > +       int offset, rc = 0;
>> > +
>> > +       offset = node;
>> > +        for (offset = fdt_first_subnode(blob, offset);
>> > +             offset >= 0 && !rc;
>> > +             offset = fdt_next_subnode(blob, offset)) {
>>
>> fdt_for_each_subnode()
>
> Got it.
>
>>
>> > +
>> > +               pathp = fdt_get_name(blob, offset, NULL);
>> > +               if (*pathp == '/')
>> > +                       pathp = kbasename(pathp);
>>
>> Seems a bit odd that you parse the name in this function. Perhaps the
>> caller should do that, or if you want subnodes matching a certain
>> name, then do the matching here. But you didn't copy me on the rest of
>> the series, so I don't know how you are using this.
>
> Hmm, it was a while since writing that part. I guess I just copied
> of_scan_flat_dt interface.
>
> Caller is in this patch:
>
> https://patchwork.ozlabs.org/patch/747262/
>
> I'll include you in subsequent post if you prefer.
>
> Thanks,
> Nick
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/1] ARM: dts: armada-xp-linksys-mamba: use wan instead of internet for DSA port
From: Andrew Lunn @ 2017-04-05 15:50 UTC (permalink / raw)
  To: Ralph Sennhauser
  Cc: Mark Rutland, devicetree, Jason Cooper, linux-kernel,
	Russell King, Rob Herring, Gregory Clement, linux-arm-kernel,
	Sebastian Hesselbarth
In-Reply-To: <20170405172224.38c1f9fb@gmail.com>

> In fact "internet" label on the case is uppercase, would this matter
> for you for new bindings, or would you still use the lowercase version?

I would use lower case, just to fix with the general convention that
interface names are lower case.

> > I would say, this is now too late. Changing an interface name will
> > break configuration scripts. We are stuck with it.
> > 
> 
> If it weren't for commit cb4f71c42988 that would have been obvious for
> me as well.

Yes, that was not nice. But it was also very earlier in the life of
38x, so it just affected a few developers with reference boards, not
real products out in the wild, as far as i remember.

     Andrew

^ permalink raw reply

* Re: [PATCH] ARM: dts: armada-38x: label USB and SATA nodes
From: Gregory CLEMENT @ 2017-04-05 15:49 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Ralph Sennhauser,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jason Cooper,
	Sebastian Hesselbarth, Rob Herring, Mark Rutland, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170331182111.GJ22609-g2DYL2Zd6BY@public.gmane.org>

Hi Andrew,
 
 On ven., mars 31 2017, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:

> On Fri, Mar 31, 2017 at 07:39:20PM +0200, Ralph Sennhauser wrote:
>> On Fri, 31 Mar 2017 18:50:15 +0200
>> Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> 
>> > > -			sata@a8000 {
>> > > +			satac0: sata@a8000 {  
>> > 
>> > Hi Ralph
>> > 
>> > Why the c in satac0?
>> 
>> For controller and to not conflict with a use case of sata0 for a port,
>> similarly to pciec and pcie1. See armada-385-synology-ds116.dts.
>
> :~/linux/arch/arm/boot/dts$ ls *ds116*
> ls: cannot access '*ds116*': No such file or directory
>
> But anyway, a few boards seem to solve this by calling the controller
> node ahci0: and the port sata0:
>
>> > > -			usb3@f0000 {
>> > > +			usb3_0: usb3@f0000 {
>> > >  				compatible =
>> > > "marvell,armada-380-xhci"; reg = <0xf0000 0x4000>,<0xf4000 0x4000>;
>> > >  				interrupts = <GIC_SPI 16
>> > > IRQ_TYPE_LEVEL_HIGH>; @@ -598,7 +598,7 @@
>> > >  				status = "disabled";
>> > >  			};
>> > >  
>> > > -			usb3@f8000 {
>> > > +			usb3_1: usb3@f8000 {
>> > >  				compatible =
>> > > "marvell,armada-380-xhci"; reg = <0xf8000 0x4000>,<0xfc000 0x4000>;
>> > >  				interrupts = <GIC_SPI 17
>> > > IRQ_TYPE_LEVEL_HIGH>;  
>> > 
>> > I can understand what you are saying. But does anybody else care? Are
>> > there other .dtsi files differentiating between USB 1.1, 2 and 3?
>> 
>> It's handled differently where ever I looked, some do some don't. A
>> case for distinguishing USB 2.0 and USB 3.0 like this is
>> armada-388-gp.dts.

Actually I care and I found confusing calling usb2 the second usb port if
it is controlled by an USB3 controller.

>
> Humm...
>
>                         /* CON4 */
>                         usb@58000 {
>                                 vcc-supply = <&reg_usb2_0_vbus>;
>                                 status = "okay";
>                         };
>
>
> 			/* CON5 */
>                         usb3@f0000 {
>                                 usb-phy = <&usb2_1_phy>;
>                                 status = "okay";
>                         };
>
>                         /* CON7 */
>                         usb3@f8000 {
>                                 usb-phy = <&usb3_phy>;
>                                 status = "okay";
>                         };
>
> Is this clear? Is CON5 a USB 3 host, but has a USB 2 PHY connected to
> it? CON7 is the only true USB 3 port? I think some comments written in

I can answer it: CON5 is indeed an USB3 host with a USB2 PHY connected
to it so we can use it only as an USB2. And indeed CON7 is the only true
USB3 port.

> schwiizerdütsch would be clearre.:-)

Actually all your assumption were correct so maybe it is not as
confusing as it looks! :)  But I can add a comment if needed.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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