Devicetree
 help / color / mirror / Atom feed
* [PATCH 2/3] libnvdimm: Add a device-tree interface
From: Oliver O'Halloran @ 2017-11-15 17:51 UTC (permalink / raw)
  To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw
In-Reply-To: <20171115175131.23631-1-oohall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

A fairly bare-bones set of device-tree bindings so libnvdimm can be used
on powerpc and other device-tree based platforms.

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Oliver O'Halloran <oohall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/nvdimm/nvdimm-bus.txt      |  69 +++++++
 MAINTAINERS                                        |   8 +
 drivers/nvdimm/Kconfig                             |  10 +
 drivers/nvdimm/Makefile                            |   1 +
 drivers/nvdimm/of_nvdimm.c                         | 202 +++++++++++++++++++++
 5 files changed, 290 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt
 create mode 100644 drivers/nvdimm/of_nvdimm.c

diff --git a/Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt b/Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt
new file mode 100644
index 000000000000..491e7c4900ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt
@@ -0,0 +1,69 @@
+Device-tree bindings for nonvolatile-memory / NVDIMMs
+-----------------------------------------------------
+
+Non-volatile DIMMs are memory modules used to provide (cacheable) main memory that
+retains its contents across power cycles. In more practical terms, they are kind
+of storage device where the contents can be accessed by the CPU directly,
+rather than indirectly via a storage controller or similar. This can provide
+substantial performance improvements for applications designed to take
+advantage of in-memory storage.
+
+This binding provides a way to describe memory regions that should be managed
+by an NVDIMM storage driver (libNVDIMM in Linux) and some of the associated
+metadata. The binding itself is split into two main parts: A container bus and
+its sub-nodes which describe which memory address ranges corresponding to
+NVDIMM backed memory.
+
+Bindings for the container bus:
+------------------------------
+
+Required properties:
+ - compatible = "nvdimm-bus";
+ - ranges;
+	A blank ranges property is required because the sub-nodes have
+	addresses in the system's physical address space.
+
+The use of a container bus is mainly to handle future expansion of the binding. For
+comparison the ACPI equivalent of this binding (NFIT) describes: Memory regions, DIMM
+control structures, Block mode DIMM control structures, interleave sets, and more. Some
+of these structures cross reference each other so everyone should be happier if we keep
+it relatively self contained.
+
+Bindings for the region nodes:
+-----------------------------
+
+Required properties:
+	- compatible = "nvdimm-persistent" or "nvdimm-volatile"
+
+		The "nvdimm-persistent" region type indicates that this memory region
+		is actually a persistent region. The volatile type is mainly useful
+		for testing and RAM disks that can persist across kexec.
+
+	- reg = <base, size>;
+		The reg property should only contain one address range.
+
+Optional properties:
+	- Any relevant NUMA assocativity properties for the target platform.
+
+A complete example:
+--------------------
+
+/ {
+	#size-cells = <1>;
+	#address-cells = <1>;
+
+	nonvolatile-memory {
+		compatible = "nonvolatile-memory";
+		ranges;
+
+		region@5000 {
+			compatible = "nvdimm-persistent";
+			reg = <0x5000 0x1000>;
+		};
+
+		region@6000 {
+			compatible = "nvdimm-volatile";
+			reg = <0x6000 0x1000>;
+		};
+	};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 65eff7857ec3..0350bf5a94d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7875,6 +7875,14 @@ Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
 S:	Supported
 F:	drivers/nvdimm/pmem*
 
+LIBNVDIMM: DEVICETREE BINDINGS
+M:	Oliver O'Halloran <oohall-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+L:	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
+Q:	https://patchwork.kernel.org/project/linux-nvdimm/list/
+S:	Supported
+F:	drivers/nvdimm/of_nvdimm.c
+F:	Documentation/devicetree/bindings/nvdimm/nvdimm-bus.txt
+
 LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
 M:	Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
 L:	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index 5bdd499b5f4f..72d147b55596 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -102,4 +102,14 @@ config NVDIMM_DAX
 
 	  Select Y if unsure
 
+config OF_NVDIMM
+	tristate "Device-tree support for NVDIMMs"
+	depends on OF
+	default LIBNVDIMM
+	help
+	  Allows byte addressable persistent memory regions to be described in the
+	  device-tree.
+
+	  Select Y if unsure.
+
 endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index ca6d325438a3..9029511a8486 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
 obj-$(CONFIG_ND_BTT) += nd_btt.o
 obj-$(CONFIG_ND_BLK) += nd_blk.o
 obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
+obj-$(CONFIG_OF_NVDIMM) += of_nvdimm.o
 
 nd_pmem-y := pmem.o
 
diff --git a/drivers/nvdimm/of_nvdimm.c b/drivers/nvdimm/of_nvdimm.c
new file mode 100644
index 000000000000..765cbbae8bcb
--- /dev/null
+++ b/drivers/nvdimm/of_nvdimm.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright 2017, IBM Corporation
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you can access it online at
+ * http://www.gnu.org/licenses/gpl-2.0.html.
+ */
+
+#define pr_fmt(fmt) "of_nvdimm: " fmt
+
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/libnvdimm.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+
+static const struct attribute_group *region_attr_groups[] = {
+	&nd_region_attribute_group,
+	&nd_device_attribute_group,
+	NULL,
+};
+
+static int of_nvdimm_add_byte(struct nvdimm_bus *bus, struct device_node *np)
+{
+	struct nd_region_desc ndr_desc;
+	struct resource temp_res;
+	struct nd_region *region;
+
+	/*
+	 * byte regions should only have one address range
+	 */
+	if (of_address_to_resource(np, 0, &temp_res)) {
+		pr_warn("Unable to parse reg[0] for %pOF\n", np);
+		return -ENXIO;
+	}
+
+	pr_debug("Found %pR for %pOF\n", &temp_res, np);
+
+	memset(&ndr_desc, 0, sizeof(ndr_desc));
+	ndr_desc.res = &temp_res;
+	ndr_desc.of_node = np;
+	ndr_desc.attr_groups = region_attr_groups;
+#ifdef CONFIG_NUMA
+	ndr_desc.numa_node = of_node_to_nid(np);
+#endif
+	set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+
+	if (of_device_is_compatible(np, "nvdimm-volatile"))
+		region = nvdimm_volatile_region_create(bus, &ndr_desc);
+	else
+		region = nvdimm_pmem_region_create(bus, &ndr_desc);
+
+	if (!region)
+		return -ENXIO;
+
+	return 0;
+}
+
+static const struct of_device_id of_nvdimm_dev_types[] = {
+	{ .compatible = "nvdimm-persistent", .data = of_nvdimm_add_byte },
+	{ .compatible = "nvdimm-volatile", .data = of_nvdimm_add_byte },
+	{ },
+};
+
+static void of_nvdimm_parse_one(struct nvdimm_bus *bus,
+		struct device_node *node)
+{
+	int (*parse_node)(struct nvdimm_bus *, struct device_node *);
+	const struct of_device_id *match;
+	int rc;
+
+	if (of_node_test_and_set_flag(node, OF_POPULATED)) {
+		pr_debug("%pOF already parsed, skipping\n", node);
+		return;
+	}
+
+	match = of_match_node(of_nvdimm_dev_types, node);
+	if (!match) {
+		pr_info("No compatible match for '%pOF'\n", node);
+		of_node_clear_flag(node, OF_POPULATED);
+		return;
+	}
+
+	of_node_get(node);
+	parse_node = match->data;
+	rc = parse_node(bus, node);
+
+	if (rc) {
+		of_node_clear_flag(node, OF_POPULATED);
+		of_node_put(node);
+	}
+
+	pr_debug("Parsed %pOF, rc = %d\n", node, rc);
+
+	return;
+}
+
+/*
+ * The nvdimm core refers to the bus descriptor structure at runtime
+ * so we need to keep it around. Note that that this is different to
+ * the other nd_*_desc types which are essentially containers for extra
+ * function parameters.
+ */
+struct of_nd_private {
+	struct nvdimm_bus_descriptor desc;
+	struct nvdimm_bus *bus;
+};
+
+static const struct attribute_group *bus_attr_groups[] = {
+	&nvdimm_bus_attribute_group,
+	NULL,
+};
+
+static int of_nvdimm_probe(struct platform_device *pdev)
+{
+	struct device_node *node, *child;
+	struct of_nd_private *priv;
+
+	node = dev_of_node(&pdev->dev);
+	if (!node)
+		return -ENXIO;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->desc.attr_groups = bus_attr_groups;
+	priv->desc.provider_name = "of_nvdimm";
+	priv->desc.module = THIS_MODULE;
+	priv->desc.of_node = node;
+
+	priv->bus = nvdimm_bus_register(&pdev->dev, &priv->desc);
+	if (!priv->bus)
+		goto err;
+
+	platform_set_drvdata(pdev, priv);
+
+	/* now walk the node bus and setup regions, etc */
+	for_each_available_child_of_node(node, child)
+		of_nvdimm_parse_one(priv->bus, child);
+
+	return 0;
+
+err:
+	nvdimm_bus_unregister(priv->bus);
+	kfree(priv);
+	return -ENXIO;
+}
+
+static int of_nvdimm_remove(struct platform_device *pdev)
+{
+	struct of_nd_private *priv = platform_get_drvdata(pdev);
+	struct device_node *node;
+
+	if (!priv)
+		return 0; /* possible? */
+
+	for_each_available_child_of_node(pdev->dev.of_node, node) {
+		if (!of_node_check_flag(node, OF_POPULATED))
+			continue;
+
+		of_node_clear_flag(node, OF_POPULATED);
+		of_node_put(node);
+		pr_debug("de-populating %s\n", node->full_name);
+	}
+
+	nvdimm_bus_unregister(priv->bus);
+	kfree(priv);
+
+	return 0;
+}
+
+static const struct of_device_id of_nvdimm_bus_match[] = {
+	{ .compatible = "nvdimm-bus" },
+	{ },
+};
+
+static const struct platform_driver of_nvdimm_driver = {
+	.probe = of_nvdimm_probe,
+	.remove = of_nvdimm_remove,
+	.driver = {
+		.name = "of_nvdimm",
+		.owner = THIS_MODULE,
+		.of_match_table = of_nvdimm_bus_match,
+	},
+};
+
+module_platform_driver(of_nvdimm_driver);
+MODULE_DEVICE_TABLE(of, of_nvdimm_bus_match);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("IBM Corporation");
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH v3 5/5] misc serdev: w2sg0004: add debugging code and Kconfig
From: Joe Perches @ 2017-11-15 17:52 UTC (permalink / raw)
  To: H. Nikolaus Schaller, Ladislav Michl
  Cc: Andreas Färber, Rob Herring, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
	Greg Kroah-Hartman, Kevin Hilman, Thierry Reding,
	Jonathan Cameron, devicetree, linux-kernel, linux-omap,
	letux-kernel, kernel, linux-arm-kernel
In-Reply-To: <2CFA9F17-0E65-4804-9971-922500F5E919@goldelico.com>

On Wed, 2017-11-15 at 18:13 +0100, H. Nikolaus Schaller wrote:
> We have automated things by calling checkpatch but it somwhow failed here.

what failed?

^ permalink raw reply

* Re: [PATCH v3 5/5] misc serdev: w2sg0004: add debugging code and Kconfig
From: H. Nikolaus Schaller @ 2017-11-15 18:08 UTC (permalink / raw)
  To: Joe Perches
  Cc: Mark Rutland, devicetree, Ladislav Michl, Arnd Bergmann,
	Tony Lindgren, Greg Kroah-Hartman, kernel, Russell King,
	linux-kernel, linux-omap, Rob Herring, linux-arm-kernel,
	Benoît Cousson, Kevin Hilman, letux-kernel, Thierry Reding,
	Andreas Färber, Jonathan Cameron
In-Reply-To: <1510768342.2006.54.camel@perches.com>


> Am 15.11.2017 um 18:52 schrieb Joe Perches <joe@perches.com>:
> 
> On Wed, 2017-11-15 at 18:13 +0100, H. Nikolaus Schaller wrote:
>> We have automated things by calling checkpatch but it somwhow failed here.
> 
> what failed?

I don't know. It didn't tell us that the signature was missing. Maybe
a glitch in our post-processing script. I tried again and checkpatch.pl
did clearly report the ERROR this time.

So nothing you can fix.

BR and thanks,
Nikolaus Schaller

^ permalink raw reply

* Re: [PATCH v1 01/10] dt-bindings: media: Add Renesas CEU bindings
From: jacopo mondi @ 2017-11-15 18:15 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jacopo Mondi, Laurent Pinchart, Magnus Damm,
	Mauro Carvalho Chehab, Hans Verkuil, Linux-Renesas,
	Linux Media Mailing List, Linux-sh list,
	linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland,
	devicetree@vger.kernel.org
In-Reply-To: <CAMuHMdUG6gAEyyC0ANBzmj3KA-940+C7e9Nv_rLkBSjtfeKR1g@mail.gmail.com>

