Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Peter Rosin @ 2016-12-13 23:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
	linux-pm, devicetree
In-Reply-To: <1481673405-4547-1-git-send-email-peda@axentia.se>

If several parallel bq24735 chargers have their ac-detect gpios wired
together (or if only one of the parallel bq24735 chargers have its
ac-detect pin wired to a gpio, and the others are assumed to react the
same), then all driver instances need to check the same gpio. But the
gpio subsystem does not allow sharing gpios, so handle that locally.

However, only do this for the polling case, sharing is not supported if
the ac detection is handled with interrupts.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/power/supply/bq24735-charger.c | 111 +++++++++++++++++++++++++++++----
 1 file changed, 100 insertions(+), 11 deletions(-)

diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index f45876927676..c2403c4d5ece 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -25,6 +25,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
@@ -43,12 +44,24 @@
 #define BQ24735_MANUFACTURER_ID		0xfe
 #define BQ24735_DEVICE_ID		0xff
 
+struct bq24735;
+
+struct bq24735_shared {
+	struct list_head		list;
+	struct bq24735			*owner;
+	struct gpio_desc		*status_gpio;
+};
+
+static DEFINE_MUTEX(shared_lock);
+static LIST_HEAD(shared_list);
+
 struct bq24735 {
 	struct power_supply		*charger;
 	struct power_supply_desc	charger_desc;
 	struct i2c_client		*client;
 	struct bq24735_platform		*pdata;
 	struct mutex			lock;
+	struct bq24735_shared		*shared;
 	struct gpio_desc		*status_gpio;
 	struct delayed_work		poll;
 	u32				poll_interval;
@@ -346,6 +359,75 @@ static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client)
 	return pdata;
 }
 
+static struct gpio_desc *bq24735_get_status_gpio(struct bq24735 *charger)
+{
+	struct device *dev = &charger->client->dev;
+	struct bq24735_shared *shared;
+	int gpio;
+	enum of_gpio_flags flags;
+	int active_low;
+	struct list_head *pos;
+
+	if (of_property_read_bool(dev->of_node, "ti,ac-detect-gpios"))
+		gpio = of_get_named_gpio_flags(dev->of_node,
+					       "ti,ac-detect-gpios", 0, &flags);
+	else if (of_property_read_bool(dev->of_node, "ti,ac-detect-gpio"))
+		gpio = of_get_named_gpio_flags(dev->of_node,
+					       "ti,ac-detect-gpio", 0, &flags);
+	else
+		return NULL;
+
+	if (!gpio_is_valid(gpio))
+		return ERR_PTR(gpio);
+	active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+	mutex_lock(&shared_lock);
+	list_for_each(pos, &shared_list) {
+		shared = list_entry(pos, struct bq24735_shared, list);
+		if (gpio != desc_to_gpio(shared->status_gpio))
+			continue;
+		if (!active_low ^ !gpiod_is_active_low(shared->status_gpio))
+			continue;
+
+		get_device(&shared->owner->client->dev);
+		dev_dbg(dev, "sharing gpio with %s\n",
+			shared->owner->pdata->name);
+		charger->shared = shared;
+		mutex_unlock(&shared_lock);
+		return shared->status_gpio;
+	}
+
+	shared = devm_kzalloc(dev, sizeof(*shared), GFP_KERNEL);
+	if (!shared) {
+		mutex_unlock(&shared_lock);
+		return ERR_PTR(-ENOMEM);
+	}
+	shared->owner = charger;
+	shared->status_gpio = gpiod_get(dev, "ti,ac-detect", GPIOD_IN);
+	if (!IS_ERR(shared->status_gpio)) {
+		charger->shared = shared;
+		list_add(&shared->list, &shared_list);
+	}
+	mutex_unlock(&shared_lock);
+
+	return shared->status_gpio;
+}
+
+static void bq24735_put_status_gpio(struct bq24735 *charger)
+{
+	if (!charger->shared)
+		return;
+
+	mutex_lock(&shared_lock);
+	if (charger->shared->owner != charger) {
+		put_device(&charger->shared->owner->client->dev);
+	} else {
+		list_del(&charger->shared->list);
+		gpiod_put(charger->shared->status_gpio);
+	}
+	mutex_unlock(&shared_lock);
+}
+
 static int bq24735_charger_probe(struct i2c_client *client,
 				 const struct i2c_device_id *id)
 {
@@ -402,9 +484,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, charger);
 
-	charger->status_gpio = devm_gpiod_get_optional(&client->dev,
-						       "ti,ac-detect",
-						       GPIOD_IN);
+	charger->status_gpio = bq24735_get_status_gpio(charger);
 	if (IS_ERR(charger->status_gpio)) {
 		ret = PTR_ERR(charger->status_gpio);
 		dev_err(&client->dev, "Getting gpio failed: %d\n", ret);
@@ -416,28 +496,30 @@ static int bq24735_charger_probe(struct i2c_client *client,
 		if (ret < 0) {
 			dev_err(&client->dev, "Failed to read manufacturer id : %d\n",
 				ret);
-			return ret;
+			goto out;
 		} else if (ret != 0x0040) {
 			dev_err(&client->dev,
 				"manufacturer id mismatch. 0x0040 != 0x%04x\n", ret);
-			return -ENODEV;
+			ret = -ENODEV;
+			goto out;
 		}
 
 		ret = bq24735_read_word(client, BQ24735_DEVICE_ID);
 		if (ret < 0) {
 			dev_err(&client->dev, "Failed to read device id : %d\n", ret);
-			return ret;
+			goto out;
 		} else if (ret != 0x000B) {
 			dev_err(&client->dev,
 				"device id mismatch. 0x000b != 0x%04x\n", ret);
-			return -ENODEV;
+			ret = -ENODEV;
+			goto out;
 		}
 	}
 
 	ret = bq24735_config_charger(charger);
 	if (ret < 0) {
 		dev_err(&client->dev, "failed in configuring charger");
-		return ret;
+		goto out;
 	}
 
 	/* check for AC adapter presence */
@@ -445,7 +527,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
 		ret = bq24735_enable_charging(charger);
 		if (ret < 0) {
 			dev_err(&client->dev, "Failed to enable charging\n");
-			return ret;
+			goto out;
 		}
 	}
 
@@ -455,7 +537,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
 		ret = PTR_ERR(charger->charger);
 		dev_err(&client->dev, "Failed to register power supply: %d\n",
 			ret);
-		return ret;
+		goto out;
 	}
 
 	if (client->irq) {
@@ -470,7 +552,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"Unable to register IRQ %d err %d\n",
 				client->irq, ret);
-			return ret;
+			goto out;
 		}
 	} else if (charger->status_gpio) {
 		ret = of_property_read_u32(client->dev.of_node,
@@ -487,6 +569,11 @@ static int bq24735_charger_probe(struct i2c_client *client,
 	}
 
 	return 0;
+
+out:
+	bq24735_put_status_gpio(charger);
+
+	return ret;
 }
 
 static int bq24735_charger_remove(struct i2c_client *client)
@@ -496,6 +583,8 @@ static int bq24735_charger_remove(struct i2c_client *client)
 	if (charger->poll_interval)
 		cancel_delayed_work_sync(&charger->poll);
 
+	bq24735_put_status_gpio(charger);
+
 	return 0;
 }
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 2/3] power: supply: bq24735-charger: optionally poll the ac-detect gpio
From: Peter Rosin @ 2016-12-13 23:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
	linux-pm, devicetree
In-Reply-To: <1481673405-4547-1-git-send-email-peda@axentia.se>

If the ac-detect gpio does not support interrupts, provide a fallback
to poll the gpio at a configurable interval.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/power/supply/ti,bq24735.txt           |  2 +
 drivers/power/supply/bq24735-charger.c             | 50 +++++++++++++++++++---
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
index 3bf55757ceec..54f0004643e2 100644
--- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
+++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
@@ -25,6 +25,8 @@ Optional properties :
  - ti,external-control : Indicates that the charger is configured externally
    and that the host should not attempt to enable/disable charging or set the
    charge voltage/current.
+ - poll-interval : In case 'interrupts' is not specified, poll AC presense
+   on the ti,ac-detect-gpios GPIO with this interval (milliseconds).
 
 Example:
 
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 1d5c9206e0ed..f45876927676 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -50,6 +50,8 @@ struct bq24735 {
 	struct bq24735_platform		*pdata;
 	struct mutex			lock;
 	struct gpio_desc		*status_gpio;
+	struct delayed_work		poll;
+	u32				poll_interval;
 	bool				charging;
 };
 
@@ -209,11 +211,8 @@ static int bq24735_charger_is_charging(struct bq24735 *charger)
 	return !(ret & BQ24735_CHG_OPT_CHARGE_DISABLE);
 }
 
