Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 3/6] remoteproc: qcom: Push reset ops, fw ops, rproc ops in to of_match data
From: Sricharan R @ 2017-11-14 10:53 UTC (permalink / raw)
  To: bjorn.andersson, ohad, robh+dt, mark.rutland, andy.gross,
	david.brown, linux-remoteproc, devicetree, linux-kernel,
	linux-arm-msm, linux-soc
  Cc: sricharan
In-Reply-To: <1510656794-3815-1-git-send-email-sricharan@codeaurora.org>

Instead of directly assigning reset, fw and rproc ops, put them
in to of_match data and get from that. Currently same ops
are used for all compatibles, but that will change when we add
q6v5-wcss support.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/remoteproc/qcom_q6v5_pil.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index df8723d..769c6af 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -116,16 +116,6 @@ struct qcom_mss_reg_res {
 	int uA;
 };
 
-struct rproc_hexagon_res {
-	const char *hexagon_mba_image;
-	struct qcom_mss_reg_res *proxy_supply;
-	struct qcom_mss_reg_res *active_supply;
-	char **proxy_clk_names;
-	char **active_clk_names;
-	int version;
-	bool need_mem_protection;
-};
-
 struct q6v5 {
 	struct device *dev;
 	struct rproc *rproc;
@@ -174,6 +164,19 @@ struct q6v5 {
 	int version;
 };
 
+struct rproc_hexagon_res {
+	const char *hexagon_mba_image;
+	struct qcom_mss_reg_res *proxy_supply;
+	struct qcom_mss_reg_res *active_supply;
+	char **proxy_clk_names;
+	char **active_clk_names;
+	int version;
+	bool need_mem_protection;
+	int (*init_reset)(struct q6v5 *qproc);
+	const struct rproc_fw_ops *fw_ops;
+	const struct rproc_ops *ops;
+};
+
 enum {
 	MSS_MSM8916,
 	MSS_MSM8974,
@@ -1128,14 +1131,14 @@ static int q6v5_probe(struct platform_device *pdev)
 	if (!desc)
 		return -EINVAL;
 
-	rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_ops,
+	rproc = rproc_alloc(&pdev->dev, pdev->name, desc->ops,
 			    desc->hexagon_mba_image, sizeof(*qproc));
 	if (!rproc) {
 		dev_err(&pdev->dev, "failed to allocate rproc\n");
 		return -ENOMEM;
 	}
 
-	rproc->fw_ops = &q6v5_fw_ops;
+	rproc->fw_ops = desc->fw_ops;
 
 	qproc = (struct q6v5 *)rproc->priv;
 	qproc->dev = &pdev->dev;
@@ -1185,7 +1188,7 @@ static int q6v5_probe(struct platform_device *pdev)
 	}
 	qproc->active_reg_count = ret;
 
-	ret = q6v5_init_reset(qproc);
+	ret = desc->init_reset(qproc);
 	if (ret)
 		goto free_rproc;
 
@@ -1258,6 +1261,9 @@ static int q6v5_remove(struct platform_device *pdev)
 	},
 	.need_mem_protection = true,
 	.version = MSS_MSM8996,
+	.init_reset = q6v5_init_reset,
+	.fw_ops = &q6v5_fw_ops,
+	.ops = &q6v5_ops,
 };
 
 static const struct rproc_hexagon_res msm8916_mss = {
@@ -1289,6 +1295,9 @@ static int q6v5_remove(struct platform_device *pdev)
 	},
 	.need_mem_protection = false,
 	.version = MSS_MSM8916,
+	.init_reset = q6v5_init_reset,
+	.fw_ops = &q6v5_fw_ops,
+	.ops = &q6v5_ops,
 };
 
 static const struct rproc_hexagon_res msm8974_mss = {
@@ -1328,6 +1337,9 @@ static int q6v5_remove(struct platform_device *pdev)
 	},
 	.need_mem_protection = false,
 	.version = MSS_MSM8974,
+	.init_reset = q6v5_init_reset,
+	.fw_ops = &q6v5_fw_ops,
+	.ops = &q6v5_ops,
 };
 
 static const struct of_device_id q6v5_of_match[] = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH v5 2/6] remoteproc: Export rproc_elf_get_boot_addr
From: Sricharan R @ 2017-11-14 10:53 UTC (permalink / raw)
  To: bjorn.andersson, ohad, robh+dt, mark.rutland, andy.gross,
	david.brown, linux-remoteproc, devicetree, linux-kernel,
	linux-arm-msm, linux-soc
  Cc: sricharan
In-Reply-To: <1510656794-3815-1-git-send-email-sricharan@codeaurora.org>

Export rproc_elf_get_boot_addr so that it can be
used by any remoteproc to get the bootaddr of the
elf type firmware images. This is used in the
subsequent patch by the q6v5 based remoteproc
while loading its elf based mdt type image.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/remoteproc/remoteproc_elf_loader.c | 2 +-
 drivers/remoteproc/remoteproc_internal.h   | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
index c523983..f6d07d9 100644
--- a/drivers/remoteproc/remoteproc_elf_loader.c
+++ b/drivers/remoteproc/remoteproc_elf_loader.c
@@ -110,13 +110,13 @@
  * Note that the boot address is not a configurable property of all remote
  * processors. Some will always boot at a specific hard-coded address.
  */
-static
 u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw)
 {
 	struct elf32_hdr *ehdr  = (struct elf32_hdr *)fw->data;
 
 	return ehdr->e_entry;
 }
+EXPORT_SYMBOL(rproc_elf_get_boot_addr);
 
 /**
  * rproc_elf_load_segments() - load firmware segments to memory
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index c1077be..1bbbfea 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -124,4 +124,7 @@ struct resource_table *rproc_find_loaded_rsc_table(struct rproc *rproc,
 
 extern const struct rproc_fw_ops rproc_elf_fw_ops;
 
+/* from remoteproc_elf_loader.c */
+u32 rproc_elf_get_boot_addr(struct rproc *rproc, const struct firmware *fw);
+
 #endif /* REMOTEPROC_INTERNAL_H */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH v5 1/6] remoteproc: qcom: mdt_loader: Make the firmware authentication optional
From: Sricharan R @ 2017-11-14 10:53 UTC (permalink / raw)
  To: bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	ohad-Ix1uc/W3ht7QT0dZR+AlfA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, andy.gross-QSEj5FYQhm4dnm+yROfE0A,
	david.brown-QSEj5FYQhm4dnm+yROfE0A,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA
  Cc: sricharan-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <1510656794-3815-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

qcom_mdt_load function loads the mdt type firmware and
initialises the secure memory as well. Make the initialisation only
when requested by the caller, so that the function can be used
by self-authenticating remoteproc as well.

Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/soc/qcom/mdt_loader.c       | 82 ++++++++++++++++++++++++++-----------
 include/linux/soc/qcom/mdt_loader.h |  3 ++
 2 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index 08bd854..65ab833 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -74,21 +74,9 @@ ssize_t qcom_mdt_get_size(const struct firmware *fw)
 }
 EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
 
-/**
- * qcom_mdt_load() - load the firmware which header is loaded as fw
- * @dev:	device handle to associate resources with
- * @fw:		firmware object for the mdt file
- * @firmware:	name of the firmware, for construction of segment file names
- * @pas_id:	PAS identifier
- * @mem_region:	allocated memory region to load firmware into
- * @mem_phys:	physical address of allocated memory region
- * @mem_size:	size of the allocated memory region
- *
- * Returns 0 on success, negative errno otherwise.
- */
-int qcom_mdt_load(struct device *dev, const struct firmware *fw,
-		  const char *firmware, int pas_id, void *mem_region,
-		  phys_addr_t mem_phys, size_t mem_size)
+static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
+			   const char *firmware, int pas_id, void *mem_region,
+			   phys_addr_t mem_phys, size_t mem_size, bool pas_init)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
@@ -119,10 +107,12 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 	if (!fw_name)
 		return -ENOMEM;
 