Hi Geert,

On Wed, Nov 15, 2017 at 02:07:31PM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> CC devicetree folks

Yeah, sorry I forgot them. Sorry about this and thanks for adding the
address back!

>
> On Wed, Nov 15, 2017 at 11:55 AM, Jacopo Mondi
> <jacopo+renesas@jmondi.org> wrote:
> > Add bindings documentation for Renesas Capture Engine Unit (CEU).
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../devicetree/bindings/media/renesas,ceu.txt      | 87 ++++++++++++++++++++++
> >  1 file changed, 87 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> > new file mode 100644
> > index 0000000..a88e9cb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> > @@ -0,0 +1,87 @@
> > +Renesas Capture Engine Unit (CEU)
> > +----------------------------------------------
> > +
> > +The Capture Engine Unit is the image capture interface found on Renesas
> > +RZ chip series and on SH Mobile ones.
> > +
> > +The interface supports a single parallel input with up 8/16bits data bus width.
>
> ... with data bus widths up to 8/16 bits?
>
> > +
> > +Required properties:
> > +- compatible
> > +       Must be "renesas,renesas-ceu".
>
> The double "renesas" part looks odd to me. What about "renesas,ceu"?

I'm totally open for better "compatible" strings here, so yeah, let's
got for the shorter one you proposed...

> Shouldn't you add SoC-specific compatible values like "renesas,r7s72100-ceu",
> too?

Well, I actually have no SoC-specific data in the driver, so I don't
need SoC specific "compatible" values. But if it's a good practice
to have them anyway, I will add those in next spin..
>
> > +- reg
> > +       Physical address base and size.
> > +- interrupts
> > +       The interrupt line number.
>
> interrupt specifier

Yeah, it's not just the line number...

>

[snip]

> > +i2c1: i2c@fcfee400 {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&i2c1_pins>;
> > +
> > +       status = "okay";
> > +       clock-frequency = <100000>;
> > +
> > +       ov7670: camera@21 {
> > +               compatible = "ovti,ov7670";
> > +               reg = <0x21>;
> > +
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&vio_pins>;
> > +
> > +               reset-gpios = <&port3 11 GPIO_ACTIVE_LOW>;
> > +               powerdown-gpios = <&port3 12 GPIO_ACTIVE_HIGH>;
> > +
> > +               clocks = <&xclk>;
> > +               clock-names = "xclk";
> > +
> > +               xclk: fixed_clk {
> > +                       compatible = "fixed-clock";
> > +                       #clock-cells = <0>;
> > +                       clock-frequency = <24000000>;
> > +               };

As Sakari pointed out in his review, this fixed clock is a detail
specific to the sensor used in the example (ov7670). For sake of
simplicity I can remove it.

Thanks
  j

^ permalink raw reply

* Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver
From: Tim Harvey @ 2017-11-15 18:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-media, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Shawn Guo,
	Steve Longerbeam, Philipp Zabel, Hans Verkuil,
	Mauro Carvalho Chehab, Hans Verkuil
In-Reply-To: <20171115155204.yhqjocdm32qunllx@rob-hp-laptop>

On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
>> Add support for the TDA1997x HDMI receivers.
>>
>> Cc: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
>> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
>> ---
>> v3:
>>  - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros
>>  - fixed missing break
>>  - use only hdmi_infoframe_log for infoframe logging
>>  - simplify tda1997x_s_stream error handling
>>  - add delayed work proc to handle hotplug enable/disable
>>  - fix set_edid (disable HPD before writing, enable after)
>>  - remove enabling edid by default
>>  - initialize timings
>>  - take quant range into account in colorspace conversion
>>  - remove vendor/product tracking (we provide this in log_status via infoframes)
>>  - add v4l_controls
>>  - add more detail to log_status
>>  - calculate vhref generator timings
>>  - timing detection fixes (rounding errors, hswidth errors)
>>  - rename configure_input/configure_conv functions
>>
>> v2:
>>  - implement dv timings enum/cap
>>  - remove deprecated g_mbus_config op
>>  - fix dv_query_timings
>>  - add EDID get/set handling
>>  - remove max-pixel-rate support
>>  - add audio codec DAI support
>>  - change audio bindings
>> ---
>>  drivers/media/i2c/Kconfig            |    9 +
>>  drivers/media/i2c/Makefile           |    1 +
>>  drivers/media/i2c/tda1997x.c         | 3485 ++++++++++++++++++++++++++++++++++
>>  include/dt-bindings/media/tda1997x.h |   78 +
>
> This belongs with the binding documentation patch.
>

Rob,

Thanks - missed that. I will move it for v4.

Regarding your previous comment to the v2 series:
> The rest of the binding looks fine, but I have some reservations about
> this. I think this should be common probably. There's been a few
> bindings for display recently that deal with the interface format. Maybe
> some vendor property is needed here to map a standard interface format
> back to pin configuration.

I take it this is not an 'Ack' for the bindings?

Which did you feel should be made common? I admit I was surprised
there wasn't a common binding for audio bus format (i2s|spdif) but if
you were referring to the video data that would probably be much more
complicated.

I was hoping one of the media/driver maintainers would respond to your
comment with thoughts as I'm not familiar with a very wide variety of
receivers.

Best regards,

Tim
--
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 v1 01/10] dt-bindings: media: Add Renesas CEU bindings
From: Geert Uytterhoeven @ 2017-11-15 18:39 UTC (permalink / raw)
  To: jacopo mondi
  Cc: Jacopo Mondi, Laurent Pinchart, Magnus Damm,
	Mauro Carvalho Chehab, Hans Verkuil, Linux-Renesas,
	Linux Media Mailing List, Linux-sh list,
	linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland,
	devicetree@vger.kernel.org
In-Reply-To: <20171115181505.GB6736@w540>

Hi Jacopo,

On Wed, Nov 15, 2017 at 7:15 PM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Wed, Nov 15, 2017 at 02:07:31PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Nov 15, 2017 at 11:55 AM, Jacopo Mondi
>> <jacopo+renesas@jmondi.org> wrote:
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
>> > @@ -0,0 +1,87 @@
>> > +Renesas Capture Engine Unit (CEU)
>> > +----------------------------------------------
>> > +
>> > +The Capture Engine Unit is the image capture interface found on Renesas
>> > +RZ chip series and on SH Mobile ones.
>> > +
>> > +The interface supports a single parallel input with up 8/16bits data bus width.
>>
>> ... with data bus widths up to 8/16 bits?
>>
>> > +
>> > +Required properties:
>> > +- compatible
>> > +       Must be "renesas,renesas-ceu".
>>
>> The double "renesas" part looks odd to me. What about "renesas,ceu"?
>
> I'm totally open for better "compatible" strings here, so yeah, let's
> got for the shorter one you proposed...
>
>> Shouldn't you add SoC-specific compatible values like "renesas,r7s72100-ceu",
>> too?
>
> Well, I actually have no SoC-specific data in the driver, so I don't
> need SoC specific "compatible" values. But if it's a good practice
> to have them anyway, I will add those in next spin..

You don't necessarily need them in the driver, but in the bindings and DTS,
just in case a difference is discovered later.

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 v3 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.
From: Rob Herring @ 2017-11-15 19:18 UTC (permalink / raw)
  To: David Daney
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
	James Hogan, netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
	Mark Rutland, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Steven J. Hill, devicetree-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
	Florian Fainelli, Carlos Munoz
In-Reply-To: <20171109192915.11912-2-david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

On Thu, Nov 09, 2017 at 11:29:08AM -0800, David Daney wrote:
> From: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> Add bindings for Common Ethernet Interface (BGX) block.
> 
> Signed-off-by: Carlos Munoz <cmunoz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Steven J. Hill <Steven.Hill-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/net/cavium-bgx.txt         | 61 ++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/cavium-bgx.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/cavium-bgx.txt b/Documentation/devicetree/bindings/net/cavium-bgx.txt
> new file mode 100644
> index 000000000000..6b1f8b994c20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/cavium-bgx.txt
> @@ -0,0 +1,61 @@
> +* Common Ethernet Interface (BGX) block
> +
> +Properties:
> +
> +- compatible: "cavium,octeon-7890-bgx": Compatibility with all cn7xxx SOCs.
> +
> +- reg: The base address of the BGX block.
> +
> +- #address-cells: Must be <1>.
> +
> +- #size-cells: Must be <0>.  BGX addresses have no size component.
> +
> +A BGX block has several children, each representing an Ethernet
> +interface.
> +
> +
> +* Ethernet Interface (BGX port) connects to PKI/PKO
> +
> +Properties:
> +
> +- compatible: "cavium,octeon-7890-bgx-port": Compatibility with all
> +	      cn7xxx SOCs.
> +
> +	      "cavium,octeon-7360-xcv": Compatibility with cn73xx SOCs
> +	      for RGMII.
> +
> +- reg: The index of the interface within the BGX block.

There's no memory mapped region associated with the sub-blocks?

> +
> +Optional properties:
> +
> +- local-mac-address: Mac address for the interface.
> +
> +- phy-handle: phandle to the phy node connected to the interface.
> +
> +- phy-mode: described in ethernet.txt.
> +
> +- fixed-link: described in fixed-link.txt.
> +
> +Example:
> +
> +	ethernet-mac-nexus@11800e0000000 {
> +		compatible = "cavium,octeon-7890-bgx";
> +		reg = <0x00011800 0xe0000000 0x00000000 0x01000000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethernet-mac@0 {

ethernet@0

> +			compatible = "cavium,octeon-7360-xcv";
> +			reg = <0>;
> +			local-mac-address = [ 00 01 23 45 67 89 ];
> +			phy-handle = <&phy3>;
> +			phy-mode = "rgmii-rxid"
> +		};
> +		ethernet-mac@1 {

ditto.

Otherwise,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> +			compatible = "cavium,octeon-7890-bgx-port";
> +			reg = <1>;
> +			local-mac-address = [ 00 01 23 45 67 8a ];
> +			phy-handle = <&phy4>;
> +			phy-mode = "sgmii"
> +		};
> +	};
> -- 
> 2.13.6
> 
--
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] Add support for bq27521 battery monitor
From: Rob Herring @ 2017-11-15 19:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: pali.rohar-Re5JQEeQqe8AvxtiuMwx3w, sre-DgEjT+Ai2ygdnm+yROfE0A,
	kernel list, linux-arm-kernel, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, khilman-DgEjT+Ai2ygdnm+yROfE0A,
	aaro.koskinen-X3B1VOXEql0,
	ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	patrikbachan-Re5JQEeQqe8AvxtiuMwx3w, serge-A9i7LUbDfNHQT0dZR+AlfA,
	abcloriens-Re5JQEeQqe8AvxtiuMwx3w, clayton-fehKsxFhGzZIf6P1QZMOBw,
	martijn-28JJ9oSIdodmR6Xm/wNWPw,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA, afd-l0cyMroinI0,
	kernel-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171109210615.GA27638@amd>

On Thu, Nov 09, 2017 at 10:06:15PM +0100, Pavel Machek wrote:
> 
> This adds basic support for BQ27521 battery monitor, used in Nokia N9
> and N950. In particular, battery voltage is important to be able to
> tell when the battery is almost empty. Emptying battery on N950 is
> pretty painful, as flasher needs to be used to recover phone in such case.
> 
> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

For the binding,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

--
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: Rob Herring @ 2017-11-15 19:22 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Linus Walleij, Laurent Pinchart, Geert Uytterhoeven,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Vladimir Barinov
In-Reply-To: <20171110181552.227541251-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

On Fri, Nov 10, 2017 at 08:59:01PM +0300, Sergei Shtylyov 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-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>.
> 
> Signed-off-by: Vladimir Barinov <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> 
> ---
>  Documentation/devicetree/bindings/pinctrl/renesas,pfc-pinctrl.txt |    1 

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/pinctrl/sh-pfc/Kconfig                                    |    5 
>  drivers/pinctrl/sh-pfc/Makefile                                   |    1 
>  drivers/pinctrl/sh-pfc/core.c                                     |    6 
>  drivers/pinctrl/sh-pfc/pfc-r8a77970.c                             | 2421 ++++++++++
>  drivers/pinctrl/sh-pfc/sh_pfc.h                                   |    1 
>  6 files changed, 2435 insertions(+)
--
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 v3 1/8] dt-bindings: Add Cavium Octeon Common Ethernet Interface.
From: David Daney @ 2017-11-15 19:30 UTC (permalink / raw)
  To: Rob Herring, David Daney
  Cc: linux-mips, ralf, James Hogan, netdev, David S. Miller,
	Mark Rutland, devel, Greg Kroah-Hartman, linux-kernel,
	Steven J. Hill, devicetree, Andrew Lunn, Florian Fainelli,
	Carlos Munoz
In-Reply-To: <20171115191857.podohd56knfd4hyh@rob-hp-laptop>