-static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+static void bq24735_update(struct bq24735 *charger)
 {
-	struct power_supply *psy = devid;
-	struct bq24735 *charger = to_bq24735(psy);
-
 	mutex_lock(&charger->lock);
 
 	if (charger->charging && bq24735_charger_is_present(charger))
@@ -223,11 +222,29 @@ static irqreturn_t bq24735_charger_isr(int irq, void *devid)
 
 	mutex_unlock(&charger->lock);
 
-	power_supply_changed(psy);
+	power_supply_changed(charger->charger);
+}
+
+static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+{
+	struct power_supply *psy = devid;
+	struct bq24735 *charger = to_bq24735(psy);
+
+	bq24735_update(charger);
 
 	return IRQ_HANDLED;
 }
 
+static void bq24735_poll(struct work_struct *work)
+{
+	struct bq24735 *charger = container_of(work, struct bq24735, poll.work);
+
+	bq24735_update(charger);
+
+	schedule_delayed_work(&charger->poll,
+			      msecs_to_jiffies(charger->poll_interval));
+}
+
 static int bq24735_charger_get_property(struct power_supply *psy,
 					enum power_supply_property psp,
 					union power_supply_propval *val)
@@ -455,11 +472,33 @@ static int bq24735_charger_probe(struct i2c_client *client,
 				client->irq, ret);
 			return ret;
 		}
+	} else if (charger->status_gpio) {
+		ret = of_property_read_u32(client->dev.of_node,
+					   "poll-interval",
+					   &charger->poll_interval);
+		if (ret)
+			return 0;
+		if (!charger->poll_interval)
+			return 0;
+
+		INIT_DELAYED_WORK(&charger->poll, bq24735_poll);
+		schedule_delayed_work(&charger->poll,
+				      msecs_to_jiffies(charger->poll_interval));
 	}
 
 	return 0;
 }
 
+static int bq24735_charger_remove(struct i2c_client *client)
+{
+	struct bq24735 *charger = i2c_get_clientdata(client);
+
+	if (charger->poll_interval)
+		cancel_delayed_work_sync(&charger->poll);
+
+	return 0;
+}
+
 static const struct i2c_device_id bq24735_charger_id[] = {
 	{ "bq24735-charger", 0 },
 	{}
@@ -478,6 +517,7 @@ static struct i2c_driver bq24735_charger_driver = {
 		.of_match_table = bq24735_match_ids,
 	},
 	.probe = bq24735_charger_probe,
+	.remove = bq24735_charger_remove,
 	.id_table = bq24735_charger_id,
 };
 
-- 
2.1.4


^ permalink raw reply related

* [PATCH v2 1/3] power: supply: bq24735-charger: simplify register update to stop charging
From: Peter Rosin @ 2016-12-13 23:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
	linux-pm, devicetree
In-Reply-To: <1481673405-4547-1-git-send-email-peda@axentia.se>

Providing value bits outside of the mask is pointless.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/power/supply/bq24735-charger.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index eb7783b42e0a..1d5c9206e0ed 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -111,8 +111,7 @@ static inline int bq24735_enable_charging(struct bq24735 *charger)
 		return 0;
 
 	return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
-				   BQ24735_CHG_OPT_CHARGE_DISABLE,
-				   ~BQ24735_CHG_OPT_CHARGE_DISABLE);
+				   BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
 }
 
 static inline int bq24735_disable_charging(struct bq24735 *charger)
-- 
2.1.4


^ permalink raw reply related

* [PATCH v2 0/3] bq24735-charger: allow gpio polling and sharing
From: Peter Rosin @ 2016-12-13 23:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
	linux-pm, devicetree

Hi!

v1 -> v2 changes:
- use poll-interval instead of ti,poll-interval-ms in the bindings
- properly initialize the shared_lock mutex
- don't share gpios on active low/high mismatch
- don't regress users specifying the old ti,ac-detect-gpio form (without
  the trailing s)

I have a board that features a couple of parallel bq24735 chargers.
However, only one of the chargers has its ACOK pin wired to a gpio,
the other parallel chargers are expected to just behave the same
as the charger that has been singled out like this. Another thing
with the board is that the gpio is not capable of generating an
interrupt.

This series fixes things for me (ok, the first patch was just a
fix for a thing that initially had me confused for a bit, and is
totally unrelated. Ignore if you want to, it's basically just churn).

One thing that I wonder about is if anything should be done to
prevent unloading of the instance that shares its gpio? I thought
about adding a device_link_add() call, but am unsure if that would
be approprite? I'm not unloading drivers at all so it's a total
non-issue for me...

Cheers,
peda

Peter Rosin (3):
  power: supply: bq24735-charger: simplify register update to stop
    charging
  power: supply: bq24735-charger: optionally poll the ac-detect gpio
  power: supply: bq24735-charger: allow chargers to share the ac-detect
    gpio

 .../bindings/power/supply/ti,bq24735.txt           |   2 +
 drivers/power/supply/bq24735-charger.c             | 164 ++++++++++++++++++---
 2 files changed, 148 insertions(+), 18 deletions(-)

-- 
2.1.4


^ permalink raw reply

* Re: Re: [PATCH v3 -next 2/2] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Alexey Kardashevskiy @ 2016-12-13 23:49 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Rob Herring, Vishnu Patekar, Andre Przywara,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Hans de Goede, Arnd Bergmann,
	Maxime Ripard, Russell King,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161207100132.1Qp0ZxGD-/icbEWb855g0PDqKvflMoHmW9unr2Ajn@public.gmane.org>

On 07/12/16 18:01, Icenowy Zheng wrote:
> 
> 2016年12月7日 05:52于 Alexey Kardashevskiy <aik-sLpHqDYs0B2HXe+LvDLADg@public.gmane.org>写道:
>>
>> On 06/12/16 18:43, Icenowy Zheng wrote: 
>>>
>>> 2016年12月6日 09:51于 Alexey Kardashevskiy <aik-sLpHqDYs0B2HXe+LvDLADg@public.gmane.org>写道: 
>>>>
>>>> On 03/12/16 02:05, Icenowy Zheng wrote: 
>>>>> Orange Pi Zero is a board that came with the new Allwinner H2+ SoC and a 
>>>>> SDIO Wi-Fi chip by Allwinner (XR819). 
>>>>>
>>>>> Add a device tree file for it. 
>>>>>
>>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 
>>>>> --- 
>>>>> Changes since v2: 
>>>>> - Merged SDIO Wi-Fi patch into it. 
>>>>> - SDIO Wi-Fi: add a ethernet1 alias to it, as it has no internal NVRAM. 
>>>>> - SDIO Wi-Fi: changed pinctrl binding to generic pinconf 
>>>>> - removed all gpio pinctrl nodes 
>>>>> - changed h2plus to h2-plus 
>>>>> Changes since v1: 
>>>>> - Convert to generic pinconf bindings. 
>>>>> - SDIO Wi-Fi: add patch. 
>>>>>
>>>>> Some notes: 
>>>>> - The uart1 and uart2 is available on the unsoldered gpio header. 
>>>>> - The onboard USB connector has its Vbus directly connected to DCIN-5V (the 
>>>>>     power jack) 
>>>>>
>>>>>    arch/arm/boot/dts/Makefile                        |   1 + 
>>>>>    arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 159 ++++++++++++++++++++++ 
>>>>>    2 files changed, 160 insertions(+) 
>>>>>    create mode 100644 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts 
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile 
>>>>> index 6447abc..59f6e86 100644 
>>>>> --- a/arch/arm/boot/dts/Makefile 
>>>>> +++ b/arch/arm/boot/dts/Makefile 
>>>>> @@ -844,6 +844,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \ 
>>>>>    sun8i-a33-sinlinx-sina33.dtb \ 
>>>>>    sun8i-a83t-allwinner-h8homlet-v2.dtb \ 
>>>>>    sun8i-a83t-cubietruck-plus.dtb \ 
>>>>> + sun8i-h2-plus-orangepi-zero.dtb \ 
>>>>>    sun8i-h3-bananapi-m2-plus.dtb \ 
>>>>>    sun8i-h3-nanopi-neo.dtb \ 
>>>>>    sun8i-h3-orangepi-2.dtb \ 
>>>>> diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts 
>>>>> new file mode 100644 
>>>>> index 0000000..d18807f 
>>>>> --- /dev/null 
>>>>> +++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts 
>>>>> @@ -0,0 +1,159 @@ 
>>>>> +/* 
>>>>> + * Copyright (C) 2016 Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 
>>>>> + * 
>>>>> + * Based on sun8i-h3-orangepi-one.dts, which is: 
>>>>> + *   Copyright (C) 2016 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 
>>>>> + * 
>>>>> + * This file is dual-licensed: you can use it either under the terms 
>>>>> + * of the GPL or the X11 license, at your option. Note that this dual 
>>>>> + * licensing only applies to this file, and not this project as a 
>>>>> + * whole. 
>>>>> + * 
>>>>> + *  a) This file 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 file 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. 
>>>>> + * 
>>>>> + * Or, alternatively, 
>>>>> + * 
>>>>> + *  b) Permission is hereby granted, free of charge, to any person 
>>>>> + *     obtaining a copy of this software and associated documentation 
>>>>> + *     files (the "Software"), to deal in the Software without 
>>>>> + *     restriction, including without limitation the rights to use, 
>>>>> + *     copy, modify, merge, publish, distribute, sublicense, and/or 
>>>>> + *     sell copies of the Software, and to permit persons to whom the 
>>>>> + *     Software is furnished to do so, subject to the following 
>>>>> + *     conditions: 
>>>>> + * 
>>>>> + *     The above copyright notice and this permission notice shall be 
>>>>> + *     included in all copies or substantial portions of the Software. 
>>>>> + * 
>>>>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>>>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES 
>>>>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND 
>>>>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT 
>>>>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, 
>>>>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>>>>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>>>>> + *     OTHER DEALINGS IN THE SOFTWARE. 
>>>>> + */ 
>>>>> + 
>>>>> +/dts-v1/; 
>>>>> +#include "sun8i-h3.dtsi" 
>>>>> +#include "sunxi-common-regulators.dtsi" 
>>>>> + 
>>>>> +#include <dt-bindings/gpio/gpio.h> 
>>>>> +#include <dt-bindings/input/input.h> 
>>>>> +#include <dt-bindings/pinctrl/sun4i-a10.h> 
>>>>> + 
>>>>> +/ { 
>>>>> + model = "Xunlong Orange Pi Zero"; 
>>>>> + compatible = "xunlong,orangepi-zero", "allwinner,sun8i-h2-plus"; 
>>>>> + 
>>>>> + aliases { 
>>>>> + serial0 = &uart0; 
>>>>> + /* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */ 
>>>>
>>>>
>>>> It is not defined there as for: 
>>>>
>>>> cef87e9 (tag: next-20161205) 20 hours ago Stephen Rothwell Add linux-next 
>>>> specific files for 20161205 
>>>
>>> The driver of H3's obfuscated DesignWare MAC is not yet mainlined, so there won't be one ethernet0 now. 
>>>
>>> But it's reserved for the onboard Ethernet. 
>>
>>
>> Could you please elaborate how you tested this patch (ideally some tree 
>> somewhere on github)? This patch added RX819, it assumes EMAC support is 
>> there, neither is there nor there is a way to test this... Thanks. 
> 
> It do not assume EMAC is there. 
> It only assume EMAC will be there someday :-)
> 
> For tree... wait for my push :-)

Has it happened yet? :)