-	ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
-	if (ret) {
-		dev_err(dev, "invalid firmware metadata\n");
-		goto out;
+	if (pas_init) {
+		ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
+		if (ret) {
+			dev_err(dev, "invalid firmware metadata\n");
+			goto out;
+		}
 	}
 
 	for (i = 0; i < ehdr->e_phnum; i++) {
@@ -142,10 +132,13 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 	}
 
 	if (relocate) {
-		ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - min_addr);
-		if (ret) {
-			dev_err(dev, "unable to setup relocation\n");
-			goto out;
+		if (pas_init) {
+			ret = qcom_scm_pas_mem_setup(pas_id, mem_phys,
+						     max_addr - min_addr);
+			if (ret) {
+				dev_err(dev, "unable to setup relocation\n");
+				goto out;
+			}
 		}
 
 		/*
@@ -197,7 +190,50 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 
 	return ret;
 }
+
+/**
+ * qcom_mdt_load() - sets up the secure memory for the firmware and
+		     loads the firmware
+ * @dev:	device handle to associate resources with
+ * @fw:		firmware object for the mdt file
+ * @firmware:	name of the firmware, for construction of segment file names
+ * @pas_id:	PAS identifier
+ * @mem_region:	allocated memory region to load firmware into
+ * @mem_phys:	physical address of allocated memory region
+ * @mem_size:	size of the allocated memory region
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int qcom_mdt_load(struct device *dev, const struct firmware *fw,
+		  const char *firmware, int pas_id, void *mem_region,
+		  phys_addr_t mem_phys, size_t mem_size)
+{
+	return __qcom_mdt_load(dev, fw, firmware, pas_id, mem_region, mem_phys,
+			       mem_size, true);
+}
 EXPORT_SYMBOL_GPL(qcom_mdt_load);
 
+/**
+ * qcom_mdt_load_no_init() - load the firmware which header is loaded as fw
+ * @dev:	device handle to associate resources with
+ * @fw:		firmware object for the mdt file
+ * @firmware:	name of the firmware, for construction of segment file names
+ * @pas_id:	PAS identifier
+ * @mem_region:	allocated memory region to load firmware into
+ * @mem_phys:	physical address of allocated memory region
+ * @mem_size:	size of the allocated memory region
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
+			  const char *firmware, int pas_id,
+			  void *mem_region, phys_addr_t mem_phys,
+			  size_t mem_size)
+{
+	return __qcom_mdt_load(dev, fw, firmware, pas_id, mem_region, mem_phys,
+			       mem_size, false);
+}
+EXPORT_SYMBOL_GPL(qcom_mdt_load_no_init);
+
 MODULE_DESCRIPTION("Firmware parser for Qualcomm MDT format");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index bd8e086..151ee6a 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -16,4 +16,7 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 		  const char *fw_name, int pas_id, void *mem_region,
 		  phys_addr_t mem_phys, size_t mem_size);
 
+int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
+			  const char *fw_name, int pas_id, void *mem_region,
+			  phys_addr_t mem_phys, size_t mem_size);
 #endif
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
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

* [PATCH v5 0/6]  Add support for Hexagon q6v5-wcss integrated core
From: Sricharan R @ 2017-11-14 10:53 UTC (permalink / raw)
  To: bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	ohad-Ix1uc/W3ht7QT0dZR+AlfA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, andy.gross-QSEj5FYQhm4dnm+yROfE0A,
	david.brown-QSEj5FYQhm4dnm+yROfE0A,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA
  Cc: sricharan-sgV2jX0FEOL9JmXXK+q4OQ

IPQ8074 has an integrated Hexagon dsp core Q6v5 and a wireless lan
(Lithium) IP. This series adds the remoteproc driver to reset, load
and boot Q6 firmware.

The first patch is to make the mdt_loader authenticate
the firmware only if required, so that the code can be reused for
self-authenticating firmware like the Q6v5 core in IPQ8074. The second
patch exports the elf header's get_boot_addr helper to reuse it.
The next couple of patches arranges the code in the original q6v5-mpss
rproc to add q6v5-wcss later. The last couple of patches add the relevant
bits for the q6v5-wcss core.

This is done on top of Avaneesh's msm8996 rproc support [1]

[1] https://lkml.org/lkml/2017/10/24/771

v5:
    No change. Just updated tags in PATCH #5

V4:
    Fixed Bjorn's comment in PATCH#1 and added his acked-by
    Rebased on top of Avinash's latest rproc for msm8996 q6 support.

V3:
    Rebased on top of latest remoteproc next

V2:
    Last time introduced this a new rproc driver, but there is lot
    of code that can be shared if it is added to the q6v5-mpss pil
    driver.

Sricharan R (6):
  remoteproc: qcom: mdt_loader: Make the firmware authentication
    optional
  remoteproc: Export rproc_elf_get_boot_addr
  remoteproc: qcom: Push reset ops, fw ops, rproc ops in to of_match
    data
  remoteproc: qcom: Split the head and tail of the q6v5-pil rproc start
    function
  remoteproc: qcom: Add support for q6v5-wcss pil
  remoteproc: qcom: Add q6v5-wcss rproc ops

 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   7 +-
 drivers/remoteproc/Kconfig                         |   1 +
 drivers/remoteproc/qcom_q6v5_pil.c                 | 469 +++++++++++++++++----
 drivers/remoteproc/remoteproc_elf_loader.c         |   2 +-
 drivers/remoteproc/remoteproc_internal.h           |   3 +
 drivers/soc/qcom/mdt_loader.c                      |  82 +++-
 include/linux/soc/qcom/mdt_loader.h                |   3 +
 7 files changed, 453 insertions(+), 114 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
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/3] Input: twl4030-vibra: fix sibling-node lookup
From: Lee Jones @ 2017-11-14 10:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Johan Hovold, linux-input, linux-kernel, stable, Peter Ujfalusi,
	Marek Belisko, Rob Herring, devicetree
In-Reply-To: <20171113215452.hqjvxjbef4dt5647@dtor-ws>

On Mon, 13 Nov 2017, Dmitry Torokhov wrote:

> On Mon, Nov 13, 2017 at 12:51:02PM +0100, Johan Hovold wrote:
> > On Mon, Nov 13, 2017 at 10:20:28AM +0000, Lee Jones wrote:
> > > On Mon, 13 Nov 2017, Johan Hovold wrote:
> > > 
> > > > On Mon, Nov 13, 2017 at 09:11:44AM +0000, Lee Jones wrote:
> > > > > On Sun, 12 Nov 2017, Johan Hovold wrote:
> > > > > 
> > > > > > [ +CC: Lee, Rob and device-tree list ]
> > > > > > 
> > > > > > On Sat, Nov 11, 2017 at 09:50:59AM -0800, Dmitry Torokhov wrote:
> > > > > > > On Sat, Nov 11, 2017 at 04:43:37PM +0100, Johan Hovold wrote:
> > > > > > > > A helper purported to look up a child node based on its name was using
> > > > > > > > the wrong of-helper and ended up prematurely freeing the parent of-node
> > > > > > > > while searching the whole device tree depth-first starting at the parent
> > > > > > > > node.
> > > > > > > 
> > > > > > > Ugh, this all is pretty ugly business. Can we teach MFD to allow
> > > > > > > specifying firmware node to be attached to the platform devices it
> > > > > > > creates in mfd_add_device() so that the leaf drivers simply call
> > > > > > > device_property_read_XXX() on their own device and not be bothered with
> > > > > > > weird OF refcount issues or what node they need to locate and parse?
> > > > > 
> > > > > If a child compatible is provided, we already set the child's
> > > > > of_node.  It's then up to the driver (set) author(s) to use it in the
> > > > > correct manner. 
> > > > > 
> > > > > > Yeah, that may have helped. You can actually specify a compatible string
> > > > > > in struct mfd_cell today which does make mfd_add_device() associate a
> > > > > > matching child node.
> > > > > > 
> > > > > > Some best practice regarding how to deal with MFD and device tree would
> > > > > > be good to determine and document too. For example, when should
> > > > > > of_platform_populate() be used in favour of mfd_add_device()?
> > > > > 
> > > > > When the device supports DT and its entire hierarchical layout, along
> > > > > with all of its attributes can be expressed in DT.
> > > > 
> > > > Ok, a follow up: When there are different variants of an MFD and that
> > > > affects the child drivers, then that should be expressed in in the child
> > > > node compatibles rather than having the child match on the parent node?
> > > > 
> > > > I'm asking because this came up recently during review and their seems
> > > > to be no precedent for matching on the parent compatible in child
> > > > drivers:
> > > > 
> > > > 	https://lkml.kernel.org/r/20171105154725.GA11226@localhost
> > > 
> > > Accessing the parent's of_device_id .data directly doesn't sit well
> > > with me.  The parent driver should pass this type of configuration
> > > though pdata IMHO.
> > 
> > The child driver is only matching on the parent-node compatible string
> > IIRC, and therefore keeps its own table of all parent compatibles with
> > its own set of (child) private match data (i.e. the parent compatible
> > property is matched first by the parent driver, and then again by the
> > child).
> > 
> > Passing through pdata here is not possible since mfd_add_device() isn't
> > used, right? It could of course be described using properties of the
> > child node (e.g. by using different child compatible strings).
> > 
> > > > > > And how best to deal with sibling nodes, which is part of the problem
> > > > > > here (I think the mfd should have provided a flag rather than having
> > > > > > subdrivers deal with sibling nodes, for example).
> > > > > 
> > > > > I disagree.  The only properties the MFD (parent) driver is interested
> > > > > in is ones which are shared across multiple child devices.
> > > > > *Everything* which pertains to only a single child device should be
> > > > > handled by its accompanying driver. 
> > > > 
> > > > Even if that means leaking details of one child driver into a sibling?
> > > 
> > > Not sure what you mean here.  Could you please elaborate or provide an
> > > example?
> > 
> > I mean that the sibling node needs to be aware of the name, compatible
> > string, or other node properties of its sibling node to be able to parse
> > sibling nodes itself (rather than the sibling or parent doing so on its
> > behalf). But it seems you answer this below.
> > 
> > > > Isn't it then cleaner to use the parent MFD to coordinate between the
> > > > cells, just as we do for IO?
> > > > 
> > > > In this case a child driver looked up a sibling node based on name, but
> > > 
> > > This should not be allowed.  If >1 sibling requires access to a
> > > particular property, this is normally evidence enough that this
> > > property should be shared and handled by the parent.
> > > 
> > > > that doesn't mean the node is active, that there's a driver bound, or
> > > > that the sibling node has some other random property for example. The
> > > > parent could be used for such coordination, if only to pass information
> > > > from one sibling to another.
> > > 
> > > Right.
> > 
> > Ok, it seems we're in agreement here.
> > 
> > Given that MFD has evolved over time and device-tree support has been
> > added retroactively to some drivers, we've ended up with a multitude of
> > different ways of dealing with such issues. I think it may still be a
> > good idea to jot down some best practices for future driver developers.
> 
> FWIW here is the patch allowing attaching fwnode to an MFD cell that is
> not using of_compatible (because if historical reasons). Completely
> untested as I do not have this hardware.

I am not familiar with the device_* OF implementation, so find it
hard to provide a solid, knowledgeable review.  It looks okay in
principle.

I'd appreciate it if Rob or one of the other DT guys could cast an
eye though.

> If this is somewhat acceptable I can untangle core from twl6040
> changes.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* RE: [v11,1/4] drivers: jtag: Add JTAG core driver
From: Oleksandr Shamray @ 2017-11-14 10:34 UTC (permalink / raw)
  To: 'Chip Bilbrey'
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org,
	jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org,
	tklauser-93Khv+1bN0NyDzI6CaY1VQ@public.gmane.org,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	mec-WqBc5aa1uDFeoWH0uzbU5w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	openocd-devel-owner-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, davem
In-Reply-To: <8760aoz78q.fsf-Gz1Ta9Qd61ZAfugRpC6u6w@public.gmane.org>

> -----Original Message-----
> From: Chip Bilbrey [mailto:chip-Gz1Ta9Qd61ZAfugRpC6u6w@public.gmane.org]
> Sent: Monday, November 6, 2017 12:33 AM
> To: Oleksandr Shamray <oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org; arnd-r2nGTMty4D4@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org;
> jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org; tklauser-93Khv+1bN0NyDzI6CaY1VQ@public.gmane.org; linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> mec-WqBc5aa1uDFeoWH0uzbU5w@public.gmane.org; Vadim Pasternak <vadimp-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; system-sw-low-
> level <system-sw-low-level-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; openocd-
> devel-owner-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Jiri Pirko <jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Subject: Re: [v11,1/4] drivers: jtag: Add JTAG core driver
> 
> 
> Oleksandr Shamray writes:

[..]

> I notice the single-open()-per-device lock was dropped by request in an earlier
> revision of your patches, but multiple processes trying to drive a single JTAG
> master could wreak serious havoc if transactions get interleaved. Would
> something like an added JTAG_LOCKCHAIN/UNLOCKCHAIN
> ioctl() for exclusive client access be reasonable to prevent this?
> 

Yes, it dropped by recommendation of Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>. 

Greg, what you can suggest about it. May be better to add again single-open()-per-device lock with right locking way like:

>if (mutex_lock_interruptible(&jtag->open_lock)) {
>	return -ERESTARTSYS;
>}
>
>if (jtag->opened) {
>	mutex_unlock(&jtag->open_lock);
>	return -EINVAL;
>}
>
>nonseekable_open(inode, file);
>file->private_data = jtag;
>jtag->opened++;
>mutex_unlock(&jtag->open_lock);
>

Thaks.

> -Chip
--
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 v4 5/6] remoteproc: qcom: Add support for q6v5-wcss pil
From: Sricharan R @ 2017-11-14  9:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	ohad-Ix1uc/W3ht7QT0dZR+AlfA, mark.rutland-5wv7dgnIgG8,
	andy.gross-QSEj5FYQhm4dnm+yROfE0A,
	david.brown-QSEj5FYQhm4dnm+yROfE0A,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171110210808.6sms52m4rrgqnf2r@rob-hp-laptop>

Hi Rob,

On 11/11/2017 2:38 AM, Rob Herring wrote:
> On Thu, Nov 09, 2017 at 08:16:14PM +0530, Sricharan R wrote:
>> IPQ8074 has an integrated Hexagon dsp core q6v5 and a wireless lan
>> (Lithium) IP. An mdt type single image format is used for the
>> firmware. So the mdt_load function can be directly used to load
>> the firmware. Also add the relevant resets required for this core.
>>
>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  7 ++-
> 
> Please add acks when posting new versions.

oops.Really sorry, missed it. will update.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

--
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 v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names
From: Fabrizio Castro @ 2017-11-14  9:38 UTC (permalink / raw)
  To: Fabrizio Castro, David Woodhouse, Brian Norris, Boris Brezillon,
	Marek Vasut, Richard Weinberger, Cyrille Pitchen, Rob Herring,
	Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Chris Paterson,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Biju Das,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1508853032-25229-1-git-send-email-fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>

Dear All,

how does this patch look like?

Thanks,
Fabrizio

> Subject: [PATCH v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names
>
> There are a few DT files that make use of sst25vf016b in their
> compatible strings, and the driver supports this chip already.
> This patch improves the documentation and therefore the result
> of ./scripts/checkpatch.pl.
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
> Signed-off-by: Chris Paterson <Chris.Paterson2-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> Thank you Rob, thank you Geert, and sorry for the delay on this one.
> Here is v2.
>
> Changes in v2:
> * fixed subject prefix
> * added changelog
>
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> index 4cab5d8..bf56d77 100644
> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> @@ -31,6 +31,7 @@ Required properties:
>                   s25sl12801
>                   s25fl008k
>                   s25fl064k
> +                 sst25vf016b
>                   sst25vf040b
>                   sst25wf040b
>                   m25p40
> --
> 2.7.4




Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
--
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

* [PATCH v8 6/6] arm: dts: stm32: remove useless clocksource nodes
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

16 bits timers aren't accurate enough to be used as
clocksource, remove them from stm32f4 and stm32f7 devicetree.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/stm32f429.dtsi | 32 --------------------------------
 arch/arm/boot/dts/stm32f746.dtsi | 32 --------------------------------
 2 files changed, 64 deletions(-)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index dd7e99b..ac9a3e6 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -108,14 +108,6 @@
 			};
 		};
 
-		timer3: timer@40000400 {
-			compatible = "st,stm32-timer";
-			reg = <0x40000400 0x400>;
-			interrupts = <29>;
-			clocks = <&rcc 0 STM32F4_APB1_CLOCK(TIM3)>;
-			status = "disabled";
-		};
-
 		timers3: timers@40000400 {
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -137,14 +129,6 @@
 			};
 		};
 
-		timer4: timer@40000800 {
-			compatible = "st,stm32-timer";
-			reg = <0x40000800 0x400>;
-			interrupts = <30>;
-			clocks = <&rcc 0 STM32F4_APB1_CLOCK(TIM4)>;
-			status = "disabled";
-		};
-
 		timers4: timers@40000800 {
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -194,14 +178,6 @@
 			};
 		};
 
-		timer6: timer@40001000 {
-			compatible = "st,stm32-timer";
-			reg = <0x40001000 0x400>;
-			interrupts = <54>;
-			clocks = <&rcc 0 STM32F4_APB1_CLOCK(TIM6)>;
-			status = "disabled";
-		};
-
 		timers6: timers@40001000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -218,14 +194,6 @@
 			};
 		};
 
-		timer7: timer@40001400 {
-			compatible = "st,stm32-timer";
-			reg = <0x40001400 0x400>;
-			interrupts = <55>;
-			clocks = <&rcc 0 STM32F4_APB1_CLOCK(TIM7)>;
-			status = "disabled";
-		};
-
 		timers7: timers@40001400 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index 5633860..a9077e6 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -82,22 +82,6 @@
 			status = "disabled";
 		};
 
-		timer3: timer@40000400 {
-			compatible = "st,stm32-timer";
-			reg = <0x40000400 0x400>;
-			interrupts = <29>;
-			clocks = <&rcc 0 STM32F7_APB1_CLOCK(TIM3)>;
-			status = "disabled";
-		};
-
-		timer4: timer@40000800 {
-			compatible = "st,stm32-timer";
-			reg = <0x40000800 0x400>;
-			interrupts = <30>;
-			clocks = <&rcc 0 STM32F7_APB1_CLOCK(TIM4)>;
-			status = "disabled";
-		};
-
 		timer5: timer@40000c00 {
 			compatible = "st,stm32-timer";
 			reg = <0x40000c00 0x400>;
@@ -105,22 +89,6 @@
 			clocks = <&rcc 0 STM32F7_APB1_CLOCK(TIM5)>;
 		};
 
-		timer6: timer@40001000 {
-			compatible = "st,stm32-timer";
-			reg = <0x40001000 0x400>;
-			interrupts = <54>;
-			clocks = <&rcc 0 STM32F7_APB1_CLOCK(TIM6)>;
-			status = "disabled";
-		};
-
-		timer7: timer@40001400 {
-			compatible = "st,stm32-timer";
-			reg = <0x40001400 0x400>;
-			interrupts = <55>;
-			clocks = <&rcc 0 STM32F7_APB1_CLOCK(TIM7)>;
-			status = "disabled";
-		};
-
 		rtc: rtc@40002800 {
 			compatible = "st,stm32-rtc";
 			reg = <0x40002800 0x400>;
-- 
2.7.4

--
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

* [PATCH v8 5/6] clocksource: stm32: add clocksource support
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

The stm32 timer hardware is currently only used as a clock event device,
but it can be utilized as a clocksource as well.

Implement this by enabling the free running counter in the hardware block
and converting the clock event part from a count down event timer to a
comparator based timer.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clocksource/timer-stm32.c | 116 +++++++++++++++++++++++++++++---------
 1 file changed, 88 insertions(+), 28 deletions(-)

diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
index 8173bcf..c0a62cd 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -16,6 +16,8 @@
 #include <linux/of_irq.h>
 #include <linux/clk.h>
 #include <linux/reset.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
 
 #include "timer-of.h"
 
@@ -23,16 +25,16 @@
 #define TIM_DIER	0x0c
 #define TIM_SR		0x10
 #define TIM_EGR		0x14
+#define TIM_CNT		0x24
 #define TIM_PSC		0x28
 #define TIM_ARR		0x2c
+#define TIM_CCR1	0x34
 
 #define TIM_CR1_CEN	BIT(0)
-#define TIM_CR1_OPM	BIT(3)
+#define TIM_CR1_UDIS	BIT(1)
 #define TIM_CR1_ARPE	BIT(7)
 
-#define TIM_DIER_UIE	BIT(0)
-
-#define TIM_SR_UIF	BIT(0)
+#define TIM_DIER_CC1IE	BIT(1)
 
 #define TIM_EGR_UG	BIT(0)
 
@@ -42,28 +44,44 @@ static int stm32_clock_event_shutdown(struct clock_event_device *evt)
 {
 	struct timer_of *to = to_timer_of(evt);
 
-	writel_relaxed(0, timer_of_base(to) + TIM_CR1);
+	writel_relaxed(0, timer_of_base(to) + TIM_DIER);
+
 	return 0;
 }
 
-static int stm32_clock_event_set_periodic(struct clock_event_device *evt)
+static int stm32_clock_event_set_next_event(unsigned long evt,
+					    struct clock_event_device *clkevt)
 {
-	struct timer_of *to = to_timer_of(evt);
+	struct timer_of *to = to_timer_of(clkevt);
+	unsigned long now, next;
 
-	writel_relaxed(timer_of_period(to), timer_of_base(to) + TIM_ARR);
-	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, timer_of_base(to) + TIM_CR1);
+	next = readl_relaxed(timer_of_base(to) + TIM_CNT) + evt;
+	writel_relaxed(next, timer_of_base(to) + TIM_CCR1);
+	now = readl_relaxed(timer_of_base(to) + TIM_CNT);
+
+	if ((next - now) > evt)
+		return -ETIME;
+
+	writel_relaxed(TIM_DIER_CC1IE, timer_of_base(to) + TIM_DIER);
 
 	return 0;
 }
 
-static int stm32_clock_event_set_next_event(unsigned long evt,
-					    struct clock_event_device *clkevt)
+static int stm32_clock_event_set_periodic(struct clock_event_device *evt)
 {
-	struct timer_of *to = to_timer_of(clkevt);
+	struct timer_of *to = to_timer_of(evt);
 
-	writel_relaxed(evt, timer_of_base(to) + TIM_ARR);
-	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_OPM | TIM_CR1_CEN,
-		       timer_of_base(to) + TIM_CR1);
+	return stm32_clock_event_set_next_event(timer_of_period(to), evt);
+}
+
+static int stm32_clock_event_set_oneshot(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+	unsigned long val;
+
+	val = readl_relaxed(timer_of_base(to) + TIM_CNT);
+	writel_relaxed(val, timer_of_base(to) + TIM_CCR1);
+	writel_relaxed(TIM_DIER_CC1IE, timer_of_base(to) + TIM_DIER);
 
 	return 0;
 }
@@ -75,12 +93,57 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
 
 	writel_relaxed(0, timer_of_base(to) + TIM_SR);
 
+	if (clockevent_state_periodic(evt))
+		stm32_clock_event_set_periodic(evt);
+	else
+		stm32_clock_event_shutdown(evt);
+
 	evt->event_handler(evt);
 
 	return IRQ_HANDLED;
 }
 
-static int __init stm32_clockevent_init(struct device_node *node)
+static void __init stm32_clockevent_init(struct timer_of *to)
+{
+	writel_relaxed(0, timer_of_base(to) + TIM_DIER);
+	writel_relaxed(0, timer_of_base(to) + TIM_SR);
+
+	clockevents_config_and_register(&to->clkevt,
+					timer_of_rate(to), MIN_DELTA, ~0U);
+}
+
+static void __iomem *stm32_timer_cnt __read_mostly;
+static u64 notrace stm32_read_sched_clock(void)
+{
+	return readl_relaxed(stm32_timer_cnt);
+}
+
+static int __init stm32_clocksource_init(struct timer_of *to)
+{
+	writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);
+	writel_relaxed(0, timer_of_base(to) + TIM_PSC);
+	writel_relaxed(0, timer_of_base(to) + TIM_SR);
+	writel_relaxed(0, timer_of_base(to) + TIM_DIER);
+	writel_relaxed(0, timer_of_base(to) + TIM_SR);
+	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_UDIS,
+		       timer_of_base(to) + TIM_CR1);
+
+	/* Make sure that registers are updated */
+	writel_relaxed(TIM_EGR_UG, timer_of_base(to) + TIM_EGR);
+
+	/* Enable controller */
+	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_UDIS | TIM_CR1_CEN,
+		       timer_of_base(to) + TIM_CR1);
+
+	stm32_timer_cnt = timer_of_base(to) + TIM_CNT;
+	sched_clock_register(stm32_read_sched_clock, 32, timer_of_rate(to));
+
+	return clocksource_mmio_init(stm32_timer_cnt, "stm32_timer",
+				     timer_of_rate(to), 250, 32,
+				     clocksource_mmio_readl_up);
+}
+
+static int __init stm32_timer_init(struct device_node *node)
 {
 	struct reset_control *rstc;
 	unsigned long max_arr;
@@ -92,12 +155,13 @@ static int __init stm32_clockevent_init(struct device_node *node)
 		return -ENOMEM;
 
 	to->flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE;
+
 	to->clkevt.name = "stm32_clockevent";
 	to->clkevt.rating = 200;
-	to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;
+	to->clkevt.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC;
 	to->clkevt.set_state_shutdown = stm32_clock_event_shutdown;
 	to->clkevt.set_state_periodic = stm32_clock_event_set_periodic;
-	to->clkevt.set_state_oneshot = stm32_clock_event_shutdown;
+	to->clkevt.set_state_oneshot = stm32_clock_event_set_oneshot;
 	to->clkevt.tick_resume = stm32_clock_event_shutdown;
 	to->clkevt.set_next_event = stm32_clock_event_set_next_event;
 
@@ -122,23 +186,19 @@ static int __init stm32_clockevent_init(struct device_node *node)
 		goto deinit;
 	}
 