On 11/15/2017 11:18 AM, Rob Herring wrote:
> On Thu, Nov 09, 2017 at 11:29:08AM -0800, David Daney wrote:
>> From: Carlos Munoz <cmunoz@cavium.com>
>>
>> Add bindings for Common Ethernet Interface (BGX) block.
>>
>> Signed-off-by: Carlos Munoz <cmunoz@cavium.com>
>> Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> ---
>>   .../devicetree/bindings/net/cavium-bgx.txt         | 61 ++++++++++++++++++++++
>>   1 file changed, 61 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/cavium-bgx.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/cavium-bgx.txt b/Documentation/devicetree/bindings/net/cavium-bgx.txt
>> new file mode 100644
>> index 000000000000..6b1f8b994c20
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/cavium-bgx.txt
>> @@ -0,0 +1,61 @@
>> +* Common Ethernet Interface (BGX) block
>> +
>> +Properties:
>> +
>> +- compatible: "cavium,octeon-7890-bgx": Compatibility with all cn7xxx SOCs.
>> +
>> +- reg: The base address of the BGX block.
>> +
>> +- #address-cells: Must be <1>.
>> +
>> +- #size-cells: Must be <0>.  BGX addresses have no size component.
>> +
>> +A BGX block has several children, each representing an Ethernet
>> +interface.
>> +
>> +
>> +* Ethernet Interface (BGX port) connects to PKI/PKO
>> +
>> +Properties:
>> +
>> +- compatible: "cavium,octeon-7890-bgx-port": Compatibility with all
>> +	      cn7xxx SOCs.
>> +
>> +	      "cavium,octeon-7360-xcv": Compatibility with cn73xx SOCs
>> +	      for RGMII.
>> +
>> +- reg: The index of the interface within the BGX block.
> 
> There's no memory mapped region associated with the sub-blocks?

No.  Many of the sub block control bits are co-located in the same 
registers.


> 
>> +
>> +Optional properties:
>> +
>> +- local-mac-address: Mac address for the interface.
>> +
>> +- phy-handle: phandle to the phy node connected to the interface.
>> +
>> +- phy-mode: described in ethernet.txt.
>> +
>> +- fixed-link: described in fixed-link.txt.
>> +
>> +Example:
>> +
>> +	ethernet-mac-nexus@11800e0000000 {
>> +		compatible = "cavium,octeon-7890-bgx";
>> +		reg = <0x00011800 0xe0000000 0x00000000 0x01000000>;
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		ethernet-mac@0 {
> 
> ethernet@0


Good point.  I will fix that.


> 
>> +			compatible = "cavium,octeon-7360-xcv";
>> +			reg = <0>;
>> +			local-mac-address = [ 00 01 23 45 67 89 ];
>> +			phy-handle = <&phy3>;
>> +			phy-mode = "rgmii-rxid"
>> +		};
>> +		ethernet-mac@1 {
> 
> ditto.
> 
> Otherwise,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks,
David Daney
> 
>> +			compatible = "cavium,octeon-7890-bgx-port";
>> +			reg = <1>;
>> +			local-mac-address = [ 00 01 23 45 67 8a ];
>> +			phy-handle = <&phy4>;
>> +			phy-mode = "sgmii"
>> +		};
>> +	};
>> -- 
>> 2.13.6
>>

^ permalink raw reply