-- 
Alexey

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

^ permalink raw reply

* Re: [PATCH 03/11] driver: clk: imx: Add clock driver for imx6sll
From: Stephen Boyd @ 2016-12-13 23:28 UTC (permalink / raw)
  To: Jacky Bai
  Cc: shawnguo@kernel.org, mturquette@baylibre.com, robh+dt@kernel.org,
	mark.rutland@arm.com, linus.walleij@linaro.org,
	kernel@pengutronix.de, Fabio Estevam, daniel.lezcano@linaro.org,
	tglx@linutronix.de, p.zabel@pengutronix.de,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	jacky.baip@gmail.com
In-Reply-To: <AM3PR04MB5302BE33BE1F2C9913FF61587980@AM3PR04MB530.eurprd04.prod.outlook.com>

On 12/12, Jacky Bai wrote:
> > On 12/02, Bai Ping wrote:
> > > diff --git a/drivers/clk/imx/clk-imx6sll.c
> > > b/drivers/clk/imx/clk-imx6sll.c new file mode 100644 index
> > > 0000000..c5219e1
> > > --- /dev/null
> > > +++ b/drivers/clk/imx/clk-imx6sll.c
> > > +0x20, 16, 1, pll7_bypass_sels, ARRAY_SIZE(pll7_bypass_sels),
> > > +CLK_SET_RATE_PARENT);
> > > +
> > > +	/* Do not bypass PLLs initially */
> > > +	clk_set_parent(clks[IMX6SLL_PLL1_BYPASS], clks[IMX6SLL_CLK_PLL1]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL2_BYPASS], clks[IMX6SLL_CLK_PLL2]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL3_BYPASS], clks[IMX6SLL_CLK_PLL3]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL4_BYPASS], clks[IMX6SLL_CLK_PLL4]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL5_BYPASS], clks[IMX6SLL_CLK_PLL5]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL6_BYPASS], clks[IMX6SLL_CLK_PLL6]);
> > > +	clk_set_parent(clks[IMX6SLL_PLL7_BYPASS], clks[IMX6SLL_CLK_PLL7]);
> > 
> > Can we just put raw register writes here instead? I'd prefer we didn't use clk
> > consumer APIs to do things to the clk tree from the providers. The problem
> > there being that:
> > 
> >  1) We're trying to move away from using consumer APIs in  provider drivers.
> > It's ok if they're used during probe, but  inside clk_ops is not preferred.
> > 
> >  2) Even if you have a clk pointer, it may be "orphaned" at the  time of
> > registration and so calling the APIs here works now but  eventually we may
> > want to return an EPROBE_DEFER error in that  case and this may block that
> > effort.
> > 
> > I suppose if this is the only clk driver on this machine then this last point isn't a
> > concern and things are probably ok here.
> > 
> 
> Using raw register writing has an issue.  The register is modified, but it seems the clock 'parent-child' relationship can 
> not match the register setting. The register setting is not bypass the pll, but in debug 'clk_summary', the
> pll is bypassed.  

So program the register settings before registering the clocks
with the framework?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v4 4/4] regulator: Prevent falling too fast
From: Matthias Kaehlcke @ 2016-12-13 23:14 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Mark Brown, Liam Girdwood, Brian Norris, Javier Martinez Canillas,
	Rob Herring, Mark Rutland, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <CAD=FV=WhUVeq5GZ5-ae4SxypHGB9jWqgvkO02S=G6Zz6cexRzQ@mail.gmail.com>

Hi,

El Tue, Dec 13, 2016 at 12:00:32PM -0800 Doug Anderson ha dit:

> On Tue, Dec 13, 2016 at 9:19 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Dec 12, 2016 at 01:15:02PM -0800, Matthias Kaehlcke wrote:
> >> El Fri, Oct 28, 2016 at 07:15:21PM +0100 Mark Brown ha dit:
> >> > On Mon, Sep 26, 2016 at 10:41:59AM -0700, Doug Anderson wrote:
> >
> >> > What you're describing to me is a discrete DCDC that has an input
> >> > voltage that sets the output voltage which happens to be set with a PWM.
> >
> >> I experimented a bit with this. Besides the question of how to model
> >> the passives I wonder how the two regulators would interact. The
> >> correct thing seems to be to specify the input regulator as a supply
> >> of the DCDC. dcdc->set_voltage breaks down a voltage transition into
> >
> > No, not unless the prior descriptions of the hardware have been wildly
> > inaccurate - my understanding had been that the DCDC was a normal DCDC
> > with an analogue input intended to be biased to set the output voltage
> > (presumably in terms of a full rail supply) and that the PWM had been
> > connected to this analogue input.  If the PWM is supplying the DCDC then
> > the hardware design just seems bizzare, I can't see how this would even
> > work.
> 
> Looking at one schematic, the discrete BUCK for at least one of the
> rails is TPS65261RHBR, which appears to be described at
> <https://store.ti.com/TPS65261RHBR.aspx>.  Data sheet appears to be at
> <http://www.ti.com/product/tps65261/technicaldocuments?HQS=TI-null-null-octopart-df-pf-null-wwe>.
> 
> As you can see from the datasheet ("Adjusting the Output Voltage"
> section), it is intended that you stuff a resistor to make a voltage
> divider and that's how you select the output voltage.  In our case the
> PWM interacts here and allows you to make a more dynamic output
> voltage.  I've always thought about the input to the "FB" pin as
> making an input voltage, but I guess it's not terribly simple since
> the voltage divider ends up dividing between ground and the output
> voltage.

I also had put my mind on seeing the output of the PWM circuitry as an
input voltage, but technically it isn't a supply of the buck
regulator. It seems we could consider it a "control voltage" instead
and thus avoid the recursive lock acquisition.

Matthias

^ permalink raw reply

* [PATCH] pinctrl: fix DT bindings for marvell,kirkwood-pinctrl
From: Andreas Klinger @ 2016-12-13 23:08 UTC (permalink / raw)
  To: devicetree
  Cc: Linus Walleij, Rob Herring, Mark Rutland, linux-gpio,
	linux-kernel, Andreas Klinger