-	writel_relaxed(0, timer_of_base(to) + TIM_ARR);
-
-	writel_relaxed(0, timer_of_base(to) + TIM_PSC);
-	writel_relaxed(TIM_EGR_UG, timer_of_base(to) + TIM_EGR);
-	writel_relaxed(TIM_DIER_UIE, timer_of_base(to) + TIM_DIER);
-	writel_relaxed(0, timer_of_base(to) + TIM_SR);
+	ret = stm32_clocksource_init(to);
+	if (ret)
+		goto deinit;
 
-	clockevents_config_and_register(&to->clkevt,
-					timer_of_period(to), MIN_DELTA, ~0U);
+	stm32_clockevent_init(to);
 
 	return 0;
 
 deinit:
-	timer_of_exit(to);
+	timer_of_cleanup(to);
 err:
 	kfree(to);
 	return ret;
 }
 
-TIMER_OF_DECLARE(stm32, "st,stm32-timer", stm32_clockevent_init);
+TIMER_OF_DECLARE(stm32, "st,stm32-timer", stm32_timer_init);
-- 
2.7.4

--
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

* [PATCH v8 4/6] clocksource: stm32: only use 32 bits timers
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

The clock driving counters is at 90MHz so the maximum period
for 16 bis counters is around 750 ms which is a short period
for a clocksource. For 32 bits counters this period is close
47 secondes which is more acceptable.