* [PATCH v2 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver
From: Dan Murphy @ 2017-11-15 19:30 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

This adds the devicetree bindings for the LM3692x
I2C LED string driver.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - No changes - https://patchwork.kernel.org/patch/10056677/

 .../devicetree/bindings/leds/leds-lm3692x.txt      | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
new file mode 100644
index 000000000000..cfef67bd4100
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -0,0 +1,28 @@
+* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver
+
+The LM36922 is an ultra-compact, highly efficient,
+two string white-LED driver designed for LCD display
+backlighting.
+
+Required properties:
+	- compatible:
+		"ti,lm3692x"
+	- reg -  I2C slave address
+
+Optional properties:
+	- label - Used for naming LEDs
+	- enable-gpio - gpio pin to enable/disable the device.
+	- supply - "vled" - LED supply
+
+Example:
+
+leds: leds@6 {
+	compatible = "ti,lm3692x";
+	reg = <0x36>;
+	label = "backlight_cluster";
+	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+}
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
-- 
2.15.0.124.g7668cbc60

^ permalink raw reply related

* [PATCH v2 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Dan Murphy @ 2017-11-15 19:30 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy
In-Reply-To: <20171115193031.28781-1-dmurphy@ti.com>

Introducing the LM3692x Dual-String white LED driver.

Data sheet is located
http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init
routine, return on fault_check failure, updated brightness calculation and
fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/

 drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 390 insertions(+)
 create mode 100644 drivers/leds/leds-lm3692x.c

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
new file mode 100644
index 000000000000..9fa7f5a20466
--- /dev/null
+++ b/drivers/leds/leds-lm3692x.c
@@ -0,0 +1,390 @@
+/*
+ * TI lm3692x LED Driver
+ *
+ * Copyright (C) 2017 Texas Instruments
+ *
+ * Author: Dan Murphy <dmurphy@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * Data sheet is located
+ * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/slab.h>
+
+#define LM3692X_LED_NAME	"lm3692x_led"
+
+#define LM3692X_REV		0x0
+#define LM3692X_RESET		0x1
+#define LM3692X_EN		0x10
+#define LM3692X_BRT_CTRL	0x11
+#define LM3692X_PWM_CTRL	0x12
+#define LM3692X_BOOST_CTRL	0x13
+#define LM3692X_AUTO_FREQ_HI	0x15
+#define LM3692X_AUTO_FREQ_LO	0x16
+#define LM3692X_BL_ADJ_THRESH	0x17
+#define LM3692X_BRT_LSB		0x18
+#define LM3692X_BRT_MSB		0x19
+#define LM3692X_FAULT_CTRL	0x1e
+#define LM3692X_FAULT_FLAGS	0x1f
+
+#define LM3692X_SW_RESET	BIT(0)
+#define LM3692X_DEVICE_EN	BIT(0)
+#define LM3692X_LED1_EN		BIT(1)
+#define LM3692X_LED2_EN		BIT(2)
+
+/* Brightness Control Bits */
+#define LM3692X_BL_ADJ_POL	BIT(0)
+#define LM3692X_RAMP_RATE_125us	0x00
+#define LM3692X_RAMP_RATE_250us	BIT(1)
+#define LM3692X_RAMP_RATE_500us BIT(2)
+#define LM3692X_RAMP_RATE_1ms	(BIT(1) | BIT(2))
+#define LM3692X_RAMP_RATE_2ms	BIT(3)
+#define LM3692X_RAMP_RATE_4ms	(BIT(3) | BIT(1))
+#define LM3692X_RAMP_RATE_8ms	(BIT(2) | BIT(3))
+#define LM3692X_RAMP_RATE_16ms	(BIT(1) | BIT(2) | BIT(3))
+#define LM3692X_RAMP_EN		BIT(4)
+#define LM3692X_BRHT_MODE_REG	0x00
+#define LM3692X_BRHT_MODE_PWM	BIT(5)
+#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6)
+#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6))
+#define LM3692X_MAP_MODE_EXP	BIT(7)
+
+/* PWM Register Bits */
+#define LM3692X_PWM_FILTER_100	BIT(0)
+#define LM3692X_PWM_FILTER_150	BIT(1)
+#define LM3692X_PWM_FILTER_200	(BIT(0) | BIT(1))
+#define LM3692X_PWM_HYSTER_1LSB BIT(2)
+#define LM3692X_PWM_HYSTER_2LSB	BIT(3)
+#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2))
+#define LM3692X_PWM_HYSTER_4LSB BIT(4)
+#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2))
+#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3))
+#define LM3692X_PWM_POLARITY	BIT(5)
+#define LM3692X_PWM_SAMP_4MHZ	BIT(6)
+#define LM3692X_PWM_SAMP_24MHZ	BIT(7)
+
+/* Boost Control Bits */
+#define LM3692X_OCP_PROT_1A	BIT(0)
+#define LM3692X_OCP_PROT_1_25A	BIT(1)
+#define LM3692X_OCP_PROT_1_5A	(BIT(0) | BIT(1))
+#define LM3692X_OVP_21V		BIT(2)
+#define LM3692X_OVP_25V		BIT(3)
+#define LM3692X_OVP_29V		(BIT(2) | BIT(3))
+#define LM3692X_MIN_IND_22UH	BIT(4)
+#define LM3692X_BOOST_SW_1MHZ	BIT(5)
+#define LM3692X_BOOST_SW_NO_SHIFT	BIT(6)
+
+/* Fault Control Bits */
+#define LM3692X_FAULT_CTRL_OVP BIT(0)
+#define LM3692X_FAULT_CTRL_OCP BIT(1)
+#define LM3692X_FAULT_CTRL_TSD BIT(2)
+#define LM3692X_FAULT_CTRL_OPEN BIT(3)
+
+/* Fault Flag Bits */
+#define LM3692X_FAULT_FLAG_OVP BIT(0)
+#define LM3692X_FAULT_FLAG_OCP BIT(1)
+#define LM3692X_FAULT_FLAG_TSD BIT(2)
+#define LM3692X_FAULT_FLAG_SHRT BIT(3)
+#define LM3692X_FAULT_FLAG_OPEN BIT(4)
+
+/**
+ * struct lm3692x_led -
+ * @lock - Lock for reading/writing the device
+ * @client - Pointer to the I2C client
+ * @led_dev - led class device pointer
+ * @regmap - Devices register map
+ * @enable_gpio - VDDIO/EN gpio to enable communication interface
+ * @regulator - LED supply regulator pointer
+ * @label - LED label
+ */
+struct lm3692x_led {
+	struct mutex lock;
+	struct i2c_client *client;
+	struct led_classdev led_dev;
+	struct regmap *regmap;
+	struct gpio_desc *enable_gpio;
+	struct regulator *regulator;
+	const char *label;
+};
+
+static const struct reg_default lm3692x_reg_defs[] = {
+	{LM3692X_EN, 0xf},
+	{LM3692X_BRT_CTRL, 0x61},
+	{LM3692X_PWM_CTRL, 0x73},
+	{LM3692X_BOOST_CTRL, 0x6f},
+	{LM3692X_AUTO_FREQ_HI, 0x0},
+	{LM3692X_AUTO_FREQ_LO, 0x0},
+	{LM3692X_BL_ADJ_THRESH, 0x0},
+	{LM3692X_BRT_LSB, 0x7},
+	{LM3692X_BRT_MSB, 0xff},
+	{LM3692X_FAULT_CTRL, 0x7},
+};
+
+static const struct regmap_config lm3692x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3692X_FAULT_FLAGS,
+	.reg_defaults = lm3692x_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs),
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int lm3692x_fault_check(struct lm3692x_led *led)
+{
+	int ret, fault;
+	unsigned int read_buf;
+
+	ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf);
+	if (ret)
+		return ret;
+
+	fault = read_buf;
+
+	if (fault)
+		dev_err(&led->client->dev, "Detected a fault 0x%X\n",
+			fault);
+
+	return ret;
+}
+
+static int lm3692x_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3692x_led *led =
+			container_of(led_cdev, struct lm3692x_led, led_dev);
+	int ret;
+	int led_brightness_lsb = (brt_val >> 5);
+
+	mutex_lock(&led->lock);
+
+	ret = lm3692x_fault_check(led);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write MSB\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write LSB\n");
+		goto out;
+	}
+out:
+	mutex_unlock(&led->lock);
+	return ret;
+}
+
+static int lm3692x_init(struct lm3692x_led *led)
+{
+	int ret;
+
+	if (led->regulator) {
+		ret = regulator_enable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to enable regulator\n");
+			goto out;
+	}
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 1);
+
+	ret = lm3692x_fault_check(led);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Fail writing BRT CTRL\n");
+		goto out;
+	}
+
+	/*
+	 * For glitch free operation, the following data should
+	 * only be written while device enable bit is 0
+	 * per Section 7.5.14 of the data sheet
+	 */
+	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
+		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
+			LM3692X_BRHT_MODE_RAMP_MULTI |
+			LM3692X_BL_ADJ_POL |
+			LM3692X_RAMP_RATE_250us);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming BL Threshold\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL,
+			LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB);
+	if (ret)
+		dev_err(&led->client->dev, "Failed programming BRT CTRL\n");
+
+	return ret;
+out:
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	return ret;
+}
+
+static int lm3692x_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int ret;
+	struct lm3692x_led *led;
+	struct device_node *np = client->dev.of_node;
+
+	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	if (client->dev.of_node) {
+		ret = of_property_read_string(np, "label", &led->label);
+		if (ret)
+			led->label = LM3692X_LED_NAME;
+	}
+
+	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(led->enable_gpio)) {
+		ret = PTR_ERR(led->enable_gpio);
+		dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret);
+		return ret;
+	}
+
+	led->regulator = devm_regulator_get(&client->dev, "vled");
+	if (IS_ERR(led->regulator))
+		led->regulator = NULL;
+
+	led->client = client;
+	led->led_dev.name = led->label;
+	led->led_dev.max_brightness = LED_FULL;
+	led->led_dev.brightness_set_blocking = lm3692x_brightness_set;
+
+	mutex_init(&led->lock);
+
+	i2c_set_clientdata(client, led);
+
+	led->regmap = devm_regmap_init_i2c(client, &lm3692x_regmap_config);
+	if (IS_ERR(led->regmap)) {
+		ret = PTR_ERR(led->regmap);
+		dev_err(&client->dev, "Failed to allocate register map: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = lm3692x_init(led);
+	if (ret)
+		return ret;
+
+	ret = led_classdev_register(&client->dev, &led->led_dev);
+	if (ret) {
+		dev_err(&client->dev, "led register err: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int lm3692x_remove(struct i2c_client *client)
+{
+	struct lm3692x_led *led = i2c_get_clientdata(client);
+	int ret;
+
+	led_classdev_unregister(&led->led_dev);
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id lm3692x_id[] = {
+	{ "lm3692x", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3692x_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_lm3692x_leds_match[] = {
+	{ .compatible = "ti,lm3692x", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match);
+#endif
+
+static struct i2c_driver lm3692x_driver = {
+	.driver = {
+		.name	= "lm3692x",
+		.of_match_table = of_match_ptr(of_lm3692x_leds_match),
+	},
+	.probe		= lm3692x_probe,
+	.remove		= lm3692x_remove,
+	.id_table	= lm3692x_id,
+};
+module_i2c_driver(lm3692x_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL");
-- 
2.15.0.124.g7668cbc60

^ permalink raw reply related

* [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver
From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy

This adds the devicetree bindings for the LM3692x
I2C LED string driver.

Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---

v3 - No changes
v2 - No changes - https://patchwork.kernel.org/patch/10056677/

 .../devicetree/bindings/leds/leds-lm3692x.txt      | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
new file mode 100644
index 000000000000..cfef67bd4100
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
@@ -0,0 +1,28 @@
+* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver
+
+The LM36922 is an ultra-compact, highly efficient,
+two string white-LED driver designed for LCD display
+backlighting.
+
+Required properties:
+	- compatible:
+		"ti,lm3692x"
+	- reg -  I2C slave address
+
+Optional properties:
+	- label - Used for naming LEDs
+	- enable-gpio - gpio pin to enable/disable the device.
+	- supply - "vled" - LED supply
+
+Example:
+
+leds: leds@6 {
+	compatible = "ti,lm3692x";
+	reg = <0x36>;
+	label = "backlight_cluster";
+	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+}
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
-- 
2.15.0.124.g7668cbc60

--
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 v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy
In-Reply-To: <20171115194203.13572-1-dmurphy-l0cyMroinI0@public.gmane.org>

Introducing the LM3692x Dual-String white LED driver.

Data sheet is located
http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf

Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---

v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/

v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init
routine, return on fault_check failure, updated brightness calculation and
fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/

 drivers/leds/Kconfig        |   7 +
 drivers/leds/Makefile       |   1 +
 drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 398 insertions(+)
 create mode 100644 drivers/leds/leds-lm3692x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 318a28fd58fe..1363e77a7d5f 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -137,6 +137,13 @@ config LEDS_LM3642
 	  converter plus 1.5A constant current driver for a high-current
 	  white LED.
 
+config LEDS_LM3692X
+	tristate "LED support for LM3692x Chips"
+	depends on LEDS_CLASS && I2C
+	select REGMAP_I2C
+	help
+	  This option enables support for the TI LM3692x family
+	  of LED drivers.
 
 config LEDS_LOCOMO
 	tristate "LED Support for Locomo device"
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index a2a6b5a4f86d..987884a5b9a5 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058)		+= leds-pm8058.o
 obj-$(CONFIG_LEDS_MLXCPLD)		+= leds-mlxcpld.o
 obj-$(CONFIG_LEDS_NIC78BX)		+= leds-nic78bx.o
 obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
+obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_DAC124S085)		+= leds-dac124s085.o
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
new file mode 100644
index 000000000000..9fa7f5a20466
--- /dev/null
+++ b/drivers/leds/leds-lm3692x.c
@@ -0,0 +1,390 @@
+/*
+ * TI lm3692x LED Driver
+ *
+ * Copyright (C) 2017 Texas Instruments
+ *
+ * Author: Dan Murphy <dmurphy-l0cyMroinI0@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
+ * version 2 as published by the Free Software Foundation.
+ *
+ * Data sheet is located
+ * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/slab.h>
+
+#define LM3692X_LED_NAME	"lm3692x_led"
+
+#define LM3692X_REV		0x0
+#define LM3692X_RESET		0x1
+#define LM3692X_EN		0x10
+#define LM3692X_BRT_CTRL	0x11
+#define LM3692X_PWM_CTRL	0x12
+#define LM3692X_BOOST_CTRL	0x13
+#define LM3692X_AUTO_FREQ_HI	0x15
+#define LM3692X_AUTO_FREQ_LO	0x16
+#define LM3692X_BL_ADJ_THRESH	0x17
+#define LM3692X_BRT_LSB		0x18
+#define LM3692X_BRT_MSB		0x19
+#define LM3692X_FAULT_CTRL	0x1e
+#define LM3692X_FAULT_FLAGS	0x1f
+
+#define LM3692X_SW_RESET	BIT(0)
+#define LM3692X_DEVICE_EN	BIT(0)
+#define LM3692X_LED1_EN		BIT(1)
+#define LM3692X_LED2_EN		BIT(2)
+
+/* Brightness Control Bits */
+#define LM3692X_BL_ADJ_POL	BIT(0)
+#define LM3692X_RAMP_RATE_125us	0x00
+#define LM3692X_RAMP_RATE_250us	BIT(1)
+#define LM3692X_RAMP_RATE_500us BIT(2)
+#define LM3692X_RAMP_RATE_1ms	(BIT(1) | BIT(2))
+#define LM3692X_RAMP_RATE_2ms	BIT(3)
+#define LM3692X_RAMP_RATE_4ms	(BIT(3) | BIT(1))
+#define LM3692X_RAMP_RATE_8ms	(BIT(2) | BIT(3))
+#define LM3692X_RAMP_RATE_16ms	(BIT(1) | BIT(2) | BIT(3))
+#define LM3692X_RAMP_EN		BIT(4)
+#define LM3692X_BRHT_MODE_REG	0x00
+#define LM3692X_BRHT_MODE_PWM	BIT(5)
+#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6)
+#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6))
+#define LM3692X_MAP_MODE_EXP	BIT(7)
+
+/* PWM Register Bits */
+#define LM3692X_PWM_FILTER_100	BIT(0)
+#define LM3692X_PWM_FILTER_150	BIT(1)
+#define LM3692X_PWM_FILTER_200	(BIT(0) | BIT(1))
+#define LM3692X_PWM_HYSTER_1LSB BIT(2)
+#define LM3692X_PWM_HYSTER_2LSB	BIT(3)
+#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2))
+#define LM3692X_PWM_HYSTER_4LSB BIT(4)
+#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2))
+#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3))
+#define LM3692X_PWM_POLARITY	BIT(5)
+#define LM3692X_PWM_SAMP_4MHZ	BIT(6)
+#define LM3692X_PWM_SAMP_24MHZ	BIT(7)
+
+/* Boost Control Bits */
+#define LM3692X_OCP_PROT_1A	BIT(0)
+#define LM3692X_OCP_PROT_1_25A	BIT(1)
+#define LM3692X_OCP_PROT_1_5A	(BIT(0) | BIT(1))
+#define LM3692X_OVP_21V		BIT(2)
+#define LM3692X_OVP_25V		BIT(3)
+#define LM3692X_OVP_29V		(BIT(2) | BIT(3))
+#define LM3692X_MIN_IND_22UH	BIT(4)
+#define LM3692X_BOOST_SW_1MHZ	BIT(5)
+#define LM3692X_BOOST_SW_NO_SHIFT	BIT(6)
+
+/* Fault Control Bits */
+#define LM3692X_FAULT_CTRL_OVP BIT(0)
+#define LM3692X_FAULT_CTRL_OCP BIT(1)
+#define LM3692X_FAULT_CTRL_TSD BIT(2)
+#define LM3692X_FAULT_CTRL_OPEN BIT(3)
+
+/* Fault Flag Bits */
+#define LM3692X_FAULT_FLAG_OVP BIT(0)
+#define LM3692X_FAULT_FLAG_OCP BIT(1)
+#define LM3692X_FAULT_FLAG_TSD BIT(2)
+#define LM3692X_FAULT_FLAG_SHRT BIT(3)
+#define LM3692X_FAULT_FLAG_OPEN BIT(4)
+
+/**
+ * struct lm3692x_led -
+ * @lock - Lock for reading/writing the device
+ * @client - Pointer to the I2C client
+ * @led_dev - led class device pointer
+ * @regmap - Devices register map
+ * @enable_gpio - VDDIO/EN gpio to enable communication interface
+ * @regulator - LED supply regulator pointer
+ * @label - LED label
+ */
+struct lm3692x_led {
+	struct mutex lock;
+	struct i2c_client *client;
+	struct led_classdev led_dev;
+	struct regmap *regmap;
+	struct gpio_desc *enable_gpio;
+	struct regulator *regulator;
+	const char *label;
+};
+
+static const struct reg_default lm3692x_reg_defs[] = {
+	{LM3692X_EN, 0xf},
+	{LM3692X_BRT_CTRL, 0x61},
+	{LM3692X_PWM_CTRL, 0x73},
+	{LM3692X_BOOST_CTRL, 0x6f},
+	{LM3692X_AUTO_FREQ_HI, 0x0},
+	{LM3692X_AUTO_FREQ_LO, 0x0},
+	{LM3692X_BL_ADJ_THRESH, 0x0},
+	{LM3692X_BRT_LSB, 0x7},
+	{LM3692X_BRT_MSB, 0xff},
+	{LM3692X_FAULT_CTRL, 0x7},
+};
+
+static const struct regmap_config lm3692x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3692X_FAULT_FLAGS,
+	.reg_defaults = lm3692x_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs),
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int lm3692x_fault_check(struct lm3692x_led *led)
+{
+	int ret, fault;
+	unsigned int read_buf;
+
+	ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf);
+	if (ret)
+		return ret;
+
+	fault = read_buf;
+
+	if (fault)
+		dev_err(&led->client->dev, "Detected a fault 0x%X\n",
+			fault);
+
+	return ret;
+}
+
+static int lm3692x_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3692x_led *led =
+			container_of(led_cdev, struct lm3692x_led, led_dev);
+	int ret;
+	int led_brightness_lsb = (brt_val >> 5);
+
+	mutex_lock(&led->lock);
+
+	ret = lm3692x_fault_check(led);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write MSB\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot write LSB\n");
+		goto out;
+	}
+out:
+	mutex_unlock(&led->lock);
+	return ret;
+}
+
+static int lm3692x_init(struct lm3692x_led *led)
+{
+	int ret;
+
+	if (led->regulator) {
+		ret = regulator_enable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to enable regulator\n");
+			goto out;
+	}
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 1);
+
+	ret = lm3692x_fault_check(led);
+	if (ret) {
+		dev_err(&led->client->dev, "Cannot read/clear faults\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Fail writing BRT CTRL\n");
+		goto out;
+	}
+
+	/*
+	 * For glitch free operation, the following data should
+	 * only be written while device enable bit is 0
+	 * per Section 7.5.14 of the data sheet
+	 */
+	ret = regmap_write(led->regmap, LM3692X_PWM_CTRL,
+		LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming PWM CTRL\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL,
+			LM3692X_BRHT_MODE_RAMP_MULTI |
+			LM3692X_BL_ADJ_POL |
+			LM3692X_RAMP_RATE_250us);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming BOOST CTRL\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed programming BL Threshold\n");
+		goto out;
+	}
+
+	ret = regmap_write(led->regmap, LM3692X_BRT_CTRL,
+			LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB);
+	if (ret)
+		dev_err(&led->client->dev, "Failed programming BRT CTRL\n");
+
+	return ret;
+out:
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	return ret;
+}
+
+static int lm3692x_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int ret;
+	struct lm3692x_led *led;
+	struct device_node *np = client->dev.of_node;
+
+	led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	if (client->dev.of_node) {
+		ret = of_property_read_string(np, "label", &led->label);
+		if (ret)
+			led->label = LM3692X_LED_NAME;
+	}
+
+	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(led->enable_gpio)) {
+		ret = PTR_ERR(led->enable_gpio);
+		dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret);
+		return ret;
+	}
+
+	led->regulator = devm_regulator_get(&client->dev, "vled");
+	if (IS_ERR(led->regulator))
+		led->regulator = NULL;
+
+	led->client = client;
+	led->led_dev.name = led->label;
+	led->led_dev.max_brightness = LED_FULL;
+	led->led_dev.brightness_set_blocking = lm3692x_brightness_set;
+
+	mutex_init(&led->lock);
+
+	i2c_set_clientdata(client, led);
+
+	led->regmap = devm_regmap_init_i2c(client, &lm3692x_regmap_config);
+	if (IS_ERR(led->regmap)) {
+		ret = PTR_ERR(led->regmap);
+		dev_err(&client->dev, "Failed to allocate register map: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = lm3692x_init(led);
+	if (ret)
+		return ret;
+
+	ret = led_classdev_register(&client->dev, &led->led_dev);
+	if (ret) {
+		dev_err(&client->dev, "led register err: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int lm3692x_remove(struct i2c_client *client)
+{
+	struct lm3692x_led *led = i2c_get_clientdata(client);
+	int ret;
+
+	led_classdev_unregister(&led->led_dev);
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	return 0;
+}
+
+static const struct i2c_device_id lm3692x_id[] = {
+	{ "lm3692x", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3692x_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_lm3692x_leds_match[] = {
+	{ .compatible = "ti,lm3692x", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match);
+#endif
+
+static struct i2c_driver lm3692x_driver = {
+	.driver = {
+		.name	= "lm3692x",
+		.of_match_table = of_match_ptr(of_lm3692x_leds_match),
+	},
+	.probe		= lm3692x_probe,
+	.remove		= lm3692x_remove,
+	.id_table	= lm3692x_id,
+};
+module_i2c_driver(lm3692x_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>");
+MODULE_LICENSE("GPL");
-- 
2.15.0.124.g7668cbc60

--
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: [PATCH v3 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: H. Nikolaus Schaller @ 2017-11-15 19:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mark Rutland, DTML, letux-kernel, Jonathan Cameron, Tony Lindgren,
	Greg Kroah-Hartman, Russell King, Linux Kernel Mailing List,
	Thierry Reding, Rob Herring, Kevin Hilman, Benoît Cousson,
	kernel, linux-omap, Andreas Färber, Linux ARM
In-Reply-To: <CAK8P3a1Ah35mmbw0qN=zp8TMN9HVgdtTSkkSCCayC-5b6Q960A@mail.gmail.com>

Hi Arnd,

> Am 15.11.2017 um 18:03 schrieb Arnd Bergmann <arnd@arndb.de>:
> 
> On Wed, Nov 15, 2017 at 5:27 PM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Am 15.11.2017 um 16:54 schrieb Arnd Bergmann <arnd@arndb.de>:
>>> On Wed, Nov 15, 2017 at 4:19 PM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>> 
>> There is one more goal. Some people are dreaming about a generic GPS interface.
>> Then, the driver wouldn't have to register a /dev/GPS tty any more but a
>> gps_interface and mangle serial data as requested by that API. This will become
>> a simple upgrade.
>> 
>> So you can consider creating a new tty as sort of temporary solution. Like we
>> had for years for UART HCI based bluetooth devices using a user-space daemon.
> 
> It shouldn't be hard to split out the tty_driver portion of your file from the
> part that registers the port, basically getting two files that each handle
> half of the work, and the second one would be generic from the start.

Hm. Sounds like a big hack to me instead of using existing API (serdev and tty_port)
and making the best out of it.

But I may have misunderstood what you mean by splitting out parts of
a tty (which one?) and why I need two files?

The structure of the driver is:

UART --> serdev magic ---> this device driver ---> register something to present data to user space ---> user space read()

Data should flow following this arrows.
And power control happens this way:

UART --> serdev magic ---> this device driver <--- register something to present data to user space <--- user space open()
GPIO <----------------------------+

So we need one serdev port to communicate with the device and something to present serial data to user-space (where gpsd runs).

> 
>>>> +       /* initialize the tty driver */
>>>> +       data->tty_drv->owner = THIS_MODULE;
>>>> +       data->tty_drv->driver_name = "w2sg0004";
>>>> +       data->tty_drv->name = "ttyGPS";
>>>> +       data->tty_drv->major = 0;
>>>> +       data->tty_drv->minor_start = 0;
>>>> +       data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
>>>> +       data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
>>>> +       data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
>>>> +       data->tty_drv->init_termios = tty_std_termios;
>>>> +       data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
>>>> +                                             HUPCL | CLOCAL;
>>>> +       /*
>>>> +        * optional:
>>>> +        * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
>>>> +                                       115200, 115200);
>>>> +        * w2sg_tty_termios(&data->tty_drv->init_termios);
>>>> +        */
>>>> +       tty_set_operations(data->tty_drv, &w2sg_serial_ops);
>>> 
>>> While I'm still not sure about why we want nested tty port, it
>>> seems that we should have only one tty_driver that gets initialized
>>> at module load time, rather than one driver structure per port.
>> 
>> If we have several such chips connected to different serdev UARTs
>> we need different /dev/GPS to separate them in user-space.
> 
> I understand that you need multiple devices, but I don't see
> what having multiple drivers that all share the same name
> "w2sg0004" helps. I would have expected that to fail in
> tty_register_driver() when you get to the second driver,
> but looking at the code, it doesn't actually try to make the name
> unique

Yes, that is missing because I copied that from other drivers
and have no full understanding what is needed to make it really
work with multiple /dev/ttyGPS0 tty ports.

Therefore each probe (for each device connected to a different uart
of the SoC) should create a different /dev/ttyGPSn. Like you can have
multiple and independent i2c clients of the same type and driver.

> proc_tty_register_driver() will fail to create the second
> procfs file, but we ignore that failure.
> 
> Why not call tty_register_driver() in the init function rather than probe()?

We have no dedicated init function. Should we have one?

And if I understand correctly it would prohibit to fix the driver for the
multiple gps-devices situation. Or makes more work if the device is to be
registered as a future GPS interface.

So if the ->driver_name or ->name should have a dynamic sequence number,
please help me to get it correct.

> 
>>>> +       /* register the tty driver */
>>>> +       err = tty_register_driver(data->tty_drv);
>>>> +       if (err) {
>>>> +               pr_err("%s - tty_register_driver failed(%d)\n",
>>>> +                       __func__, err);
>>>> +               put_tty_driver(data->tty_drv);
>>>> +               goto err_rfkill;
>>>> +       }
>>>> +
>>>> +       tty_port_init(&data->port);
>>>> +       data->port.ops = &w2sg_port_ops;
>>>> +
>>>> +/*
>>>> + * FIXME: this appears to reenter this probe() function a second time
>>>> + * which only fails because the gpio is already assigned
>>>> + */
>>>> +
>>>> +       data->dev = tty_port_register_device(&data->port,
>>>> +                       data->tty_drv, minor, &serdev->dev);
>>> 
>>> This seems to be a result of having nested tty ports, and both
>>> ports point to the same device.
>> 
>> The UART tty would be e.g. /dev/ttyO2 (on OMAP3) if no driver is
>> installed. And the new one that is registered is /dev/GPS0. So the
>> tty subsystem doesn't (or shouldn't) know they are related. They
>> are only related/connected inside this driver. So I assume that
>> some locking or reentrancy happens in tty_port_register_device().
> 
> I meant the serdev->dev pointer that you pass into
> tty_port_register_device() seems to be the same one that
> got passed into the first tty_port_register_device() in the
> parent uart_port.

Ah, interesting!

Well, I copied that from other drivers registering a tty without
understanding all side-effects of everything.

Documentations of tty_port_register_device() says:
@device: parent if exists, otherwise NULL

Do we really need a "parent" here? Could we safely pass NULL?

> 
> I just checked the current mainline code, and it doesn't seem
> to actually call serdev_tty_port_register() from
> tty_port_register_device(), so maybe the comment was
> based on an older version of the serdev framework?

Maybe. We rewrote the driver in parallel to v4.11-rc where this
was observed. Then we only rebased it to now v4.14 but didn't
verify this detail.

I did now test the driver with debugging enabled (after removing
the pdata stuff) on top of v4.14.

But I could not find a trace of this issue (there was a
double w2sg_probe() right after tty_port_register_device):

dmesg|fgrep w2sg
[    8.039184] w2sg_probe()
[    8.039184] w2sg serdev_device_set_drvdata
[    8.039398] w2sg_probe() lna_regulator = dc944c80
[    8.039398] w2sg devm_gpio_request
[    8.039703] w2sg rfkill_alloc
[    8.039733] w2sg register rfkill
[    8.039886] w2sg alloc_tty_driver
[    8.039916] w2sg tty_register_driver
[    8.039916] w2sg call tty_port_init
[    8.039916] w2sg call tty_port_register_device
[    8.040130] w2sg_rfkill_set_block: blocked: 0
[    8.040161] w2sg_set_lna_power: off
[    8.040222] w2sg tty_port_register_device -> ddeda800
[    8.040222] w2sg port.tty =   (null)
[    8.040222] w2sg probed
[    8.040222] w2sg DEBUGGING MODE enabled
[    8.040222] w2sg power gpio ON
[    8.252227] w2sg power gpio OFF
[    8.876617] w2sg_set_power to state=0 (requested=0)
[    9.127410] w2sg00x4 has sent 124 characters data although it should be off!
[    9.127471] w2sg_set_lna_power: off
[    9.127471] w2sg: power gpio ON
[    9.142700] w2sg: power gpio OFF
[    9.162689] w2sg: idle
[  239.280212] w2sg_tty_install() tty = ddfa3a00
[  239.284759] w2sg_tty_install() data = dc858810
[  239.290344] w2sg_tty_open() data = dc858810 open_count = ++0
[  239.296264] w2sg_set_power to state=1 (requested=0)
[  239.301940] w2sg00x4 scheduled for 1
[  239.305725] w2sg_set_lna_power: on
[  239.310913] w2sg: power gpio ON
[  239.327362] w2sg: power gpio OFF
[  239.347351] w2sg: idle
[  239.385162] w2sg00x4: push 1 chars to tty port
[  239.390228] w2sg00x4: push 4 chars to tty port
[  239.395141] w2sg00x4: push 5 chars to tty port
[  239.401184] w2sg00x4: push 5 chars to tty port
[  239.406097] w2sg00x4: push 4 chars to tty port
[  239.412994] w2sg00x4: push 6 chars to tty port
[  239.418731] w2sg00x4: push 5 chars to tty port
[  240.241821] w2sg_tty_close()
[  240.244873] w2sg_set_power to state=0 (requested=1)
[  240.251281] w2sg00x4 scheduled for 0
[  240.255065] w2sg_set_lna_power: off
[  240.261322] w2sg: power gpio ON
[  240.277435] w2sg: power gpio OFF
[  240.297424] w2sg: idle

So it is probed only once. Maybe we did note a bug in early
serdev that already has been fixed?

Hence we are discussing a problem that already has disappeared.

> 
> It seems like something that should be fixed, so maybe
> put a WARN_ON() at the beginning of the probe
> function to see where we come from.

Well, I'd say this notice can be removed as well.

So I'll post a v4 asap.

BR,
Nikolaus

^ permalink raw reply

* Re: [PATCH 1/2] ASoC: codecs: Add PCM186x binding documentation
From: Rob Herring @ 2017-11-15 19:56 UTC (permalink / raw)
  To: Andrew F. Davis
  Cc: Liam Girdwood, Mark Brown, Mark Rutland,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171110195711.24210-1-afd-l0cyMroinI0@public.gmane.org>

On Fri, Nov 10, 2017 at 01:57:10PM -0600, Andrew F. Davis wrote:
> Add the dt-binding documentation for the TI PCM186x 2ch and 4ch Audio
> ADCs With Universal Front End.
> 
> Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/sound/pcm186x.txt          | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/pcm186x.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/pcm186x.txt b/Documentation/devicetree/bindings/sound/pcm186x.txt
> new file mode 100644
> index 000000000000..0bc7d265c7bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/pcm186x.txt
> @@ -0,0 +1,42 @@
> +Texas Instruments PCM186x Universal Audio ADC
> +
> +These devices support both I2C and SPI (configured with pin strapping
> +on the board).
> +
> +Required properties:
> +
> + - compatible : "ti,pcm1862",
> +                "ti,pcm1863",
> +                "ti,pcm1864",
> +                "ti,pcm1865"
> +
> + - reg : The I2C address of the device for I2C, the chip select
> +         number for SPI.
> +
> + - avdd-supply: Analog core power supply (3.3v)
> + - dvdd-supply: Digital core power supply
> + - iovdd-supply: Digital IO power supply
> +        See regulator/regulator.txt for more information
> +
> +CODEC input pins:
> + * VINL1
> + * VINR1
> + * VINL2
> + * VINR2
> + * VINL3
> + * VINR3
> + * VINL4
> + * VINR4
> +
> +The pins can be used in referring sound node's audio-routing property.
> +
> +Example:
> +
> +	pcm186x: pcm186x@4a {

audio-codec@4a

With that,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> +		compatible = "ti,pcm1865";
> +		reg = <0x4a>;
> +
> +		avdd-supply = <&reg_3v3_analog>;
> +		dvdd-supply = <&reg_3v3>;
> +		iovdd-supply = <&reg_1v8>;
> +	};
> -- 
> 2.15.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

* Re: [PATCH v11 1/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver documentation
From: Rob Herring @ 2017-11-15 19:59 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, linux-renesas-soc,
	tomoharu.fukawa.eb, Kieran Bingham, Sakari Ailus,
	Geert Uytterhoeven, devicetree
In-Reply-To: <20171111002526.2646-2-niklas.soderlund+renesas@ragnatech.se>

On Sat, Nov 11, 2017 at 01:25:25AM +0100, Niklas Söderlund wrote:
> Documentation for Renesas R-Car MIPI CSI-2 receiver. The CSI-2 receivers
> are located between the video sources (CSI-2 transmitters) and the video
> grabbers (VIN) on Gen3 of Renesas R-Car SoC.
> 
> Each CSI-2 device is connected to more then one VIN device which
> simultaneously can receive video from the same CSI-2 device. Each VIN
> device can also be connected to more then one CSI-2 device. The routing
> of which link are used are controlled by the VIN devices. There are only
> a few possible routes which are set by hardware limitations, which are
> different for each SoC in the Gen3 family.
> 
> To work with the limitations of routing possibilities it is necessary
> for the DT bindings to describe which VIN device is connected to which
> CSI-2 device. This is why port 1 needs to to assign reg numbers for each
> VIN device that be connected to it. To setup and to know which links are
> valid for each SoC is the responsibility of the VIN driver since the
> register to configure it belongs to the VIN hardware.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  .../bindings/media/renesas,rcar-csi2.txt           | 104 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> new file mode 100644
> index 0000000000000000..24705d8997b14a10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -0,0 +1,104 @@
> +Renesas R-Car MIPI CSI-2
> +------------------------
> +
> +The rcar-csi2 device provides MIPI CSI-2 capabilities for the Renesas R-Car
> +family of devices. It is to be used in conjunction with the R-Car VIN module,
> +which provides the video capture capabilities.
> +
> +Mandatory properties
> +--------------------
> + - compatible: Must be one or more of the following
> +   - "renesas,r8a7795-csi2" for the R8A7795 device.
> +   - "renesas,r8a7796-csi2" for the R8A7796 device.
> +
> + - reg: the register base and size for the device registers
> + - interrupts: the interrupt for the device
> + - clocks: Reference to the parent clock
> +
> +The device node shall contain two 'port' child nodes according to the
> +bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt. Port 0 shall connect the node that is the video
> +source for to the CSI-2. Port 1 shall connect all the R-Car VIN
> +modules, which can make use of the CSI-2 module.
> +
> +- Port 0 - Video source (Mandatory)
> +	- Endpoint 0 - sub-node describing the endpoint that is the video source
> +
> +- Port 1 - VIN instances (Mandatory for all VIN present in the SoC)
> +	- Endpoint 0 - sub-node describing the endpoint that is VIN0
> +	- Endpoint 1 - sub-node describing the endpoint that is VIN1
> +	- Endpoint 2 - sub-node describing the endpoint that is VIN2
> +	- Endpoint 3 - sub-node describing the endpoint that is VIN3
> +	- Endpoint 4 - sub-node describing the endpoint that is VIN4
> +	- Endpoint 5 - sub-node describing the endpoint that is VIN5
> +	- Endpoint 6 - sub-node describing the endpoint that is VIN6
> +	- Endpoint 7 - sub-node describing the endpoint that is VIN7
> +
> +Example:
> +
> +	csi20: csi2@fea80000 {
> +		compatible = "renesas,r8a7796-csi2", "renesas,rcar-gen3-csi2";
> +		reg = <0 0xfea80000 0 0x10000>;
> +		interrupts = <0 184 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 714>;
> +		power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
> +		resets = <&cpg 714>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				reg = <0>;
> +
> +				csi20_in: endpoint@0 {

unit-address without reg property is not valid.

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>


> +					clock-lanes = <0>;
> +					data-lanes = <1>;
> +					remote-endpoint = <&adv7482_txb>;
> +				};
> +			};
> +
> +			port@1 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				reg = <1>;
> +
> +				csi20vin0: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&vin0csi20>;
> +				};
> +				csi20vin1: endpoint@1 {
> +					reg = <1>;
> +					remote-endpoint = <&vin1csi20>;
> +				};
> +				csi20vin2: endpoint@2 {
> +					reg = <2>;
> +					remote-endpoint = <&vin2csi20>;
> +				};
> +				csi20vin3: endpoint@3 {
> +					reg = <3>;
> +					remote-endpoint = <&vin3csi20>;
> +				};
> +				csi20vin4: endpoint@4 {
> +					reg = <4>;
> +					remote-endpoint = <&vin4csi20>;
> +				};
> +				csi20vin5: endpoint@5 {
> +					reg = <5>;
> +					remote-endpoint = <&vin5csi20>;
> +				};
> +				csi20vin6: endpoint@6 {
> +					reg = <6>;
> +					remote-endpoint = <&vin6csi20>;
> +				};
> +				csi20vin7: endpoint@7 {
> +					reg = <7>;
> +					remote-endpoint = <&vin7csi20>;
> +				};
> +			};
> +		};
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index adbf69306e9ee3d2..fe999e9de76e3cb3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8565,6 +8565,7 @@ L:	linux-media@vger.kernel.org
>  L:	linux-renesas-soc@vger.kernel.org
>  T:	git git://linuxtv.org/media_tree.git
>  S:	Supported
> +F:	Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
>  F:	Documentation/devicetree/bindings/media/rcar_vin.txt
>  F:	drivers/media/platform/rcar-vin/
>  
> -- 
> 2.15.0
> 

^ permalink raw reply

* Re: [PATCH v7 01/25] rcar-vin: add Gen3 devicetree bindings documentation
From: Rob Herring @ 2017-11-15 20:02 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Laurent Pinchart, Hans Verkuil, linux-media, linux-renesas-soc,
	tomoharu.fukawa.eb, Kieran Bingham, devicetree
In-Reply-To: <20171111003835.4909-2-niklas.soderlund+renesas@ragnatech.se>

On Sat, Nov 11, 2017 at 01:38:11AM +0100, Niklas Söderlund wrote:
> Document the devicetree bindings for the CSI-2 inputs available on Gen3.
> 
> There is a need to add a custom property 'renesas,id' and to define
> which CSI-2 input is described in which endpoint under the port@1 node.
> This information is needed since there are a set of predefined routes
> between each VIN and CSI-2 block. This routing table will be kept
> inside the driver but in order for it to act on it it must know which
> VIN and CSI-2 is which.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  .../devicetree/bindings/media/rcar_vin.txt         | 116 ++++++++++++++++++---
>  1 file changed, 104 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 6e4ef8caf759e5d3..df1abd0fb20386f8 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -2,8 +2,12 @@ Renesas R-Car Video Input driver (rcar_vin)
>  -------------------------------------------
>  
>  The rcar_vin device provides video input capabilities for the Renesas R-Car
> -family of devices. The current blocks are always slaves and suppot one input
> -channel which can be either RGB, YUYV or BT656.
> +family of devices.
> +
> +Each VIN instance has a single parallel input that supports RGB and YUV video,
> +with both external synchronization and BT.656 synchronization for the latter.
> +Depending on the instance the VIN input is connected to external SoC pins, or
> +on Gen3 to a CSI-2 receiver.
>  
>   - compatible: Must be one or more of the following
>     - "renesas,vin-r8a7795" for the R8A7795 device
> @@ -28,21 +32,38 @@ channel which can be either RGB, YUYV or BT656.
>  Additionally, an alias named vinX will need to be created to specify
>  which video input device this is.
>  
> -The per-board settings:
> +The per-board settings Gen2:
>   - port sub-node describing a single endpoint connected to the vin
>     as described in video-interfaces.txt[1]. Only the first one will
>     be considered as each vin interface has one input port.
>  
> -   These settings are used to work out video input format and widths
> -   into the system.
> +The per-board settings Gen3:
> +
> +Gen3 can support both a single connected parallel input source from
> +external SoC pins (port0) and/or multiple parallel input sources from
> +local SoC CSI-2 receivers (port1) depending on SoC.
>  
> +- renesas,id - ID number of the VIN, VINx in the documentation.

Why is this needed? We try to avoid indexes unless that's the only way a 
device is addressed (and then we use reg).

^ permalink raw reply

* Re: [PATCH v2 1/5] DT: vendor prefix for Wi2Wi, Inc.
From: Rob Herring @ 2017-11-15 20:04 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <8fb996b8bd96e4a9205193ce712da9deff090771.1510520397.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

On Sun, Nov 12, 2017 at 09:59:54PM +0100, H. Nikolaus Schaller wrote:
> Introduce vendor prefix for Wi2Wi, Inc. for W2SG00x4 GPS modules
> and W2CBW003 Bluetooth/WiFi combo (CSR/Marvell).
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

"dt-bindings: ..." is the preferred subject prefix. Otherwise,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 1/5] DT: vendor prefix for Wi2Wi, Inc.
From: H. Nikolaus Schaller @ 2017-11-15 20:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <20171115200432.6azjwbco4r5y3e7z@rob-hp-laptop>


> Am 15.11.2017 um 21:04 schrieb Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> 
> On Sun, Nov 12, 2017 at 09:59:54PM +0100, H. Nikolaus Schaller wrote:
>> Introduce vendor prefix for Wi2Wi, Inc. for W2SG00x4 GPS modules
>> and W2CBW003 Bluetooth/WiFi combo (CSR/Marvell).
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>> 1 file changed, 1 insertion(+)
> 
> "dt-bindings: ..." is the preferred subject prefix. Otherwise,
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Ok,
changed for v4.

BR and thanks,
Nikolaus Schaller
--
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 2/5] DT: misc: w2sg0004: add bindings documentation (GPS module with serdev UART)
From: Rob Herring @ 2017-11-15 20:08 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <069ce12d5609c4e968f2a468e6c58a57f59f3ffb.1510520397.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

On Sun, Nov 12, 2017 at 09:59:55PM +0100, H. Nikolaus Schaller wrote:
> add bindings documentation for Wi2Wi W2SG00x4 GPS module

Add ... module.

> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    | 24 ++++++++++++++++++++++

The path doesn't have to match the kernel. Use bindings/gps/...

>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> new file mode 100644
> index 000000000000..ccec9361a1a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> @@ -0,0 +1,24 @@
> +Wi2Wi GPS module connected through UART
> +
> +Should be a subnode of the SoC UART it is connected to (serdev).

Could be connected to any kind of UART and serdev is a Linuxism.

With those fixed,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 2/2] leds: lm3692x: Introduce LM3692x dual string driver
From: Dan Murphy @ 2017-11-15 20:12 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <20171115193031.28781-2-dmurphy@ti.com>

On 11/15/2017 01:30 PM, Dan Murphy wrote:
> Introducing the LM3692x Dual-String white LED driver.
> 
> Data sheet is located
> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init
> routine, return on fault_check failure, updated brightness calculation and
> fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/
> 
>  drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 390 insertions(+)
>  create mode 100644 drivers/leds/leds-lm3692x.c
> 
> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
> new file mode 100644
> index 000000000000..9fa7f5a20466

Forgot to add the Makefile and Kconfig so submitting v3 with them included.

<snip>
-- 
------------------
Dan Murphy

^ permalink raw reply

* Re: [PATCH v3 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: Arnd Bergmann @ 2017-11-15 20:12 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, DTML, linux-omap, Tony Lindgren, Greg Kroah-Hartman,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf, Russell King,
	Linux Kernel Mailing List, Rob Herring, Linux ARM,
	Benoît Cousson, Kevin Hilman,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w, Thierry Reding,
	Andreas Färber, Jonathan Cameron
In-Reply-To: <AFF4418B-740D-4FFE-A3A6-A92E73F0B901-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

On Wed, Nov 15, 2017 at 8:55 PM, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
>> Am 15.11.2017 um 18:03 schrieb Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>> On Wed, Nov 15, 2017 at 5:27 PM, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
>>>> Am 15.11.2017 um 16:54 schrieb Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>:
>>>> On Wed, Nov 15, 2017 at 4:19 PM, H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
>>>>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>>>
>>> There is one more goal. Some people are dreaming about a generic GPS interface.
>>> Then, the driver wouldn't have to register a /dev/GPS tty any more but a
>>> gps_interface and mangle serial data as requested by that API. This will become
>>> a simple upgrade.
>>>
>>> So you can consider creating a new tty as sort of temporary solution. Like we
>>> had for years for UART HCI based bluetooth devices using a user-space daemon.
>>
>> It shouldn't be hard to split out the tty_driver portion of your file from the
>> part that registers the port, basically getting two files that each handle
>> half of the work, and the second one would be generic from the start.
>
> Hm. Sounds like a big hack to me instead of using existing API (serdev and tty_port)
> and making the best out of it.
>
> But I may have misunderstood what you mean by splitting out parts of
> a tty (which one?) and why I need two files?
>
> The structure of the driver is:
>
> UART --> serdev magic ---> this device driver ---> register something to present data to user space ---> user space read()
>
> Data should flow following this arrows.
> And power control happens this way:
>
> UART --> serdev magic ---> this device driver <--- register something to present data to user space <--- user space open()
> GPIO <----------------------------+
>
> So we need one serdev port to communicate with the device and something to present serial data to user-space (where gpsd runs).

My suggestion was directed at replacing the big hack (adding one
driver per port, and a different driver for each type of GPS hardware)
with a smaller hack, using one driver to manage the ttyGPS name
space for any implementation. That same driver could later even be
expanded to offer additional ioctl interfaces, e.g. for reading the
current position or time.

Let's ignore that point for the moment though and finish the
discussion below, I think it will become clearer what I meant here
when the split between init() and probe() function is resolved.

>>>>> +       /* initialize the tty driver */
>>>>> +       data->tty_drv->owner = THIS_MODULE;
>>>>> +       data->tty_drv->driver_name = "w2sg0004";
>>>>> +       data->tty_drv->name = "ttyGPS";
>>>>> +       data->tty_drv->major = 0;
>>>>> +       data->tty_drv->minor_start = 0;
>>>>> +       data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
>>>>> +       data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
>>>>> +       data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
>>>>> +       data->tty_drv->init_termios = tty_std_termios;
>>>>> +       data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
>>>>> +                                             HUPCL | CLOCAL;
>>>>> +       /*
>>>>> +        * optional:
>>>>> +        * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
>>>>> +                                       115200, 115200);
>>>>> +        * w2sg_tty_termios(&data->tty_drv->init_termios);
>>>>> +        */
>>>>> +       tty_set_operations(data->tty_drv, &w2sg_serial_ops);
>>>>
>>>> While I'm still not sure about why we want nested tty port, it
>>>> seems that we should have only one tty_driver that gets initialized
>>>> at module load time, rather than one driver structure per port.
>>>
>>> If we have several such chips connected to different serdev UARTs
>>> we need different /dev/GPS to separate them in user-space.
>>
>> I understand that you need multiple devices, but I don't see
>> what having multiple drivers that all share the same name
>> "w2sg0004" helps. I would have expected that to fail in
>> tty_register_driver() when you get to the second driver,
>> but looking at the code, it doesn't actually try to make the name
>> unique
>
> Yes, that is missing because I copied that from other drivers
> and have no full understanding what is needed to make it really
> work with multiple /dev/ttyGPS0 tty ports.
>
> Therefore each probe (for each device connected to a different uart
> of the SoC) should create a different /dev/ttyGPSn. Like you can have
> multiple and independent i2c clients of the same type and driver.

Right, that's what I mean. But each of those devices should
use the same driver, just like each omap tty port uses the same
'serial_omap_reg' (which includes a tty driver).

>> proc_tty_register_driver() will fail to create the second
>> procfs file, but we ignore that failure.
>>
>> Why not call tty_register_driver() in the init function rather than probe()?
>
> We have no dedicated init function. Should we have one?
>
> And if I understand correctly it would prohibit to fix the driver for the
> multiple gps-devices situation. Or makes more work if the device is to be
> registered as a future GPS interface.
>
> So if the ->driver_name or ->name should have a dynamic sequence number,
> please help me to get it correct.

There should only be one driver structure, as in any other tty driver
implementation. You have an init function inside of the
module_serdev_device_driver() macro. When you open-code that,
you can register the tty driver there before registering the serdev driver,
see serial_omap_init() for a similar example using uart_driver and
platform_driver.

>>> The UART tty would be e.g. /dev/ttyO2 (on OMAP3) if no driver is
>>> installed. And the new one that is registered is /dev/GPS0. So the
>>> tty subsystem doesn't (or shouldn't) know they are related. They
>>> are only related/connected inside this driver. So I assume that
>>> some locking or reentrancy happens in tty_port_register_device().
>>
>> I meant the serdev->dev pointer that you pass into
>> tty_port_register_device() seems to be the same one that
>> got passed into the first tty_port_register_device() in the
>> parent uart_port.
>
> Ah, interesting!
>
> Well, I copied that from other drivers registering a tty without
> understanding all side-effects of everything.
>
> Documentations of tty_port_register_device() says:
> @device: parent if exists, otherwise NULL
>
> Do we really need a "parent" here? Could we safely pass NULL?

There should be a parent, to make it show up in the right place in sysfs.

Ideally the parent should point to a device representing the original
uart port. It probably is that right now, so I'd leave it like that if it
works without getting you into a double probe.

       Arnd
--
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 v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver
From: Jacek Anaszewski @ 2017-11-15 20:15 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w, pavel-+ZI9xUNit7I,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Daniel Thompson,
	Jingoo Han
In-Reply-To: <20171115194203.13572-1-dmurphy-l0cyMroinI0@public.gmane.org>

Hi Dan,

Thanks for the patch.

On 11/15/2017 08:42 PM, Dan Murphy wrote:
> This adds the devicetree bindings for the LM3692x
> I2C LED string driver.
> 
> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
> ---
> 
> v3 - No changes
> v2 - No changes - https://patchwork.kernel.org/patch/10056677/
> 
>  .../devicetree/bindings/leds/leds-lm3692x.txt      | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> new file mode 100644
> index 000000000000..cfef67bd4100
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt
> @@ -0,0 +1,28 @@
> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver
> +
> +The LM36922 is an ultra-compact, highly efficient,
> +two string white-LED driver designed for LCD display
> +backlighting.

Shouldn't the driver be targeted for backlight subsystem then?

Adding backlight maintainers.

Best regards,
Jacek Anaszewski

> +Required properties:
> +	- compatible:
> +		"ti,lm3692x"
> +	- reg -  I2C slave address
> +
> +Optional properties:
> +	- label - Used for naming LEDs
> +	- enable-gpio - gpio pin to enable/disable the device.
> +	- supply - "vled" - LED supply
> +
> +Example:
> +
> +leds: leds@6 {
> +	compatible = "ti,lm3692x";
> +	reg = <0x36>;
> +	label = "backlight_cluster";
> +	enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +	vled-supply = <&vbatt>;
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf
> 

--
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 3/5] misc: Add w2sg0004 (gps receiver) power control driver
From: Rob Herring @ 2017-11-15 20:17 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Thierry Reding, Jonathan Cameron, Maxime Ripard, Jarkko Sakkinen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <1dd7e2ccf3d0361c0bb947bacbc2c00066e528c4.1510520397.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

On Sun, Nov 12, 2017 at 09:59:56PM +0100, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
> 
> Use serdev API hooks to monitor and forward the UART traffic to /dev/GPSn
> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
> 
> Additionally, rfkill block/unblock can be used to control an external LNA
> (and power down the module if not needed).
> 
> The driver concept is based on code developed by NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
> but simplified and adapted to use the new serdev API introduced in 4.11.
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> ---
>  drivers/misc/Kconfig     |  10 +
>  drivers/misc/Makefile    |   1 +
>  drivers/misc/w2sg0004.c  | 565 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/w2sg0004.h |  27 +++
>  4 files changed, 603 insertions(+)
>  create mode 100644 drivers/misc/w2sg0004.c
>  create mode 100644 include/linux/w2sg0004.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8136dc7e863d..09d171d68408 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>  source "drivers/misc/genwqe/Kconfig"
>  source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> +	tristate "W2SG00x4 on/off control"
> +	depends on GPIOLIB && SERIAL_DEV_BUS
> +	help
> +          Enable on/off control of W2SG00x4 GPS moduled connected
> +	  to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> +	  is opened/closed.
> +	  It also provides a rfkill gps name to control the LNA power.
> +
>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index ad0e64fdba34..abcb667e0ff0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC)		+= sram-exec.o
>  obj-y				+= mic/
>  obj-$(CONFIG_GENWQE)		+= genwqe/
>  obj-$(CONFIG_ECHO)		+= echo/
> +obj-$(CONFIG_W2SG0004)		+= w2sg0004.o
>  obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
>  obj-$(CONFIG_CXL_BASE)		+= cxl/
>  obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index 000000000000..55101aa6beeb
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,565 @@
> +/*
> + * w2sg0004.c

Listing the filename is pointless IMO. You need a license too. Use SPDX 
tag (on first line with C++ style comment).

> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
> + *
> + * This receiver has an ON/OFF pin which must be toggled to
> + * turn the device 'on' of 'off'.  A high->low->high toggle
> + * will switch the device on if it is off, and off if it is on.
> + *
> + * To enable receiving on/off requests we register with the
> + * UART power management notifications.
> + *
> + * It is not possible to directly detect the state of the device.
> + * However when it is on it will send characters on a UART line
> + * regularly.
> + *
> + * To detect that the power state is out of sync (e.g. if GPS
> + * was enabled before a reboot), we register for UART data received
> + * notifications.
> + *
> + * In addition we register as a rfkill client so that we can
> + * control the LNA power.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/rfkill.h>
> +#include <linux/serdev.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/w2sg0004.h>
> +#include <linux/workqueue.h>
> +
> +/*
> + * There seems to be restrictions on how quickly we can toggle the
> + * on/off line.  data sheets says "two rtc ticks", whatever that means.
> + * If we do it too soon it doesn't work.
> + * So we have a state machine which uses the common work queue to ensure
> + * clean transitions.
> + * When a change is requested we record that request and only act on it
> + * once the previous change has completed.
> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
> + * one change per second.
> + */
> +
> +enum w2sg_state {
> +	W2SG_IDLE,	/* is not changing state */
> +	W2SG_PULSE,	/* activate on/off impulse */
> +	W2SG_NOPULSE	/* deactivate on/off impulse */
> +};
> +
> +struct w2sg_data {
> +	struct		rfkill *rf_kill;
> +	struct		regulator *lna_regulator;
> +	int		lna_blocked;	/* rfkill block gps active */
> +	int		lna_is_off;	/* LNA is currently off */
> +	int		is_on;		/* current state (0/1) */
> +	unsigned long	last_toggle;
> +	unsigned long	backoff;	/* time to wait since last_toggle */
> +	int		on_off_gpio;	/* the on-off gpio number */
> +	struct		serdev_device *uart;	/* uart connected to the chip */
> +	struct		tty_driver *tty_drv;	/* this is the user space tty */
> +	struct		device *dev;	/* from tty_port_register_device() */
> +	struct		tty_port port;
> +	int		open_count;	/* how often we were opened */
> +	enum		w2sg_state state;
> +	int		requested;	/* requested state (0/1) */
> +	int		suspended;
> +	struct delayed_work work;
> +	int		discard_count;
> +};
> +
> +static struct w2sg_data *w2sg_by_minor[1];
> +
> +static int w2sg_set_lna_power(struct w2sg_data *data)
> +{
> +	int ret = 0;
> +	int off = data->suspended || !data->requested || data->lna_blocked;
> +
> +	pr_debug("%s: %s\n", __func__, off ? "off" : "on");
> +
> +	if (off != data->lna_is_off) {
> +		data->lna_is_off = off;
> +		if (!IS_ERR_OR_NULL(data->lna_regulator)) {
> +			if (off)
> +				regulator_disable(data->lna_regulator);
> +			else
> +				ret = regulator_enable(data->lna_regulator);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void w2sg_set_power(void *pdata, int val)
> +{
> +	struct w2sg_data *data = (struct w2sg_data *) pdata;
> +
> +	pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
> +
> +	if (val && !data->requested) {
> +		data->requested = true;
> +	} else if (!val && data->requested) {
> +		data->backoff = HZ;
> +		data->requested = false;
> +	} else
> +		return;
> +
> +	pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
> +
> +	if (!data->suspended)
> +		schedule_delayed_work(&data->work, 0);
> +}
> +
> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
> +
> +static int w2sg_uart_receive_buf(struct serdev_device *serdev,
> +				const unsigned char *rxdata,
> +				size_t count)
> +{
> +	struct w2sg_data *data =
> +		(struct w2sg_data *) serdev_device_get_drvdata(serdev);
> +
> +	if (!data->requested && !data->is_on) {
> +		/*
> +		 * we have received characters while the w2sg
> +		 * should have been be turned off
> +		 */
> +		data->discard_count += count;
> +		if ((data->state == W2SG_IDLE) &&
> +		    time_after(jiffies,
> +		    data->last_toggle + data->backoff)) {
> +			/* Should be off by now, time to toggle again */
> +			pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
> +				data->discard_count);
> +
> +			data->discard_count = 0;
> +
> +			data->is_on = true;
> +			data->backoff *= 2;
> +			if (!data->suspended)
> +				schedule_delayed_work(&data->work, 0);
> +		}
> +	} else if (data->open_count > 0) {
> +		int n;
> +
> +		pr_debug("w2sg00x4: push %d chars to tty port\n", count);
> +
> +		/* pass to user-space */
> +		n = tty_insert_flip_string(&data->port, rxdata, count);
> +		if (n != count)
> +			pr_err("w2sg00x4: did loose %d characters\n", count - n);
> +		tty_flip_buffer_push(&data->port);
> +		return n;
> +	}
> +
> +	/* assume we have processed everything */
> +	return count;
> +}
> +
> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
> +
> +static void toggle_work(struct work_struct *work)
> +{
> +	struct w2sg_data *data = container_of(work, struct w2sg_data,
> +					      work.work);
> +
> +	switch (data->state) {
> +	case W2SG_IDLE:
> +		if (data->requested == data->is_on)
> +			return;
> +
> +		w2sg_set_lna_power(data);	/* update LNA power state */
> +		gpio_set_value_cansleep(data->on_off_gpio, 0);
> +		data->state = W2SG_PULSE;
> +
> +		pr_debug("w2sg: power gpio ON\n");
> +
> +		schedule_delayed_work(&data->work,
> +				      msecs_to_jiffies(10));
> +		break;
> +
> +	case W2SG_PULSE:
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->last_toggle = jiffies;
> +		data->state = W2SG_NOPULSE;
> +		data->is_on = !data->is_on;
> +
> +		pr_debug("w2sg: power gpio OFF\n");
> +
> +		schedule_delayed_work(&data->work,
> +				      msecs_to_jiffies(10));
> +		break;
> +
> +	case W2SG_NOPULSE:
> +		data->state = W2SG_IDLE;
> +
> +		pr_debug("w2sg: idle\n");
> +
> +		break;
> +
> +	}
> +}
> +
> +static int w2sg_rfkill_set_block(void *pdata, bool blocked)
> +{
> +	struct w2sg_data *data = pdata;
> +
> +	pr_debug("%s: blocked: %d\n", __func__, blocked);
> +
> +	data->lna_blocked = blocked;
> +
> +	return w2sg_set_lna_power(data);
> +}
> +
> +static struct rfkill_ops w2sg0004_rfkill_ops = {
> +	.set_block = w2sg_rfkill_set_block,
> +};
> +
> +static struct serdev_device_ops serdev_ops = {
> +	.receive_buf = w2sg_uart_receive_buf,
> +};
> +
> +/*
> + * we are a man-in the middle between the user-space visible tty port
> + * and the serdev tty where the chip is connected.
> + * This allows us to recognise when the device should be powered on
> + * or off and handle the "false" state that data arrives while no
> + * users-space tty client exists.
> + */
> +
> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
> +{
> +	return w2sg_by_minor[minor];
> +}
> +
> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	struct w2sg_data *data;
> +	int retval;
> +
> +	pr_debug("%s() tty = %p\n", __func__, tty);
> +
> +	data = w2sg_get_by_minor(tty->index);
> +
> +	if (!data)
> +		return -ENODEV;
> +
> +	retval = tty_standard_install(driver, tty);
> +	if (retval)
> +		goto error_init_termios;
> +
> +	tty->driver_data = data;
> +
> +	return 0;
> +
> +error_init_termios:
> +	tty_port_put(&data->port);
> +	return retval;
> +}
> +
> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
> +
> +	w2sg_set_power(data, ++data->open_count > 0);
> +
> +	return tty_port_open(&data->port, tty, file);
> +}
> +
> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	w2sg_set_power(data, --data->open_count > 0);
> +
> +	tty_port_close(&data->port, tty, file);
> +}
> +
> +static int w2sg_tty_write(struct tty_struct *tty,
> +		const unsigned char *buffer, int count)
> +{
> +	struct w2sg_data *data = tty->driver_data;
> +
> +	/* simply pass down to UART */
> +	return serdev_device_write_buf(data->uart, buffer, count);
> +}
> +
> +
> +static const struct tty_operations w2sg_serial_ops = {
> +	.install = w2sg_tty_install,
> +	.open = w2sg_tty_open,
> +	.close = w2sg_tty_close,
> +	.write = w2sg_tty_write,
> +};
> +
> +static const struct tty_port_operations w2sg_port_ops = {
> +};
> +
> +static int w2sg_probe(struct serdev_device *serdev)
> +{
> +	struct w2sg_pdata *pdata = NULL;
> +	struct w2sg_data *data;
> +	struct rfkill *rf_kill;
> +	int err;
> +	int minor;
> +
> +	pr_debug("%s()\n", __func__);
> +
> +	minor = 0;
> +	if (w2sg_by_minor[minor]) {
> +		pr_err("w2sg minor is already in use!\n");
> +		return -ENODEV;
> +	}
> +
> +	if (serdev->dev.of_node) {
> +		struct device *dev = &serdev->dev;
> +		enum of_gpio_flags flags;
> +
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata)
> +			return -ENOMEM;
> +
> +		pdata->on_off_gpio = of_get_named_gpio_flags(dev->of_node,
> +							     "enable-gpios", 0,
> +							     &flags);
> +
> +		/* defer until we have all gpios */
> +		if (pdata->on_off_gpio == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		pdata->lna_regulator = devm_regulator_get_optional(dev, "lna");
> +		if (IS_ERR(pdata->lna_regulator)) {
> +			/* defer until we can get the regulator */
> +			if (PTR_ERR(pdata->lna_regulator) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +
> +			pdata->lna_regulator = NULL;
> +		}
> +
> +		pr_debug("%s() lna_regulator = %p\n", __func__,
> +			pdata->lna_regulator);
> +
> +		serdev->dev.platform_data = pdata;
> +	}
> +
> +	data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	w2sg_by_minor[minor] = data;
> +
> +	serdev_device_set_drvdata(serdev, data);
> +
> +	data->lna_regulator = pdata->lna_regulator;

NULL ptr dereference if you have no of_node.

> +	data->lna_blocked = true;
> +	data->lna_is_off = true;
> +
> +	data->on_off_gpio = pdata->on_off_gpio;
> +
> +	data->is_on = false;
> +	data->requested = false;
> +	data->state = W2SG_IDLE;
> +	data->last_toggle = jiffies;
> +	data->backoff = HZ;
> +
> +	data->uart = serdev;
> +
> +	INIT_DELAYED_WORK(&data->work, toggle_work);
> +
> +	err = devm_gpio_request(&serdev->dev, data->on_off_gpio,
> +				"w2sg0004-on-off");
> +	if (err < 0)
> +		goto out;
> +
> +	gpio_direction_output(data->on_off_gpio, false);
> +
> +	serdev_device_set_client_ops(data->uart, &serdev_ops);
> +	serdev_device_open(data->uart);
> +
> +	serdev_device_set_baudrate(data->uart, 9600);
> +	serdev_device_set_flow_control(data->uart, false);
> +
> +	rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
> +				&w2sg0004_rfkill_ops, data);
> +	if (rf_kill == NULL) {
> +		err = -ENOMEM;
> +		goto err_rfkill;
> +	}
> +
> +	err = rfkill_register(rf_kill);
> +	if (err) {
> +		dev_err(&serdev->dev, "Cannot register rfkill device\n");
> +		goto err_rfkill;
> +	}
> +
> +	data->rf_kill = rf_kill;
> +
> +	/* allocate the tty driver */
> +	data->tty_drv = alloc_tty_driver(1);

Is there an advantage or reason to make this a tty driver rather than 
just a char driver?

Perhaps this should be under drivers/tty if it is a tty driver.

> +	if (!data->tty_drv)
> +		return -ENOMEM;
> +
> +	/* initialize the tty driver */
> +	data->tty_drv->owner = THIS_MODULE;
> +	data->tty_drv->driver_name = "w2sg0004";
> +	data->tty_drv->name = "ttyGPS";
> +	data->tty_drv->major = 0;
> +	data->tty_drv->minor_start = 0;
> +	data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
> +	data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
> +	data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
> +	data->tty_drv->init_termios = tty_std_termios;
> +	data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
> +					      HUPCL | CLOCAL;
> +	/*
> +	 * optional:
> +	 * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
> +					115200, 115200);
> +	 * w2sg_tty_termios(&data->tty_drv->init_termios);
> +	 */
> +	tty_set_operations(data->tty_drv, &w2sg_serial_ops);
> +
> +	/* register the tty driver */
> +	err = tty_register_driver(data->tty_drv);
> +	if (err) {
> +		pr_err("%s - tty_register_driver failed(%d)\n",
> +			__func__, err);
> +		put_tty_driver(data->tty_drv);
> +		goto err_rfkill;
> +	}
> +
> +	tty_port_init(&data->port);
> +	data->port.ops = &w2sg_port_ops;
> +
> +/*
> + * FIXME: this appears to reenter this probe() function a second time
> + * which only fails because the gpio is already assigned
> + */
> +
> +	data->dev = tty_port_register_device(&data->port,
> +			data->tty_drv, minor, &serdev->dev);
> +
> +	pr_debug("w2sg probed\n");
> +
> +	/* keep off until user space requests the device */
> +	w2sg_set_power(data, false);
> +
> +	return 0;
> +
> +err_rfkill:
> +	rfkill_destroy(rf_kill);
> +	serdev_device_close(data->uart);
> +out:
> +	return err;
> +}
> +
> +static void w2sg_remove(struct serdev_device *serdev)
> +{
> +	struct w2sg_data *data = serdev_device_get_drvdata(serdev);
> +	int minor;
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	/* what is the right sequence to avoid problems? */
> +	serdev_device_close(data->uart);
> +
> +	minor = 0;
> +	tty_unregister_device(data->tty_drv, minor);
> +
> +	tty_unregister_driver(data->tty_drv);
> +}
> +
> +static int w2sg_suspend(struct device *dev)
> +{
> +	struct w2sg_data *data = dev_get_drvdata(dev);
> +
> +	data->suspended = true;
> +
> +	cancel_delayed_work_sync(&data->work);
> +
> +	w2sg_set_lna_power(data);	/* shuts down if needed */
> +
> +	if (data->state == W2SG_PULSE) {
> +		msleep(10);
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->last_toggle = jiffies;
> +		data->is_on = !data->is_on;
> +		data->state = W2SG_NOPULSE;
> +	}
> +
> +	if (data->state == W2SG_NOPULSE) {
> +		msleep(10);
> +		data->state = W2SG_IDLE;
> +	}
> +
> +	if (data->is_on) {
> +		pr_info("GPS off for suspend %d %d %d\n", data->requested,
> +			data->is_on, data->lna_is_off);
> +
> +		gpio_set_value_cansleep(data->on_off_gpio, 0);
> +		msleep(10);
> +		gpio_set_value_cansleep(data->on_off_gpio, 1);
> +		data->is_on = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int w2sg_resume(struct device *dev)
> +{
> +	struct w2sg_data *data = dev_get_drvdata(dev);
> +
> +	data->suspended = false;
> +
> +	pr_info("GPS resuming %d %d %d\n", data->requested,
> +		data->is_on, data->lna_is_off);
> +
> +	schedule_delayed_work(&data->work, 0);	/* enables LNA if needed */
> +
> +	return 0;
> +}
> +
> +#if defined(CONFIG_OF)
> +static const struct of_device_id w2sg0004_of_match[] = {
> +	{ .compatible = "wi2wi,w2sg0004" },
> +	{ .compatible = "wi2wi,w2sg0084" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
> +#endif
> +
> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_suspend, w2sg_resume);
> +
> +static struct serdev_device_driver w2sg_driver = {
> +	.probe		= w2sg_probe,
> +	.remove		= w2sg_remove,
> +	.driver = {
> +		.name	= "w2sg0004",
> +		.owner	= THIS_MODULE,
> +		.pm	= &w2sg_pm_ops,
> +		.of_match_table = of_match_ptr(w2sg0004_of_match)
> +	},
> +};
> +
> +module_serdev_device_driver(w2sg_driver);
> +
> +MODULE_ALIAS("w2sg0004");
> +
> +MODULE_AUTHOR("NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>");
> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/w2sg0004.h b/include/linux/w2sg0004.h
> new file mode 100644
> index 000000000000..ad0c4a18e01d
> --- /dev/null
> +++ b/include/linux/w2sg0004.h
> @@ -0,0 +1,27 @@
> +/*
> + * UART slave to allow ON/OFF control of w2sg0004 GPS receiver.
> + *
> + * Copyright (C) 2011 Neil Brown <neil-+NVA1uvv1dVBDLzU/O5InQ@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
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +
> +
> +#ifndef __LINUX_W2SG0004_H
> +#define __LINUX_W2SG0004_H
> +
> +#include <linux/regulator/consumer.h>
> +
> +struct w2sg_pdata {

You don't really support platform_data, so you can move this into the 
driver.

> +	struct regulator *lna_regulator;	/* enable LNA power */
> +	int	on_off_gpio;	/*  on-off input of the GPS module */
> +};
> +#endif /* __LINUX_W2SG0004_H */
> -- 
> 2.12.2
> 
--
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