On Marvell mv88f6180 mpp pins range from 0 to 19 as well as from 35 to 44.
This is already fixed in commit: 9573e7923007961799beff38bc5c5a7635634eef

This is the documentation change for above commit.

Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 .../bindings/pinctrl/marvell,kirkwood-pinctrl.txt    | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,kirkwood-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,kirkwood-pinctrl.txt
index 730444a9a4de..6c0ea155b708 100644
--- a/Documentation/devicetree/bindings/pinctrl/marvell,kirkwood-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,kirkwood-pinctrl.txt
@@ -44,16 +44,16 @@ mpp16         16       gpio, sdio(d2), uart0(cts), uart1(rxd), mii(crs)
 mpp17         17       gpio, sdio(d3)
 mpp18         18       gpo, nand(io0)
 mpp19         19       gpo, nand(io1)
-mpp20         20       gpio, mii(rxerr)
-mpp21         21       gpio, audio(spdifi)
-mpp22         22       gpio, audio(spdifo)
-mpp23         23       gpio, audio(rmclk)
-mpp24         24       gpio, audio(bclk)
-mpp25         25       gpio, audio(sdo)
-mpp26         26       gpio, audio(lrclk)
-mpp27         27       gpio, audio(mclk)
-mpp28         28       gpio, audio(sdi)
-mpp29         29       gpio, audio(extclk)
+mpp35         35       gpio, mii(rxerr)
+mpp36         36       gpio, audio(spdifi)
+mpp37         37       gpio, audio(spdifo)
+mpp38         38       gpio, audio(rmclk)
+mpp39         39       gpio, audio(bclk)
+mpp40         40       gpio, audio(sdo)
+mpp41         41       gpio, audio(lrclk)
+mpp42         42       gpio, audio(mclk)
+mpp43         43       gpio, audio(sdi)
+mpp44         44       gpio, audio(extclk)
 
 * Marvell Kirkwood 88f6190
 
-- 
2.1.4


^ permalink raw reply related

* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Rob Herring @ 2016-12-13 22:10 UTC (permalink / raw)
  To: Brian Norris
  Cc: Benjamin Tissoires, Jiri Kosina, Dmitry Torokhov, Doug Anderson,
	Caesar Wang, open list:ARM/Rockchip SoC...,
	linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Mark Rutland
In-Reply-To: <20161212183422.GA138477@google.com>