This patch remove 16 bits counters support and makes sure that
they won't be probed anymore.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clocksource/timer-stm32.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
index ae41a19..8173bcf 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -83,9 +83,9 @@ static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
 static int __init stm32_clockevent_init(struct device_node *node)
 {
 	struct reset_control *rstc;
-	unsigned long max_delta;
-	int ret, bits, prescaler = 1;
+	unsigned long max_arr;
 	struct timer_of *to;
+	int ret;
 
 	to = kzalloc(sizeof(*to), GFP_KERNEL);
 	if (!to)
@@ -115,29 +115,27 @@ static int __init stm32_clockevent_init(struct device_node *node)
 
 	/* Detect whether the timer is 16 or 32 bits */
 	writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);
-	max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);
-	if (max_delta == ~0U) {
-		prescaler = 1;
-		bits = 32;
-	} else {
-		prescaler = 1024;
-		bits = 16;
+	max_arr = readl_relaxed(timer_of_base(to) + TIM_ARR);
+	if (max_arr != ~0U) {
+		pr_err("32 bits timer is needed\n");
+		ret = -EINVAL;
+		goto deinit;
 	}
+
 	writel_relaxed(0, timer_of_base(to) + TIM_ARR);
 
-	writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);
+	writel_relaxed(0, timer_of_base(to) + TIM_PSC);
 	writel_relaxed(TIM_EGR_UG, timer_of_base(to) + TIM_EGR);
 	writel_relaxed(TIM_DIER_UIE, timer_of_base(to) + TIM_DIER);
 	writel_relaxed(0, timer_of_base(to) + TIM_SR);
 
 	clockevents_config_and_register(&to->clkevt,
-					timer_of_period(to), MIN_DELTA, max_delta);
-
-	pr_info("%pOF: STM32 clockevent driver initialized (%d bits)\n",
-			node, bits);
+					timer_of_period(to), MIN_DELTA, ~0U);
 
 	return 0;
 