On Mon, Dec 12, 2016 at 12:34 PM, Brian Norris <briannorris@chromium.org> wrote:
> Hi all,
>
> On Mon, Dec 12, 2016 at 08:47:06AM -0600, Rob Herring wrote:
>
> [Snip Benjamin's proposal; I agree we don't really want a multi-level DT
> layout purely for making the driver look a little nicer (I'm not sure
> this would really be nicer anyway). And I think, as Rob notes here, our
> disagreement is smaller than appears. But I might be wrong.]
>
>> Anyway, we're only debating this:
>
> OK, so I think we might have a consensus of sorts? I'll describe it
> here, in case I'm wrong. Otherwise, I'll send another rev.
>
>>          i2c-hid-dev@2c {
>>                  compatible = "wacom,w9013", "hid-over-i2c";
>
> I plan to document the above, but not treat "wacom,w9013" specially in
> the driver, apart from possibly listing it in the driver of_match_table.
> This was mentioned by Dmitry earlier, and I didn't see any objection.
>
> (Note that there are problems with module autoload when using multiple
> compatible strings like above. May not be supremely relevant to the
> documentation, but it *is* practically important.)

I'm not sure what is not working here exactly. We emit all the
compatible strings in the uevent. However, it looks like this is only
called for platform devices. In the case of i2c, I don't think any
compatible string is emitted. It looks to me like i2c_device_uevent
just needs a call to of_device_get_modalias to fix this. There's some
issues in the I2C core in how it does matching and maybe this is
related? I would guess if it was that easy, then it would be fixed
already. Maybe not.

>>                  reg = <0x2c>;
>>                  hid-descr-addr = <0x0020>;
>>                  interrupt-parent = <&gpx3>;
>>                  interrupts = <3 2>;
>>                  vdd-supply = <sth>;
>
> Document and support 'vdd-supply', optionally.
>
>>                  init-delay-ms = <100>;
>
> Per Rob's mention below, support this as 'post-power-on-delay-ms',
> optionally.
>
> We can use either of these properties on any device, with the
> intention that if there are future needs for divergent bindings, the
> aforementioned compatible property can help us differentiate.

TBC, from a DT perspective (and what the binding should say), is the
properties are only valid with a wacom compatible present (or any
others you want to add). If the driver doesn't enforce that and
supports having those properties with just "hid-over-i2c", then
downstream dts's can use that for all I care.

Rob

^ permalink raw reply

* Re: [PATCH 5/5] Documentation: fsl-quadspi: Add fsl, ls1012a-qspi compatible string
From: Rob Herring @ 2016-12-13 21:14 UTC (permalink / raw)
  To: Yao Yuan
  Cc: Yuan Yao, shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <DB6PR0402MB2902DADEB4E6D06418AD311F899B0-2mNvjAGDOPmZPt/6Y1Ah+Y3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>

On Mon, Dec 12, 2016 at 8:47 PM, Yao Yuan <yao.yuan-3arQi8VN3Tc@public.gmane.org> wrote:
> On Thu, Dec 13, 2016 at 05:23:02PM +0800, Rob Herring wrote:
>> On Thu, Dec 08, 2016 at 05:23:04PM +0800, Yuan Yao wrote:
>> > From: Yuan Yao <yao.yuan-3arQi8VN3Tc@public.gmane.org>
>>
>> Same problem in this subject too.
>>
>> >
>> > new compatible string: "fsl,ls1012a-qspi".
>> >
>> > Signed-off-by: Yuan Yao <yao.yuan-3arQi8VN3Tc@public.gmane.org>
>> > ---
>> >  Documentation/devicetree/bindings/mtd/fsl-quadspi.txt | 1 +
>> >  1 file changed, 1 insertion(+)
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Thanks for your review.
> And do you have any suggestion for this subject?

The problem is you have a space in the compatible string: "fsl,
ls1012a-qspi" rather than "fsl,ls1012a-qspi"

Also, I prefer "dt/bindings: " as the beginning of binding patch subjects.

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

^ permalink raw reply

* Re: [PATCH 2/4] dt-bindings: mfd: Remove TPS65217 interrupts
From: Rob Herring @ 2016-12-13 21:09 UTC (permalink / raw)
  To: Milo Kim
  Cc: Arnd Bergmann, Benoit Cousson, Tony Lindgren, Lee Jones,
	Sebastian Reichel, Dmitry Torokhov, linux-omap,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <fe4985c0-d60a-2562-12a7-064404efd64c@gmail.com>

On Mon, Dec 12, 2016 at 5:24 PM, Milo Kim <woogyom.kim@gmail.com> wrote:
> On 12/13/2016 02:25 AM, Rob Herring wrote:
>>
>> On Fri, Dec 09, 2016 at 03:28:31PM +0900, Milo Kim wrote:
>>>
>>> Interrupt numbers are from the datasheet, so no need to keep them in
>>> the ABI. Use the number in the DT file.
>>
>> I don't see the purpose of ripping this out. The headers have always
>> been for convienence, not whether the values come from the datasheet or
>> not.
>
>
> My understanding is it's a same rule as other interrupt controllers.

Oh yes, that's true. We never use defines for interrupts. In that case:

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

Rob

^ permalink raw reply

* Re: [PATCH v6 1/8] MFD: add bindings for STM32 Timers driver
From: Rob Herring @ 2016-12-13 21:07 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Lee Jones, Mark Rutland, Alexandre Torgue,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, Thierry Reding, Linux PWM List,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
	Linus Walleij <linus.walleij@
In-Reply-To: <CA+M3ks4ukP14YE5-6+gAzJBjEmjEyGyVbsVGOm8ehVm0EfzO-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Dec 13, 2016 at 3:29 AM, Benjamin Gaignard
<benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> 2016-12-12 19:51 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>> On Fri, Dec 09, 2016 at 03:15:12PM +0100, Benjamin Gaignard wrote:
>>> Add bindings information for STM32 Timers
>>>
>>> version 6:
>>> - rename stm32-gtimer to stm32-timers
>>> - change compatible
>>> - add description about the IPs
>>>
>>> version 2:
>>> - rename stm32-mfd-timer to stm32-gptimer
>>> - only keep one compatible string
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/mfd/stm32-timers.txt       | 46 ++++++++++++++++++++++
>>>  1 file changed, 46 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timers.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>>> new file mode 100644
>>> index 0000000..b30868e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>>> @@ -0,0 +1,46 @@
>>> +STM32 Timers driver bindings
>>> +
>>> +This IP provides 3 types of timer along with PWM functionality:
>>> +- advanced-control timers consist of a 16-bit auto-reload counter driven by a programmable
>>> +  prescaler, break input feature, PWM outputs and complementary PWM ouputs channels.
>>> +- general-purpose timers consist of a 16-bit or 32-bit auto-reload counter driven by a
>>> +  programmable prescaler and PWM outputs.
>>> +- basic timers consist of a 16-bit auto-reload counter driven by a programmable prescaler.
>>> +
>>> +Required parameters:
>>> +- compatible: must be "st,stm32-timers"
>>> +
>>> +- reg:                       Physical base address and length of the controller's
>>> +                     registers.
>>> +- clock-names:               Set to "clk_int".
>>
>> 'clk' is redundant. Also, you don't really need -names when there is
>> only one of them.
>
> I use devm_regmap_init_mmio_clk() which get the clock by it name so
> I have to define it in DT.

Are you sure NULL is not allowed? I don't know, but at least clk_get()
allows NULL.

It's fine to keep, just drop the "clk_" part.

>
>>> +- clocks:            Phandle to the clock used by the timer module.
>>> +                     For Clk properties, please refer to ../clock/clock-bindings.txt
>>> +
>>> +Optional parameters:
>>> +- resets:            Phandle to the parent reset controller.
>>> +                     See ../reset/st,stm32-rcc.txt
>>> +
>>> +Optional subnodes:
>>> +- pwm:                       See ../pwm/pwm-stm32.txt
>>> +- timer:             See ../iio/timer/stm32-timer-trigger.txt
>>> +
>>> +Example:
>>> +     timers@40010000 {
>>> +             #address-cells = <1>;
>>> +             #size-cells = <0>;
>>> +             compatible = "st,stm32-timers";
>>> +             reg = <0x40010000 0x400>;
>>> +             clocks = <&rcc 0 160>;
>>> +             clock-names = "clk_int";
>>> +
>>> +             pwm {
>>> +                     compatible = "st,stm32-pwm";
>>> +                     pinctrl-0       = <&pwm1_pins>;
>>> +                     pinctrl-names   = "default";
>>> +             };
>>> +
>>> +             timer {
>>> +                     compatible = "st,stm32-timer-trigger";
>>> +                     reg = <0>;
>>
>> You don't need reg here as there is only one. In turn, you don't need
>> #address-cells or #size-cells.
>
> I use "reg" to set each timer configuration.
> From hardware point of view they are all the same except for which hardware
> signals they could consume and/or send.

This sounds okay, but...

> "reg" is used as index of the two tables in driver code.

this statement doesn't really sound like valid use of reg.

If you keep reg, then the node needs a unit address (timer@0).

Rob

^ permalink raw reply

* Re: [PATCH 2/6] clk: sunxi-ng: set the parent rate when adjustin CPUX clock on A33
From: Icenowy Zheng @ 2016-12-13 20:54 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: devicetree@vger.kernel.org, Quentin Schulz, Michael Turquette,
	Stephen Boyd, Russell King, linux-kernel@vger.kernel.org,
	Hans de Goede, Chen-Yu Tsai, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Jorik Jonker
In-Reply-To: <20161213154451.y4wcrqhtcc5sqli7@lukather>



13.12.2016, 23:44, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Tue, Dec 13, 2016 at 11:22:48PM +0800, Icenowy Zheng wrote:
>>  The CPUX clock on A33, which is for the Cortex-A7 cores, is designed to
>>  be changeable by changing the rate of PLL_CPUX.
>>
>>  Add CLK_SET_RATE_PARENT flag to this clock.
>>
>>  Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Excuse me, have you merged this patch?

If merged, I won't contain it in my PATCH v2, thus the PATCH v2 will contain
only an updated OPP patch.

>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

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

^ permalink raw reply

* Re: [PATCH 2/2] iio: adc: hx711: Add IIO driver for AVIA HX711
From: Peter Meerwald-Stadler @ 2016-12-13 20:22 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, jic23-DgEjT+Ai2ygdnm+yROfE0A,
	knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw
In-Reply-To: <20161213180254.GA3185@andreas>


> This is the IIO driver for AVIA HX711 ADC which ist mostly used in weighting
> cells.

comments below
 
> The protocol is quite simple and using GPIO's:

GPIOs

> One GPIO is used as clock (SCK) while another GPIO is read (DOUT)
> 
> Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
> ---
>  drivers/iio/adc/Kconfig  |  13 +++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/hx711.c  | 269 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 283 insertions(+)
>  create mode 100644 drivers/iio/adc/hx711.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 932de1f9d1e7..7902b50fcf32 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -205,6 +205,19 @@ config HI8435
>  	  This driver can also be built as a module. If so, the module will be
>  	  called hi8435.
>  
> +config HX711
> +	tristate "AVIA HX711 ADC for weight cells"
> +	depends on GPIOLIB
> +	help
> +	  If you say yes here you get support for AVIA HX711 ADC which is used
> +	  for weight cells
> +
> +	  This driver uses two GPIO's, one for setting the clock and the other

GPIOs

> +	  one for getting the data
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called hx711.
> +
>  config INA2XX_ADC
>  	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
>  	depends on I2C && !SENSORS_INA2XX
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index b1aa456e6af3..d46e289900ef 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>  obj-$(CONFIG_HI8435) += hi8435.o
> +obj-$(CONFIG_HX711) += hx711.o
>  obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
>  obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> new file mode 100644
> index 000000000000..cbc89e467985
> --- /dev/null
> +++ b/drivers/iio/adc/hx711.c
> @@ -0,0 +1,269 @@
> +/*
> + * HX711: analog to digital converter for weight sensor module
> + *
> + * Copyright (c) Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + *
> + */
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +
> +#define HX711_GAIN_32		2	/* gain = 32 for channel B  */
> +#define HX711_GAIN_64		3	/* gain = 64 for channel A  */
> +#define HX711_GAIN_128		1	/* gain = 128 for channel A */
> +

only one newline here please

> +
> +struct hx711_data {
> +	struct device		*dev;
> +	dev_t			devt;

devt is not used, delete

> +	struct gpio_desc	*gpiod_sck;
> +	struct gpio_desc	*gpiod_dout;
> +	int			gain_pulse;
> +	struct mutex		lock;
> +};
> +
> +static void hx711_reset(struct hx711_data *hx711_data)
> +{
> +	int val;
> +	int i;
> +
> +	val = gpiod_get_value(hx711_data->gpiod_dout);
> +	if (val) {
> +		dev_warn(hx711_data->dev, "RESET-HX711\n");

message needed?

> +
> +		gpiod_set_value(hx711_data->gpiod_sck, 1);
> +		udelay(80);
> +		gpiod_set_value(hx711_data->gpiod_sck, 0);
> +
> +		for (i = 0; i < 1000; i++) {
> +			val = gpiod_get_value(hx711_data->gpiod_dout);
> +			if (!val)
> +				break;
> +			/* sleep at least 1 ms*/

add space before */

> +			msleep(1);
> +		}

can this fail? what is it takes longer than 1000 * 1ms?

> +	}
> +}
> +
> +static int hx711_cycle(struct hx711_data *hx711_data)
> +{
> +	int val;
> +
> +	/* if preempted for more then 60us while SCK is high:
> +	 * hx711 is going in reset
> +	 * ==> measuring is false
> +	 */
> +	preempt_disable();
> +	gpiod_set_value(hx711_data->gpiod_sck, 1);
> +	val = gpiod_get_value(hx711_data->gpiod_dout);
> +	gpiod_set_value(hx711_data->gpiod_sck, 0);
> +	preempt_enable();
> +
> +	return val;
> +}
> +
> +static unsigned int hx711_read(struct hx711_data *hx711_data)
> +{
> +	int i;
> +	int ret;
> +	int wert = 0;

val or value maybe?
should be unsigned int to match the return type of the function

> +
> +	hx711_reset(hx711_data);
> +
> +	for (i = 0; i < 24; i++) {
> +		wert <<= 1;
> +		ret = hx711_cycle(hx711_data);
> +		if (ret)
> +			wert++;
> +	}
> +
> +	wert ^= 0x800000;
> +
> +	for (i = 0; i < hx711_data->gain_pulse; i++)
> +		ret = hx711_cycle(hx711_data);
> +
> +	return wert;
> +}
> +

delete newline

> +
> +static int hx711_read_raw(struct iio_dev *iio_dev,
> +				const struct iio_chan_spec *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct hx711_data *hx711_data = iio_priv(iio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			mutex_lock(&hx711_data->lock);
> +			*val = (int)hx711_read(hx711_data);
> +			mutex_unlock(&hx711_data->lock);
> +			ret = IIO_VAL_INT;

return IIO_VAL_INT;

> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;

delete, dead code

> +}
> +
> +static const struct iio_info hx711_iio_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= hx711_read_raw,
> +};
> +
> +static const struct iio_chan_spec hx711_chan_spec[] = {
> +	{ .type = IIO_VOLTAGE,
> +		.channel = 0,

.channel = 0 not needed, there is only one channel

> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW),
> +		.scan_type = {

scan_type not needed, driver does not support buffered reads

> +			.sign = 'u',
> +			.realbits = 24,
> +			.storagebits = 32,
> +			.shift = 0,
> +		}
> +	},
> +};
> +
> +static int hx711_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct hx711_data *hx711_data = NULL;
> +	struct iio_dev *iio;
> +	int ret = 0, ival;
> +
> +	iio = devm_iio_device_alloc(dev, sizeof(struct hx711_data));
> +	if (!iio) {
> +		dev_err(dev, "failed to allocate IIO device\n");
> +		return -ENOMEM;
> +	}
> +
> +	hx711_data = iio_priv(iio);
> +	hx711_data->dev = dev;
> +
> +	mutex_init(&hx711_data->lock);
> +
> +	hx711_data->gpiod_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_HIGH);
> +	if (IS_ERR(hx711_data->gpiod_sck)) {
> +		ret = PTR_ERR(hx711_data->gpiod_sck);
> +		goto err;
> +	}
> +
> +	hx711_data->gpiod_dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_HIGH);
> +	if (IS_ERR(hx711_data->gpiod_dout)) {
> +		ret = PTR_ERR(hx711_data->gpiod_dout);
> +		goto err;
> +	}
> +
> +	ret = of_property_read_u32 (node, "gain", &ival);
> +	if (!ret) {
> +		switch (ival) {
> +		case 32:
> +			hx711_data->gain_pulse = HX711_GAIN_32;
> +			break;
> +		case 64:
> +			hx711_data->gain_pulse = HX711_GAIN_64;
> +			break;
> +		case 128:

merge default and case 128?

> +			hx711_data->gain_pulse = HX711_GAIN_128;
> +			break;
> +		default:
> +			hx711_data->gain_pulse = HX711_GAIN_128;
> +		}
> +	} else
> +		hx711_data->gain_pulse = HX711_GAIN_128;
> +	dev_dbg(hx711_data->dev, "gain: %d\n", hx711_data->gain_pulse);
> +
> +	ret = gpiod_direction_input(hx711_data->gpiod_dout);
> +	if (ret < 0) {
> +		dev_err(hx711_data->dev, "gpiod_direction_input: %d\n", ret);
> +		goto err;
> +	}
> +
> +	ret = gpiod_direction_output(hx711_data->gpiod_sck, 0);
> +	if (ret < 0) {
> +		dev_err(hx711_data->dev, "gpiod_direction_output: %d\n", ret);
> +		goto err;
> +	}
> +
> +	platform_set_drvdata(pdev, iio);
> +
> +	iio->name = pdev->name;
> +	iio->dev.parent = &pdev->dev;
> +	iio->info = &hx711_iio_info;
> +	iio->modes = INDIO_DIRECT_MODE;
> +	iio->channels = hx711_chan_spec;
> +	iio->num_channels = ARRAY_SIZE(hx711_chan_spec);
> +
> +	dev_err(hx711_data->dev, "initialized\n");