+deinit:
+	timer_of_exit(to);
 err:
 	kfree(to);
 	return ret;
-- 
2.7.4

--
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

* [PATCH v8 3/6] clocksource: stm32: increase min delta value
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

The CPU is a CortexM4 @ 200MHZ and the clocks driving
the timers are at 90MHZ with a min delta at 1 you could
have an interrupt each 0.01 ms which is really to much.
By increase it to 0x60 it give more time (around 1 ms)
to CPU to handle the interrupt.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clocksource/timer-stm32.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
index fc61fd1..ae41a19 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -36,6 +36,8 @@
 
 #define TIM_EGR_UG	BIT(0)
 
+#define MIN_DELTA	0x60
+
 static int stm32_clock_event_shutdown(struct clock_event_device *evt)
 {
 	struct timer_of *to = to_timer_of(evt);
@@ -129,7 +131,7 @@ static int __init stm32_clockevent_init(struct device_node *node)
 	writel_relaxed(0, timer_of_base(to) + TIM_SR);
 
 	clockevents_config_and_register(&to->clkevt,
-					timer_of_period(to), 0x1, max_delta);
+					timer_of_period(to), MIN_DELTA, max_delta);
 
 	pr_info("%pOF: STM32 clockevent driver initialized (%d bits)\n",
 			node, bits);
-- 
2.7.4

--
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

* [PATCH v8 2/6] clocksource: stm32: convert driver to timer_of
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Convert driver to use timer_of helpers. This allow to remove
custom proprietary structure.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clocksource/Kconfig       |   1 +
 drivers/clocksource/timer-stm32.c | 160 ++++++++++++++------------------------
 2 files changed, 58 insertions(+), 103 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index c729a88..28bc5595 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -269,6 +269,7 @@ config CLKSRC_STM32
 	bool "Clocksource for STM32 SoCs" if !ARCH_STM32
 	depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
 	select CLKSRC_MMIO
+	select TIMER_OF
 
 config CLKSRC_MPS2
 	bool "Clocksource for MPS2 SoCs" if COMPILE_TEST
diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
index 8f24237..fc61fd1 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -17,6 +17,8 @@
 #include <linux/clk.h>
 #include <linux/reset.h>
 
+#include "timer-of.h"
+
 #define TIM_CR1		0x00
 #define TIM_DIER	0x0c
 #define TIM_SR		0x10
@@ -34,117 +36,84 @@
 
 #define TIM_EGR_UG	BIT(0)
 
-struct stm32_clock_event_ddata {
-	struct clock_event_device evtdev;
-	unsigned periodic_top;
-	void __iomem *base;
-};
-
-static int stm32_clock_event_shutdown(struct clock_event_device *evtdev)
+static int stm32_clock_event_shutdown(struct clock_event_device *evt)
 {
-	struct stm32_clock_event_ddata *data =
-		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
-	void *base = data->base;
+	struct timer_of *to = to_timer_of(evt);
 
-	writel_relaxed(0, base + TIM_CR1);
+	writel_relaxed(0, timer_of_base(to) + TIM_CR1);
 	return 0;
 }
 
-static int stm32_clock_event_set_periodic(struct clock_event_device *evtdev)
+static int stm32_clock_event_set_periodic(struct clock_event_device *evt)
 {
-	struct stm32_clock_event_ddata *data =
-		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
-	void *base = data->base;
+	struct timer_of *to = to_timer_of(evt);
+
+	writel_relaxed(timer_of_period(to), timer_of_base(to) + TIM_ARR);
+	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, timer_of_base(to) + TIM_CR1);
 
-	writel_relaxed(data->periodic_top, base + TIM_ARR);
-	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, base + TIM_CR1);
 	return 0;
 }
 
 static int stm32_clock_event_set_next_event(unsigned long evt,
-					    struct clock_event_device *evtdev)
+					    struct clock_event_device *clkevt)
 {
-	struct stm32_clock_event_ddata *data =
-		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
+	struct timer_of *to = to_timer_of(clkevt);
 
-	writel_relaxed(evt, data->base + TIM_ARR);
+	writel_relaxed(evt, timer_of_base(to) + TIM_ARR);
 	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_OPM | TIM_CR1_CEN,
-		       data->base + TIM_CR1);
+		       timer_of_base(to) + TIM_CR1);
 
 	return 0;
 }
 
 static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
 {
-	struct stm32_clock_event_ddata *data = dev_id;
+	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+	struct timer_of *to = to_timer_of(evt);
 
-	writel_relaxed(0, data->base + TIM_SR);
+	writel_relaxed(0, timer_of_base(to) + TIM_SR);
 
-	data->evtdev.event_handler(&data->evtdev);
+	evt->event_handler(evt);
 
 	return IRQ_HANDLED;
 }
 
-static struct stm32_clock_event_ddata clock_event_ddata = {
-	.evtdev = {
-		.name = "stm32 clockevent",
-		.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
-		.set_state_shutdown = stm32_clock_event_shutdown,
-		.set_state_periodic = stm32_clock_event_set_periodic,
-		.set_state_oneshot = stm32_clock_event_shutdown,
-		.tick_resume = stm32_clock_event_shutdown,
-		.set_next_event = stm32_clock_event_set_next_event,
-		.rating = 200,
-	},
-};
-
-static int __init stm32_clockevent_init(struct device_node *np)
+static int __init stm32_clockevent_init(struct device_node *node)
 {
-	struct stm32_clock_event_ddata *data = &clock_event_ddata;
-	struct clk *clk;
 	struct reset_control *rstc;
-	unsigned long rate, max_delta;
-	int irq, ret, bits, prescaler = 1;
-
-	clk = of_clk_get(np, 0);
-	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		pr_err("failed to get clock for clockevent (%d)\n", ret);
-		goto err_clk_get;
-	}
-
-	ret = clk_prepare_enable(clk);
-	if (ret) {
-		pr_err("failed to enable timer clock for clockevent (%d)\n",
-		       ret);
-		goto err_clk_enable;
-	}
-
-	rate = clk_get_rate(clk);
-
-	rstc = of_reset_control_get(np, NULL);
+	unsigned long max_delta;
+	int ret, bits, prescaler = 1;
+	struct timer_of *to;
+
+	to = kzalloc(sizeof(*to), GFP_KERNEL);
+	if (!to)
+		return -ENOMEM;
+
+	to->flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE;
+	to->clkevt.name = "stm32_clockevent";
+	to->clkevt.rating = 200;
+	to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC;
+	to->clkevt.set_state_shutdown = stm32_clock_event_shutdown;
+	to->clkevt.set_state_periodic = stm32_clock_event_set_periodic;
+	to->clkevt.set_state_oneshot = stm32_clock_event_shutdown;
+	to->clkevt.tick_resume = stm32_clock_event_shutdown;
+	to->clkevt.set_next_event = stm32_clock_event_set_next_event;
+
+	to->of_irq.handler = stm32_clock_event_handler;
+
+	ret = timer_of_init(node, to);
+	if (ret)
+		goto err;
+
+	rstc = of_reset_control_get(node, NULL);
 	if (!IS_ERR(rstc)) {
 		reset_control_assert(rstc);
 		reset_control_deassert(rstc);
 	}
 
-	data->base = of_iomap(np, 0);
-	if (!data->base) {
-		ret = -ENXIO;
-		pr_err("failed to map registers for clockevent\n");
-		goto err_iomap;
-	}
-
-	irq = irq_of_parse_and_map(np, 0);
-	if (!irq) {
-		ret = -EINVAL;
-		pr_err("%pOF: failed to get irq.\n", np);
-		goto err_get_irq;
-	}
-
 	/* Detect whether the timer is 16 or 32 bits */
-	writel_relaxed(~0U, data->base + TIM_ARR);
-	max_delta = readl_relaxed(data->base + TIM_ARR);
+	writel_relaxed(~0U, timer_of_base(to) + TIM_ARR);
+	max_delta = readl_relaxed(timer_of_base(to) + TIM_ARR);
 	if (max_delta == ~0U) {
 		prescaler = 1;
 		bits = 32;
@@ -152,38 +121,23 @@ static int __init stm32_clockevent_init(struct device_node *np)
 		prescaler = 1024;
 		bits = 16;
 	}
-	writel_relaxed(0, data->base + TIM_ARR);
-
-	writel_relaxed(prescaler - 1, data->base + TIM_PSC);
-	writel_relaxed(TIM_EGR_UG, data->base + TIM_EGR);
-	writel_relaxed(TIM_DIER_UIE, data->base + TIM_DIER);
-	writel_relaxed(0, data->base + TIM_SR);
+	writel_relaxed(0, timer_of_base(to) + TIM_ARR);
 
-	data->periodic_top = DIV_ROUND_CLOSEST(rate, prescaler * HZ);
+	writel_relaxed(prescaler - 1, timer_of_base(to) + TIM_PSC);
+	writel_relaxed(TIM_EGR_UG, timer_of_base(to) + TIM_EGR);
+	writel_relaxed(TIM_DIER_UIE, timer_of_base(to) + TIM_DIER);
+	writel_relaxed(0, timer_of_base(to) + TIM_SR);
 
-	clockevents_config_and_register(&data->evtdev,
-					DIV_ROUND_CLOSEST(rate, prescaler),
-					0x1, max_delta);
-
-	ret = request_irq(irq, stm32_clock_event_handler, IRQF_TIMER,
-			"stm32 clockevent", data);
-	if (ret) {
-		pr_err("%pOF: failed to request irq.\n", np);
-		goto err_get_irq;
-	}
+	clockevents_config_and_register(&to->clkevt,
+					timer_of_period(to), 0x1, max_delta);
 
 	pr_info("%pOF: STM32 clockevent driver initialized (%d bits)\n",
-			np, bits);
+			node, bits);
 
-	return ret;
+	return 0;
 
-err_get_irq:
-	iounmap(data->base);
-err_iomap:
-	clk_disable_unprepare(clk);
-err_clk_enable:
-	clk_put(clk);
-err_clk_get:
+err:
+	kfree(to);
 	return ret;
 }
 
-- 
2.7.4

--
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

* [PATCH v8 1/6] clocksource: timer_of: rename timer_of_exit to timer_of_cleanup
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard
In-Reply-To: <1510649563-22975-1-git-send-email-benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Change function name to something more explicit since it is only
used in init error cases.
Add __init annotation and description about the function usage.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clocksource/timer-of.c | 9 ++++++++-
 drivers/clocksource/timer-of.h | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 7c64a5c1..a319904 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -177,7 +177,14 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
 	return ret;
 }
 
-void timer_of_exit(struct timer_of *to)
+/**
+ * timer_of_cleanup - release timer_of ressources
+ * @to: timer_of structure
+ *
+ * Release the ressources that has been used in timer_of_init().
+ * This function should be called in init error cases
+ */
+void __init timer_of_cleanup(struct timer_of *to)
 {
 	if (to->flags & TIMER_OF_IRQ)
 		timer_irq_exit(&to->of_irq);
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index 44f57e0..f521477 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -67,6 +67,6 @@ static inline unsigned long timer_of_period(struct timer_of *to)
 extern int __init timer_of_init(struct device_node *np,
 				struct timer_of *to);
 
-extern void timer_of_exit(struct timer_of *to);
+extern void __init timer_of_cleanup(struct timer_of *to);
 
 #endif
-- 
2.7.4

--
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

* [PATCH v8 0/6] stm32 clocksource driver rework
From: Benjamin Gaignard @ 2017-11-14  8:52 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ, ludovic.barre-qxv4g6HH51o,
	julien.thierry-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	arnd-r2nGTMty4D4
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Benjamin Gaignard

version 8:
 - rebased on timers/core
 - change timer_of_exit() name to timer_of_cleanup()
 - update stm32 clocksource driver to use this function

version 7:
 - reword "clocksource: stm32: only use 32 bits timers" commit message
   to give more details about why 16 bits are problematics.

version 6:
 - add dedicated patch min delta change
 - rework commit messages, I hope it will be better now
 - change new function name from timer_of_deinit to timer_of_exit
 - make stm32_clock_event_set_next_event() safer like done in other
   drivers

version 6:
- add timer_of_deinit function in core
- rework failure cases in probe function

version 5:
- rebase on top of timer/core branch
- rework commit message of the first patch

version 4:
- split patch in 3 parts
  - convert code to timer_of
  - only use 32 bits timers
  - add clocksource support

version 3:
- fix comments done by Daniel
- use timer_of helper functions

version 2:
- fix uninitialized variable

Benjamin Gaignard (6):
  clocksource: timer_of: rename timer_of_exit to timer_of_cleanup
  clocksource: stm32: convert driver to timer_of
  clocksource: stm32: increase min delta value
  clocksource: stm32: only use 32 bits timers
  clocksource: stm32: add clocksource support
  arm: dts: stm32: remove useless clocksource nodes

 arch/arm/boot/dts/stm32f429.dtsi  |  32 -----
 arch/arm/boot/dts/stm32f746.dtsi  |  32 -----
 drivers/clocksource/Kconfig       |   1 +
 drivers/clocksource/timer-of.c    |   9 +-
 drivers/clocksource/timer-of.h    |   2 +-
 drivers/clocksource/timer-stm32.c | 242 ++++++++++++++++++++------------------
 6 files changed, 138 insertions(+), 180 deletions(-)

-- 
2.7.4

--
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 2/2] pinctrl: sh-pfc: add R8A77970 PFC support
From: Geert Uytterhoeven @ 2017-11-14  8:48 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Linus Walleij, Rob Herring, Laurent Pinchart, Geert Uytterhoeven,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	Linux-Renesas, Mark Rutland, Vladimir Barinov
In-Reply-To: <20171110181552.227541251@cogentembedded.com>

Hi Sergei,

On Fri, Nov 10, 2017 at 6:59 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the PFC support for the R8A77970 SoC including pin groups for some
> on-chip devices such as CAN-FD, EtherAVB, [H]SCIF, I2C, INTC-EX, MMC,
> MSIOF, PWM, VIN...
>
> Based on the original (and large) patch by Daisuke Matsushita
> <daisuke.matsushita.ns@hitachi.com>.
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> --- /dev/null
> +++ renesas-drivers/drivers/pinctrl/sh-pfc/pfc-r8a77970.c

> +/* - EtherAVB --------------------------------------------------------------- */

> +static const unsigned int avb0_mdio_pins[] = {
> +       /* AVB0_MDC, AVB0_MDIO */
> +       RCAR_GP_PIN(1, 15), RCAR_GP_PIN(1, 14),
> +};
> +static const unsigned int avb0_mdio_mux[] = {
> +       AVB0_MDC_MARK, AVB0_MDIO_MARK,
> +};

Upon second look: this group is called "avb(0)_mdc" instead of "avb(0)_mdio"
on all other R-Car Gen3 SoCs?

Gr{oetje,eeting}s,

                        Geert

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

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

^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: renesas: eagle: add EtherAVB pins
From: Geert Uytterhoeven @ 2017-11-14  8:44 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
	Linux-Renesas, devicetree@vger.kernel.org, Magnus Damm,
	Mark Rutland, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20171110200714.406365231@cogentembedded.com>