excessive logging, please remove

> +
> +	return devm_iio_device_register(dev, iio);
> +
> +err:
> +	return ret;

just return directly without goto?

> +}
> +
> +
> +static int hx711_suspend(struct device *dev)
> +{

pointless, just don't do PM support

> +	return 0;
> +}
> +
> +static int hx711_resume(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(hx711_pm_ops, hx711_suspend, hx711_resume);
> +
> +
> +static const struct of_device_id of_hx711_match[] = {
> +	{ .compatible = "avia,hx711", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, of_hx711_match);
> +
> +static struct platform_driver hx711_driver = {
> +	.probe		= hx711_probe,
> +	.driver		= {
> +		.name		= "hx711-gpio",
> +		.pm		= &hx711_pm_ops,
> +		.of_match_table	= of_hx711_match,
> +	},
> +};
> +
> +module_platform_driver(hx711_driver);
> +
> +MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
> +MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:hx711-gpio");
> +
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

^ permalink raw reply

* Re: [PATCH v1 07/12] scsi: ufs: add option to change default UFS power management level
From: Subhash Jadavani @ 2016-12-13 20:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: vinholikatti-Re5JQEeQqe8AvxtiuMwx3w,
	jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	martin.petersen-QHcLZuEGTsvQT0dZR+AlfA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Hannes Reinecke,
	Yaniv Gardi, Joao Pinto,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20161213200427.vzdwyzajcftvvcgu@rob-hp-laptop>

On 2016-12-13 12:04, Rob Herring wrote:
> On Mon, Dec 12, 2016 at 04:54:20PM -0800, Subhash Jadavani wrote:
>> UFS device and link can be put in multiple different low power modes 
>> hence
>> UFS driver supports multiple different low power modes. By default UFS
>> driver selects the default (optimal) low power mode (which gives 
>> moderate
>> power savings and have relatively less enter and exit latencies) but
>> we might have to tune this default power mode for different chipset
>> platforms to meet the low power requirements/goals. Hence this patch
>> adds option to change default UFS low power mode (level).
>> 
>> Reviewed-by: Yaniv Gardi <ygardi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Subhash Jadavani <subhashj-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 10 ++++++
>>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 ++++++++
>>  drivers/scsi/ufs/ufshcd.c                          | 39 
>> ++++++++++++++++++++++
>>  drivers/scsi/ufs/ufshcd.h                          |  4 +--
>>  4 files changed, 65 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index a99ed55..c3836c5 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,14 @@ Optional properties:
>>  -lanes-per-direction	: number of lanes available per direction - 
>> either 1 or 2.
>>  			  Note that it is assume same number of lanes is used both
>>  			  directions at once. If not specified, default is 2 lanes per 
>> direction.
>> +- rpm-level		: UFS Runtime power management level. Following PM 
>> levels are supported:
>> +			  0 - Both UFS device and Link in active state (Highest power 
>> consumption)
>> +			  1 - UFS device in active state but Link in Hibern8 state
>> +			  2 - UFS device in Sleep state but Link in active state
>> +			  3 - UFS device in Sleep state and Link in hibern8 state (default 
>> PM level)
>> +			  4 - UFS device in Power-down state and Link in Hibern8 state
>> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest 
>> power consumption)
>> +- spm-level		: UFS System power management level. Allowed PM levels 
>> are same as rpm-level.
> 
> This looks like you are putting policy for Linux into DT.
> 
> What I would expect to see here is disabling of states that don't work
> due to some h/w limitation. Otherwise, it is a user decision for what
> modes to go into. Also, I think link and device states should be
> separate.

Yes, generally default level (3) is good enough (and recommended) for 
all platforms and most likely user is only expected to change this if 
they see issues (most H/W) on their platform or they want even more 
aggressive power state (level-4 or level-5) and ready to take the 
performance hit associated with resume latencies.

Also, I think it is better to keep Link and device states tied, one 
reason is that we can't keep device in sleep/active state when Link is 
in OFF state.

> 
> Rob

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] iio: adc: hx711: Add DT binding for avia,hx711
From: Peter Meerwald-Stadler @ 2016-12-13 20:11 UTC (permalink / raw)
  To: Andreas Klinger; +Cc: devicetree, linux-iio, linux-kernel, robh+dt, jic23
In-Reply-To: <20161213180142.GA3162@andreas>


> Add DT bindings for avia,hx711
> Add vendor avia to vendor list
> 
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> ---
>  .../devicetree/bindings/iio/adc/avia-hx711.txt     | 23 ++++++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  2 files changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/avia-hx711.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/avia-hx711.txt b/Documentation/devicetree/bindings/iio/adc/avia-hx711.txt
> new file mode 100644
> index 000000000000..d56f95705fd2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/avia-hx711.txt
> @@ -0,0 +1,23 @@
> +* AVIA HX711 ADC chip for weight cells
> +  Bit-banging driver
> +
> +Required properties:
> + - compatible: Should be "avia,hx711"
> + - sck-gpios:	Definition of the GPIO for the clock
> + - dout-gpios:	Definition of the GPIO for Data-Out

data-out (lowercase)


> +		See Documentation/devicetree/bindings/gpio/gpio.txt
> +
> +Recommended properties:
> + - gain:	Gain select, can be 32, 64 or 128
> +		default is 128
> +
> +Optional properties:

maybe delete this empty section?

> +
> +Example:
> +weight@0 {
> +	compatible = "avia,hx711";
> +	sck-gpios = <&gpio3 10 GPIO_ACTIVE_HIGH>;
> +	dout-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
> +	gain = <32>
> +};
> +
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 44ddc980b085..4696bb5c2198 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -32,6 +32,7 @@ atlas	Atlas Scientific LLC
>  atmel	Atmel Corporation
>  auo	AU Optronics Corporation
>  avago	Avago Technologies
> +avia	avia semiconductor
>  avic	Shanghai AVIC Optoelectronics Co., Ltd.
>  axis	Axis Communications AB
>  boe	BOE Technology Group Co., Ltd.
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

^ permalink raw reply

* Re: [PATCH 3/9] dt-bindings: stm32-dma: Fix typo regarding DMA client binding
From: Rob Herring @ 2016-12-13 20:09 UTC (permalink / raw)
  To: M'boumba Cedric Madianga
  Cc: mark.rutland, devicetree, alexandre.torgue, vinod.koul,
	linux-kernel, mcoquelin.stm32, dmaengine, dan.j.williams,
	linux-arm-kernel
In-Reply-To: <1481636451-27863-4-git-send-email-cedric.madianga@gmail.com>

On Tue, Dec 13, 2016 at 02:40:45PM +0100, M'boumba Cedric Madianga wrote:
> Only four cells are required for dma client binding not five.
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
> ---
>  Documentation/devicetree/bindings/dma/stm32-dma.txt | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

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

^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: arm: update Armada CP110 system controller binding
From: Rob Herring @ 2016-12-13 20:07 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Mark Rutland, devicetree, Yehuda Yitschak, Jason Cooper,
	Pawel Moll, Ian Campbell, Michael Turquette, Hanna Hawa,
	Stephen Boyd, Nadav Haklai, Andrew Lunn, Kumar Gala,
	Gregory Clement, Marcin Wojtas, linux-clk, linux-arm-kernel,
	Sebastian Hesselbarth
In-Reply-To: <1481632880-9198-2-git-send-email-thomas.petazzoni@free-electrons.com>

On Tue, Dec 13, 2016 at 01:41:18PM +0100, Thomas Petazzoni wrote:
> It turns out that in the CP110 HW block present in Marvell Armada
> 7K/8K SoCs, gatable clock n°18 not only controls SD/MMC, but also the
> GOP block. This commit updates the Device Tree binding for this piece
> of hardware accordingly.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  .../devicetree/bindings/arm/marvell/cp110-system-controller0.txt    | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

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

^ permalink raw reply

* Re: [PATCH v2 2/2] crypto: mediatek - add DT bindings documentation
From: Rob Herring @ 2016-12-13 20:06 UTC (permalink / raw)
  To: Ryder Lee
  Cc: Herbert Xu, David S. Miller, Matthias Brugger, devicetree,
	linux-mediatek, linux-kernel, linux-crypto, linux-arm-kernel,
	Sean Wang, Roy Luo
In-Reply-To: <1481592676-2248-3-git-send-email-ryder.lee@mediatek.com>

On Tue, Dec 13, 2016 at 09:31:16AM +0800, Ryder Lee wrote:
> Add DT bindings documentation for the crypto driver
> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  .../devicetree/bindings/crypto/mediatek-crypto.txt | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
> new file mode 100644
> index 0000000..47a786e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
> @@ -0,0 +1,32 @@
> +MediaTek cryptographic accelerators
> +
> +Required properties:
> +- compatible: Should be "mediatek,eip97-crypto"
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain the five crypto engines interrupts in numeric
> +	order. These are global system and four descriptor rings.
> +- clocks: the clock used by the core
> +- clock-names: the names of the clock listed in the clocks property. These are
> +	"ethif", "cryp"
> +- power-domains: Must contain a reference to the PM domain.
> +
> +
> +Optional properties:
> +- interrupt-parent: Should be the phandle for the interrupt controller
> +  that services interrupts for this device

This is not optional. It's perhaps inherited from the parent. You can 
drop it as it's implied by interrupts property.

Rob

^ permalink raw reply

* Re: [PATCH v1 07/12] scsi: ufs: add option to change default UFS power management level
From: Rob Herring @ 2016-12-13 20:04 UTC (permalink / raw)
  To: Subhash Jadavani
  Cc: vinholikatti, jejb, martin.petersen, linux-scsi, Mark Rutland,
	Hannes Reinecke, Yaniv Gardi, Joao Pinto,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <1481590462-9981-1-git-send-email-subhashj@codeaurora.org>

On Mon, Dec 12, 2016 at 04:54:20PM -0800, Subhash Jadavani wrote:
> UFS device and link can be put in multiple different low power modes hence
> UFS driver supports multiple different low power modes. By default UFS
> driver selects the default (optimal) low power mode (which gives moderate
> power savings and have relatively less enter and exit latencies) but
> we might have to tune this default power mode for different chipset
> platforms to meet the low power requirements/goals. Hence this patch
> adds option to change default UFS low power mode (level).
> 
> Reviewed-by: Yaniv Gardi <ygardi@codeaurora.org>
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 10 ++++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 ++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 39 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                          |  4 +--
>  4 files changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index a99ed55..c3836c5 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,14 @@ Optional properties:
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> +- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
> +			  0 - Both UFS device and Link in active state (Highest power consumption)
> +			  1 - UFS device in active state but Link in Hibern8 state
> +			  2 - UFS device in Sleep state but Link in active state
> +			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
> +			  4 - UFS device in Power-down state and Link in Hibern8 state
> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
> +- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.

This looks like you are putting policy for Linux into DT.

What I would expect to see here is disabling of states that don't work 
due to some h/w limitation. Otherwise, it is a user decision for what 
modes to go into. Also, I think link and device states should be 
separate.

Rob

^ permalink raw reply

* Re: [PATCH v4 4/4] regulator: Prevent falling too fast
From: Doug Anderson @ 2016-12-13 20:00 UTC (permalink / raw)
  To: Mark Brown
  Cc: Matthias Kaehlcke, Liam Girdwood, Brian Norris,
	Javier Martinez Canillas, Rob Herring, Mark Rutland,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20161213171904.ohsxc3xhscdinctt@sirena.org.uk>

Hi,

On Tue, Dec 13, 2016 at 9:19 AM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Dec 12, 2016 at 01:15:02PM -0800, Matthias Kaehlcke wrote:
>> El Fri, Oct 28, 2016 at 07:15:21PM +0100 Mark Brown ha dit:
>> > On Mon, Sep 26, 2016 at 10:41:59AM -0700, Doug Anderson wrote:
>
>> > What you're describing to me is a discrete DCDC that has an input
>> > voltage that sets the output voltage which happens to be set with a PWM.
>
>> I experimented a bit with this. Besides the question of how to model
>> the passives I wonder how the two regulators would interact. The
>> correct thing seems to be to specify the input regulator as a supply
>> of the DCDC. dcdc->set_voltage breaks down a voltage transition into
>
> No, not unless the prior descriptions of the hardware have been wildly
> inaccurate - my understanding had been that the DCDC was a normal DCDC
> with an analogue input intended to be biased to set the output voltage
> (presumably in terms of a full rail supply) and that the PWM had been
> connected to this analogue input.  If the PWM is supplying the DCDC then
> the hardware design just seems bizzare, I can't see how this would even
> work.

Looking at one schematic, the discrete BUCK for at least one of the
rails is TPS65261RHBR, which appears to be described at
<https://store.ti.com/TPS65261RHBR.aspx>.  Data sheet appears to be at
<http://www.ti.com/product/tps65261/technicaldocuments?HQS=TI-null-null-octopart-df-pf-null-wwe>.

As you can see from the datasheet ("Adjusting the Output Voltage"
section), it is intended that you stuff a resistor to make a voltage
divider and that's how you select the output voltage.  In our case the
PWM interacts here and allows you to make a more dynamic output
voltage.  I've always thought about the input to the "FB" pin as
making an input voltage, but I guess it's not terribly simple since
the voltage divider ends up dividing between ground and the output
voltage.

Also as you can see, the datasheet never talks about using a PWM to
control the feedback and as I understand it the BUCK wasn't designed
for this, but it (mostly) works just fine.

...you can also see details about the Over Voltage Protection (at last
for this BUCK) in the TPS65261RHBR datasheet.


-Doug

^ permalink raw reply

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
From: Rob Herring @ 2016-12-13 19:54 UTC (permalink / raw)
  To: Guy Shapiro
  Cc: Fabio Estevam, Mark Rutland, devicetree@vger.kernel.org,
	Haibo Chen, Dmitry Torokhov, linux-input@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1481440003-27168-1-git-send-email-guy.shapiro@mobi-wize.com>