Hi Sergei,

On Fri, Nov 10, 2017 at 9:02 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the (previously omitted) EtherAVB pin data to the Eagle board's
> device tree.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks for your patch!

> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> @@ -34,6 +34,9 @@
>  };
>
>  &avb {
> +       pinctrl-0 = <&avb_pins>;
> +       pinctrl-names = "default";
> +
>         renesas,no-ether-link;
>         phy-handle = <&phy0>;
>         status = "okay";
> @@ -53,6 +56,11 @@
>  };
>
>  &pfc {
> +       avb_pins: avb {
> +               groups = "avb0_mdio", "avb0_mii";

Oh no, its'called "avb0_mdio" here, but "avb(0)_mdc" on all other
R-Car Gen3 SoCs?

What about "avb0_link", which is connected through "L_LNK"?

> +               function = "avb0";
> +       };
> +

Gr{oetje,eeting}s,

                        Geert

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

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

^ permalink raw reply

* Re: [PATCH 2/3] arm64: dts: renesas: eagle: add SCIF0 pins
From: Geert Uytterhoeven @ 2017-11-14  8:34 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
	Linux-Renesas, devicetree@vger.kernel.org, Magnus Damm,
	Mark Rutland, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20171110200710.769271617@cogentembedded.com>

On Fri, Nov 10, 2017 at 9:02 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Add the (previously omitted) SCIF0 pin data to the Eagle board's
> device tree.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

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

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

^ permalink raw reply

* Re: [PATCH 1/3] arm64: dts: renesas: r8a77970: add PFC support
From: Geert Uytterhoeven @ 2017-11-14  8:27 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Simon Horman, Rob Herring, Catalin Marinas, Will Deacon,
	Linux-Renesas, devicetree@vger.kernel.org, Magnus Damm,
	Mark Rutland, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20171110200705.772746681@cogentembedded.com>

On Fri, Nov 10, 2017 at 9:02 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the generic R8A77970 part of the PFC device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

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

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

^ permalink raw reply

* RE: [PATCH v2] arm64: dts: ls1088a: Add USB support
From: Yinbo Zhu @ 2017-11-14  8:00 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Rob Herring, Mark Rutland, Catalin Marinas ), Will Deacon ),
	Harninder Rai, Raghav Dogra, Ashish Kumar, Andy Tang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	open list
In-Reply-To: <VI1PR04MB1262D160BA8625330E32CAF9E9470-mr6QIVyDiCGbtYzA8xQqo89NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>



-----Original Message-----
From: Yinbo Zhu 
Sent: Tuesday, October 24, 2017 5:15 PM
To: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; Catalin Marinas ) <catalin.marinas-5wv7dgnIgG8@public.gmane.org>; Will Deacon ) <will.deacon@arm.com>; Harninder Rai <harninder.rai-3arQi8VN3Tc@public.gmane.org>; Raghav Dogra <raghav.dogra@nxp.com>; Ashish Kumar <ashish.kumar-3arQi8VN3Tc@public.gmane.org>; Andy Tang <andy.tang-3arQi8VN3Tc@public.gmane.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; open list <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] arm64: dts: ls1088a: Add USB support


-----Original Message-----
From: Shawn Guo [mailto:shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org] 
Sent: Friday, September 22, 2017 2:55 PM
To: Yinbo Zhu <yinbo.zhu-3arQi8VN3Tc@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; Catalin Marinas ) <catalin.marinas-5wv7dgnIgG8@public.gmane.org>; Will Deacon ) <will.deacon@arm.com>; Harninder Rai <harninder.rai-3arQi8VN3Tc@public.gmane.org>; Raghav Dogra <raghav.dogra@nxp.com>; Ashish Kumar <ashish.kumar-3arQi8VN3Tc@public.gmane.org>; Andy Tang <andy.tang-3arQi8VN3Tc@public.gmane.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] arm64: dts: ls1088a: Add USB support

On Wed, Sep 13, 2017 at 05:10:09PM +0800, yinbo.zhu-3arQi8VN3Tc@public.gmane.org wrote:
> From: "yinbo.zhu" <yinbo.zhu-3arQi8VN3Tc@public.gmane.org>
> 
> Fix the issue that usb is not detected on ls1088ardb

>It's not really about fixing issue but adding support.

The patch had been tested on upstream 4.14 code, it can fix the issue. 
> 
> Signed-off-by: yinbo.zhu <yinbo.zhu-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Ran Wang <ran.wang_1-3arQi8VN3Tc@public.gmane.org>
> ---

>You should better have a version history here to tell what's changed between version.

>I will add a version history on next v3 patch
Hi,

I had modified the code as v4 version, 
https://patchwork.kernel.org/patch/10027393/
please check.

Thanks,
BRs
>  arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts |  8 ++++++++
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi    | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts 
> b/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
> index 213abb72de93..6c3c3bc4b681 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a-rdb.dts
> @@ -118,6 +118,14 @@
>  	status = "okay";
>  };
>  
> +&usb0 {
> +	status = "okay";
> +};
> +
> +&usb1 {
> +	status = "okay";
> +};
> +
>  &esdhc {

>Please sort these labeled nodes alphabetically.

>Shawn

>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi 
> b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index c144d06a6e33..c23fede8cf5d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -359,6 +359,26 @@
>  			status = "disabled";
>  		};
>  
> +		usb0: usb3@3100000 {
> +			compatible = "snps,dwc3";
> +			reg = <0x0 0x3100000 0x0 0x10000>;
> +			interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
> +			dr_mode = "host";
> +			snps,quirk-frame-length-adjustment = <0x20>;
> +			snps,dis_rxdet_inp3_quirk;
> +			status = "disabled";
> +		};
> +
> +		usb1: usb3@3110000 {
> +			compatible = "snps,dwc3";
> +			reg = <0x0 0x3110000 0x0 0x10000>;
> +			interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
> +			dr_mode = "host";
> +			snps,quirk-frame-length-adjustment = <0x20>;
> +			snps,dis_rxdet_inp3_quirk;
> +			status = "disabled";
> +		};
> +
>  		sata: sata@3200000 {
>  			compatible = "fsl,ls1088a-ahci";
>  			reg = <0x0 0x3200000 0x0 0x10000>,
> --
> 2.14.1
> 
--
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 0/2] backlight: pwm_bl: prevent backlight flicker when switching PWM on
From: Lothar Waßmann @ 2017-11-14  7:56 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Daniel Thompson, Bartlomiej Zolnierkiewicz, Jacek Anaszewski,
	Jingoo Han, Lee Jones, Mark Rutland, Pavel Machek, Richard Purdie,
	Rob Herring, Thierry Reding, devicetree, linux-fbdev,
	linux-kernel, linux-leds, linux-pwm
In-Reply-To: <8682e9c4-518d-5dce-55c5-c221dddd5fd6@collabora.com>

Hi,

On Fri, 10 Nov 2017 12:22:15 +0100 Enric Balletbo i Serra wrote:
> Hi all,
> 
> On 08/11/17 11:48, Daniel Thompson wrote:
> > On 26/10/17 13:49, Lothar Waßmann wrote:
> >> These patches implement some measures to prevent backlight flicker
> >> when the backlight is being switched on for a display with an active
> >> low brightness control pin.
> >> GIT: [PATCH 1/2] backlight: pwm_bl: Enable PWM before switching regulator
> >> GIT: [PATCH 2/2] backlight: pwm_bl: add configurable delay between
> > 
> > Other than hoisting the pwm_enable() even earlier in the setup sequence this
> > patchset seems to have a significant overlap with Enric's recent posting:
> > 
> >   https://lkml.org/lkml/2017/7/21/211
> > 
> > Any chance of a shared view on this, especially on the DT bindings?
> > 
> 
> The DT binding were discussed some time ago for the patch series I sent, though
> there isn't a final ack from DT maintainer.
> 
> Lothar, the series I sent have been reviewed and acked, can you test if the
> series fixes the problem for you too?
> 
I'll try to test it within the next couple of days and will report back.


Lothar Waßmann

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: phy: Add Cygnus usb phy binding
From: Raveendra Padasalagi @ 2017-11-14  5:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Kishon Vijay Abraham I, Russell King, Scott Branden,
	Ray Jui, Srinath Mannam, Vikram Prakash, Jon Mason,
	Florian Fainelli, Yoshihiro Shimoda, Raviteja Garimella,
	Rafal Milecki, Arnd Bergmann, Viresh Kumar, Jaehoon Chung,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-ke
In-Reply-To: <CAL_JsqL+KaxAHb_u31U07-e+Nm2cW=Wi-oHyTP42B--hBiEyig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On Mon, Nov 13, 2017 at 11:23 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sun, Nov 12, 2017 at 10:23 PM, Raveendra Padasalagi
> <raveendra.padasalagi-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> Hi,
>>
>> On Sat, Nov 11, 2017 at 3:14 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Nov 08, 2017 at 01:16:41PM +0530, Raveendra Padasalagi wrote:
>>>> Add devicetree binding document for broadcom's
>>>> Cygnus SoC specific usb phy controller driver.
>>>>
>>>> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>>  .../bindings/phy/brcm,cygnus-usb-phy.txt           | 106 +++++++++++++++++++++
>>>>  1 file changed, 106 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>>> new file mode 100644
>>>> index 0000000..bbc4b94
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>>> @@ -0,0 +1,106 @@
>>>> +BROADCOM CYGNUS USB PHY
>>>> +
>>>> +Required Properties:
>>>> +- compatible:  brcm,cygnus-usb-phy
>>>> +- reg : the register start address and length for
>>>> +        crmu_usbphy_aon_ctrl,
>>>> +        cdru usb phy control,
>>>> +        usb host idm registers,
>>>> +        usb device idm registers.
>>>> +- reg-names: a list of the names corresponding to the previous register ranges
>>>> +  Should contain
>>>> +        "crmu-usbphy-aon-ctrl",
>>>> +        "cdru-usbphy",
>>>> +        "usb2h-idm",
>>>> +        "usb2d-idm".
>>>> +- address-cells: should be 1
>>>> +- size-cells: should be 0
>>>> +
>>>> +Sub-nodes:
>>>> +  Each port's PHY should be represented as a sub-node.
>>>> +
>>>> +Sub-nodes required properties:
>>>> +- reg: the PHY number
>>>> +- #phy-cells must be 1
>>>> +  The node that uses the phy must provide 1 integer argument specifying
>>>> +  port number.
>>>> +
>>>> +Optional Properties:
>>>> +- vbus-p#-supply : The regulator for vbus out control for the host
>>>
>>> Is this a literal # or something else?
>>
>> Yes, this is a literal. It's assumed # will replace numeric 0-2 for
>> each of the ports.
>
> I'm still confused. Which is valid? "vbus-p#-supply" or "vbus-p0-supply"
>
I agree, it's creating confusion. Instead of enumerating
"vbus-p0-supply", "vbus-p1-supply", "vbus-p2-supply" kept "vbus-p#-supply".

Yes, as suggested by you "vbus-supply" should be sufficient as it's in each
of phy sub node.

> If the latter, you need to enumerate all valid options. But these are
> in sub nodes for each port, so just "vbus-supply" should be
> sufficient.

Keeping "vbus-supply" should not create any confusion. Will send out the
change in next version of the patch.

> One more question, does Vbus actually supply power to the phy or you
> are just associating the Vbus supply to a connector with a port? The
> latter needs a connector node instead and Vbus should be part of that.
> There's been some attempts at USB connectors, but we don't really have
> one yet (the extcon binding is not it).

Vbus is not supplied to phy, it's been given to the devices connected on
the port and in our platform vbus is controlled through an external regulator
which is controlled through gpio.
So "vbus-supply" shown above actually points to the phandle of vbus regulator
node.

>> In the example it's not shown as the regulators specified in vbus-p#-supply
>> are board specific.
>
> Please show in the example. Examples should be complete.

Ok. Sure.

> Rob
--
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 v9 3/7] mailbox: qcom: Move the apcs struct into a separate header
From: Bjorn Andersson @ 2017-11-14  4:47 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Georgi Djakov, jassisinghbrar, robh+dt, mturquette, linux-clk,
	linux-kernel, linux-arm-msm, devicetree
In-Reply-To: <20171114021255.GY11955@codeaurora.org>

On Mon 13 Nov 18:12 PST 2017, Stephen Boyd wrote:

> On 10/27, Georgi Djakov wrote:
> > Hi Bjorn,
> > 
> > Thanks for reviewing!
> > 
> > On 10/26/2017 07:28 AM, Bjorn Andersson wrote:
> > > On Thu 21 Sep 09:49 PDT 2017, Georgi Djakov wrote:
> > > 
> > >> Move the structure shared by the APCS IPC device and its subdevices
> > >> into a separate header file.
> > >>
> > > 
> > > As you're creating the apcs regmap with devm_regmap_init_mmio() you can
> > > just call dev_get_regmap(dev->parent) in your child to get the handle.
> > 
> > Ok, thanks!
> > 
> > > 
> > > But I would prefer that you just add the clock code to the existing
> > > driver.
> > 
> > This will require an ack from Stephen, and i got the impression that he
> > prefers a separate clk driver [1].
> > 
> > Stephen, are you ok with registering the clocks from the apcs mailbox
> > driver?
> > 
> > [1] https://lkml.org/lkml/2017/6/26/750
> 
> The parent regmap "trick" was the plan. Is something wrong with
> that?
> 

Not at all, but then this patch (moving apcs context to a shared header
file) shouldn't be needed, or am I missing something?

> Not having random clk drivers scattered throughout the tree is
> sort of nice because it makes for an easier time finding things
> that are similar. Maybe that's an abuse of the driver model
> though? Just to get things into some same directory. I'm fine
> either way.
> 

Keeping the clock driver in the clock subsystem does make sense. I see
now that there is a include of a local header file as well, so that
would just be messy to keep split.

I'm fine with the extra driver instance, it's the DT that I don't think
should describe the fact that we want to keep the clock-part in the
clock subsystem.

Do you see any problems spawning the clock driver programmatically and
then calling of_clk_add_hw_provider() on the parent's of_node?

Regards,
Bjorn

^ permalink raw reply

* Re: [PATCH] MIPS: implement a "bootargs-append" DT property
From: Daniel Gimpelevich @ 2017-11-14  3:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: James Hogan, Linux-MIPS, Frank Rowand,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Geert Uytterhoeven
In-Reply-To: <CAL_JsqJRVB928DVOAVQGrtT_EOuQBHkBhcd9+XFzqemutG65GA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, 2017-11-13 at 10:34 -0600, Rob Herring wrote:
> This is a kernel problem. What's the use case where you want the DT to
> override the kernel?
> 
> One way you could handle this is make bootargs be multiple strings.
> Well 2 specifically, the first string is prepended and the 2nd is
> appended. That complicates how you'd implement /append-property/
> though as you'd probably want to support both string cat and 2
> strings. Though the latter works more generically without knowing the
> data type.

Let's say the bootloader tells the kernel that the command line is "foo
bar" and that "baz" is in the dtb. Currently, there are kernel
configuration options to control whether this means the kernel's command
line will be "baz" or "baz foo bar" instead, but there is no way to turn
it into "foo bar baz" without either creating new kernel configuration
options or this patch. Implementing it via this patch would allow a
theoretical distro to use a generic kernel across different devices that
need the arguments from their bootloader manipulated in different ways.

Ideally, the MIPS-specific kernel configuration options for how to treat
the dtb's bootargs with respect to the bootloader's bootargs should go
away in favor of an analogous device tree property "bootargs-prepend"
for this reason. The kernel configuration options to supply a hard-coded
default command line if the bootloader does not supply one, and to
override what the bootloader and dtb supply, would remain. This would
separate the need for an alternate or "recovery" kernel to supply its
own bootargs to override the dtb from the need to have device-specific
bootargs in the dtb override a legacy bootloader, where the manner its
bootargs are overridden may be device-specific.

--
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 v9 3/7] mailbox: qcom: Move the apcs struct into a separate header
From: Stephen Boyd @ 2017-11-14  2:12 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: Bjorn Andersson, jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5bad4c4b-362b-ba9f-3072-1cced7a004dd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 10/27, Georgi Djakov wrote:
> Hi Bjorn,
> 
> Thanks for reviewing!
> 
> On 10/26/2017 07:28 AM, Bjorn Andersson wrote:
> > On Thu 21 Sep 09:49 PDT 2017, Georgi Djakov wrote:
> > 
> >> Move the structure shared by the APCS IPC device and its subdevices
> >> into a separate header file.
> >>
> > 
> > As you're creating the apcs regmap with devm_regmap_init_mmio() you can
> > just call dev_get_regmap(dev->parent) in your child to get the handle.
> 
> Ok, thanks!
> 
> > 
> > But I would prefer that you just add the clock code to the existing
> > driver.
> 
> This will require an ack from Stephen, and i got the impression that he
> prefers a separate clk driver [1].
> 
> Stephen, are you ok with registering the clocks from the apcs mailbox
> driver?
> 
> [1] https://lkml.org/lkml/2017/6/26/750

The parent regmap "trick" was the plan. Is something wrong with
that?

Not having random clk drivers scattered throughout the tree is
sort of nice because it makes for an easier time finding things
that are similar. Maybe that's an abuse of the driver model
though? Just to get things into some same directory. I'm fine
either way.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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