On Sun, Dec 11, 2016 at 1:06 AM, Guy Shapiro <guy.shapiro@mobi-wize.com> wrote:
> Make the avarage-samples property a general touchscreen property
> rather than imx6ul device specific.
>
> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
> ---
>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>  drivers/input/touchscreen/imx6ul_tsc.c             | 46 ++++++++++++++++------
>  3 files changed, 41 insertions(+), 19 deletions(-)

[...]

> +       switch (average_samples) {
> +       case 1:
> +               tsc->average_enable = false;
> +               tsc->average_select = 0; /* value unused; initialize anyway */
> +               break;
> +       case 4:
> +               tsc->average_enable = true;
> +               tsc->average_select = 0;
> +               break;
> +       case 8:
> +               tsc->average_enable = true;
> +               tsc->average_select = 1;
> +               break;
> +       case 16:
> +               tsc->average_enable = true;
> +               tsc->average_select = 2;
> +               break;
> +       case 32:
> +               tsc->average_enable = true;
> +               tsc->average_select = 3;
> +               break;

This could be more efficiently written as

tsc->average_select = log2(average_samples) - 2;

Then enable if >=0.

Rob

^ permalink raw reply

* [PATCH v2 07/12] scsi: ufs: add option to change default UFS power management level
From: Subhash Jadavani @ 2016-12-13 19:51 UTC (permalink / raw)
  To: vinholikatti, jejb, martin.petersen
  Cc: linux-scsi, Subhash Jadavani, Rob Herring, Mark Rutland,
	Hannes Reinecke, Yaniv Gardi, Gilad Broner, Joao Pinto,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

UFS device and link can be put in multiple different low power modes hence
UFS driver supports multiple different low power modes. By default UFS
driver selects the default (optimal) low power mode (which gives moderate
power savings and have relatively less enter and exit latencies) but
we might have to tune this default power mode for different chipset
platforms to meet the low power requirements/goals. Hence this patch
adds option to change default UFS low power mode (level).

Reviewed-by: Yaniv Gardi <ygardi@codeaurora.org>
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 10 ++++++
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 ++++++++
 drivers/scsi/ufs/ufshcd.c                          | 39 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h                          |  4 +--
 4 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index a99ed55..c3836c5 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -41,6 +41,14 @@ Optional properties:
 -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
 			  Note that it is assume same number of lanes is used both
 			  directions at once. If not specified, default is 2 lanes per direction.
+- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
+			  0 - Both UFS device and Link in active state (Highest power consumption)
+			  1 - UFS device in active state but Link in Hibern8 state
+			  2 - UFS device in Sleep state but Link in active state
+			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
+			  4 - UFS device in Power-down state and Link in Hibern8 state
+			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
+- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.
 
 Note: If above properties are not defined it can be assumed that the supply
 regulators or clocks are always on.
@@ -66,4 +74,6 @@ Example:
 		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
 		phys = <&ufsphy1>;
 		phy-names = "ufsphy";
+		rpm-level = <3>;
+		spm-level = <5>;
 	};
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index a72a4ba..896943d 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -223,6 +223,19 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
 	return err;
 }
 
+static void ufshcd_parse_pm_levels(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+	struct device_node *np = dev->of_node;
+
+	if (np) {
+		if (of_property_read_u32(np, "rpm-level", &hba->rpm_lvl))
+			hba->rpm_lvl = -1;
+		if (of_property_read_u32(np, "spm-level", &hba->spm_lvl))
+			hba->spm_lvl = -1;
+	}
+}
+
 #ifdef CONFIG_PM
 /**
  * ufshcd_pltfrm_suspend - suspend power management function
@@ -342,6 +355,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 		goto dealloc_host;
 	}
 
+	ufshcd_parse_pm_levels(hba);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 982802c..c5e4b86 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -191,6 +191,22 @@ enum {
 	return ufs_pm_lvl_states[lvl].link_state;
 }
 
+static inline enum ufs_pm_level
+ufs_get_desired_pm_lvl_for_dev_link_state(enum ufs_dev_pwr_mode dev_state,
+					enum uic_link_state link_state)
+{
+	enum ufs_pm_level lvl;
+
+	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++) {
+		if ((ufs_pm_lvl_states[lvl].dev_state == dev_state) &&
+			(ufs_pm_lvl_states[lvl].link_state == link_state))
+			return lvl;
+	}
+
+	/* if no match found, return the level 0 */
+	return UFS_PM_LVL_0;
+}
+
 static struct ufs_dev_fix ufs_fixups[] = {
 	/* UFS cards deviations table */
 	UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
@@ -215,6 +231,14 @@ enum {
 	END_FIX
 };
 
+static inline bool ufshcd_is_valid_pm_lvl(int lvl)
+{
+	if (lvl >= 0 && lvl < ARRAY_SIZE(ufs_pm_lvl_states))
+		return true;
+	else
+		return false;
+}
+
 static void ufshcd_tmc_handler(struct ufs_hba *hba);
 static void ufshcd_async_scan(void *data, async_cookie_t cookie);
 static int ufshcd_reset_and_restore(struct ufs_hba *hba);
@@ -7290,6 +7314,21 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		ufshcd_clkscaling_init_sysfs(hba);
 	}
 
+	/*
+	 * If rpm_lvl and and spm_lvl are not already set to valid levels,
+	 * set the default power management level for UFS runtime and system
+	 * suspend. Default power saving mode selected is keeping UFS link in
+	 * Hibern8 state and UFS device in sleep.
+	 */
+	if (!ufshcd_is_valid_pm_lvl(hba->rpm_lvl))
+		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
+							UFS_SLEEP_PWR_MODE,
+							UIC_LINK_HIBERN8_STATE);
+	if (!ufshcd_is_valid_pm_lvl(hba->spm_lvl))
+		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
+							UFS_SLEEP_PWR_MODE,
+							UIC_LINK_HIBERN8_STATE);
+
 	/* Hold auto suspend until async scan completes */
 	pm_runtime_get_sync(dev);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 787323b..97fbe4a 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -432,9 +432,9 @@ struct ufs_hba {
 	enum ufs_dev_pwr_mode curr_dev_pwr_mode;
 	enum uic_link_state uic_link_state;
 	/* Desired UFS power management level during runtime PM */
-	enum ufs_pm_level rpm_lvl;
+	int rpm_lvl;
 	/* Desired UFS power management level during system PM */
-	enum ufs_pm_level spm_lvl;
+	int spm_lvl;
 	struct device_attribute rpm_lvl_attr;
 	struct device_attribute spm_lvl_attr;
 	int pm_op_in_progress;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: [PATCH] Input: imx6ul_tsc - generalize the averaging property
From: Rob Herring @ 2016-12-13 19:48 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Fabio Estevam, Mark Rutland, Guy Shapiro,
	devicetree@vger.kernel.org, Haibo Chen,
	linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1CDAF0C6-25E1-4CE9-8F98-F07333827B98@gmail.com>

On Tue, Dec 13, 2016 at 1:14 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On December 13, 2016 11:10:50 AM PST, Rob Herring <robh@kernel.org> wrote:
>>On Sun, Dec 11, 2016 at 09:06:43AM +0200, Guy Shapiro wrote:
>>> Make the avarage-samples property a general touchscreen property
>>> rather than imx6ul device specific.
>>>
>>> Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
>>> ---
>>>  .../bindings/input/touchscreen/imx6ul_tsc.txt      | 11 ++----
>>>  .../bindings/input/touchscreen/touchscreen.txt     |  3 ++
>>>  drivers/input/touchscreen/imx6ul_tsc.c             | 46
>>++++++++++++++++------
>>>  3 files changed, 41 insertions(+), 19 deletions(-)
>>
>>You can't just switch existing bindings as that breaks compatibility
>>with old dtbs. The kernel driver would need to support both. Just
>>introduce the new common property and use it for your device.
>>
>
> The old binding is only in my tree at the moment, so I do not think there is compatibility concerns.
>
> Are you OK with the new binding itself?

Ah, then yes. For the binding:

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

^ permalink raw reply

* Re: [PATCH v2 2/2] ASoC: cs43130: Add devicetree bindings for CS43130
From: Rob Herring @ 2016-12-13 19:46 UTC (permalink / raw)
  To: Li Xu
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	brian.austin-jGc1dHjMKG3QT0dZR+AlfA,
	Paul.Handrigan-jGc1dHjMKG3QT0dZR+AlfA
In-Reply-To: <63e40838-0226-4f9d-ab41-5815c724e9ef-XU/xxMRwCJnfk+Ne4bZl5AC/G2K4zDHf@public.gmane.org>

On Mon, Dec 12, 2016 at 02:17:31PM -0600, Li Xu wrote:
> Add devicetree bindings documentation file for Cirrus
> Logic CS43130 codec.
> 
> Signed-off-by: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/sound/cs43130.txt          | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/cs43130.txt

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


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